[GR-17457] Refactor reference processing and data holders to be more lightweight.

PullRequest: truffleruby/3243

Author: aardvark179

CHANGELOG.md

@@ -22,6 +22,8 @@ Compatibility:
 Performance:
 
 * Reimplement `Float#to_s` for better performance (#1584, @aardvark179).
+* Improve reference processing by making C object free functions and other finalizers more lightweight (@aardvark179).
+* Improve performance of `RSTRING_PTR` for interned strings (@aardvark179).
 
 Changes:
 

lib/truffle/truffle/cext.rb

@@ -183,7 +183,7 @@ def rb_type(value)
 
   def rb_tr_cached_type(value, type)
     if type == T_NONE
-      if Primitive.object_hidden_var_get(value, DATA_HOLDER)
+      if Primitive.data_holder_is_holder?(Primitive.object_hidden_var_get(value, DATA_HOLDER))
         T_DATA
       else
         T_OBJECT
@@ -1432,44 +1432,35 @@ def rb_set_end_proc(func, data)
   def rb_data_object_wrap(ruby_class, data, mark, free)
     ruby_class = Object unless ruby_class
     object = ruby_class.__send__(:__layout_allocate__)
-    data_holder = DataHolder.new(data)
+    data_holder = Primitive.data_holder_create(data)
     Primitive.object_hidden_var_set object, DATA_HOLDER, data_holder
-    ObjectSpace.define_finalizer object, data_finalizer(free, data_holder) unless free.nil?
-    define_marker object, data_marker(mark, data_holder) unless mark.nil?
+
+    Primitive.object_space_define_data_finalizer object, free, data_holder unless Truffle::Interop.null?(free)
+
+    define_marker object, data_marker(mark, data_holder) unless Truffle::Interop.null?(mark)
+
     object
   end
 
   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__)
-    data_holder = DataHolder.new(data)
+    data_holder = Primitive.data_holder_create(data)
     Primitive.object_hidden_var_set object, DATA_TYPE, data_type
     Primitive.object_hidden_var_set object, DATA_HOLDER, data_holder
-    Primitive.object_hidden_var_set object, DATA_MEMSIZER, data_sizer(size, data_holder) unless size.nil?
+    Primitive.object_hidden_var_set object, DATA_MEMSIZER, data_sizer(size, data_holder) unless Truffle::Interop.null?(size)
 
-    ObjectSpace.define_finalizer object, data_finalizer(free, data_holder) unless free.nil?
+    Primitive.object_space_define_data_finalizer object, free, data_holder unless Truffle::Interop.null?(free)
 
-    define_marker object, data_marker(mark, data_holder) unless mark.nil?
+    define_marker object, data_marker(mark, data_holder) unless Truffle::Interop.null?(mark)
     object
   end
 
