[GR-16199] Refactor C block access for better performance.

PullRequest: truffleruby/870

Author: aardvark179

lib/truffle/truffle/cext.rb

@@ -200,11 +200,11 @@ def to_native
     end
 
     def [](index)
-      Truffle::CExt.rb_tr_wrap(array[index])
+      Truffle.invoke_primitive(:cext_wrap, array[index])
     end
 
     def []=(index, value)
-      array[index] = Truffle::CExt.rb_tr_unwrap(value)
+      array[index] = Truffle.invoke_primitive(:cext_unwrap, value)
     end
 
     def native?
@@ -962,7 +962,7 @@ def rb_funcallv(recv, meth, args)
 
   def rb_funcall(recv, meth, n, *args)
     # see #call_with_thread_locally_stored_block
-    thread_local_block           = Thread.current[:__C_BLOCK__]
+    thread_local_block = Thread.current[:__C_BLOCK__]
     Thread.current[:__C_BLOCK__] = nil
     recv.__send__(meth, *args, &thread_local_block)
   ensure
@@ -1032,7 +1032,7 @@ def rb_hash_set_ifnone(hash, value)
 
   def rb_hash_foreach(hash, func, farg)
     hash.each do |key, value|
-      st_result = Truffle::Interop.execute_without_conversion(func, rb_tr_wrap(key), rb_tr_wrap(value), farg)
+      st_result = Truffle::Interop.execute_without_conversion(func, Truffle.invoke_primitive(:cext_wrap, key), Truffle.invoke_primitive(:cext_wrap, value), farg)
 
       case st_result
       when ST_CONTINUE
@@ -1055,8 +1055,8 @@ def rb_path_to_class(path)
 
   def rb_proc_new(function, value)
     Proc.new do |*args|
-      Truffle::CExt.rb_tr_unwrap(
-        Truffle.invoke_primitive(:call_with_c_mutex, function, args.map! { |arg| Truffle::CExt.rb_tr_wrap(arg) }))
+      Truffle.invoke_primitive(:cext_unwrap,
+        Truffle.invoke_primitive(:call_with_c_mutex, function, args.map! { |arg| Truffle.invoke_primitive( :cext_wrap, arg) }))
     end
   end
 
@@ -1088,7 +1088,7 @@ def rb_protect_with_block(function, arg)
     # function called will do that. In general we try not to touch the
     # values passed in or out of protected functions as C extensions
     # may accept or return arbitrary pointers rather than ruby VALUEs.
-    res = rb_tr_wrap(nil)
+    res = Truffle.invoke_primitive(:cext_wrap, nil)
     pos = 0
     e = capture_exception do
       res = Truffle::Interop.execute_without_conversion(function, arg)
@@ -1271,7 +1271,7 @@ def rb_enumeratorize(obj, meth, args)
 
   def rb_enumeratorize_with_size(obj, meth, args, size_fn)
     return rb_enumeratorize(obj, meth, args) if size_fn.nil?
-    enum = obj.to_enum(meth, *args) { rb_tr_unwrap(Truffle.invoke_primitive(:call_with_c_mutex, size_fn, [rb_tr_wrap(obj), rb_tr_wrap(args), rb_tr_wrap(enum)])) }
+    enum = obj.to_enum(meth, *args) { rb_tr_unwrap(Truffle.invoke_primitive(:call_with_c_mutex, size_fn, [Truffle.invoke_primitive(:cext_wrap, obj), Truffle.invoke_primitive(:cext_wrap, args), Truffle.invoke_primitive(:cext_wrap, enum)])) }
     enum
   end
 
@@ -1281,7 +1281,7 @@ def rb_eval_string(str)
 
   def rb_define_alloc_func(ruby_class, function)
     ruby_class.singleton_class.define_method(:__allocate__) do
-      Truffle::CExt.rb_tr_unwrap(Truffle.invoke_primitive(:call_with_c_mutex, function, [Truffle::CExt.rb_tr_wrap(self)]))
+      Truffle.invoke_primitive( :cext_unwrap, Truffle.invoke_primitive(:call_with_c_mutex, function, [Truffle.invoke_primitive( :cext_wrap, self)]))
     end
     class << ruby_class
       private :__allocate__
