Skip to content

Commit

Permalink
unlock the gvl when calculating hashes / salts
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
tenderlove committed Nov 27, 2019
1 parent 2121626 commit e1320b0
Show file tree
Hide file tree
Showing 2 changed files with 77 additions and 17 deletions.
93 changes: 76 additions & 17 deletions ext/mri/bcrypt_ext.c
Original file line number Diff line number Diff line change
@@ -1,20 +1,57 @@
#include <ruby.h>
#include <ow-crypt.h>

#ifdef HAVE_RUBY_THREAD_H
#include <ruby/thread.h>
#endif

/* Delete this when 1.8 support is dropped. */
#ifdef HAVE_RB_STR_NEW_FROZEN
#define RB_STR_NEW_FROZEN rb_str_new_frozen
#else
#define RB_STR_NEW_FROZEN rb_str_new4
#endif

static VALUE mBCrypt;
static VALUE cBCryptEngine;

struct bc_salt_args {
const char * prefix;
unsigned long count;
const char * input;
int size;
};

static void * bc_salt_nogvl(void * ptr) {
struct bc_salt_args * args = ptr;

return crypt_gensalt_ra(args->prefix, args->count, args->input, args->size);
}

/* Given a logarithmic cost parameter, generates a salt for use with +bc_crypt+.
*/
static VALUE bc_salt(VALUE self, VALUE prefix, VALUE count, VALUE input) {
char * salt;
VALUE str_salt;

salt = crypt_gensalt_ra(
StringValuePtr(prefix),
NUM2ULONG(count),
NIL_P(input) ? NULL : StringValuePtr(input),
NIL_P(input) ? 0 : RSTRING_LEN(input));
struct bc_salt_args args;

/* duplicate the parameters for thread safety. If another thread has a
* reference to the parameters and mutates them while we are working,
* that would be very bad. Duping the strings means that the reference
* isn't shared. */
prefix = RB_STR_NEW_FROZEN(prefix);
input = RB_STR_NEW_FROZEN(input);

args.prefix = StringValuePtr(prefix);
args.count = NUM2ULONG(count);
args.input = NIL_P(input) ? NULL : StringValuePtr(input);
args.size = NIL_P(input) ? 0 : RSTRING_LEN(input);

#ifdef HAVE_RUBY_THREAD_H
salt = rb_thread_call_without_gvl(bc_salt_nogvl, &args, NULL, NULL);
#else
salt = bc_salt_nogvl((void *)&args);
#endif

if(!salt) return Qnil;

Expand All @@ -24,30 +61,52 @@ static VALUE bc_salt(VALUE self, VALUE prefix, VALUE count, VALUE input) {
return str_salt;
}

struct bc_crypt_args {
const char * key;
const char * setting;
void * data;
int size;
};

static void * bc_crypt_nogvl(void * ptr) {
struct bc_crypt_args * args = ptr;

return crypt_ra(args->key, args->setting, &args->data, &args->size);
}

/* Given a secret and a salt, generates a salted hash (which you can then store safely).
*/
static VALUE bc_crypt(VALUE self, VALUE key, VALUE setting) {
char * value;
void * data;
int size;
VALUE out;

data = NULL;
size = 0xDEADBEEF;
struct bc_crypt_args args;

if(NIL_P(key) || NIL_P(setting)) return Qnil;

value = crypt_ra(
NIL_P(key) ? NULL : StringValuePtr(key),
NIL_P(setting) ? NULL : StringValuePtr(setting),
&data,
&size);
/* duplicate the parameters for thread safety. If another thread has a
* reference to the parameters and mutates them while we are working,
* that would be very bad. Duping the strings means that the reference
* isn't shared. */
key = RB_STR_NEW_FROZEN(key);
setting = RB_STR_NEW_FROZEN(setting);

args.data = NULL;
args.size = 0xDEADBEEF;
args.key = NIL_P(key) ? NULL : StringValuePtr(key);
args.setting = NIL_P(setting) ? NULL : StringValuePtr(setting);

#ifdef HAVE_RUBY_THREAD_H
value = rb_thread_call_without_gvl(bc_crypt_nogvl, &args, NULL, NULL);
#else
value = bc_crypt_nogvl((void *)&args);
#endif

if(!value) return Qnil;

out = rb_str_new2(value);
out = rb_str_new(args.data, args.size - 1);

xfree(data);
xfree(args.data);

return out;
}
Expand Down
1 change: 1 addition & 0 deletions ext/mri/extconf.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,5 +18,6 @@

$defs << "-D__SKIP_GNU"
dir_config("bcrypt_ext")
have_func 'rb_str_new_frozen' # Ruby 1.8 support.
create_makefile("bcrypt_ext")
end

0 comments on commit e1320b0

Please sign in to comment.