Use Primitive.use_cext_lock? to find out if the C ext lock should be used

Author: andrykonchin

lib/truffle/truffle/cext.rb

@@ -1772,12 +1772,13 @@ def rb_data_object_wrap(ruby_class, data, mark, free)
   def rb_data_typed_object_wrap(ruby_class, data, data_type, mark, free, size)
     ruby_class = Object unless ruby_class
     object = ruby_class.__send__(:__layout_allocate__)
+    use_cext_lock = Primitive.use_cext_lock?
 
     rtypeddata = LIBTRUFFLERUBY.rb_tr_rtypeddata_create(data_type, data)
     Primitive.object_hidden_var_set object, DATA_STRUCT, rtypeddata
     Primitive.object_hidden_var_set object, DATA_MARKER, data_marker(LIBTRUFFLERUBY[:rb_tr_rtypeddata_run_marker], rtypeddata)
     # Could use a simpler finalizer if Truffle::Interop.null?(free)
-    Primitive.objectspace_define_data_finalizer object, LIBTRUFFLERUBY[:rb_tr_rtypeddata_run_finalizer], rtypeddata
+    Primitive.objectspace_define_data_finalizer object, LIBTRUFFLERUBY[:rb_tr_rtypeddata_run_finalizer], rtypeddata, use_cext_lock
 
     unless Truffle::Interop.null?(size)
       Primitive.object_hidden_var_set object, DATA_MEMSIZER, data_sizer(LIBTRUFFLERUBY[:rb_tr_rtypeddata_run_memsizer], rtypeddata)
@@ -1789,10 +1790,8 @@ def rb_data_typed_object_wrap(ruby_class, data, data_type, mark, free, size)
     object
   end
 
-  def run_data_finalizer(function, data)
-    # TODO: Maybe we should not hold the C extension lock here, and instead save the result of Primitive.use_cext_lock?
-    # in DataObjectFinalizerReference from rb_data_object_wrap/rb_data_typed_object_wrap.
-    Primitive.call_with_cext_lock_and_frame POINTER_TO_VOID_WRAPPER, [function, data], nil, nil, CEXT_LOCK
+  def run_data_finalizer(function, data, use_cext_lock)
+    Primitive.call_with_cext_lock_and_frame POINTER_TO_VOID_WRAPPER, [function, data], nil, nil, use_cext_lock
   end
 
   def run_marker(obj)
@@ -1848,6 +1847,8 @@ def send_splatted(object, method, args)
   end
 
   def rb_block_call(object, method, args, func, data)
+    use_cext_lock = Primitive.use_cext_lock?
+
     object.__send__(method, *args) do |*block_args|
       Primitive.cext_unwrap(Primitive.call_with_cext_lock(RB_BLOCK_CALL_FUNC_WRAPPER, [ # Probably need to save the frame here for blocks.
         func,
@@ -1856,7 +1857,7 @@ def rb_block_call(object, method, args, func, data)
         block_args.size, # argc
         Truffle::CExt.RARRAY_PTR(block_args), # argv
         nil, # blockarg
-      ]))
+      ], use_cext_lock))
     end
   end
 
@@ -1933,6 +1934,8 @@ def rb_exec_recursive(func, obj, arg)
   Truffle::Graal.always_split instance_method(:rb_exec_recursive)
 
   def rb_catch_obj(tag, func, data)
+    use_cext_lock = Primitive.use_cext_lock?
+
     catch tag do |caught|
       Primitive.cext_unwrap(Primitive.call_with_cext_lock(RB_BLOCK_CALL_FUNC_WRAPPER, [
         func,
@@ -1941,7 +1944,7 @@ def rb_catch_obj(tag, func, data)
         0, # argc
         nil, # argv
         nil, # blockarg
-      ]))
+      ], use_cext_lock))
     end
   end
 
@@ -2026,7 +2029,8 @@ def rb_thread_create(fn, args)
   end
 
   def rb_thread_call_with_gvl(function, data)
