Skip to content

Commit e02b148

Browse files
tenderlovetenderlove
committed
unlock the gvl when calculating hashes / salts
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.
1 parent 0612960 commit e02b148

2 files changed

Lines changed: 80 additions & 17 deletions

File tree

‎ext/mri/bcrypt_ext.c‎

Lines changed: 79 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,60 @@
11
#include <ruby.h>
22
#include <ow-crypt.h>
33

4+
#ifdef HAVE_RUBY_THREAD_H
5+
#include <ruby/thread.h>
6+
#endif
7+
8+
/* For Ruby 1.8 support. This doesn't actually return a frozen string, but
9+
* we don't release the GVL on Ruby 1.8, so it doesn't matter. Delete this
10+
* when 1.8 support is dropped. */
11+
#ifndef HAVE_RB_STR_NEW_FROZEN
12+
static VALUE rb_str_new_frozen(VALUE str)
13+
{
14+
return str;
15+
}
16+
#endif
17+
418
static VALUE mBCrypt;
519
static VALUE cBCryptEngine;
620

21+
struct bc_salt_args {
22+
const char * prefix;
23+
unsigned long count;
24+
const char * input;
25+
int size;
26+
};
27+
28+
static void * bc_salt_nogvl(void * ptr) {
29+
struct bc_salt_args * args = ptr;
30+
31+
return crypt_gensalt_ra(args->prefix, args->count, args->input, args->size);
32+
}
33+
734
/* Given a logarithmic cost parameter, generates a salt for use with +bc_crypt+.
835
*/
936
static VALUE bc_salt(VALUE self, VALUE prefix, VALUE count, VALUE input) {
1037
char * salt;
1138
VALUE str_salt;
12-
13-
salt = crypt_gensalt_ra(
14-
StringValuePtr(prefix),
15-
NUM2ULONG(count),
16-
NIL_P(input) ? NULL : StringValuePtr(input),
17-
NIL_P(input) ? 0 : RSTRING_LEN(input));
39+
struct bc_salt_args args;
40+
41+
/* duplicate the parameters for thread safety. If another thread has a
42+
* reference to the parameters and mutates them while we are working,
43+
* that would be very bad. Duping the strings means that the reference
44+
* isn't shared. */
45+
prefix = rb_str_new_frozen(prefix);
46+
input = rb_str_new_frozen(input);
47+
48+
args.prefix = StringValuePtr(prefix);
49+
args.count = NUM2ULONG(count);
50+
args.input = NIL_P(input) ? NULL : StringValuePtr(input);
51+
args.size = NIL_P(input) ? 0 : RSTRING_LEN(input);
52+
53+
#ifdef HAVE_RUBY_THREAD_H
54+
salt = rb_thread_call_without_gvl(bc_salt_nogvl, &args, NULL, NULL);
55+
#else
56+
salt = bc_salt_nogvl((void *)&args);
57+
#endif
1858

1959
if(!salt) return Qnil;
2060

@@ -24,30 +64,52 @@ static VALUE bc_salt(VALUE self, VALUE prefix, VALUE count, VALUE input) {
2464
return str_salt;
2565
}
2666

67+
struct bc_crypt_args {
68+
const char * key;
69+
const char * setting;
70+
void * data;
71+
int size;
72+
};
73+
74+
static void * bc_crypt_nogvl(void * ptr) {
75+
struct bc_crypt_args * args = ptr;
76+
77+
return crypt_ra(args->key, args->setting, &args->data, &args->size);
78+
}
79+
2780
/* Given a secret and a salt, generates a salted hash (which you can then store safely).
2881
*/
2982
static VALUE bc_crypt(VALUE self, VALUE key, VALUE setting) {
3083
char * value;
31-
void * data;
32-
int size;
3384
VALUE out;
3485

35-
data = NULL;
36-
size = 0xDEADBEEF;
86+
struct bc_crypt_args args;
3787

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

40-
value = crypt_ra(
41-
NIL_P(key) ? NULL : StringValuePtr(key),
42-
NIL_P(setting) ? NULL : StringValuePtr(setting),
43-
&data,
44-
&size);
90+
/* duplicate the parameters for thread safety. If another thread has a
91+
* reference to the parameters and mutates them while we are working,
92+
* that would be very bad. Duping the strings means that the reference
93+
* isn't shared. */
94+
key = rb_str_new_frozen(key);
95+
setting = rb_str_new_frozen(setting);
96+
97+
args.data = NULL;
98+
args.size = 0xDEADBEEF;
99+
args.key = NIL_P(key) ? NULL : StringValuePtr(key);
100+
args.setting = NIL_P(setting) ? NULL : StringValuePtr(setting);
101+
102+
#ifdef HAVE_RUBY_THREAD_H
103+
value = rb_thread_call_without_gvl(bc_crypt_nogvl, &args, NULL, NULL);
104+
#else
105+
value = bc_crypt_nogvl((void *)&args);
106+
#endif
45107

46108
if(!value) return Qnil;
47109

48-
out = rb_str_new(data, size - 1);
110+
out = rb_str_new(args.data, args.size - 1);
49111

50-
xfree(data);
112+
xfree(args.data);
51113

52114
return out;
53115
}

‎ext/mri/extconf.rb‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,5 +12,6 @@
1212
else
1313
require "mkmf"
1414
dir_config("bcrypt_ext")
15+
have_func 'rb_str_new_frozen' # Ruby 1.8 support.
1516
create_makefile("bcrypt_ext")
1617
end

0 commit comments

Comments
 (0)