GROOVY-11935: Set invokedynamic call site target immediately for static method calls to enable earlier JIT inlining

Author: daniellansun

src/main/java/org/codehaus/groovy/vmplugin/v8/IndyInterface.java

@@ -21,7 +21,6 @@
 import groovy.lang.GroovySystem;
 import org.apache.groovy.util.SystemUtil;
 import org.codehaus.groovy.GroovyBugError;
-import org.codehaus.groovy.reflection.ClassInfo;
 import org.codehaus.groovy.runtime.GeneratedClosure;
 
 import java.lang.invoke.CallSite;
@@ -31,6 +30,7 @@
 import java.lang.invoke.MethodType;
 import java.lang.invoke.MutableCallSite;
 import java.lang.invoke.SwitchPoint;
+import java.lang.reflect.Modifier;
 import java.util.Map;
 import java.util.function.Function;
 import java.util.logging.Level;
@@ -320,48 +320,46 @@ public static Object fromCache(CacheableCallSite callSite, Class<?> sender, Stri
      */
     private static MethodHandle fromCacheHandle(CacheableCallSite callSite, Class<?> sender, String methodName, int callID, Boolean safeNavigation, Boolean thisCall, Boolean spreadCall, Object dummyReceiver, Object[] arguments) throws Throwable {
         FallbackSupplier fallbackSupplier = new FallbackSupplier(callSite, sender, methodName, callID, safeNavigation, thisCall, spreadCall, dummyReceiver, arguments);
+        final Object receiver = arguments[0];
 
-        MethodHandleWrapper mhw;
-        if (bypassCache(spreadCall, arguments)) {
-            mhw = NULL_METHOD_HANDLE_WRAPPER;
-        } else {
-            Object receiver = arguments[0];
-            String receiverClassName = receiver != null ? receiver.getClass().getName() : NULL_OBJECT_CLASS_NAME;
-
-            mhw = callSite.getAndPut(receiverClassName, (theName) -> {
-                MethodHandleWrapper fallback = fallbackSupplier.get();
-                if (fallback.isCanSetTarget()) return fallback;
-                return NULL_METHOD_HANDLE_WRAPPER;
-            });
-        }
+        String receiverClassName = receiver != null ? receiver.getClass().getName() : NULL_OBJECT_CLASS_NAME;
+        MethodHandleWrapper mhw = callSite.getAndPut(receiverClassName, (theName) -> {
+            MethodHandleWrapper fallback = fallbackSupplier.get();
+            if (fallback.isCanSetTarget()) return fallback;
+            return NULL_METHOD_HANDLE_WRAPPER;
+        });
 
         if (mhw == NULL_METHOD_HANDLE_WRAPPER) {
             mhw = fallbackSupplier.get();
         }
 
-        if (mhw.isCanSetTarget() && (callSite.getTarget() != mhw.getTargetMethodHandle()) && (mhw.getLatestHitCount() > INDY_OPTIMIZE_THRESHOLD)) {
-            if (callSite.getFallbackRound().get() > INDY_FALLBACK_CUTOFF) {
-                if (callSite.getTarget() != callSite.getDefaultTarget()) {
-                    // reset the call site target to default forever to avoid JIT deoptimization storm further
-                    callSite.setTarget(callSite.getDefaultTarget());
+        if (mhw.isCanSetTarget() && (callSite.getTarget() != mhw.getTargetMethodHandle())) {
+            // GROOVY-11935: Set invokedynamic call site target immediately for static method calls to enable earlier JIT inlining
+            if (receiver instanceof Class) {
+                var method = mhw.getMethod();
+                if (method != null && Modifier.isStatic(method.getModifiers())) {
+                    callSite.setTarget(mhw.getTargetMethodHandle());
                 }
-            } else {
-                callSite.setTarget(mhw.getTargetMethodHandle());
-                if (LOG_ENABLED) LOG.info("call site target set, preparing outside invocation");
             }
 
-            mhw.resetLatestHitCount();
+            if (mhw.getLatestHitCount() > INDY_OPTIMIZE_THRESHOLD) {
+                if (callSite.getFallbackRound().get() > INDY_FALLBACK_CUTOFF) {
+                    if (callSite.getTarget() != callSite.getDefaultTarget()) {
+                        // reset the call site target to default forever to avoid JIT deoptimization storm further
+                        callSite.setTarget(callSite.getDefaultTarget());
+                    }
+                } else {
+                    callSite.setTarget(mhw.getTargetMethodHandle());
+                    if (LOG_ENABLED) LOG.info("call site target set, preparing outside invocation");
+                }
+
+                mhw.resetLatestHitCount();
+            }
         }
 
         return mhw.getCachedMethodHandle();
     }
 
-    private static boolean bypassCache(Boolean spreadCall, Object[] arguments) {
-        if (spreadCall) return true;
-        Object receiver = arguments[0];
-        return receiver != null && ClassInfo.getClassInfo(receiver.getClass()).hasPerInstanceMetaClasses();
-    }
-
     /**
      * Core method for indy method selection using runtime types.
      * @deprecated Use the new bootHandle-based approach instead.
@@ -391,7 +389,9 @@ private static MethodHandle selectMethodHandle(CacheableCallSite callSite, Class
             // it is important but impacts the performance somehow when cache misses frequently
             Object receiver = arguments[0];
             String key = receiver != null ? receiver.getClass().getName() : NULL_OBJECT_CLASS_NAME;
-            callSite.put(key, mhw);
+
+            // Avoid PIC pollution: don't write back uncached wrappers, e.g. for instance-level metaClass dispatches.
+            callSite.put(key, mhw.isCanSetTarget() ? mhw : NULL_METHOD_HANDLE_WRAPPER);
         }
 
         return mhw.getCachedMethodHandle();
@@ -404,6 +404,7 @@ private static MethodHandleWrapper fallback(CacheableCallSite callSite, Class<?>
         return new MethodHandleWrapper(
                 selector.handle.asSpreader(Object[].class, arguments.length).asType(MethodType.methodType(Object.class, Object[].class)),
                 selector.handle,
+                selector.method,
                 selector.cache
         );
     }

src/main/java/org/codehaus/groovy/vmplugin/v8/MethodHandleWrapper.java

@@ -18,6 +18,8 @@
  */
 package org.codehaus.groovy.vmplugin.v8;
 
+import groovy.lang.MetaMethod;
+
 import java.lang.invoke.MethodHandle;
 import java.util.concurrent.atomic.AtomicLong;
 
@@ -29,12 +31,14 @@
 class MethodHandleWrapper {
     private final MethodHandle cachedMethodHandle;
     private final MethodHandle targetMethodHandle;
+    private final MetaMethod method;
     private final boolean canSetTarget;
     private final AtomicLong latestHitCount = new AtomicLong(0);
 
-    public MethodHandleWrapper(MethodHandle cachedMethodHandle, MethodHandle targetMethodHandle, boolean canSetTarget) {
+    public MethodHandleWrapper(MethodHandle cachedMethodHandle, MethodHandle targetMethodHandle, MetaMethod method, boolean canSetTarget) {
         this.cachedMethodHandle = cachedMethodHandle;
         this.targetMethodHandle = targetMethodHandle;
+        this.method = method;
         this.canSetTarget = canSetTarget;
     }
 
@@ -46,6 +50,10 @@ public MethodHandle getTargetMethodHandle() {
         return targetMethodHandle;
     }
 
+    public MetaMethod getMethod() {
+        return method;
+    }
+
     public boolean isCanSetTarget() {
         return canSetTarget;
     }
@@ -67,10 +75,10 @@ public static MethodHandleWrapper getNullMethodHandleWrapper() {
     }
 
     private static class NullMethodHandleWrapper extends MethodHandleWrapper {
-        public static final NullMethodHandleWrapper INSTANCE = new NullMethodHandleWrapper(null, null, false);
+        public static final NullMethodHandleWrapper INSTANCE = new NullMethodHandleWrapper();
 
-        private NullMethodHandleWrapper(MethodHandle cachedMethodHandle, MethodHandle targetMethodHandle, boolean canSetTarget) {
-            super(cachedMethodHandle, targetMethodHandle, canSetTarget);
+        private NullMethodHandleWrapper() {
+            super(null, null, null, false);
         }
     }
 }