-    Primitive.call_with_cext_lock(POINTER_TO_POINTER_WRAPPER, [function, data])
+    use_cext_lock = Primitive.use_cext_lock?
+    Primitive.call_with_cext_lock(POINTER_TO_POINTER_WRAPPER, [function, data], use_cext_lock)
   end
 
   def rb_thread_call_without_gvl(function, data1, unblock, data2)

lib/truffle/truffle/cext_ruby.rb

@@ -36,11 +36,7 @@ def rb_define_method(mod, name, function, argc)
 
       # We must set block argument if given here so that the
       # `rb_block_*` functions will be able to find it by walking the stack.
-      if use_cext_lock
-        Primitive.call_with_cext_lock_and_frame_and_unwrap(wrapper, args, Primitive.caller_special_variables_if_available, block)
-      else
-        Primitive.call_with_frame_and_unwrap(wrapper, args, Primitive.caller_special_variables_if_available, block)
-      end
+      Primitive.call_with_cext_lock_and_frame_and_unwrap(wrapper, args, Primitive.caller_special_variables_if_available, block, use_cext_lock)
     end
 
     # Even if the argc is -2, the arity number

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

@@ -192,6 +192,7 @@ Object callWithCExtLockAndFrame(
                 @CachedLibrary(limit = "getCacheLimit()") InteropLibrary receivers,
                 @Cached ArrayToObjectArrayNode arrayToObjectArrayNode,
                 @Cached TranslateInteropExceptionNode translateInteropExceptionNode,
+                @Cached InlinedConditionProfile lockCExtProfile,
                 @Cached InlinedConditionProfile ownedProfile,
                 @Cached RunMarkOnExitNode runMarksNode) {
             final ExtensionCallStack extensionStack = getLanguage()
@@ -201,8 +202,10 @@ Object callWithCExtLockAndFrame(
             extensionStack.push(keywordsGiven, specialVariables, block);
             try {
                 final Object[] args = arrayToObjectArrayNode.executeToObjectArray(argsArray);
+                final boolean lockCExt = lockCExtProfile.profile(this,
+                        getContext().getOptions().CEXT_LOCK && useCExtLock);
 
-                if (getContext().getOptions().CEXT_LOCK && useCExtLock) {
+                if (lockCExt) {
                     final ReentrantLock lock = getContext().getCExtensionsLock();
                     boolean owned = ownedProfile.profile(this, lock.isHeldByCurrentThread());
 
@@ -235,47 +238,21 @@ protected int getCacheLimit() {
         }
     }
 
-    @Primitive(name = "call_with_frame_and_unwrap")
-    public abstract static class CallWithFrameAndUnwrapNode extends PrimitiveArrayArgumentsNode {
-
-        @Specialization
-        Object callWithFrame(
-                VirtualFrame frame, Object receiver, RubyArray argsArray, Object specialVariables, Object block,
-                @CachedLibrary(limit = "getCacheLimit()") InteropLibrary receivers,
-                @Cached ArrayToObjectArrayNode arrayToObjectArrayNode,
-                @Cached TranslateInteropExceptionNode translateInteropExceptionNode,
-                @Cached RunMarkOnExitNode runMarksNode,
-                @Cached UnwrapNode unwrapNode) {
-            final ExtensionCallStack extensionStack = getLanguage().getCurrentFiber().extensionCallStack;
-            final boolean keywordsGiven = RubyArguments.getDescriptor(frame) instanceof KeywordArgumentsDescriptor;
-            extensionStack.push(keywordsGiven, specialVariables, block);
-            try {
-                final Object[] args = arrayToObjectArrayNode.executeToObjectArray(argsArray);
-                try {
-                    return unwrapNode.execute(this,
-                            InteropNodes.execute(this, receiver, args, receivers, translateInteropExceptionNode));
-                } finally {
-                    runMarksNode.execute(this, extensionStack);
-                }
-            } finally {
-                extensionStack.pop();
-            }
-        }
-
-        protected int getCacheLimit() {
-            return getLanguage().options.DISPATCH_CACHE;
-        }
-    }
-
     @Primitive(name = "call_with_cext_lock_and_frame_and_unwrap")
     public abstract static class CallWithCExtLockAndFrameAndUnwrapNode extends PrimitiveArrayArgumentsNode {
 
         @Specialization
         Object callWithCExtLockAndFrame(
-                VirtualFrame frame, Object receiver, RubyArray argsArray, Object specialVariables, Object block,
+                VirtualFrame frame,
+                Object receiver,
+                RubyArray argsArray,
+                Object specialVariables,
+                Object block,
+                boolean useCExtLock,
                 @CachedLibrary(limit = "getCacheLimit()") InteropLibrary receivers,
                 @Cached ArrayToObjectArrayNode arrayToObjectArrayNode,
                 @Cached TranslateInteropExceptionNode translateInteropExceptionNode,
+                @Cached InlinedConditionProfile lockCExtProfile,
                 @Cached InlinedConditionProfile ownedProfile,
                 @Cached RunMarkOnExitNode runMarksNode,
                 @Cached UnwrapNode unwrapNode) {
@@ -284,8 +261,10 @@ Object callWithCExtLockAndFrame(
             extensionStack.push(keywordsGiven, specialVariables, block);
             try {
                 final Object[] args = arrayToObjectArrayNode.executeToObjectArray(argsArray);
+                final boolean lockCExt = lockCExtProfile.profile(this,
+                        getContext().getOptions().CEXT_LOCK && useCExtLock);
 
-                if (getContext().getOptions().CEXT_LOCK) {
+                if (lockCExt) {
                     final ReentrantLock lock = getContext().getCExtensionsLock();
                     boolean owned = ownedProfile.profile(this, lock.isHeldByCurrentThread());
 
@@ -323,17 +302,19 @@ protected int getCacheLimit() {
     @Primitive(name = "call_with_cext_lock")
     public abstract static class CallWithCExtLockNode extends PrimitiveArrayArgumentsNode {
 
-        public abstract Object execute(Object receiver, RubyArray argsArray);
+        public abstract Object execute(Object receiver, RubyArray argsArray, boolean useCExtLock);
 
         @Specialization
-        Object callWithCExtLock(Object receiver, RubyArray argsArray,
+        Object callWithCExtLock(Object receiver, RubyArray argsArray, boolean useCExtLock,
                 @CachedLibrary(limit = "getCacheLimit()") InteropLibrary receivers,
                 @Cached ArrayToObjectArrayNode arrayToObjectArrayNode,
                 @Cached TranslateInteropExceptionNode translateInteropExceptionNode,
+                @Cached InlinedConditionProfile lockCExtProfile,
                 @Cached InlinedConditionProfile ownedProfile) {
             Object[] args = arrayToObjectArrayNode.executeToObjectArray(argsArray);
+            final boolean lockCExt = lockCExtProfile.profile(this, getContext().getOptions().CEXT_LOCK && useCExtLock);
 
-            if (getContext().getOptions().CEXT_LOCK) {
+            if (lockCExt) {
                 final ReentrantLock lock = getContext().getCExtensionsLock();
                 boolean owned = ownedProfile.profile(this, lock.isHeldByCurrentThread());
 
@@ -496,15 +477,15 @@ boolean startNewHandleBlock() {
         }
     }
 
-    @CoreMethod(names = "cext_lock_owned?", onSingleton = true)
+    @Primitive(name = "cext_lock_owned?")
     public abstract static class IsCExtLockOwnedPrimitiveNode extends PrimitiveArrayArgumentsNode {
         @Specialization
         boolean isCExtLockOwned() {
             return getContext().getCExtensionsLock().isHeldByCurrentThread();
         }
     }
 
-    @CoreMethod(names = "use_cext_lock?", onSingleton = true)
+    @Primitive(name = "use_cext_lock?")
     public abstract static class UseCExtLockNode extends PrimitiveArrayArgumentsNode {
         @Specialization
         boolean useCExtLock() {

src/main/java/org/truffleruby/core/DataObjectFinalizationService.java

@@ -50,7 +50,7 @@ public Object execute(DataObjectFinalizerReference ref) {
             if (!getContext().isFinalizing()) {
                 try {
                     callNode.invokeMember(getContext().getCoreLibrary().truffleCExtModule, "run_data_finalizer",
-                            ref.finalizerCFunction, ref.dataStruct);
+                            ref.finalizerCFunction, ref.dataStruct, ref.useCExtLock);
                 } catch (InteropException e) {
                     throw CompilerDirectives.shouldNotReachHere(e);
                 }
@@ -73,17 +73,19 @@ public DataObjectFinalizationService(RubyLanguage language, ReferenceProcessor r
 
     @TruffleBoundary
     public DataObjectFinalizerReference addFinalizer(RubyContext context, Object object, Object finalizerCFunction,
-            Object dataStruct) {
-        final DataObjectFinalizerReference newRef = createRef(object, finalizerCFunction, dataStruct);
+            Object dataStruct, boolean useCExtLock) {
+        final DataObjectFinalizerReference newRef = createRef(object, finalizerCFunction, dataStruct, useCExtLock);
 
         add(newRef);
         context.getReferenceProcessor().processReferenceQueue(this);
 
         return newRef;
     }
 
-    public DataObjectFinalizerReference createRef(Object object, Object finalizerCFunction, Object dataStruct) {
-        return new DataObjectFinalizerReference(object, processingQueue, this, finalizerCFunction, dataStruct);
+    public DataObjectFinalizerReference createRef(Object object, Object finalizerCFunction, Object dataStruct,
+            boolean useCExtLock) {
+        return new DataObjectFinalizerReference(object, processingQueue, this, finalizerCFunction, dataStruct,
+                useCExtLock);
     }
 
     @Override

src/main/java/org/truffleruby/core/DataObjectFinalizerReference.java

@@ -19,15 +19,19 @@ public final class DataObjectFinalizerReference
 
     public final Object finalizerCFunction;
     public final Object dataStruct;
+    /** Run a finalizer with or without c-extension lock. It depends on a context where a finalizer was registered. */
+    public final boolean useCExtLock;
 
     DataObjectFinalizerReference(
             Object object,
             ReferenceQueue<? super Object> queue,
             DataObjectFinalizationService service,
             Object finalizerCFunction,
-            Object dataStruct) {
+            Object dataStruct,
+            boolean useCExtLock) {
         super(object, queue, service);
         this.finalizerCFunction = finalizerCFunction;
         this.dataStruct = dataStruct;
+        this.useCExtLock = useCExtLock;
     }
 }

src/main/java/org/truffleruby/core/objectspace/ObjectSpaceNodes.java

@@ -263,11 +263,12 @@ private void defineFinalizer(RubyDynamicObject object, Object finalizer) {
     public abstract static class DefineDataObjectFinalizerNode extends PrimitiveArrayArgumentsNode {
 
         @Specialization
-        Object defineFinalizer(RubyDynamicObject object, Object finalizerCFunction, Object dataStruct,
+        Object defineFinalizer(
+                RubyDynamicObject object, Object finalizerCFunction, Object dataStruct, boolean useCExtLock,
                 @CachedLibrary(limit = "1") DynamicObjectLibrary objectLibrary) {
             DataObjectFinalizerReference newRef = getContext()
                     .getDataObjectFinalizationService()
-                    .addFinalizer(getContext(), object, finalizerCFunction, dataStruct);
+                    .addFinalizer(getContext(), object, finalizerCFunction, dataStruct, useCExtLock);
 
             objectLibrary.put(object, Layouts.DATA_OBJECT_FINALIZER_REF_IDENTIFIER, newRef);