Merge pull request #18913 from uschindler/painless_megamorphic_opto

Remove useless dropArguments in megamorphic cache

modules/lang-painless/src/main/java/org/elasticsearch/painless/DefBootstrap.java

@@ -149,7 +149,7 @@ public final class DefBootstrap {
          * Creates the {@link MethodHandle} for the megamorphic call site
          * using {@link ClassValue} and {@link MethodHandles#exactInvoker(MethodType)}:
          */
-        private MethodHandle createMegamorphicHandle() throws Throwable {
+        private MethodHandle createMegamorphicHandle() {
             final MethodType type = type();
             final ClassValue<MethodHandle> megamorphicCache = new ClassValue<MethodHandle>() {
                 @Override
@@ -163,10 +163,8 @@ public final class DefBootstrap {
                     }
                 }
             };
-            MethodHandle cacheLookup = MEGAMORPHIC_LOOKUP.bindTo(megamorphicCache);
-            cacheLookup = MethodHandles.dropArguments(cacheLookup,
-                    1, type.parameterList().subList(1, type.parameterCount()));
-            return MethodHandles.foldArguments(MethodHandles.exactInvoker(type), cacheLookup);            
+            return MethodHandles.foldArguments(MethodHandles.exactInvoker(type),
+                    MEGAMORPHIC_LOOKUP.bindTo(megamorphicCache));            
         }
 
         /**
@@ -268,7 +266,7 @@ public final class DefBootstrap {
             }
         }
         
-        private MethodHandle lookupGeneric() throws Throwable {
+        private MethodHandle lookupGeneric() {
             if ((flags & OPERATOR_COMPOUND_ASSIGNMENT) != 0) {
                 return DefMath.lookupGenericWithCast(name);
             } else {
@@ -277,8 +275,8 @@ public final class DefBootstrap {
         }
         
         /**
-         * Called when a new type is encountered (or, when we have encountered more than {@code MAX_DEPTH}
-         * types at this call site and given up on caching).
+         * Called when a new type is encountered or if cached type does not match.
+         * In that case we revert to a generic, but slower operator handling.
          */
         @SuppressForbidden(reason = "slow path")
         Object fallback(Object[] args) throws Throwable {

modules/lang-painless/src/test/java/org/elasticsearch/painless/DefBootstrapTests.java

@@ -25,6 +25,7 @@ import java.lang.invoke.MethodHandles;
 import java.lang.invoke.MethodType;
 import java.util.Arrays;
 import java.util.Collections;
+import java.util.HashMap;
 
 import org.elasticsearch.test.ESTestCase;
 
@@ -92,7 +93,7 @@ public class DefBootstrapTests extends ESTestCase {
         assertDepthEquals(site, 5);
     }
     
-    /** test that we really revert to a "generic" method that can handle any receiver types */
+    /** test that we revert to the megamorphic classvalue cache and that it works as expected */
     public void testMegamorphic() throws Throwable {
         DefBootstrap.PIC site = (DefBootstrap.PIC) DefBootstrap.bootstrap(MethodHandles.publicLookup(), 
                                                                           "size", 
@@ -102,6 +103,22 @@ public class DefBootstrapTests extends ESTestCase {
         MethodHandle handle = site.dynamicInvoker();
         assertEquals(2, (int)handle.invokeExact((Object) Arrays.asList("1", "2")));
         assertEquals(1, (int)handle.invokeExact((Object) Collections.singletonMap("a", "b")));
+        assertEquals(3, (int)handle.invokeExact((Object) Arrays.asList("x", "y", "z")));
+        assertEquals(2, (int)handle.invokeExact((Object) Arrays.asList("u", "v")));
+        
+        final HashMap<String,String> map = new HashMap<String,String>();
+        map.put("x", "y");
+        map.put("a", "b");
+        assertEquals(2, (int)handle.invokeExact((Object) map));
+        
+        final IllegalArgumentException iae = expectThrows(IllegalArgumentException.class, () -> {
+            Integer.toString((int)handle.invokeExact(new Object()));
+        });
+        assertEquals("Unable to find dynamic method [size] with [0] arguments for class [java.lang.Object].", iae.getMessage());
+        assertTrue("Does not fail inside ClassValue.computeValue()", Arrays.stream(iae.getStackTrace()).anyMatch(e -> {
+            return e.getMethodName().equals("computeValue") &&
+                   e.getClassName().startsWith("org.elasticsearch.painless.DefBootstrap$PIC$");
+        }));
     }
     
     // test operators with null guards