paulk-asert commented on code in PR #2673: URL: https://github.com/apache/groovy/pull/2673#discussion_r3549140460
########## src/main/java/org/codehaus/groovy/vmplugin/v8/ColdReflectiveMethodHandleWrapper.java: ########## @@ -0,0 +1,196 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.codehaus.groovy.vmplugin.v8; + +import groovy.lang.GroovyObject; +import groovy.lang.MetaClass; +import groovy.lang.MetaMethod; +import org.codehaus.groovy.reflection.CachedMethod; +import org.codehaus.groovy.runtime.GroovyCategorySupport; +import org.codehaus.groovy.runtime.wrappers.Wrapper; +import org.codehaus.groovy.vmplugin.VMPluginFactory; + +import java.lang.invoke.MethodHandle; +import java.lang.invoke.SwitchPoint; +import java.lang.reflect.Method; +import java.lang.reflect.Modifier; + +/** + * Experimental reflective cold-tier wrapper, guarded by the + * {@code groovy.indy.cold.reflection} system property. + * <p> + * The regular cold tier builds a fully guarded MethodHandle chain on the + * first invocation of every receiver shape — paying the one-time LambdaForm + * creation cost for a MethodType shape determined by the call site's arity + * and primitive pattern. This wrapper instead carries only the selected + * {@link MetaMethod} plus the data needed to re-validate the selection in + * plain Java; invocation happens reflectively via + * {@link IndyInterface#invokeColdReflective}. Its bound invocation handle has + * the same {@code (Object[])Object} shape for every call site, so the cold + * tier spins no per-shape LambdaForms. The full guarded chain is built only + * at promotion time (see {@code IndyInterface.fromCacheHandle}), leaving the + * hot path unchanged. + * <p> + * The plain-Java validity checks in {@link #isValidFor} mirror the guards the + * full chain would install: switch-point freshness (which also covers + * category enter/leave, as those invalidate the switch point), exact argument + * classes (at least as strict as the chain's same-class guards), and + * metaclass identity for {@code GroovyObject} receivers. + */ +class ColdReflectiveMethodHandleWrapper extends MethodHandleWrapper { + + final CacheableCallSite callSite; + final Class<?> sender; + final String methodName; + final int callID; + final Boolean safeNavigation; + final Boolean thisCall; + final Boolean spreadCall; + private final MetaClass selectionMetaClass; + private final SwitchPoint validity; + private final Class<?>[] argClasses; + private final MethodHandle reflectiveHandle; + /** + * Cumulative reflective invocations, independent of the PIC's + * consecutive-hit counter (which resets whenever receivers alternate, so + * polymorphic sites would otherwise stay on the reflective tier forever). + * Racy increments only delay promotion slightly, which is harmless. + */ + private int reflectiveHits; + + private ColdReflectiveMethodHandleWrapper(MetaMethod method, CacheableCallSite callSite, Class<?> sender, + String methodName, int callID, Boolean safeNavigation, Boolean thisCall, Boolean spreadCall, + MetaClass selectionMetaClass, SwitchPoint validity, Class<?>[] argClasses) { + super(null, null, method, true); + this.callSite = callSite; + this.sender = sender; + this.methodName = methodName; + this.callID = callID; + this.safeNavigation = safeNavigation; + this.thisCall = thisCall; + this.spreadCall = spreadCall; + this.selectionMetaClass = selectionMetaClass; + this.validity = validity; + this.argClasses = argClasses; + this.reflectiveHandle = IndyInterface.COLD_REFLECTIVE_INVOKER.bindTo(this); + } + + @Override + public MethodHandle getCachedMethodHandle() { + return reflectiveHandle; + } + + /** + * Counts a reflective invocation. + * + * @return the cumulative reflective invocation count + */ + int incrementReflectiveHits() { + return ++reflectiveHits; + } + + /** + * Plain-Java equivalent of the guards the full handle chain would install. + * + * @param arguments receiver-first invocation arguments + * @return {@code true} if the cached selection is still valid for them + */ + boolean isValidFor(Object[] arguments) { + if (validity.hasBeenInvalidated()) return false; + Class<?>[] classes = argClasses; + if (arguments.length != classes.length) return false; + for (int i = 0; i < classes.length; i++) { + Object a = arguments[i]; + if (a == null ? classes[i] != null : a.getClass() != classes[i]) return false; + } + Object receiver = arguments[0]; + return !(receiver instanceof GroovyObject go) || go.getMetaClass() == selectionMetaClass; + } + + /** + * Attempts a reflective cold-tier selection for a plain method call. + * "Plain" means: real receiver (non-null, not a {@code Class}, not + * {@code GroovyInterceptable}), no spread/safe-null semantics, cacheable + * selection resolving to a public, non-static, non-category + * {@link CachedMethod} on a public class that is not caller-sensitive. + * Anything else returns {@code null} and takes the full handle path. + * + * @return the reflective wrapper, or {@code null} if unsupported + */ + static MethodHandleWrapper tryBuild(Selector selector, CacheableCallSite callSite, Class<?> sender, + String methodName, int callID, Boolean safeNavigation, Boolean thisCall, Boolean spreadCall, + Object[] arguments) { + MetaMethod selected = selector.selectForColdReflection(); + if (selected == null || selected instanceof GroovyCategorySupport.CategoryMethod) return null; + MetaClass mc = selector.getSelectionMetaClass(); + if (mc == null || !selector.cache) return null; + MetaMethod transformed = VMPluginFactory.getPlugin().transformMetaMethod(mc, selected, sender); + if (!(transformed instanceof CachedMethod cm)) return null; + int modifiers = cm.getModifiers(); + if (Modifier.isStatic(modifiers) || !Modifier.isPublic(modifiers)) return null; + if (!Modifier.isPublic(cm.getDeclaringClass().getModifiers())) return null; + if (isCallerSensitive(cm, arguments[0].getClass())) return null; + Class<?>[] argClasses = new Class[arguments.length]; + for (int i = 0; i < arguments.length; i++) { + Object a = arguments[i]; + if (a instanceof Wrapper) return null; // cast markers need the full coercion path + argClasses[i] = (a == null) ? null : a.getClass(); + } + return new ColdReflectiveMethodHandleWrapper(cm, callSite, sender, methodName, callID, + safeNavigation, thisCall, spreadCall, mc, IndyInterface.switchPoint, argClasses); + } + + /** + * Reflective invocation would report the wrong caller for caller-sensitive + * methods (and promotion would then change the observed caller mid-run), + * so such targets stay on the full handle path. The per-method annotation + * probe lives on {@link CachedMethod#isCallerSensitive()} so its cost is + * paid once per method rather than per call site. Two extensions are + * handled here: some JDK methods are caller-context sensitive via stack + * walking without carrying the annotation — notably object serialization, + * whose {@code latestUserDefinedLoader} would see the reflective frames + * instead of the calling script's class loader — and the metaclass may + * select an <em>interface</em> method (e.g. {@code ObjectInput.readObject()}) + * whose implementation is the sensitive one, so abstract selections are + * probed against the runtime receiver's implementation. When a probe + * cannot decide, the method is treated as sensitive. + */ + private static boolean isCallerSensitive(CachedMethod cm, Class<?> receiverClass) { + Class<?> declaringClass = cm.getDeclaringClass().getTheClass(); + if (java.io.ObjectInput.class.isAssignableFrom(declaringClass) + || java.io.ObjectOutput.class.isAssignableFrom(declaringClass) + || java.io.ObjectInputStream.class.isAssignableFrom(declaringClass) + || java.io.ObjectOutputStream.class.isAssignableFrom(declaringClass)) { + return true; + } + if (cm.isCallerSensitive()) return true; + Method method = cm.getCachedMethod(); + if (declaringClass.isInterface() || Modifier.isAbstract(method.getModifiers())) { + // virtual dispatch may land on a caller-sensitive implementation + try { + Method implementation = receiverClass.getMethod(method.getName(), method.getParameterTypes()); + CachedMethod implCached = CachedMethod.find(implementation); + if (implCached != null) return implCached.isCallerSensitive(); + } catch (Throwable ignore) { + } + return true; // could not resolve the implementation: stay safe + } + return false; Review Comment: (isCallerSensitive): Restructured as you suggest: everything method-determined — the annotation probe and the declaring-class cases like serialization — is now behind cm.isCallerSensitive(), cached per method. And yes, abstract selections really occur: the metaclass picks ObjectInput.readObject() for a call on an ObjectInputStream (that exact case was found by a failing serialization test), and the sensitivity then belongs to the receiver's implementation, which is the one receiver-dependent check left in the wrapper. Good catch on defaults: the old isInterface() clause could conservatively mark a default method when implementation lookup failed — now non-abstract interface methods are probed directly, and a default method verifiably takes the reflective tier. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
