dumping ground for random patches and texts
 help / color / mirror / Atom feed
* [PATCH] fiddle: release GVL for ffi_call
@ 2015-10-19 21:44 Eric Wong
  0 siblings, 0 replies; 2+ messages in thread
From: Eric Wong @ 2015-10-19 21:44 UTC (permalink / raw)
  To: spew

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/closure.c         | 76 ++++++++++++++++++++++++++++++--------------
 ext/fiddle/depend            |  4 ++-
 ext/fiddle/extconf.rb        |  1 +
 ext/fiddle/function.c        | 36 +++++++++++++++------
 internal.h                   |  3 ++
 test/fiddle/test_function.rb | 19 +++++++++++
 vm_core.h                    |  1 -
 7 files changed, 105 insertions(+), 35 deletions(-)

diff --git a/ext/fiddle/closure.c b/ext/fiddle/closure.c
index ca1b10a..bf12bab 100644
--- a/ext/fiddle/closure.c
+++ b/ext/fiddle/closure.c
@@ -1,4 +1,6 @@
 #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 = {
     {0, dealloc, closure_memsize,},
 };
 
+struct callback_args {
+    ffi_cif *cif;
+    void *resp;
+    void **args;
+    void *ctx;
+};
+
 static void
-callback(ffi_cif *cif, void *resp, void **args, void *ctx)
+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,36 +142,36 @@ 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:
@@ -168,6 +179,23 @@ callback(ffi_cif *cif, void *resp, void **args, void *ctx)
     }
 }
 
+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
 allocate(VALUE klass)
 {
diff --git a/ext/fiddle/depend b/ext/fiddle/depend
index 0052787..bf264a1 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 5cb6386..c264a33 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 e0da8b6..8b08e0f 100644
--- a/ext/fiddle/function.c
+++ b/ext/fiddle/function.c
@@ -1,4 +1,5 @@
 #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 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;
@@ -149,7 +165,7 @@ function_call(int argc, VALUE argv[], VALUE self)
 		argc, RARRAY_LENINT(types));
     }
 
-    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++) {
@@ -162,7 +178,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_PTR(types)[i];
@@ -178,11 +195,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)
@@ -191,7 +209,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
diff --git a/internal.h b/internal.h
index 2322186..43e3597 100644
--- a/internal.h
+++ b/internal.h
@@ -1288,6 +1288,9 @@ VALUE rb_gcd_gmp(VALUE x, VALUE y);
 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[];
diff --git a/test/fiddle/test_function.rb b/test/fiddle/test_function.rb
index de7c958..7da73de 100644
--- a/test/fiddle/test_function.rb
+++ b/test/fiddle/test_function.rb
@@ -73,6 +73,25 @@ module Fiddle
       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'
diff --git a/vm_core.h b/vm_core.h
index 2bdb029..1130d60 100644
--- a/vm_core.h
+++ b/vm_core.h
@@ -1013,7 +1013,6 @@ rb_vm_living_threads_remove(rb_vm_t *vm, rb_thread_t *th)
     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);
-- 
EW


^ permalink raw reply related	[flat|nested] 2+ messages in thread
* [PATCH] fiddle: release GVL for ffi_call
@ 2015-10-21 20:50 Eric Wong
  0 siblings, 0 replies; 2+ messages in thread
From: Eric Wong @ 2015-10-21 20:50 UTC (permalink / raw)
  To: spew

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/closure.c         | 79 ++++++++++++++++++++++++++++++--------------
 ext/fiddle/depend            |  4 ++-
 ext/fiddle/extconf.rb        |  1 +
 ext/fiddle/function.c        | 36 +++++++++++++++-----
 internal.h                   |  3 ++
 test/fiddle/test_function.rb | 19 +++++++++++
 vm_core.h                    |  1 -
 7 files changed, 107 insertions(+), 36 deletions(-)

diff --git a/ext/fiddle/closure.c b/ext/fiddle/closure.c
index ca1b10a..30bf940 100644
--- a/ext/fiddle/closure.c
+++ b/ext/fiddle/closure.c
@@ -1,4 +1,6 @@
 #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 = {
     {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 0052787..bf264a1 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 5cb6386..c264a33 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 e0da8b6..8b08e0f 100644
--- a/ext/fiddle/function.c
+++ b/ext/fiddle/function.c
@@ -1,4 +1,5 @@
 #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 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;
@@ -149,7 +165,7 @@ function_call(int argc, VALUE argv[], VALUE self)
 		argc, RARRAY_LENINT(types));
     }
 
-    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++) {
@@ -162,7 +178,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_PTR(types)[i];
@@ -178,11 +195,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)
@@ -191,7 +209,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
diff --git a/internal.h b/internal.h
index 6941586..e4a6520 100644
--- a/internal.h
+++ b/internal.h
@@ -1291,6 +1291,9 @@ VALUE rb_gcd_gmp(VALUE x, VALUE y);
 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[];
diff --git a/test/fiddle/test_function.rb b/test/fiddle/test_function.rb
index de7c958..7da73de 100644
--- a/test/fiddle/test_function.rb
+++ b/test/fiddle/test_function.rb
@@ -73,6 +73,25 @@ module Fiddle
       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'
diff --git a/vm_core.h b/vm_core.h
index 2bdb029..1130d60 100644
--- a/vm_core.h
+++ b/vm_core.h
@@ -1013,7 +1013,6 @@ rb_vm_living_threads_remove(rb_vm_t *vm, rb_thread_t *th)
     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);
-- 
EW


^ permalink raw reply related	[flat|nested] 2+ messages in thread
end of thread, other threads:[~2015-10-21 20:50 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-10-19 21:44 [PATCH] fiddle: release GVL for ffi_call Eric Wong
  -- strict thread matches above, loose matches on Subject: below --
2015-10-21 20:50 Eric Wong

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).