Always split core methods taking a block

Author: eregon

lib/mri/monitor.rb

@@ -217,6 +217,7 @@ def mon_owned?
   def mon_synchronize(&block)
     Primitive.monitor_synchronize(@mon_mutex, block)
   end
+  Truffle::Graal.always_split instance_method(:mon_synchronize)
   alias synchronize mon_synchronize
 
   #

lib/truffle/truffle/cext.rb

@@ -12,6 +12,8 @@
 require_relative 'cext_constants'
 require_relative 'cext_structs'
 
+# TODO (eregon, 1 Sep 2022): should we always_split or use separate CallTargets for usages of Primitive.call_with* in this file?
+
 module Truffle::CExt
   DATA_TYPE = Primitive.object_hidden_var_create :data_type
   DATA_HOLDER = Primitive.object_hidden_var_create :data_holder
@@ -978,6 +980,7 @@ def rb_hash_foreach(hash, func, farg)
       end
     end
   end
+  Truffle::Graal.always_split instance_method(:rb_hash_foreach)
 
   def rb_path_to_class(path)
     begin
@@ -1046,6 +1049,7 @@ def rb_protect(function, arg, write_status, status)
     Truffle::Interop.execute_without_conversion(write_status, status, pos)
     res
   end
+  Truffle::Graal.always_split instance_method(:rb_protect)
 
   def rb_jump_tag(pos)
     if pos > 0
@@ -1064,10 +1068,12 @@ def rb_jump_tag(pos)
   def rb_yield(value)
     Primitive.interop_execute(rb_block_proc, [value])
   end
+  Truffle::Graal.always_split instance_method(:rb_yield)
 
   def rb_yield_splat(values)
     Primitive.interop_execute(rb_block_proc, values)
   end
+  Truffle::Graal.always_split instance_method(:rb_yield_splat)
 
   def rb_ivar_lookup(object, name, default_value)
     # TODO CS 24-Jul-16 races - needs a new primitive or be defined in Java?
@@ -1396,6 +1402,7 @@ def rb_mutex_synchronize(mutex, func, arg)
       Primitive.cext_unwrap(Primitive.interop_execute(func, [Primitive.cext_wrap(arg)]))
     end
   end
+  Truffle::Graal.always_split instance_method(:rb_mutex_synchronize)
 
   def rb_gc_enable
     GC.enable
@@ -1547,6 +1554,7 @@ def rb_ensure(b_proc, data1, e_proc, data2)
       Primitive.interop_execute(e_proc, [data2])
     end
   end
+  Truffle::Graal.always_split instance_method(:rb_ensure)
 
   def rb_rescue(b_proc, data1, r_proc, data2)
     begin
@@ -1559,6 +1567,7 @@ def rb_rescue(b_proc, data1, r_proc, data2)
       end
     end
   end
+  Truffle::Graal.always_split instance_method(:rb_rescue)
 
   def rb_rescue2(b_proc, data1, r_proc, data2, rescued)
     begin
@@ -1567,6 +1576,7 @@ def rb_rescue2(b_proc, data1, r_proc, data2, rescued)
       Primitive.interop_execute(r_proc, [data2, Primitive.cext_wrap(e)])
     end
   end
+  Truffle::Graal.always_split instance_method(:rb_rescue2)
 
   def rb_exec_recursive(func, obj, arg)
     result = nil
@@ -1581,6 +1591,7 @@ def rb_exec_recursive(func, obj, arg)
       result
     end
   end
+  Truffle::Graal.always_split instance_method(:rb_exec_recursive)
 
   def rb_catch_obj(tag, func, data)
     catch tag do |caught|

src/annotations/java/org/truffleruby/builtins/CoreMethod.java

@@ -51,6 +51,13 @@
      * RubyHash, and the arguments descriptor will tell if it was kwargs or positional, as always. */
     boolean rest() default false;
 
