Merge pull request #7732 from headius/more_indy_testing

headius · web-flow · commit 95c5be48f23b · 2023-03-24T13:38:21.000-05:00
Add more testing for invokedynamic modes
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -15,7 +15,7 @@ jobs:
 
     strategy:
       matrix:
-        target: ['test:mri:core:jit', 'spec:ruby:fast', 'spec:ruby:fast:jit', 'spec:ji', 'spec:ffi']
+        target: ['test:jruby:int', 'spec:ruby:fast', 'spec:ji', 'spec:ffi']
         java-version: ['8', '17']
       fail-fast: false
 
@@ -52,7 +52,7 @@ jobs:
 
     strategy:
       matrix:
-        target: ['test:mri:core:int', 'test:mri:extra', 'test:jruby:int', 'test:mri:stdlib', 'spec:ruby:slow', 'spec:ruby:debug', 'test:jruby:aot', 'test:slow_suites', 'spec:compiler', 'spec:regression', 'spec:jruby', 'spec:jrubyc', 'spec:profiler']
+        target: ['test:mri:core:jit', 'test:mri:extra', 'spec:ruby:fast:jit', 'test:mri:stdlib', 'spec:ruby:slow', 'spec:ruby:debug', 'test:jruby:aot', 'test:slow_suites', 'spec:compiler', 'spec:regression', 'spec:jruby', 'spec:jrubyc', 'spec:profiler']
       fail-fast: false
 
     name: rake ${{ matrix.target }} (Java 8)
@@ -118,12 +118,12 @@ jobs:
       - name: rake test:jruby
         run: bin/jruby --dev -S rake test:jruby
 
-  rake-test-indy:
+  rake-test-17-indy:
     runs-on: ubuntu-latest
 
     strategy:
       matrix:
-        target: ['test:jruby:jit', 'spec:compiler', 'spec:ruby:fast:jit']
+        target: ['test:mri:core:jit', 'test:jruby:jit', 'spec:compiler', 'spec:ruby:fast:jit']
         java-version: ['17']
       fail-fast: false
 
@@ -165,7 +165,7 @@ jobs:
       matrix:
         package-flags: ['-Pmain', '-Pdist', '-Pcomplete', '-Posgi', '-Ptest', '-Pmain,test -Dinvoker.test=extended']
         # dist, complete, and osgi do not pass on 17 yet
-        java-version: ['8', '11']
+        java-version: ['11']
       fail-fast: false
 
     name: mvn ${{ matrix.package-flags }} (Java ${{ matrix.java-version }})
@@ -233,7 +233,7 @@ jobs:
         env:
           PHASE: 'package ${{ matrix.package-flags }}'
 
-  ji-specs-indy:
+  ji-specs-8-indy:
     runs-on: ubuntu-latest
 
     strategy:
@@ -555,7 +555,7 @@ jobs:
   maven-test-openj9-8:
     runs-on: ubuntu-latest
 
-    name: mvn -Ptest (OpenJ9 Java 8)
+    name: mvn -Ptest (OpenJ9 Java 8; disabled)
 
     steps:
       - name: checkout
@@ -624,7 +624,7 @@ jobs:
     permissions:
       contents: none
     if: ${{ github.ref == 'refs/heads/master' || github.ref == 'refs/heads/jruby-9.3' }}
-    needs: [mvn-test, mvn-test-8, mvn-test-windows, dependency-check, rake-test, rake-test-indy, rake-test-8, test-versions, sequel, concurrent-ruby, jruby-tests-dev, ji-specs-indy, regression-specs-jit, mvn-test-m1]
+    needs: [mvn-test, mvn-test-8, mvn-test-windows, dependency-check, rake-test, rake-test-17-indy, rake-test-8, test-versions, sequel, concurrent-ruby, jruby-tests-dev, ji-specs-8-indy, regression-specs-jit, mvn-test-m1]
     uses: jruby/jruby/.github/workflows/snapshot-publish.yml@6cd0d4d96d9406635183d81cf91acc82cd78245f
     secrets:
       SONATYPE_USERNAME: ${{ secrets.SONATYPE_USERNAME }}
diff --git a/core/src/main/java/org/jruby/internal/runtime/methods/HandleMethod.java b/core/src/main/java/org/jruby/internal/runtime/methods/HandleMethod.java
@@ -29,8 +29,10 @@
 package org.jruby.internal.runtime.methods;
 
 import java.lang.invoke.MethodHandle;
