Skip to content

Commit c800cf0

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 c800cf0

2 files changed

Lines changed: 77 additions & 17 deletions

File tree

‎ext/mri/bcrypt_ext.c‎

Lines changed: 76 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,57 @@
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+
/* Delete this when 1.8 support is dropped. */
9+
#ifdef HAVE_RB_STR_NEW_FROZEN
10+
#define RB_STR_NEW_FROZEN rb_str_new_frozen
11+
#else
12+
#define RB_STR_NEW_FROZEN rb_str_new4
13+
#endif
14+
415
static VALUE mBCrypt;
516
static VALUE cBCryptEngine;
617

18+
struct bc_salt_args {
19+
const char * prefix;
20+
unsigned long count;
21+
const char * input;
22+
int size;
23+
};
24+
25+
static void * bc_salt_nogvl(void * ptr) {
26+
struct bc_salt_args * args = ptr;
27+
28+
return crypt_gensalt_ra(args->prefix, args->count, args->input, args->size);
29+
}
30+
731
/* Given a logarithmic cost parameter, generates a salt for use with +bc_crypt+.
832
*/
933
static VALUE bc_salt(VALUE self, VALUE prefix, VALUE count, VALUE input) {
1034
char * salt;
1135
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));
36+
struct bc_salt_args args;
37+
38+
/* duplicate the parameters for thread safety. If another thread has a
39+
* reference to the parameters and mutates them while we are working,
40+
* that would be very bad. Duping the strings means that the reference
41+
* isn't shared. */
42+
prefix = RB_STR_NEW_FROZEN(prefix);
43+
input = RB_STR_NEW_FROZEN(input);
44+
45+
args.prefix = StringValuePtr(prefix);
46+
args.count = NUM2ULONG(count);
47+
args.input = NIL_P(input) ? NULL : StringValuePtr(input);
48+
args.size = NIL_P(input) ? 0 : RSTRING_LEN(input);
49+
50+
#ifdef HAVE_RUBY_THREAD_H
51+
salt = rb_thread_call_without_gvl(bc_salt_nogvl, &args, NULL, NULL);
52+
#else
53+
salt = bc_salt_nogvl((void *)&args);
54+
#endif
1855

1956
if(!salt) return Qnil;
2057

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

64+
struct bc_crypt_args {
65+
const char * key;
66+
const char * setting;
67+
void * data;
68+
int size;
69+
};
70+
71+
static void * bc_crypt_nogvl(void * ptr) {
72+
struct bc_crypt_args * args = ptr;
73+
74+
return crypt_ra(args->key, args->setting, &args->data, &args->size);
75+
}
76+
2777
/* Given a secret and a salt, generates a salted hash (which you can then store safely).
2878
*/
2979
static VALUE bc_crypt(VALUE self, VALUE key, VALUE setting) {
3080
char * value;
31-
void * data;
32-
int size;
3381
VALUE out;
3482

35-
data = NULL;
36-
size = 0xDEADBEEF;
83+
struct bc_crypt_args args;
3784

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

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

46105
if(!value) return Qnil;
47106

48-
out = rb_str_new(data, size - 1);
107+
out = rb_str_new(args.data, args.size - 1);
49108

50-
xfree(data);
109+
xfree(args.data);
51110

52111
return out;
53112
}

‎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)