-  # These data function are created in separate methods to ensure they
-  # will not accidentally capture the objects they operate on, which
-  # might prevent garbage collection.
-  def data_finalizer(free, data_holder)
-    raise unless free.respond_to?(:call)
-    proc {
-      data = data_holder.data
-      # This call does not require a frame as the finalisation processor will push one.
-      Primitive.call_with_c_mutex(free, [data]) unless Truffle::Interop.null?(data)
-    }
-  end
-
   def data_marker(mark, data_holder)
     raise unless mark.respond_to?(:call)
     proc { |obj|
       create_mark_list(obj)
-      data = data_holder.data
+      data = Primitive.data_holder_get_data(data_holder)
       # This call is done without pushing a new frame as the marking service manages frames itself.
       Primitive.call_with_c_mutex(mark, [data]) unless Truffle::Interop.null?(data)
       set_mark_list_on_object(obj)
@@ -1479,7 +1470,7 @@ def data_marker(mark, data_holder)
   def data_sizer(sizer, data_holder)
     raise unless sizer.respond_to?(:call)
     proc {
-      Primitive.call_with_c_mutex_and_frame(sizer, [data_holder.data], Primitive.caller_special_variables_if_available, nil)
+      Primitive.call_with_c_mutex_and_frame(sizer, [Primitive.data_holder_get_data(data_holder)], Primitive.caller_special_variables_if_available, nil)
     }
   end
 

lib/truffle/truffle/cext_structs.rb

@@ -29,7 +29,7 @@ def RDATA_PTR(object)
       raise TypeError, "wrong argument type #{object.class} (expected T_DATA)"
     end
 
-    data_holder.data
+    Primitive.data_holder_get_data(data_holder)
   end
 
   def RBASIC(object)
@@ -48,15 +48,6 @@ def RFILE(file)
   end
 end
 
-# Used by RData.
-class Truffle::CExt::DataHolder
-  attr_accessor :data
-
-  def initialize(data)
-    @data = data
-  end
-end
-
 # ruby.h: `struct RData` and struct `RTypedData`
 class Truffle::CExt::RData
   def initialize(object, data_holder)
@@ -77,7 +68,7 @@ def polyglot_members(internal)
   def polyglot_read_member(name)
     case name
     when 'data'
-      @data_holder.data
+      Primitive.data_holder_get_data(@data_holder)
     when 'type'
       type
     when 'typed_flag'
@@ -91,7 +82,7 @@ def polyglot_read_member(name)
 
   def polyglot_write_member(name, value)
     raise Truffle::Interop::UnknownIdentifierException unless name == 'data'
-    @data_holder.data = value
+    Primitive.data_holder_set_data(@data_holder, value)
   end
 
   def polyglot_remove_member(name)

src/main/java/org/truffleruby/Layouts.java

@@ -24,6 +24,7 @@ public abstract class Layouts {
     public static final HiddenKey OBJECT_LOCK = new HiddenKey("object_lock"); // ReentrantLock
     public static final HiddenKey ASSOCIATED_IDENTIFIER = new HiddenKey("associated"); // Pointer[]
     public static final HiddenKey FINALIZER_REF_IDENTIFIER = new HiddenKey("finalizerRef"); // FinalizerReference
+    public static final HiddenKey DATA_OBJECT_FINALIZER_REF_IDENTIFIER = new HiddenKey("data_object_finalizerRef"); // FinalizerReference
     public static final HiddenKey MARKED_OBJECTS_IDENTIFIER = new HiddenKey("marked_objects"); // Object[]
     public static final HiddenKey VALUE_WRAPPER_IDENTIFIER = new HiddenKey("value_wrapper"); // ValueWrapper
     public static final HiddenKey ALLOCATION_TRACE_IDENTIFIER = new HiddenKey("allocation_trace"); // AllocationTrace

src/main/java/org/truffleruby/RubyContext.java

@@ -38,6 +38,7 @@
 import org.truffleruby.cext.ValueWrapperManager;
 import org.truffleruby.collections.SharedIndicesMap.ContextArray;
 import org.truffleruby.core.CoreLibrary;
+import org.truffleruby.core.DataObjectFinalizationService;
 import org.truffleruby.core.FinalizationService;
 import org.truffleruby.core.Hashing;
 import org.truffleruby.core.MarkingService;
@@ -56,13 +57,11 @@
 import org.truffleruby.core.objectspace.ObjectSpaceManager;
 import org.truffleruby.core.proc.ProcOperations;
 import org.truffleruby.core.proc.RubyProc;
-import org.truffleruby.core.rope.NativeRope;
 import org.truffleruby.core.symbol.RubySymbol;
 import org.truffleruby.core.thread.ThreadManager;
 import org.truffleruby.core.time.GetTimeZoneNode;
 import org.truffleruby.debug.MetricsProfiler;
 import org.truffleruby.language.CallStackManager;
-import org.truffleruby.core.string.ImmutableRubyString;
 import org.truffleruby.language.LexicalScope;
 import org.truffleruby.language.RubyBaseNode;
 import org.truffleruby.language.SafepointManager;
@@ -112,6 +111,7 @@ public class RubyContext {
     private final TraceManager traceManager;
     private final ReferenceProcessor referenceProcessor;
     private final FinalizationService finalizationService;
+    private final DataObjectFinalizationService dataObjectFinalizationService;
     private final MarkingService markingService;
     private final ObjectSpaceManager objectSpaceManager = new ObjectSpaceManager();
     private final SharedObjects sharedObjects = new SharedObjects(this);
@@ -126,7 +126,6 @@ public class RubyContext {
     private final Map<Source, Integer> sourceLineOffsets = Collections.synchronizedMap(new WeakHashMap<>());
     /** (Symbol, refinements) -> Proc for Symbol#to_proc */
     public final Map<Pair<RubySymbol, Map<RubyModule, RubyModule[]>>, RootCallTarget> cachedSymbolToProcTargetsWithRefinements = new ConcurrentHashMap<>();
-    private final Map<ImmutableRubyString, NativeRope> immutableNativeRopes = new ConcurrentHashMap<>();
     /** Default signal handlers for Ruby, only SIGINT and SIGALRM, see {@code core/main.rb} */
     public final ConcurrentMap<String, SignalHandler> defaultRubySignalHandlers = new ConcurrentHashMap<>();
 
@@ -190,6 +189,7 @@ public RubyContext(RubyLanguage language, TruffleLanguage.Env env) {
         referenceProcessor = new ReferenceProcessor(this);
         finalizationService = new FinalizationService(referenceProcessor);
         markingService = new MarkingService(referenceProcessor);
+        dataObjectFinalizationService = new DataObjectFinalizationService(language, referenceProcessor);
 
         // We need to construct this at runtime
         random = createRandomInstance();
@@ -598,6 +598,10 @@ public FinalizationService getFinalizationService() {
         return finalizationService;
     }
 
+    public DataObjectFinalizationService getDataObjectFinalizationService() {
+        return dataObjectFinalizationService;
+    }
+
     public MarkingService getMarkingService() {
         return markingService;
     }
@@ -714,10 +718,6 @@ public Map<Source, Integer> getSourceLineOffsets() {
         return sourceLineOffsets;
     }
 
-    public Map<ImmutableRubyString, NativeRope> getImmutableNativeRopes() {
-        return immutableNativeRopes;
-    }
-
     private static SecureRandom createRandomInstance() {
         try {
             /* We want to use a non-blocking source because this is what MRI does (via /dev/urandom) and it's been found

src/main/java/org/truffleruby/RubyLanguage.java

@@ -11,7 +11,7 @@
 
 import java.io.File;
 import java.io.IOException;
-import java.lang.ref.ReferenceQueue;
+import java.lang.ref.Cleaner;
 import java.util.Arrays;
 import java.util.Objects;
 import java.util.Optional;
@@ -36,7 +36,6 @@
 import org.truffleruby.cext.ValueWrapperManager;
 import org.truffleruby.collections.SharedIndicesMap;
 import org.truffleruby.collections.SharedIndicesMap.LanguageArray;
-import org.truffleruby.core.FinalizationService;
 import org.truffleruby.core.RubyHandle;
 import org.truffleruby.core.array.RubyArray;
 import org.truffleruby.core.basicobject.RubyBasicObject;
@@ -101,6 +100,7 @@
 import org.truffleruby.interop.RubyInnerContext;
 import org.truffleruby.interop.RubySourceLocation;
 import org.truffleruby.language.LexicalScope;
+import org.truffleruby.language.Nil;
 import org.truffleruby.language.RubyDynamicObject;
 import org.truffleruby.language.RubyEvalInteractiveRootNode;
 import org.truffleruby.language.RubyInlineParsingRequestNode;
@@ -182,6 +182,11 @@ public final class RubyLanguage extends TruffleLanguage<RubyContext> {
 
     public static final TruffleLogger LOGGER = TruffleLogger.getLogger(TruffleRuby.LANGUAGE_ID);
 
+    /** This is a truly empty frame descriptor and should only by dummy root nodes which require no variables. Any other
+     * root nodes should should use
+     * {@link TranslatorEnvironment#newFrameDescriptorBuilder(org.truffleruby.parser.ParentFrameDescriptor, boolean)}. */
+    public static final FrameDescriptor EMPTY_FRAME_DESCRIPTOR = new FrameDescriptor(Nil.INSTANCE);
+
     /** We need an extra indirection added to ContextThreadLocal due to multiple Fibers of different Ruby Threads
      * sharing the same Java Thread when using the fiber pool. */
     public static final class ThreadLocalState {
@@ -212,9 +217,9 @@ public static final class ThreadLocalState {
     public final SymbolTable symbolTable;
     public final KeywordArgumentsDescriptorManager keywordArgumentsDescriptorManager = new KeywordArgumentsDescriptorManager();
     public final FrozenStringLiterals frozenStringLiterals;
+    public final Cleaner cleaner = Cleaner.create();
+
 
-    public final ReferenceQueue<Object> sharedReferenceQueue = new ReferenceQueue<>();
-    public final FinalizationService sharedFinzationService = new FinalizationService(sharedReferenceQueue);
     public volatile ValueWrapperManager.HandleBlockWeakReference[] handleBlockSharedMap = new ValueWrapperManager.HandleBlockWeakReference[0];
     public final ValueWrapperManager.HandleBlockAllocator handleBlockAllocator = new ValueWrapperManager.HandleBlockAllocator();
 
@@ -831,5 +836,4 @@ private static String buildCoreLoadPath(String coreLoadPath) {
             throw CompilerDirectives.shouldNotReachHere(e);
         }
     }
-
 }

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

@@ -29,7 +29,6 @@
 import org.truffleruby.cext.CExtNodesFactory.CallWithCExtLockNodeFactory;
 import org.truffleruby.cext.CExtNodesFactory.StringToNativeNodeGen;
 import org.truffleruby.cext.UnwrapNode.UnwrapCArrayNode;
-import org.truffleruby.collections.ConcurrentOperations;
 import org.truffleruby.core.CoreLibrary;
 import org.truffleruby.core.MarkingService.ExtensionCallStack;
 import org.truffleruby.core.MarkingServiceNodes;
@@ -62,7 +61,6 @@
 import org.truffleruby.core.proc.RubyProc;
 import org.truffleruby.core.rope.Bytes;
 import org.truffleruby.core.rope.CodeRange;
-import org.truffleruby.core.rope.LeafRope;
 import org.truffleruby.core.rope.NativeRope;
 import org.truffleruby.core.rope.Rope;
 import org.truffleruby.core.rope.RopeNodes;
@@ -710,7 +708,7 @@ public abstract static class RbStrNewNulNode extends CoreMethodArrayArgumentsNod
         @Specialization
         protected RubyString rbStrNewNul(int byteLength,
                 @Cached StringNodes.MakeStringNode makeStringNode) {
-            final Rope rope = NativeRope.newBuffer(getContext(), byteLength, byteLength);
+            final Rope rope = NativeRope.newBuffer(getLanguage(), byteLength, byteLength);
 
             return makeStringNode.fromRope(rope, Encodings.BINARY);
         }
@@ -768,7 +766,7 @@ protected RubyString rbStrResize(RubyString string, int newByteLength,
                 nativeRope.clearCodeRange();
                 return string;
             } else {
-                final NativeRope newRope = nativeRope.resize(getContext(), newByteLength);
+                final NativeRope newRope = nativeRope.resize(getLanguage(), newByteLength);
 
                 // Like MRI's rb_str_resize()
                 newRope.clearCodeRange();
@@ -790,7 +788,7 @@ protected RubyString trStrCapaResize(RubyString string, int newCapacity,
             if (nativeRope.getCapacity() == newCapacity) {
                 return string;
             } else {
-                final NativeRope newRope = nativeRope.expandCapacity(getContext(), newCapacity);
+                final NativeRope newRope = nativeRope.expandCapacity(getLanguage(), newCapacity);
                 string.setRope(newRope);
                 return string;
             }
@@ -1214,7 +1212,7 @@ protected NativeRope toNative(RubyString string,
                 nativeRope = (NativeRope) currentRope;
             } else {
                 nativeRope = new NativeRope(
-                        getContext(),
+                        getLanguage(),
                         bytesNode.execute(currentRope),
                         currentRope.getEncoding(),
                         characterLengthNode.execute(currentRope),
@@ -1225,18 +1223,9 @@ protected NativeRope toNative(RubyString string,
             return nativeRope;
         }
 
-        @TruffleBoundary
         @Specialization
         protected NativeRope toNativeImmutable(ImmutableRubyString string) {
-            return ConcurrentOperations.getOrCompute(getContext().getImmutableNativeRopes(), string, s -> {
-                final LeafRope currentRope = s.rope;
-                return new NativeRope(
-                        getContext(),
-                        currentRope.getBytes(),
-                        currentRope.getEncoding(),
-                        currentRope.characterLength(),
-                        currentRope.getCodeRange());
-            });
+            return string.getNativeRope(getLanguage());
         }
 
     }
@@ -1283,7 +1272,7 @@ protected boolean isNative(RubyString string) {
         @TruffleBoundary
         @Specialization
         protected boolean isNative(ImmutableRubyString string) {
-            return getContext().getImmutableNativeRopes().containsKey(string);
+            return string.isNative();
         }
 
     }
@@ -1962,4 +1951,41 @@ protected RubyArray zlibGetCRCTable() {
             return createArray(ZLibCRCTable.TABLE.clone());
         }
     }
+
+    @Primitive(name = "data_holder_create")
+    public abstract static class DataHolderCreate extends PrimitiveArrayArgumentsNode {
+
+        @Specialization
+        protected DataHolder create(Object address) {
+            return new DataHolder(address);
+        }
+    }
+
+    @Primitive(name = "data_holder_get_data")
+    public abstract static class DataHolderGetData extends PrimitiveArrayArgumentsNode {
+
+        @Specialization
+        protected Object getData(DataHolder data) {
+            return data.getPointer();
+        }
+    }
+
+    @Primitive(name = "data_holder_set_data")
+    public abstract static class DataHolderSetData extends PrimitiveArrayArgumentsNode {
+
+        @Specialization
+        protected Object setData(DataHolder data, Object address) {
+            data.setPointer(address);
+            return nil;
+        }
+    }
+
+    @Primitive(name = "data_holder_is_holder?")
+    public abstract static class DataHolderIsHolder extends PrimitiveArrayArgumentsNode {
+
+        @Specialization
+        protected boolean setData(Object dataHolder) {
+            return dataHolder instanceof DataHolder;
+        }
+    }
 }