@@ -1399,7 +1399,7 @@ def rb_mutex_sleep(mutex, timeout)
 
   def rb_mutex_synchronize(mutex, func, arg)
     mutex.synchronize do
-      rb_tr_unwrap(Truffle.invoke_primitive(:call_with_c_mutex, func, [rb_tr_wrap(arg)]))
+      rb_tr_unwrap(Truffle.invoke_primitive(:call_with_c_mutex, func, [Truffle.invoke_primitive(:cext_wrap, arg)]))
     end
   end
 
@@ -1513,7 +1513,7 @@ def send_splatted(object, method, args)
 
   def rb_block_call(object, method, args, func, data)
     object.__send__(method, *args) do |*block_args|
-      rb_tr_unwrap(Truffle.invoke_primitive(:call_with_c_mutex, func, [rb_tr_wrap(block_args.first), data, block_args.size, RARRAY_PTR(block_args), nil]))
+      rb_tr_unwrap(Truffle.invoke_primitive(:call_with_c_mutex, func, [Truffle.invoke_primitive(:cext_wrap, block_args.first), data, block_args.size, RARRAY_PTR(block_args), nil]))
     end
   end
 
@@ -1529,35 +1529,35 @@ def rb_rescue(b_proc, data1, r_proc, data2)
     begin
       Truffle.invoke_primitive(:call_with_c_mutex, b_proc, [data1])
     rescue StandardError => e
-      Truffle.invoke_primitive(:call_with_c_mutex, r_proc, [data2, rb_tr_wrap(e)])
+      Truffle.invoke_primitive(:call_with_c_mutex, r_proc, [data2, Truffle.invoke_primitive(:cext_wrap, e)])
     end
   end
 
   def rb_rescue2(b_proc, data1, r_proc, data2, rescued)
     begin
       Truffle.invoke_primitive(:call_with_c_mutex, b_proc, [data1])
     rescue *rescued => e
-      Truffle.invoke_primitive(:call_with_c_mutex, r_proc, [data2, rb_tr_wrap(e)])
+      Truffle.invoke_primitive(:call_with_c_mutex, r_proc, [data2, Truffle.invoke_primitive(:cext_wrap, e)])
     end
   end
 
   def rb_exec_recursive(func, obj, arg)
     result = nil
 
     recursive = Thread.detect_recursion(obj) do
-      result = rb_tr_unwrap(Truffle.invoke_primitive(:call_with_c_mutex, func, [rb_tr_wrap(obj), rb_tr_wrap(arg), 0]))
+      result = rb_tr_unwrap(Truffle.invoke_primitive(:call_with_c_mutex, func, [Truffle.invoke_primitive(:cext_wrap, obj), Truffle.invoke_primitive(:cext_wrap, arg), 0]))
     end
 
     if recursive
-      rb_tr_unwrap(Truffle.invoke_primitive(:call_with_c_mutex, func, [rb_tr_wrap(obj), rb_tr_wrap(arg), 1]))
+      rb_tr_unwrap(Truffle.invoke_primitive(:call_with_c_mutex, func, [Truffle.invoke_primitive(:cext_wrap, obj), Truffle.invoke_primitive(:cext_wrap, arg), 1]))
     else
       result
     end
   end
 
   def rb_catch_obj(tag, func, data)
     catch tag do |caught|
-      rb_tr_unwrap(Truffle.invoke_primitive(:call_with_c_mutex, func, [rb_tr_wrap(caught), rb_tr_wrap(data), rb_tr_wrap(nil)]))
+      rb_tr_unwrap(Truffle.invoke_primitive(:call_with_c_mutex, func, [Truffle.invoke_primitive(:cext_wrap, caught), Truffle.invoke_primitive(:cext_wrap, data), Truffle.invoke_primitive(:cext_wrap, nil)]))
     end
   end
 
@@ -1656,7 +1656,7 @@ def rb_iterate(iteration, iterated_object, callback, callback_arg)
       end
     else
       call_with_thread_locally_stored_block iteration, iterated_object do |block_arg|
-        rb_tr_unwrap Truffle.invoke_primitive(:call_with_c_mutex, callback, [rb_tr_wrap(block_arg), rb_tr_wrap(callback_arg), rb_tr_wrap(nil)])
+        rb_tr_unwrap Truffle.invoke_primitive(:call_with_c_mutex, callback, [Truffle.invoke_primitive(:cext_wrap, block_arg), Truffle.invoke_primitive(:cext_wrap, callback_arg), Truffle.invoke_primitive(:cext_wrap, nil)])
       end
     end
   end
