Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Why bcrypt has both sync and async methods? #9

Open
yorkie opened this issue Sep 20, 2015 · 0 comments
Open

Why bcrypt has both sync and async methods? #9

yorkie opened this issue Sep 20, 2015 · 0 comments
Labels

Comments

@yorkie
Copy link
Owner

yorkie commented Sep 20, 2015

Note: This is another proven case to use Node.js properly.

Because of working with loopback, which uses bcrypt as its algorithm to encrypt user’s password, in these recent weeks. Then I watched something test result in speed aspect, found some tests that always create user then encrypt password would cost over 3s, which is very weird as I was thinking.

So I was about to take what the issue is, and want to know why the cases are that slow. Then I got an answer is the today’s topic: BCRYPT. Yea, maybe you have got known about this word, and I did know it costs much CPU resources to avoid the rainbow from other hackers. But after I looked the loopback’s following line

this.$password = bcrypt.hashSync(plain, salt);

I’m little confusion on the usage from loopback, this is in an async routine, right? as an opposite of Node.js best practice, shouldn’t we use async functions here, too?

I was still thinking that loopback is wrong and gonna submit a PR, but just now, when I decided to write this post, I got I’m wrong.

The story just looks like I get known that this is an absolute CPU consuming operator, so there is no such that async I/O method. Do we should call it as ASYNC CPU function? I’m quite not sure what’s the answer, but let’s take a look at how bcrypt.js handles with the arguments in async function.

In the [email protected], the async hash function’s normal usage should be:

crypt.hash(
    'password', 
    'salt',
    function ondone (err) {
        // get fired when done or fail
    },
    function onprogress (num) {
        // get fired so many times to notify what’s the progress number
    }
)

Internally, the bcryptjs library just splits the whole stuff into some little subtasks by declared rounds, that is a number in 4 - 64. And it would do the following thing when one round is done:

  • Check if the onprogress is provided, it’s optional.
  • Wrap the next round into the Node.js nextTick utility.

Such that, I can give you an example to simplify the workflow, if I decided to use 31 as the rounds number, then the onprogress function would be called 32 times, and will executes over 32 ticks minimally. Now if I use the same sync function, this should only be executed without wrapping by nextTick.

Ok, this is a preparation for us to enter the final part, is there a necessarity to use async copy in loopback? I have to tell you that I still have no answer here. Without the async calls, when server is going to encrypt someone’s password, the any other requests would be blocked, right? So the sequence of users would looks like: A1-A2-A3-B1-B2-B3-C1-C2-C3. In opposite of this, async function enables we did the same thing like: A1-B1-C1-A2-B2-C2-A3-B3-C3. This is still confusion, no one solution is getting less cost than another one, the former just can let the first one to quit at first, the latter is going to do not block others.

But the difference is from the other requests, assuming the encryption would consume 3s to achieve, that is when someone is going to login or register, the other requests without requiring to execute the algorithm would be blocked, this is unfair to them. And this is why Node.js or event-loop-based server are not able to handle with the large CPU tasks, it’s not about the JavaScript or other dynamic language is not good at doing CPU stuff (even the JavaScript/v8 has been proven that did align with C/C++ on speed). The best practice of using event-loop just is we procedure should be split into these small enough micro tasks which costs extreme less CPU time.

@yorkie yorkie added the Notes label Sep 20, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant