Limit this binding to arity <= 3

We could support more arities here, but it is difficult to predict whether that might bind methods that should not be bound (e.g. if a multiple-arity method had one common uber method with > 3 args it would be bound, skipping the annotated arities). In order to land this without introducing too much complexity, I'm limiting it to arities <= 3. There may be some overhead from trying and failing to acquire a specific-arity target repeatedly. I will be refactoring the indy binding logic to live close to (or inside) the DynamicMethod types in order to let them slowly cache all arities with any handle adaptations necessary. This will eliminate the extra overhead by failing to find a given arity exactly once.
headius · Dec 19, 2024 · 10c6383 · 10c6383
1 parent 776e811
commit 10c6383
Showing 1 changed file with 21 additions and 17 deletions.
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
@@ -570,34 +570,34 @@ private static DynamicMethod.NativeCall buildExactNativeCall(DynamicMethod.Nativ
         }
 
         // rebuild with requested arity
-        Class[] params;
+        Class[] params = null;
 
         if (nativeCall.isStatic()) {
             if (hasContext) {
                 if (hasBlock) {
                     if (arity == -1) {
                         params = CodegenUtils.params(ThreadContext.class, IRubyObject.class, IRubyObject[].class, Block.class);
-                    } else {
+                    } else if (arity <= 3) {
                         params = CodegenUtils.params(ThreadContext.class, IRubyObject.class, IRubyObject.class, arity, Block.class);
                     }
                 } else {
                     if (arity == -1) {
                         params = CodegenUtils.params(ThreadContext.class, IRubyObject.class, IRubyObject[].class);
-                    } else {
+                    } else if (arity <= 3) {
                         params = CodegenUtils.params(ThreadContext.class, IRubyObject.class, IRubyObject.class, arity);
                     }
                 }
             } else {
                 if (hasBlock) {
                     if (arity == -1) {
                         params = CodegenUtils.params(IRubyObject.class, IRubyObject[].class, Block.class);
-                    } else {
+                    } else if (arity <= 3) {
                         params = CodegenUtils.params(IRubyObject.class, IRubyObject.class, arity, Block.class);
                     }
                 } else {
                     if (arity == -1) {
                         params = CodegenUtils.params(IRubyObject.class, IRubyObject[].class);
-                    } else {
+                    } else if (arity <= 3) {
                         params = CodegenUtils.params(IRubyObject.class, IRubyObject.class, arity);
                     }
                 }
@@ -607,43 +607,47 @@ private static DynamicMethod.NativeCall buildExactNativeCall(DynamicMethod.Nativ
                 if (hasBlock) {
                     if (arity == -1) {
                         params = CodegenUtils.params(ThreadContext.class, IRubyObject[].class, Block.class);
-                    } else {
+                    } else if (arity <= 3) {
                         params = CodegenUtils.params(ThreadContext.class, IRubyObject.class, arity, Block.class);
                     }
                 } else {
                     if (arity == -1) {
                         params = CodegenUtils.params(ThreadContext.class, IRubyObject[].class);
-                    } else {
+                    } else if (arity <= 3) {
                         params = CodegenUtils.params(ThreadContext.class, IRubyObject.class, arity);
                     }
                 }
             } else {
                 if (hasBlock) {
                     if (arity == -1) {
                         params = CodegenUtils.params(IRubyObject[].class, Block.class);
-                    } else {
+                    } else if (arity <= 3) {
                         params = CodegenUtils.params(IRubyObject.class, arity, Block.class);
                     }
                 } else {
                     if (arity == -1) {
                         params = CodegenUtils.params(IRubyObject[].class);
-                    } else {
+                    } else if (arity <= 3) {
                         params = CodegenUtils.params(IRubyObject.class, arity);
                     }
                 }
             }
         }
-        try {
-            if (nativeCall.isStatic()) {
-                lookup().findStatic(nativeCall.getNativeTarget(), nativeCall.getNativeName(), methodType(nativeCall.getNativeReturn(), params));
-            } else {
-                lookup().findVirtual(nativeCall.getNativeTarget(), nativeCall.getNativeName(), methodType(nativeCall.getNativeReturn(), params));
+
+        if (params != null) {
+            try {
+                if (nativeCall.isStatic()) {
+                    lookup().findStatic(nativeCall.getNativeTarget(), nativeCall.getNativeName(), methodType(nativeCall.getNativeReturn(), params));
+                } else {
+                    lookup().findVirtual(nativeCall.getNativeTarget(), nativeCall.getNativeName(), methodType(nativeCall.getNativeReturn(), params));
+                }
+
+                return new DynamicMethod.NativeCall(nativeCall.getNativeTarget(), nativeCall.getNativeName(), nativeCall.getNativeReturn(), params, nativeCall.isStatic(), false);
+            } catch (NoSuchMethodException | IllegalAccessException e) {
             }
-        } catch (NoSuchMethodException | IllegalAccessException e) {
-            return null;
         }
 
-        return new DynamicMethod.NativeCall(nativeCall.getNativeTarget(), nativeCall.getNativeName(), nativeCall.getNativeReturn(), params, nativeCall.isStatic(), false);
+        return null;
     }
 
     private static int getArgCount(Class[] args, boolean isStatic) {