@@ -1762,11 +1762,11 @@ def rb_define_hooked_variable(name, gvar, getter, setter)
     id = name.to_sym
 
     getter_proc = -> {
-      rb_tr_unwrap Truffle.invoke_primitive(:call_with_c_mutex, getter, [rb_tr_wrap(id), gvar, rb_tr_wrap(nil)])
+      rb_tr_unwrap Truffle.invoke_primitive(:call_with_c_mutex, getter, [Truffle.invoke_primitive(:cext_wrap, id), gvar, Truffle.invoke_primitive(:cext_wrap, nil)])
     }
 
     setter_proc = -> value {
-      Truffle.invoke_primitive(:call_with_c_mutex, setter, [rb_tr_wrap(value), rb_tr_wrap(id), gvar, rb_tr_wrap(nil)])
+      Truffle.invoke_primitive(:call_with_c_mutex, setter, [Truffle.invoke_primitive(:cext_wrap, value), Truffle.invoke_primitive(:cext_wrap, id), gvar, Truffle.invoke_primitive(:cext_wrap, nil)])
     }
 
     Truffle::KernelOperations.define_hooked_variable id, getter_proc, setter_proc
@@ -1837,4 +1837,12 @@ def rb_sprintf(format, *args)
     sprintf(f, *args) rescue raise ArgumentError, "Bad format string #{f}."
   end
 
+  def rb_tr_wrap(obj)
+    Truffle.invoke_primitive :cext_wrap, obj
+  end
+
+  def rb_tr_unwrap(wrapper)
+    Truffle.invoke_primitive :cext_unwrap, wrapper
+  end
+
 end

lib/truffle/truffle/cext_ruby.rb

@@ -19,30 +19,22 @@ def rb_define_method(mod, name, function, argc)
 
     method_body = Truffle::Graal.copy_captured_locals -> *args, &block do
       if argc == -1 # (int argc, VALUE *argv, VALUE obj)
-        args = [args.size, Truffle::CExt.RARRAY_PTR(args), Truffle::CExt.rb_tr_wrap(self)]
+        args = [args.size, Truffle::CExt.RARRAY_PTR(args), Truffle.invoke_primitive(:cext_wrap, self)]
       elsif argc == -2 # (VALUE obj, VALUE rubyArrayArgs)
-        args = [Truffle::CExt.rb_tr_wrap(self), Truffle::CExt.rb_tr_wrap(args)]
+        args = [Truffle.invoke_primitive(:cext_wrap, self), Truffle.invoke_primitive(:cext_wrap, args)]
       elsif argc >= 0 # (VALUE obj); (VALUE obj, VALUE arg1); (VALUE obj, VALUE arg1, VALUE arg2); ...
         if args.size != argc
           raise ArgumentError, "wrong number of arguments (given #{args.size}, expected #{argc})"
         end
-        args = [Truffle::CExt.rb_tr_wrap(self), *args.map! { |arg| Truffle::CExt.rb_tr_wrap(arg) }]
+        args = [Truffle.invoke_primitive(:cext_wrap, self), *args.map! { |arg| Truffle.invoke_primitive(:cext_wrap, arg) }]
       end
 
       # Using raw execute instead of #call here to avoid argument conversion
-      Truffle::CExt.push_preserving_frame
-      begin
-        # We must call explicitly with the block argument if given
-        # here so that the `rb_block_*` functions will be able to find
-        # it by walking the stack.
-        if block
-          Truffle::CExt.rb_tr_unwrap(Truffle.invoke_primitive(:call_with_c_mutex, function, args, &block))
-        else
-          Truffle::CExt.rb_tr_unwrap(Truffle.invoke_primitive(:call_with_c_mutex, function, args))
-        end
-      ensure
-        Truffle::CExt.pop_preserving_frame
-      end
+
+      # We must set block argument if given here so that the
+      # `rb_block_*` functions will be able to find it by walking the
+      # stack.
+      Truffle.invoke_primitive(:cext_unwrap, Truffle.invoke_primitive(:call_with_c_mutex_and_frame, function, args, block))
     end
 
     mod.define_method(name, method_body)
