unlock the gvl when calculating hashes / salts#124
Merged
tenderlove merged 4 commits intobcrypt-ruby:masterfrom Nov 27, 2019
Merged
unlock the gvl when calculating hashes / salts#124tenderlove merged 4 commits intobcrypt-ruby:masterfrom
tenderlove merged 4 commits intobcrypt-ruby:masterfrom
Conversation
e02b148 to
c800cf0
Compare
|
So is this mergeable yet? :) |
bdewater
reviewed
Mar 27, 2019
Collaborator
Author
|
I'm going to merge this. I should have merged it a long time ago. 🙇♂️ Bechmarks in 2019: Without this patch: With this patch: |
Holding on to the GVL means we can't do anything in parallel. Since
these are CPU-only operations that do not involve the ruby interpreter,
we can safely unlock the GVL when calculating hashes.
This program demonstrates the difference:
```ruby
require 'bcrypt'
require 'thread'
GUESSES = (ENV['GUESSES'] || 100).to_i
THREADS = (ENV['THREADS'] || 1).to_i
p GUESSES: GUESSES, THREADS: THREADS
password = BCrypt::Password.create 'hello world!'
queue = Queue.new
GUESSES.times { queue << "x" * 90 }
THREADS.times { queue << nil }
THREADS.times.map {
Thread.new {
while guess = queue.pop
password == guess
end
}
}.each(&:join)
```
Without this patch:
```
[aaron@TC bcrypt-ruby (master)]$ time THREADS=4 ruby test.rb
{:GUESSES=>100, :THREADS=>4}
real 0m30.014s
user 0m29.739s
sys 0m0.153s
```
With the patch:
```
[aaron@TC bcrypt-ruby (master)]$ time THREADS=4 ruby -Ilib test.rb
{:GUESSES=>100, :THREADS=>4}
real 0m12.293s
user 0m42.382s
sys 0m0.169s
```
If you run this program with the patch, you can see Ruby use nearly 400% CPU,
as long as you have a 4 core machine.
The crypt library is almost certainly using `malloc` and not `xmalloc`. We should use `free` instead of `xfree` to ensure the malloc / free calls are correctly balanced.
Contributor
|
@tenderlove may be mention this in CHANGELOG? |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Holding on to the GVL means we can't do anything in parallel. Since
these are CPU-only operations that do not involve the ruby interpreter,
we can safely unlock the GVL when calculating hashes.
This program demonstrates the difference:
Without this patch:
With the patch:
If you run this program with the patch, you can see Ruby use nearly 400% CPU,
as long as you have a 4 core machine.
Sending this as a PR because I'm not sure if the feature detection is enough to work on all versions of Ruby we support and I figured I'd let the CI test it. If something fails, I'll add better feature detection.