+    /** Passes the optional block as an argument to the node. Also causes {@link Split#DEFAULT} to be resolved to
+     * {@link Split#ALWAYS}, unless {@link #split()} is set to a non-default value, since if a block is passed and the
+     * block is called it is necessary to split for the best performance. Splitting eagerly instead of lazily via the
+     * heuristic avoids an extra copy and works better with {@code RubyRootNode#getParentFrameDescriptor()} when the
+     * possible loop in this core method does many iterations on the first method call as it then preserves the
+     * single-caller chain from the beginning, instead of breaking it until split and executed again for the first and
+     * second call sites. {@link Split#NEVER} should be used if this method never calls the block but just stores it. */
     boolean needsBlock() default false;
 
     /** Try to lower argument <code>i</code> (starting at 0) to an int if its value is a long. The 0 is reserved for
@@ -73,6 +80,6 @@
     /** Use these names in Ruby core methods stubs, ignore argument names in Java specializations. */
     String[] argumentNames() default {};
 
-    Split split() default Split.HEURISTIC;
+    Split split() default Split.DEFAULT;
 
 }

src/annotations/java/org/truffleruby/language/methods/Split.java

@@ -10,6 +10,8 @@
 package org.truffleruby.language.methods;
 
 public enum Split {
+    /** Special value for the CoreMethod annotation, resolved to either HEURISTIC or ALWAYS, based on needsBlock */
+    DEFAULT,
     ALWAYS,
     HEURISTIC,
     /** Disallow splitting for this CallTarget, which avoids making a eager uninitialized copy of the AST. Useful

src/main/java/org/truffleruby/builtins/CoreMethodNodeManager.java

@@ -121,6 +121,16 @@ private RubyClass getSingletonClass(Object object) {
         return SingletonClassNode.getUncached().executeSingletonClass(object);
     }
 
+    private Split effectiveSplit(Split split, boolean needsBlock) {
+        if (context.getOptions().CORE_ALWAYS_CLONE) {
+            return Split.ALWAYS;
+        } else if (split == Split.DEFAULT) {
+            return needsBlock ? Split.ALWAYS : Split.HEURISTIC;
+        } else {
+            return split;
+        }
+    }
+
     private void addCoreMethod(RubyModule module, MethodDetails methodDetails) {
         final CoreMethod annotation = methodDetails.getMethodAnnotation();
 
@@ -134,7 +144,7 @@ private void addCoreMethod(RubyModule module, MethodDetails methodDetails) {
         final NodeFactory<? extends RubyBaseNode> nodeFactory = methodDetails.getNodeFactory();
         final boolean onSingleton = annotation.onSingleton() || annotation.constructor();
         final boolean isModuleFunc = annotation.isModuleFunction();
-        final Split split = context.getOptions().CORE_ALWAYS_CLONE ? Split.ALWAYS : annotation.split();
+        final Split split = effectiveSplit(annotation.split(), annotation.needsBlock());
 
         final Function<SharedMethodInfo, RootCallTarget> callTargetFactory = sharedMethodInfo -> {
             return createCoreMethodRootNode(nodeFactory, language, sharedMethodInfo, split, annotation);
@@ -164,6 +174,7 @@ public void addLazyCoreMethod(
             int required,
             int optional,
             boolean rest,
+            boolean needsBlock,
             String... names) {
 
         final RubyModule module = getModule(moduleName, isClass);
@@ -172,7 +183,7 @@ public void addLazyCoreMethod(
         if (alwaysInlined) {
             finalSplit = Split.NEVER;
         } else {
-            finalSplit = context.getOptions().CORE_ALWAYS_CLONE ? Split.ALWAYS : split;
+            finalSplit = effectiveSplit(split, needsBlock);
         }
 
         final Function<SharedMethodInfo, RootCallTarget> callTargetFactory = sharedMethodInfo -> {
@@ -329,7 +340,7 @@ public RootCallTarget createCoreMethodRootNode(NodeFactory<? extends RubyBaseNod
 
             if (method.lowerFixnum().length > 0 || method.raiseIfFrozenSelf() || method.raiseIfNotMutableSelf() ||
                     method.returnsEnumeratorIfNoBlock() || !method.enumeratorSize().isEmpty() ||
-                    method.split() != Split.HEURISTIC) {
+                    method.split() != Split.DEFAULT) {
                 throw new Error("Always-inlined methods do not support all @CoreMethod attributes for " + nodeClass);
             }
 

src/main/java/org/truffleruby/core/array/ArrayNodes.java

@@ -662,7 +662,6 @@ protected RubyArray concatManyGeneral(RubyArray array, Object first, Object[] re
 
     @CoreMethod(names = "delete", required = 1, needsBlock = true)
     @ImportStatic(ArrayGuards.class)
-    @ReportPolymorphism
     public abstract static class DeleteNode extends YieldingCoreMethodNode {
 
         @Child private SameOrEqualNode sameOrEqualNode = SameOrEqualNode.create();
@@ -1005,7 +1004,6 @@ protected Object eqlNotArray(RubyArray a, Object b) {
     }
 
     @CoreMethod(names = "fill", rest = true, needsBlock = true, raiseIfFrozenSelf = true)
-    @ReportPolymorphism
     public abstract static class FillNode extends ArrayCoreMethodNode {
 
         @Specialization(
@@ -1331,21 +1329,18 @@ protected RubyArray initializeCopy(RubyArray self, RubyArray from,
 
     @Primitive(name = "array_inject")
     @ImportStatic(ArrayGuards.class)
-    @ReportPolymorphism
     public abstract static class InjectNode extends YieldingCoreMethodNode {
 
         @Child private DispatchNode dispatch = DispatchNode.create(PUBLIC);
 
         // Uses block
 
         @Specialization(guards = { "isEmptyArray(array)", "wasProvided(initialOrSymbol)" })
-        @ReportPolymorphism.Exclude
         protected Object injectEmptyArray(RubyArray array, Object initialOrSymbol, NotProvided symbol, RubyProc block) {
             return initialOrSymbol;
         }
 
         @Specialization(guards = { "isEmptyArray(array)" })
-        @ReportPolymorphism.Exclude
         protected Object injectEmptyArrayNoInitial(
                 RubyArray array, NotProvided initialOrSymbol, NotProvided symbol, RubyProc block) {
             return nil;
@@ -1484,7 +1479,6 @@ public Object injectSymbolHelper(VirtualFrame frame, RubyArray array, String sym
 
     @CoreMethod(names = { "map", "collect" }, needsBlock = true, enumeratorSize = "size")
     @ImportStatic(ArrayGuards.class)
-    @ReportPolymorphism
     public abstract static class MapNode extends YieldingCoreMethodNode {
 
         @Specialization(limit = "storageStrategyLimit()")
@@ -1757,7 +1751,6 @@ protected RubyArray pushMany(VirtualFrame frame, RubyArray array, Object value,
 
     @CoreMethod(names = "reject", needsBlock = true, enumeratorSize = "size")
     @ImportStatic(ArrayGuards.class)
-    @ReportPolymorphism
     public abstract static class RejectNode extends YieldingCoreMethodNode {
 
         @Specialization(limit = "storageStrategyLimit()")
@@ -1792,7 +1785,6 @@ protected Object reject(RubyArray array, RubyProc block,
 
     @CoreMethod(names = "reject!", needsBlock = true, enumeratorSize = "size", raiseIfFrozenSelf = true)
     @ImportStatic(ArrayGuards.class)
-    @ReportPolymorphism
     public abstract static class RejectInPlaceNode extends YieldingCoreMethodNode {
 
         @Child private BooleanCastNode booleanCastNode = BooleanCastNode.create();
@@ -2048,7 +2040,6 @@ private void reverse(ArrayStoreLibrary stores,
 
     @CoreMethod(names = { "select", "filter" }, needsBlock = true, enumeratorSize = "size")
     @ImportStatic(ArrayGuards.class)
-    @ReportPolymorphism
     public abstract static class SelectNode extends YieldingCoreMethodNode {
 
         @Specialization(limit = "storageStrategyLimit()")
@@ -2159,11 +2150,9 @@ protected int size(RubyArray array,
 
     @CoreMethod(names = "sort", needsBlock = true)
     @ImportStatic(ArrayGuards.class)
-    @ReportPolymorphism
     public abstract static class SortNode extends ArrayCoreMethodNode {
 
         @Specialization(guards = "isEmptyArray(array)")
-        @ReportPolymorphism.Exclude
         protected RubyArray sortEmpty(RubyArray array, Object unusedBlock) {
             return createEmptyArray();
         }

src/main/java/org/truffleruby/core/basicobject/BasicObjectNodes.java

@@ -56,6 +56,7 @@
 import org.truffleruby.language.methods.DeclarationContext.SingletonClassOfSelfDefaultDefinee;
 import org.truffleruby.language.methods.InternalMethod;
 import org.truffleruby.language.methods.LookupMethodNode;
+import org.truffleruby.language.methods.Split;
 import org.truffleruby.language.objects.AllocationTracing;
 import org.truffleruby.language.objects.MetaClassNode;
 import org.truffleruby.language.objects.ObjectIDOperations;
@@ -465,7 +466,8 @@ protected Object instanceExec(Object receiver, Object[] arguments, Nil block) {
 
     }
 
-    @CoreMethod(names = "method_missing", needsBlock = true, rest = true, optional = 1, visibility = Visibility.PRIVATE)
+    @CoreMethod(names = "method_missing", needsBlock = true, rest = true, optional = 1, visibility = Visibility.PRIVATE,
+            split = Split.NEVER)
     public abstract static class MethodMissingNode extends CoreMethodArrayArgumentsNode {
 
         @Specialization

src/main/java/org/truffleruby/core/hash/HashNodes.java

@@ -39,6 +39,7 @@
 import org.truffleruby.language.Visibility;
 import org.truffleruby.language.control.RaiseException;
 import org.truffleruby.language.dispatch.DispatchNode;
+import org.truffleruby.language.methods.Split;
 import org.truffleruby.language.objects.AllocationTracing;
 import org.truffleruby.language.objects.shared.PropagateSharingNode;
 import org.truffleruby.language.yield.CallBlockNode;
@@ -362,7 +363,8 @@ protected boolean isEmpty(RubyHash hash) {
         }
     }
 
-    @CoreMethod(names = "initialize", needsBlock = true, optional = 1, raiseIfFrozenSelf = true)
+    @CoreMethod(names = "initialize", needsBlock = true, optional = 1, raiseIfFrozenSelf = true,
+            split = Split.HEURISTIC)
     @ImportStatic(HashGuards.class)
     public abstract static class InitializeNode extends CoreMethodArrayArgumentsNode {
 

src/main/java/org/truffleruby/core/kernel/KernelNodes.java

@@ -121,6 +121,7 @@
 import org.truffleruby.language.locals.FindDeclarationVariableNodes.FindAndReadDeclarationVariableNode;
 import org.truffleruby.language.methods.GetMethodObjectNode;
 import org.truffleruby.language.methods.InternalMethod;
+import org.truffleruby.language.methods.Split;
 import org.truffleruby.language.objects.AllocationTracing;
 import org.truffleruby.language.objects.CheckIVarNameNode;
 import org.truffleruby.language.objects.IsANode;
@@ -1153,7 +1154,7 @@ protected boolean isATypeError(Object self, Object module) {
 
     }
 
-    @CoreMethod(names = "lambda", isModuleFunction = true, needsBlock = true)
+    @CoreMethod(names = "lambda", isModuleFunction = true, needsBlock = true, split = Split.HEURISTIC)
     public abstract static class LambdaNode extends CoreMethodArrayArgumentsNode {
 
         @Specialization
@@ -1322,7 +1323,7 @@ protected RubyArray privateMethods(Object self, boolean includeAncestors) {
 
     }
 
-    @CoreMethod(names = "proc", isModuleFunction = true, needsBlock = true)
+    @CoreMethod(names = "proc", isModuleFunction = true, needsBlock = true, split = Split.HEURISTIC)
     public abstract static class ProcNode extends CoreMethodArrayArgumentsNode {
 
         @Specialization

src/main/java/org/truffleruby/core/kernel/TruffleKernelNodes.java

@@ -52,6 +52,7 @@
 import org.truffleruby.language.loader.FileLoader;
 import org.truffleruby.language.locals.FindDeclarationVariableNodes;
 import org.truffleruby.language.methods.DeclarationContext;
+import org.truffleruby.language.methods.Split;
 import org.truffleruby.language.threadlocal.SpecialVariableStorage;
 import org.truffleruby.parser.ParserContext;
 import org.truffleruby.parser.RubySource;
@@ -72,7 +73,7 @@
 @CoreModule("Truffle::KernelOperations")
 public abstract class TruffleKernelNodes {
 
-    @CoreMethod(names = "at_exit", onSingleton = true, needsBlock = true, required = 1)
+    @CoreMethod(names = "at_exit", onSingleton = true, needsBlock = true, required = 1, split = Split.NEVER)
     public abstract static class AtExitSystemNode extends CoreMethodArrayArgumentsNode {
 
         @TruffleBoundary