riverrun / bcrypt_elixir

Bcrypt password hashing for Elixir
Other
162 stars 26 forks source link

Consider dirty schedulers #5

Closed michalmuskala closed 7 years ago

michalmuskala commented 7 years ago

Now that dirty schedulers are out of the experimental phase, I see no reason not to use them.

It's worth pointing out that the library significantly exceeds the advised run time for NIFs. Even though each bcrypt round is a separate NIF - a single round takes about 30ms on my machine. The recommended maximum runtime for a NIF is 1ms.

Running on dirty schedulers should significantly improve the responsiveness of the programs using the NIF.

riverrun commented 7 years ago

I'm working on this at the moment :)

What I'll probably do is keep the current implementation at the current version and then make the dirty scheduler version 1.0, so that there's a clear distinction between versions, as not everyone is using Erlang 20.

It will be interesting to see if, or how much, the responsiveness will improve. I know there was a discussion about this on the slack channel recently, and that discussion centered on the responsiveness under a heavy load. It's important to remember, though, that the bcrypt function is meant to take a long time, and that it is cpu-intensive, and so under a really heavy load, there need to be enough cpus to do all the work.

I should be able to release something within a week.

michalmuskala commented 7 years ago

That's great to hear. :+1:

The solution I had in mind to keep both compatibility and take advantage of dirty schedulers would be to run using dirty schedulers when they are available and otherwise fall back to a regular NIF just like it's done today.

riverrun commented 7 years ago

Work in progress on dirtyable branch.

michalmuskala commented 7 years ago

I looked around a bit. It seems like you're moving more work to the C side from the Elixir side. Any particular reason? If the reason was to get rid of the Base64 module - Elixir has Base.decode64 in stdlib (which probably wasn't the case when the code was initially created).

riverrun commented 7 years ago

There are a few reasons for moving more work to the C side:

As for the Base module in the stdlib, that does not help much because Bcrypt uses a non-standard base64 alphabet.

If you have any other questions, please let me know.

riverrun commented 7 years ago

I just merged the changes to the master branch.

Now version 1 of bcrypt_elixir is the version with dirty scheduler support, and version 0.12 is for anyone not yet on Erlang 20.