-import java.util.concurrent.Callable;
+import java.lang.invoke.MethodHandles;
+import java.util.function.Supplier;
 
+import com.headius.invokebinder.SmartBinder;
 import org.jruby.RubyModule;
 import org.jruby.runtime.Arity;
 import org.jruby.runtime.Block;
@@ -40,6 +42,7 @@
 import org.jruby.runtime.Visibility;
 import org.jruby.runtime.builtin.IRubyObject;
 
+import static org.jruby.runtime.Helpers.arrayOf;
 import static org.jruby.util.StringSupport.EMPTY_STRING_ARRAY;
 import static org.jruby.util.StringSupport.split;
 
@@ -56,11 +59,11 @@
  * @author headius
  */
 public class HandleMethod extends DynamicMethod implements MethodArgs2, Cloneable {
-    private Callable<MethodHandle> maker0;
-    private Callable<MethodHandle> maker1;
-    private Callable<MethodHandle> maker2;
-    private Callable<MethodHandle> maker3;
-    private Callable<MethodHandle> maker4;
+    private Supplier<MethodHandle> maker0;
+    private Supplier<MethodHandle> maker1;
+    private Supplier<MethodHandle> maker2;
+    private Supplier<MethodHandle> maker3;
+    private Supplier<MethodHandle> maker4;
     private MethodHandle target0;
     private MethodHandle target1;
     private MethodHandle target2;
@@ -74,6 +77,16 @@ public class HandleMethod extends DynamicMethod implements MethodArgs2, Cloneabl
     private final boolean builtin;
     private final boolean notImplemented;
 
+    private static final com.headius.invokebinder.Signature ARITY_0 =
+            com.headius.invokebinder.Signature.from(
+                    IRubyObject.class,
+                    arrayOf(ThreadContext.class, IRubyObject.class, RubyModule.class, String.class, Block.class),
+                    "context", "self", "selfType", "name", "block");
+    private static final com.headius.invokebinder.Signature ARITY_1 = ARITY_0.insertArg(4, "arg0", IRubyObject.class);
+    private static final com.headius.invokebinder.Signature ARITY_2 = ARITY_1.insertArg(5, "arg1", IRubyObject.class);
+    private static final com.headius.invokebinder.Signature ARITY_3 = ARITY_2.insertArg(6, "arg2", IRubyObject.class);
+    private static final com.headius.invokebinder.Signature[] ARITIES = {ARITY_0, ARITY_1, ARITY_2, ARITY_3};
+
     public HandleMethod(
             RubyModule implementationClass,
             Visibility visibility,
@@ -84,11 +97,11 @@ public HandleMethod(
             String parameterDesc,
             final int min,
             final int max,
-            final Callable<MethodHandle> maker0,
-            final Callable<MethodHandle> maker1,
-            final Callable<MethodHandle> maker2,
-            final Callable<MethodHandle> maker3,
-            final Callable<MethodHandle> maker4) {
+            final Supplier<MethodHandle> maker0,
+            final Supplier<MethodHandle> maker1,
+            final Supplier<MethodHandle> maker2,
+            final Supplier<MethodHandle> maker3,
+            final Supplier<MethodHandle> maker4) {
 
         super(implementationClass, visibility, name);
         this.signature = Signature.decode(encodedSignature);
@@ -130,74 +143,118 @@ public boolean isNative() {
     }
 
     private MethodHandle ensureTarget0() {
+        MethodHandle target0;
         if (!initialized0) {
-            this.target0 = safeCall(maker0);
+            Supplier<MethodHandle> maker0 = this.maker0;
+            if (maker0 == null) {
+                target0 = adaptSpecificToVarargs(ensureTarget4(), 0);
+            } else {
+                target0 = maker0.get();
+            }
+            this.target0 = target0;
+            this.maker0 = null;
             initialized0 = true;
-            maker0 = null;
+        } else {
+            target0 = this.target0;
         }
-        return this.target0;
+        return target0;
     }
 
     private MethodHandle ensureTarget1() {
+        MethodHandle target1;
         if (!initialized1) {
-            this.target1 = safeCall(maker1);
+            Supplier<MethodHandle> maker1 = this.maker1;
+            if (maker1 == null) {
+                target1 = adaptSpecificToVarargs(ensureTarget4(), 1);
+            } else {
+                target1 = maker1.get();
+            }
+            this.target1 = target1;
+            this.maker1 = null;
             initialized1 = true;
-            maker1 = null;
+        } else {
+            target1 = this.target1;
         }
-        return this.target1;
+        return target1;
     }
 
     private MethodHandle ensureTarget2() {
+        MethodHandle target2;
         if (!initialized2) {
-            this.target2 = safeCall(maker2);
+            Supplier<MethodHandle> maker2 = this.maker2;
+            if (maker2 == null) {
+                target2 = adaptSpecificToVarargs(ensureTarget4(), 2);
+            } else {
+                target2 = maker2.get();
+            }
+            this.target2 = target2;
+            this.maker2 = null;
             initialized2 = true;
-            maker2 = null;
+        } else {
+            target2 = this.target2;
         }
-        return this.target2;
+        return target2;
     }
 
     private MethodHandle ensureTarget3() {
+        MethodHandle target3;
         if (!initialized3) {
-            this.target3 = safeCall(maker3);
+            Supplier<MethodHandle> maker3 = this.maker3;
+            if (maker3 == null) {
+                target3 = adaptSpecificToVarargs(ensureTarget4(), 3);
+            } else {
+                target3 = maker3.get();
+            }
+            this.target3 = target3;
+            this.maker3 = null;
             initialized3 = true;
-            maker3 = null;
+        } else {
+            target3 = this.target3;
         }
-        return this.target3;
+        return target3;
     }
 
     private MethodHandle ensureTarget4() {
+        MethodHandle target4;
         if (!initialized4) {
-            this.target4 = safeCall(maker4);
+            Supplier<MethodHandle> maker4 = this.maker4;
+            if (maker4 == null) {
+                target4 = null;
+            } else {
+                target4 = maker4.get();
+            }
+            this.target4 = target4;
             initialized4 = true;
-            maker4 = null;
+            this.maker4 = null;
         }
         return this.target4;
     }
 
-    private static MethodHandle safeCall(Callable<MethodHandle> maker) {
-        try {
-            if (maker == null) return null;
-            return maker.call();
-        } catch (Exception e) {
-            Helpers.throwException(e);
-            return null;
+    private MethodHandle adaptSpecificToVarargs(MethodHandle varargs, int arity) {
+        if (arity == 0) {
+            return MethodHandles.insertArguments(varargs, 4, new Object[] {IRubyObject.NULL_ARRAY});
         }
+
+        return SmartBinder.from(ARITIES[arity])
+                .permute("context", "self", "type", "name", "block", "arg.*")
+                .collect("args", "arg.*")
+                .permute("context", "self", "type", "name", "args", "block")
+                .invoke(varargs).handle();
     }
 
     @Override
     public IRubyObject call(ThreadContext context, IRubyObject self, RubyModule clazz, String name, IRubyObject[] args, Block block) {
         try {
             MethodHandle target4 = ensureTarget4();
             if (target4 != null) {
-                Arity.checkArgumentCount(context, args.length, min, max);
                 return (IRubyObject) target4.invokeExact(context, self, clazz, name, args, block);
             } else {
                 int arity = Arity.checkArgumentCount(context, args.length, min, max);
                 switch (args.length) {
-                    case 0: return (IRubyObject) ensureTarget0().invokeExact(context, self, clazz, name, block);
-                    case 1: return (IRubyObject) ensureTarget1().invokeExact(context, self, clazz, name, args[0], block);
-                    case 2: return (IRubyObject) ensureTarget2().invokeExact(context, self, clazz, name, args[0], args[1], block);
-                    case 3: return (IRubyObject) ensureTarget3().invokeExact(context, self, clazz, name, args[0], args[1], args[2], block);
+                    case 0: return call(context, self, clazz, name, block);
+                    case 1: return call(context, self, clazz, name, args[0], block);
+                    case 2: return call(context, self, clazz, name, args[0], args[1], block);
+                    case 3: return call(context, self, clazz, name, args[0], args[1], args[2], block);
                     default:
                         throw new RuntimeException("invalid arity for call: " + arity);
                 }
diff --git a/core/src/main/java/org/jruby/internal/runtime/methods/InvokeDynamicMethodFactory.java b/core/src/main/java/org/jruby/internal/runtime/methods/InvokeDynamicMethodFactory.java
@@ -44,7 +44,7 @@
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
-import java.util.concurrent.Callable;
+import java.util.function.Supplier;
 
 import org.jruby.Ruby;
 import org.jruby.anno.JavaMethodDescriptor;
@@ -116,7 +116,7 @@ public DynamicMethod getAnnotatedMethod(final RubyModule implementationClass, fi
             notImplemented = notImplemented || desc.anno.notImplemented();
         }
 
-        Callable<MethodHandle>[] generators = buildAnnotatedMethodHandles(implementationClass.getRuntime(), descs, implementationClass);
+        Supplier<MethodHandle>[] generators = buildAnnotatedMethodHandles(implementationClass.getRuntime(), descs, implementationClass);
 
         return new HandleMethod(
                 implementationClass,
@@ -137,14 +137,14 @@ public DynamicMethod getAnnotatedMethod(final RubyModule implementationClass, fi
                 generators[4]);
     }
 
-    private Callable<MethodHandle>[] buildAnnotatedMethodHandles(Ruby runtime, List<JavaMethodDescriptor> descs, RubyModule implementationClass) {
+    private Supplier<MethodHandle>[] buildAnnotatedMethodHandles(Ruby runtime, List<JavaMethodDescriptor> descs, RubyModule implementationClass) {
         int min = Integer.MAX_VALUE;
         int max = 0;
 
         // FIXME: Using desc.anno.name()[0] for super may super up the wrong name
         final String rubyName = descs.get(0).rubyName;
 
-        Callable<MethodHandle>[] targets = new Callable[5];
+        Supplier<MethodHandle>[] targets = new Supplier[5];
 
         for (JavaMethodDescriptor desc: descs) {
             MethodHandle method;
@@ -155,7 +155,7 @@ private Callable<MethodHandle>[] buildAnnotatedMethodHandles(Ruby runtime, List<
                 method = Binder.from(desc.returnClass, desc.declaringClass, desc.parameters).invokeVirtualQuiet(LOOKUP, desc.name);
             }
 
-            Callable<MethodHandle> target = adaptHandle(method, runtime, desc.actualRequired, desc.required, desc.optional, desc.rest, rubyName, desc.declaringClass, desc.isStatic, desc.hasContext, desc.hasBlock, desc.anno.frame(), implementationClass);
+            Supplier<MethodHandle> target = adaptHandle(method, runtime, desc.actualRequired, desc.required, desc.optional, desc.rest, rubyName, desc.declaringClass, desc.isStatic, desc.hasContext, desc.hasBlock, desc.anno.frame(), implementationClass);
             int specificArity = -1;
             if (desc.required < 4 && desc.optional == 0 && !desc.rest) {
                 if (desc.required == 0) {
@@ -284,27 +284,22 @@ public static MethodHandle finishAdapting(final SmartBinder binder, final RubyMo
         return target;
     }
 
-    public static Callable<MethodHandle> adaptHandle(final MethodHandle method, final Ruby runtime, final int actualRequired, final int required, final int optional, final boolean rest, final String rubyName, final Class declaringClass, final boolean isStatic, final boolean hasContext, final boolean hasBlock, final boolean frame, final RubyModule implementationClass) {
-        return new Callable<MethodHandle>() {
-            @Override
-            public MethodHandle call() throws Exception {
-                //Class returnClass = method.type().returnType();
-
-                int specificArity = -1;
-                if (optional == 0 && !rest) {
-                    if (required == 0) {
-                        if (actualRequired <= 3) {
-                            specificArity = actualRequired;
-                        }
-                    } else if (required >= 0 && required <= 3) {
-                        specificArity = required;
+    public static Supplier<MethodHandle> adaptHandle(final MethodHandle method, final Ruby runtime, final int actualRequired, final int required, final int optional, final boolean rest, final String rubyName, final Class declaringClass, final boolean isStatic, final boolean hasContext, final boolean hasBlock, final boolean frame, final RubyModule implementationClass) {
+        return () -> {
+            int specificArity = -1;
+            if (optional == 0 && !rest) {
+                if (required == 0) {
+                    if (actualRequired <= 3) {
+                        specificArity = actualRequired;
                     }
+                } else if (required >= 0 && required <= 3) {
+                    specificArity = required;
                 }
+            }
 
-                SmartBinder targetBinder = getBinder(specificArity, isStatic, hasContext, hasBlock);
+            SmartBinder targetBinder = getBinder(specificArity, isStatic, hasContext, hasBlock);
 
-                return finishAdapting(targetBinder, implementationClass, rubyName, method, declaringClass, runtime, isStatic, frame);
-            }
+            return finishAdapting(targetBinder, implementationClass, rubyName, method, declaringClass, runtime, isStatic, frame);
         };
     }
 
diff --git a/core/src/main/java/org/jruby/ir/targets/indy/Bootstrap.java b/core/src/main/java/org/jruby/ir/targets/indy/Bootstrap.java
@@ -1096,11 +1096,11 @@ static MethodHandle buildNativeHandle(InvokeSite site, CacheEntry entry, boolean
                     if (Options.INVOKEDYNAMIC_LOG_BINDING.load()) {
                         LOG.info(site.name() + "\tbound directly to JVM method " + Bootstrap.logMethod(method));
                     }
-                }
 
-                JRubyMethod anno = nativeCall.getMethod().getAnnotation(JRubyMethod.class);
-                if (anno != null && anno.frame()) {
-                    mh = InvocationLinker.wrapWithFrameOnly(site.signature, entry.sourceModule, site.name(), mh);
+                    JRubyMethod anno = nativeCall.getMethod().getAnnotation(JRubyMethod.class);
+                    if (anno != null && anno.frame()) {
+                        mh = InvocationLinker.wrapWithFrameOnly(site.signature, entry.sourceModule, site.name(), mh);
+                    }
                 }
             }
         }
diff --git a/core/src/main/java/org/jruby/ir/targets/indy/InvokeSite.java b/core/src/main/java/org/jruby/ir/targets/indy/InvokeSite.java
@@ -252,7 +252,7 @@ private IRubyObject performIndirectCall(ThreadContext context, IRubyObject self,
         if (literalClosure) {
             try {
                 if (passSymbol) {
-                    return method.call(context, self, sourceModule, methodName, Helpers.arrayOf(context.runtime.newSymbol(methodName), args), block);
+                    return method.call(context, self, sourceModule, "method_missing", Helpers.arrayOf(context.runtime.newSymbol(methodName), args), block);
                 } else {
                     return method.call(context, self, sourceModule, methodName, args, block);
                 }

Original file line number	Diff line number	Diff line change
`@@ -1096,11 +1096,11 @@ static MethodHandle buildNativeHandle(InvokeSite site, CacheEntry entry, boolean`
`1096`	`1096`	`if (Options.INVOKEDYNAMIC_LOG_BINDING.load()) {`
`1097`	`1097`	`LOG.info(site.name() + "\tbound directly to JVM method " + Bootstrap.logMethod(method));`
`1098`	`1098`	`}`
`1099`		`- }`
`1100`	`1099`
`1101`		`- JRubyMethod anno = nativeCall.getMethod().getAnnotation(JRubyMethod.class);`
`1102`		`- if (anno != null && anno.frame()) {`
`1103`		`- mh = InvocationLinker.wrapWithFrameOnly(site.signature, entry.sourceModule, site.name(), mh);`
	`1100`	`+ JRubyMethod anno = nativeCall.getMethod().getAnnotation(JRubyMethod.class);`
	`1101`	`+ if (anno != null && anno.frame()) {`
	`1102`	`+ mh = InvocationLinker.wrapWithFrameOnly(site.signature, entry.sourceModule, site.name(), mh);`
	`1103`	`+ }`
`1104`	`1104`	`}`
`1105`	`1105`	`}`
`1106`	`1106`	`}`
Original file line number	Diff line number	Diff line change
`@@ -252,7 +252,7 @@ private IRubyObject performIndirectCall(ThreadContext context, IRubyObject self,`
`252`	`252`	`if (literalClosure) {`
`253`	`253`	`try {`
`254`	`254`	`if (passSymbol) {`
`255`		`- return method.call(context, self, sourceModule, methodName, Helpers.arrayOf(context.runtime.newSymbol(methodName), args), block);`
	`255`	`+ return method.call(context, self, sourceModule, "method_missing", Helpers.arrayOf(context.runtime.newSymbol(methodName), args), block);`
`256`	`256`	`} else {`
`257`	`257`	`return method.call(context, self, sourceModule, methodName, args, block);`
`258`	`258`	`}`