@@ -51,17 +43,18 @@ def rb_define_method(mod, name, function, argc)
   private
 
   def rb_iterate_call_block(callback, block_arg, callback_arg, &block)
-    rb_tr_unwrap(Truffle.invoke_primitive(:call_with_c_mutex, callback, [rb_tr_wrap(block_arg), rb_tr_wrap(callback_arg)]))
+    rb_tr_unwrap(Truffle.invoke_primitive(:call_with_c_mutex_and_frame, callback, [rb_tr_wrap(block_arg), rb_tr_wrap(callback_arg)], block))
   end
 
   def call_with_thread_locally_stored_block(function, *args, &block)
     # MRI puts the block to a thread local th->passed_block and later rb_funcall reads it,
     # we have to do the same
     # TODO (pitr-ch 14-Dec-2017): This is fixed just for rb_iterate with a rb_funcall in it combination
+
     previous_block = Thread.current[:__C_BLOCK__]
     begin
       Thread.current[:__C_BLOCK__] = block
-      rb_tr_unwrap(Truffle.invoke_primitive(:call_with_c_mutex, function, args.map! { |arg| Truffle::CExt.rb_tr_wrap(arg) }, &block))
+      rb_tr_unwrap(Truffle.invoke_primitive(:call_with_c_mutex_and_frame, function, args.map! { |arg| Truffle.invoke_primitive(:cext_wrap, arg) }, block))
     ensure
       Thread.current[:__C_BLOCK__] = previous_block
     end

src/main/java/org/truffleruby/cext/CExtNodes.java

@@ -44,9 +44,12 @@
 import org.truffleruby.builtins.Primitive;
 import org.truffleruby.builtins.PrimitiveArrayArgumentsNode;
 import org.truffleruby.builtins.YieldingCoreMethodNode;
+import org.truffleruby.cext.CExtNodesFactory.CallCWithMutexNodeFactory;
 import org.truffleruby.cext.CExtNodesFactory.StringToNativeNodeGen;
+import org.truffleruby.cext.CExtNodesFactory.CallCWithMutexNodeFactory.CallCWithMutexNodeGen;
 import org.truffleruby.core.CoreLibrary;
 import org.truffleruby.core.MarkingServiceNodes;
+import org.truffleruby.core.MarkingService.ExtensionCallStack;
 import org.truffleruby.core.array.ArrayHelpers;
 import org.truffleruby.core.array.ArrayOperationNodes;
 import org.truffleruby.core.array.ArrayStrategy;
