From 0f1073ca088f01a40e5c8c3f030760177d82a278 Mon Sep 17 00:00:00 2001 From: normal Date: Mon, 23 Nov 2015 21:20:56 +0000 Subject: 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 git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/trunk@52723 b2dd03c8-39d4-4d8f-98ff-823fe69b080e --- ext/fiddle/closure.c | 79 +++++++++++++++++++++++++++++++++++---------------- ext/fiddle/depend | 4 ++- ext/fiddle/extconf.rb | 1 + ext/fiddle/function.c | 36 +++++++++++++++++------ 4 files changed, 85 insertions(+), 35 deletions(-) (limited to 'ext/fiddle') diff --git a/ext/fiddle/closure.c b/ext/fiddle/closure.c index 5d8a3d200f..2263b410e8 100644 --- a/ext/fiddle/closure.c +++ b/ext/fiddle/closure.c @@ -1,4 +1,6 @@ #include +#include +#include "internal.h" /* rb_thread_has_gvl_p */ VALUE cFiddleClosure; @@ -55,10 +57,19 @@ const rb_data_type_t closure_data_type = { {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 **args, void *ctx) 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 **args, void *ctx) 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 diff --git a/ext/fiddle/depend b/ext/fiddle/depend index 0052787934..bf264a18eb 100644 --- a/ext/fiddle/depend +++ b/ext/fiddle/depend @@ -9,7 +9,9 @@ CONFIGURE_LIBFFI = \ $(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) diff --git a/ext/fiddle/extconf.rb b/ext/fiddle/extconf.rb index 5cb638611e..c264a33429 100644 --- a/ext/fiddle/extconf.rb +++ b/ext/fiddle/extconf.rb @@ -143,6 +143,7 @@ if libffi $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" diff --git a/ext/fiddle/function.c b/ext/fiddle/function.c index 209c03f71e..164e5f3df6 100644 --- a/ext/fiddle/function.c +++ b/ext/fiddle/function.c @@ -1,4 +1,5 @@ #include +#include #ifdef PRIsVALUE # define RB_OBJ_CLASSNAME(obj) rb_obj_class(obj) @@ -128,13 +129,28 @@ initialize(int argc, VALUE argv[], VALUE self) 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[], VALUE self) 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[], VALUE self) 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[], VALUE self) } 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[], VALUE self) ALLOCV_END(alloc_buffer); - return GENERIC2VALUE(rb_iv_get(self, "@return_type"), retval); + return GENERIC2VALUE(rb_iv_get(self, "@return_type"), args.retval); } void -- cgit v1.2.3