ruby-changes:40644
From: normal <ko1@a...>
Date: Tue, 24 Nov 2015 06:21:17 +0900 (JST)
Subject: [ruby-changes:40644] normal:r52723 (trunk): fiddle: release GVL for ffi_call
normal 2015-11-24 06:20:56 +0900 (Tue, 24 Nov 2015) New Revision: 52723 http://svn.ruby-lang.org/cgi-bin/viewvc.cgi?view=revision&revision=52723 Log: fiddle: release GVL for ffi_call Some external functions I wish to call may take a long time and unnecessarily block other threads. This may lead to performance regressions for fast functions as releasing/acquiring the GVL is not cheap, but can improve performance for long-running functions in multi-threaded applications. This also means we must reacquire the GVL when calling Ruby-defined callbacks for Fiddle::Closure, meaning we must detect whether the current thread has the GVL by exporting ruby_thread_has_gvl_p in internal.h * ext/fiddle/function.c (struct nogvl_ffi_call_args): new struct for GVL release (nogvl_ffi_call): new function (function_call): adjust for GVL release [ruby-core:71642] [Feature #11607] * ext/fiddle/closure.c (struct callback_args): new struct for GVL acquire (with_gvl_callback): adjusted original callback function (callback): wrapper for conditional GVL acquire * ext/fiddle/depend: add dependencies * ext/fiddle/extconf.rb: include top_srcdir for internal.h * internal.h (ruby_thread_has_gvl_p): expose for fiddle * vm_core.h (ruby_thread_has_gvl_p): moved to internal.h * test/fiddle/test_function.rb (test_nogvl_poll): new test Modified files: trunk/ChangeLog trunk/ext/fiddle/closure.c trunk/ext/fiddle/depend trunk/ext/fiddle/extconf.rb trunk/ext/fiddle/function.c trunk/internal.h trunk/test/fiddle/test_function.rb trunk/vm_core.h Index: ChangeLog =================================================================== --- ChangeLog (revision 52722) +++ ChangeLog (revision 52723) @@ -1,3 +1,20 @@ https://github.com/ruby/ruby/blob/trunk/ChangeLog#L1 +Tue Nov 24 05:13:35 2015 Eric Wong <e@8...> + + * ext/fiddle/function.c (struct nogvl_ffi_call_args): + new struct for GVL release + (nogvl_ffi_call): new function + (function_call): adjust for GVL release + [ruby-core:71642] [Feature #11607] + * ext/fiddle/closure.c (struct callback_args): + new struct for GVL acquire + (with_gvl_callback): adjusted original callback function + (callback): wrapper for conditional GVL acquire + * ext/fiddle/depend: add dependencies + * ext/fiddle/extconf.rb: include top_srcdir for internal.h + * internal.h (ruby_thread_has_gvl_p): expose for fiddle + * vm_core.h (ruby_thread_has_gvl_p): moved to internal.h + * test/fiddle/test_function.rb (test_nogvl_poll): new test + Mon Nov 23 19:53:12 2015 Naohisa Goto <ngotogenome@g...> * configure.in: On Solaris, with gcc, "-std=iso9899:1999" Index: vm_core.h =================================================================== --- vm_core.h (revision 52722) +++ vm_core.h (revision 52723) @@ -1036,7 +1036,6 @@ rb_vm_living_threads_remove(rb_vm_t *vm, https://github.com/ruby/ruby/blob/trunk/vm_core.h#L1036 vm->living_thread_num--; } -int ruby_thread_has_gvl_p(void); typedef int rb_backtrace_iter_func(void *, VALUE, int, VALUE); rb_control_frame_t *rb_vm_get_ruby_level_next_cfp(const rb_thread_t *th, const rb_control_frame_t *cfp); rb_control_frame_t *rb_vm_get_binding_creatable_next_cfp(const rb_thread_t *th, const rb_control_frame_t *cfp); Index: ext/fiddle/depend =================================================================== --- ext/fiddle/depend (revision 52722) +++ ext/fiddle/depend (revision 52723) @@ -9,7 +9,9 @@ CONFIGURE_LIBFFI = \ https://github.com/ruby/ruby/blob/trunk/ext/fiddle/depend#L9 $(OBJS): $(HDRS) $(ruby_headers) \ $(hdrdir)/ruby/io.h \ $(hdrdir)/ruby/encoding.h \ - $(hdrdir)/ruby/oniguruma.h + $(hdrdir)/ruby/oniguruma.h \ + $(hdrdir)/ruby/thread.h \ + $(top_srcdir)/internal.h $(STATIC_LIB) $(RUBYARCHDIR)/$(DLLIB) $(DLLIB): $(LIBFFI_A) Index: ext/fiddle/function.c =================================================================== --- ext/fiddle/function.c (revision 52722) +++ ext/fiddle/function.c (revision 52723) @@ -1,4 +1,5 @@ https://github.com/ruby/ruby/blob/trunk/ext/fiddle/function.c#L1 #include <fiddle.h> +#include <ruby/thread.h> #ifdef PRIsVALUE # define RB_OBJ_CLASSNAME(obj) rb_obj_class(obj) @@ -128,13 +129,28 @@ initialize(int argc, VALUE argv[], VALUE https://github.com/ruby/ruby/blob/trunk/ext/fiddle/function.c#L129 return self; } +struct nogvl_ffi_call_args { + ffi_cif *cif; + void (*fn)(void); + void **values; + fiddle_generic retval; +}; + +static void * +nogvl_ffi_call(void *ptr) +{ + struct nogvl_ffi_call_args *args = ptr; + + ffi_call(args->cif, args->fn, &args->retval, args->values); + + return NULL; +} + static VALUE function_call(int argc, VALUE argv[], VALUE self) { - ffi_cif * cif; - fiddle_generic retval; + struct nogvl_ffi_call_args args = { 0 }; fiddle_generic *generic_args; - void **values; VALUE cfunc, types, cPointer; int i; VALUE alloc_buffer = 0; @@ -148,7 +164,7 @@ function_call(int argc, VALUE argv[], VA https://github.com/ruby/ruby/blob/trunk/ext/fiddle/function.c#L164 rb_error_arity(argc, i, i); } - TypedData_Get_Struct(self, ffi_cif, &function_data_type, cif); + TypedData_Get_Struct(self, ffi_cif, &function_data_type, args.cif); if (rb_safe_level() >= 1) { for (i = 0; i < argc; i++) { @@ -161,7 +177,8 @@ function_call(int argc, VALUE argv[], VA https://github.com/ruby/ruby/blob/trunk/ext/fiddle/function.c#L177 generic_args = ALLOCV(alloc_buffer, (size_t)(argc + 1) * sizeof(void *) + (size_t)argc * sizeof(fiddle_generic)); - values = (void **)((char *)generic_args + (size_t)argc * sizeof(fiddle_generic)); + args.values = (void **)((char *)generic_args + + (size_t)argc * sizeof(fiddle_generic)); for (i = 0; i < argc; i++) { VALUE type = RARRAY_AREF(types, i); @@ -177,11 +194,12 @@ function_call(int argc, VALUE argv[], VA https://github.com/ruby/ruby/blob/trunk/ext/fiddle/function.c#L194 } VALUE2GENERIC(NUM2INT(type), src, &generic_args[i]); - values[i] = (void *)&generic_args[i]; + args.values[i] = (void *)&generic_args[i]; } - values[argc] = NULL; + args.values[argc] = NULL; + args.fn = NUM2PTR(rb_Integer(cfunc)); - ffi_call(cif, NUM2PTR(rb_Integer(cfunc)), &retval, values); + (void)rb_thread_call_without_gvl(nogvl_ffi_call, &args, 0, 0); rb_funcall(mFiddle, rb_intern("last_error="), 1, INT2NUM(errno)); #if defined(_WIN32) @@ -190,7 +208,7 @@ function_call(int argc, VALUE argv[], VA https://github.com/ruby/ruby/blob/trunk/ext/fiddle/function.c#L208 ALLOCV_END(alloc_buffer); - return GENERIC2VALUE(rb_iv_get(self, "@return_type"), retval); + return GENERIC2VALUE(rb_iv_get(self, "@return_type"), args.retval); } void Index: ext/fiddle/extconf.rb =================================================================== --- ext/fiddle/extconf.rb (revision 52722) +++ ext/fiddle/extconf.rb (revision 52723) @@ -143,6 +143,7 @@ if libffi https://github.com/ruby/ruby/blob/trunk/ext/fiddle/extconf.rb#L143 $LOCAL_LIBS.prepend("./#{libffi.a} ").strip! # to exts.mk $INCFLAGS.gsub!(/-I#{libffi.dir}/, '-I$(LIBFFI_DIR)') end +$INCFLAGS << " -I$(top_srcdir)" create_makefile 'fiddle' do |conf| if !libffi next conf << "LIBFFI_CLEAN = none\n" Index: ext/fiddle/closure.c =================================================================== --- ext/fiddle/closure.c (revision 52722) +++ ext/fiddle/closure.c (revision 52723) @@ -1,4 +1,6 @@ https://github.com/ruby/ruby/blob/trunk/ext/fiddle/closure.c#L1 #include <fiddle.h> +#include <ruby/thread.h> +#include "internal.h" /* rb_thread_has_gvl_p */ VALUE cFiddleClosure; @@ -55,10 +57,19 @@ const rb_data_type_t closure_data_type = https://github.com/ruby/ruby/blob/trunk/ext/fiddle/closure.c#L57 {0, dealloc, closure_memsize,}, }; -static void -callback(ffi_cif *cif, void *resp, void **args, void *ctx) +struct callback_args { + ffi_cif *cif; + void *resp; + void **args; + void *ctx; +}; + +static void * +with_gvl_callback(void *ptr) { - VALUE self = (VALUE)ctx; + struct callback_args *x = ptr; + + VALUE self = (VALUE)x->ctx; VALUE rbargs = rb_iv_get(self, "@args"); VALUE ctype = rb_iv_get(self, "@ctype"); int argc = RARRAY_LENINT(rbargs); @@ -76,46 +87,46 @@ callback(ffi_cif *cif, void *resp, void https://github.com/ruby/ruby/blob/trunk/ext/fiddle/closure.c#L87 argc = 0; break; case TYPE_INT: - rb_ary_push(params, INT2NUM(*(int *)args[i])); + rb_ary_push(params, INT2NUM(*(int *)x->args[i])); break; case -TYPE_INT: - rb_ary_push(params, UINT2NUM(*(unsigned int *)args[i])); + rb_ary_push(params, UINT2NUM(*(unsigned int *)x->args[i])); break; case TYPE_VOIDP: rb_ary_push(params, rb_funcall(cPointer, rb_intern("[]"), 1, - PTR2NUM(*(void **)args[i]))); + PTR2NUM(*(void **)x->args[i]))); break; case TYPE_LONG: - rb_ary_push(params, LONG2NUM(*(long *)args[i])); + rb_ary_push(params, LONG2NUM(*(long *)x->args[i])); break; case -TYPE_LONG: - rb_ary_push(params, ULONG2NUM(*(unsigned long *)args[i])); + rb_ary_push(params, ULONG2NUM(*(unsigned long *)x->args[i])); break; case TYPE_CHAR: - rb_ary_push(params, INT2NUM(*(signed char *)args[i])); + rb_ary_push(params, INT2NUM(*(signed char *)x->args[i])); break; case -TYPE_CHAR: - rb_ary_push(params, UINT2NUM(*(unsigned char *)args[i])); + rb_ary_push(params, UINT2NUM(*(unsigned char *)x->args[i])); break; case TYPE_SHORT: - rb_ary_push(params, INT2NUM(*(signed short *)args[i])); + rb_ary_push(params, INT2NUM(*(signed short *)x->args[i])); break; case -TYPE_SHORT: - rb_ary_push(params, UINT2NUM(*(unsigned short *)args[i])); + rb_ary_push(params, UINT2NUM(*(unsigned short *)x->args[i])); break; case TYPE_DOUBLE: - rb_ary_push(params, rb_float_new(*(double *)args[i])); + rb_ary_push(params, rb_float_new(*(double *)x->args[i])); break; case TYPE_FLOAT: - rb_ary_push(params, rb_float_new(*(float *)args[i])); + rb_ary_push(params, rb_float_new(*(float *)x->args[i])); break; #if HAVE_LONG_LONG case TYPE_LONG_LONG: - rb_ary_push(params, LL2NUM(*(LONG_LONG *)args[i])); + rb_ary_push(params, LL2NUM(*(LONG_LONG *)x->args[i])); break; case -TYPE_LONG_LONG: - rb_ary_push(params, ULL2NUM(*(unsigned LONG_LONG *)args[i])); + rb_ary_push(params, ULL2NUM(*(unsigned LONG_LONG *)x->args[i])); break; #endif default: @@ -131,41 +142,59 @@ callback(ffi_cif *cif, void *resp, void https://github.com/ruby/ruby/blob/trunk/ext/fiddle/closure.c#L142 case TYPE_VOID: break; case TYPE_LONG: - *(long *)resp = NUM2LONG(ret); + *(long *)x->resp = NUM2LONG(ret); break; case -TYPE_LONG: - *(unsigned long *)resp = NUM2ULONG(ret); + *(unsigned long *)x->resp = NUM2ULONG(ret); break; case TYPE_CHAR: case TYPE_SHORT: case TYPE_INT: - *(ffi_sarg *)resp = NUM2INT(ret); + *(ffi_sarg *)x->resp = NUM2INT(ret); break; case -TYPE_CHAR: case -TYPE_SHORT: case -TYPE_INT: - *(ffi_arg *)resp = NUM2UINT(ret); + *(ffi_arg *)x->resp = NUM2UINT(ret); break; case TYPE_VOIDP: - *(void **)resp = NUM2PTR(ret); + *(void **)x->resp = NUM2PTR(ret); break; case TYPE_DOUBLE: - *(double *)resp = NUM2DBL(ret); + *(double *)x->resp = NUM2DBL(ret); break; case TYPE_FLOAT: - *(float *)resp = (float)NUM2DBL(ret); + *(float *)x->resp = (float)NUM2DBL(ret); break; #if HAVE_LONG_LONG case TYPE_LONG_LONG: - *(LONG_LONG *)resp = NUM2LL(ret); + *(LONG_LONG *)x->resp = NUM2LL(ret); break; case -TYPE_LONG_LONG: - *(unsigned LONG_LONG *)resp = NUM2ULL(ret); + *(unsigned LONG_LONG *)x->resp = NUM2ULL(ret); break; #endif default: rb_raise(rb_eRuntimeError, "closure retval: %d", type); } + return 0; +} + +static void +callback(ffi_cif *cif, void *resp, void **args, void *ctx) +{ + struct callback_args x; + + x.cif = cif; + x.resp = resp; + x.args = args; + x.ctx = ctx; + + if (ruby_thread_has_gvl_p()) { + (void)with_gvl_callback(&x); + } else { + (void)rb_thread_call_with_gvl(with_gvl_callback, &x); + } } static VALUE Index: internal.h =================================================================== --- internal.h (revision 52722) +++ internal.h (revision 52723) @@ -1320,6 +1320,9 @@ VALUE rb_gcd_gmp(VALUE x, VALUE y); https://github.com/ruby/ruby/blob/trunk/internal.h#L1320 VALUE rb_setup_fake_str(struct RString *fake_str, const char *name, long len, rb_encoding *enc); #endif +/* thread.c (export) */ +int ruby_thread_has_gvl_p(void); /* for ext/fiddle/closure.c */ + /* util.c (export) */ extern const signed char ruby_digit36_to_number_table[]; extern const char ruby_hexdigits[]; Index: test/fiddle/test_function.rb =================================================================== --- test/fiddle/test_function.rb (revision 52722) +++ test/fiddle/test_function.rb (revision 52723) @@ -73,6 +73,25 @@ module Fiddle https://github.com/ruby/ruby/blob/trunk/test/fiddle/test_function.rb#L73 assert_equal("123", str.to_s) end + def test_nogvl_poll + begin + poll = @libc['poll'] + rescue Fiddle::DLError + skip 'poll(2) not available' + end + f = Function.new(poll, [TYPE_VOIDP, TYPE_INT, TYPE_INT], TYPE_INT) + + msec = 200 + t0 = Process.clock_gettime(Process::CLOCK_MONOTONIC, :millisecond) + th = Thread.new { f.call(nil, 0, msec) } + n1 = f.call(nil, 0, msec) + n2 = th.value + t1 = Process.clock_gettime(Process::CLOCK_MONOTONIC, :millisecond) + assert_in_delta(msec, t1 - t0, 100, 'slept correct amount of time') + assert_equal(0, n1, 'poll(2) called correctly main-thread') + assert_equal(0, n2, 'poll(2) called correctly in sub-thread') + end + def test_no_memory_leak prep = 'r = Fiddle::Function.new(Fiddle.dlopen(nil)["rb_obj_tainted"], [Fiddle::TYPE_UINTPTR_T], Fiddle::TYPE_UINTPTR_T); a = "a"' code = 'begin r.call(a); rescue TypeError; end' -- ML: ruby-changes@q... Info: http://www.atdot.net/~ko1/quickml/