mirror of
				https://github.com/ruby/ruby.git
				synced 2022-11-09 12:17:21 -05:00 
			
		
		
		
	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
This commit is contained in:
		
							parent
							
								
									6965964df6
								
							
						
					
					
						commit
						15476c695d
					
				
					 8 changed files with 124 additions and 36 deletions
				
			
		
							
								
								
									
										17
									
								
								ChangeLog
									
										
									
									
									
								
							
							
						
						
									
										17
									
								
								ChangeLog
									
										
									
									
									
								
							| 
						 | 
				
			
			@ -1,3 +1,20 @@
 | 
			
		|||
Tue Nov 24 05:13:35 2015  Eric Wong  <e@80x24.org>
 | 
			
		||||
 | 
			
		||||
	* 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@gmail.com>
 | 
			
		||||
 | 
			
		||||
	* configure.in: On Solaris, with gcc, "-std=iso9899:1999"
 | 
			
		||||
| 
						 | 
				
			
			
 | 
			
		|||
| 
						 | 
				
			
			@ -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
 | 
			
		||||
| 
						 | 
				
			
			
 | 
			
		|||
| 
						 | 
				
			
			@ -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)
 | 
			
		||||
 | 
			
		||||
| 
						 | 
				
			
			
 | 
			
		|||
| 
						 | 
				
			
			@ -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"
 | 
			
		||||
| 
						 | 
				
			
			
 | 
			
		|||
| 
						 | 
				
			
			@ -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;
 | 
			
		||||
| 
						 | 
				
			
			@ -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
 | 
			
		||||
| 
						 | 
				
			
			
 | 
			
		|||
| 
						 | 
				
			
			@ -1320,6 +1320,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[];
 | 
			
		||||
| 
						 | 
				
			
			
 | 
			
		|||
| 
						 | 
				
			
			@ -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'
 | 
			
		||||
| 
						 | 
				
			
			
 | 
			
		|||
| 
						 | 
				
			
			@ -1036,7 +1036,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);
 | 
			
		||||
| 
						 | 
				
			
			
 | 
			
		|||
		Loading…
	
	Add table
		Add a link
		
	
		Reference in a new issue