@@ -116,6 +119,8 @@ public class CExtNodes {
     @Primitive(name = "call_with_c_mutex")
     public abstract static class CallCWithMutexNode extends PrimitiveArrayArgumentsNode {
 
+        public abstract Object execute(TruffleObject receiverm, DynamicObject argsArray);
+
         @Specialization
         public Object callCWithMutex(TruffleObject receiver, DynamicObject argsArray,
                 @Cached("create()") ArrayToObjectArrayNode arrayToObjectArrayNode,
@@ -155,6 +160,26 @@ private Object execute(TruffleObject receiver, Object[] args, Node executeNode,
 
     }
 
+    @ImportStatic(Message.class)
+    @Primitive(name = "call_with_c_mutex_and_frame")
+    public abstract static class CallCWithMuteAndFramexNode extends PrimitiveArrayArgumentsNode {
+
+        @Child protected CallCWithMutexNode callCextNode = CallCWithMutexNodeFactory.create(EMPTY_ARRAY);
+
+        @Specialization
+        public Object callCWithMutex(VirtualFrame frame, TruffleObject receiver, DynamicObject argsArray, DynamicObject block,
+                @Cached("create()") MarkingServiceNodes.GetMarkerThreadLocalDataNode getDataNode) {
+            ExtensionCallStack extensionStack = getDataNode.execute(frame).getExtensionCallStack();
+            extensionStack.push(block);
+
+            try {
+                return callCextNode.execute(receiver, argsArray);
+            } finally {
+                extensionStack.pop();
+            }
+        }
+    }
+
     @ImportStatic(Message.class)
     @Primitive(name = "call_without_c_mutex")
     public abstract static class CallCWithoutMutexNode extends PrimitiveArrayArgumentsNode {
@@ -590,31 +615,11 @@ public DynamicObject rbStrResize(DynamicObject string, int newByteLength,
     @CoreMethod(names = "rb_block_proc", onSingleton = true)
     public abstract static class BlockProcNode extends CoreMethodArrayArgumentsNode {
 
-        // TODO (pitr-ch 04-Dec-2017): needs optimising
-        @TruffleBoundary
         @Specialization
-        public DynamicObject blockProc() {
-            return Truffle.getRuntime().iterateFrames(frameInstance -> {
-                final Node callNode = frameInstance.getCallNode();
-
-                if (callNode != null) {
-                    final RootNode rootNode = callNode.getRootNode();
-                    // Skip Ruby frames in cext.rb file since they are implementing methods which are implemented
-                    // with C in MRI, and therefore are also implicitly skipped when when looking up the block passed
-                    // to a C API function.
-                    if (rootNode instanceof RubyRootNode &&
-                            rootNode.getSourceSection().isAvailable() &&
-                            !rootNode.getSourceSection().getSource().getName().endsWith("truffle/cext.rb")) {
-
-                        final DynamicObject block = RubyArguments.getBlock(frameInstance.getFrame(FrameAccess.READ_ONLY));
-                        return block == null ? nil() : block;
-                    }
-                }
-
-                return null;
-            });
+        public DynamicObject block(VirtualFrame frame,
+                @Cached("create()") MarkingServiceNodes.GetMarkerThreadLocalDataNode getDataNode) {
+            return getDataNode.execute(frame).getExtensionCallStack().getBlock();
         }
-
     }
 
     @CoreMethod(names = "rb_check_frozen", onSingleton = true, required = 1)
@@ -1355,8 +1360,8 @@ public DynamicObject newManagedStruct(Object type) {
 
     }
 
-    @CoreMethod(names = "rb_tr_wrap", onSingleton = true, required = 1)
-    public abstract static class WrapValueNode extends CoreMethodArrayArgumentsNode {
+    @Primitive(name = "cext_wrap", needsSelf = false)
+    public abstract static class WrapValueNode extends PrimitiveArrayArgumentsNode {
 
         @Specialization
         public TruffleObject wrapInt(Object value,
@@ -1369,8 +1374,8 @@ protected WrapNode createWrapNode() {
         }
     }
 
-    @CoreMethod(names = "rb_tr_unwrap", onSingleton = true, required = 1)
-    public abstract static class UnwrapValueNode extends CoreMethodArrayArgumentsNode {
+    @Primitive(name = "cext_unwrap", needsSelf = false)
+    public abstract static class UnwrapValueNode extends PrimitiveArrayArgumentsNode {
 
         @Specialization
         public Object unwrap(TruffleObject value,
@@ -1520,24 +1525,24 @@ protected void addObjectToMarkingService(DynamicObject object, DynamicObject mar
         }
     }
 
-    @CoreMethod(names = "push_preserving_frame", onSingleton = true, required = 0)
+    @CoreMethod(names = "push_extension_call_frame", onSingleton = true, required = 1)
     public abstract static class PushPreservingFrame extends CoreMethodArrayArgumentsNode {
 
         @Specialization
-        public DynamicObject pushFrame(VirtualFrame frame,
+        public DynamicObject pushFrame(VirtualFrame frame, DynamicObject block,
                 @Cached("create()") MarkingServiceNodes.GetMarkerThreadLocalDataNode getDataNode) {
-            getDataNode.execute(frame).getPreservationStack().push();
+            getDataNode.execute(frame).getExtensionCallStack().push(block);
             return nil();
         }
     }
 
-    @CoreMethod(names = "pop_preserving_frame", onSingleton = true, required = 0)
+    @CoreMethod(names = "pop_extension_call_frame", onSingleton = true, required = 0)
     public abstract static class PopPreservingFrame extends CoreMethodArrayArgumentsNode {
 
         @Specialization
         public DynamicObject popFrame(VirtualFrame frame,
                 @Cached("create()") MarkingServiceNodes.GetMarkerThreadLocalDataNode getDataNode) {
-            getDataNode.execute(frame).getPreservationStack().pop();
+            getDataNode.execute(frame).getExtensionCallStack().pop();
             return nil();
         }
     }