[GR-38296] [GR-65175] Remove deprecated OSR frame transfer.

PullRequest: graal/21068

Author: DSouzaM

substratevm/src/com.oracle.svm.truffle/src/com/oracle/svm/truffle/TruffleBaseFeature.java

@@ -300,7 +300,7 @@ static <T> T invokeStaticMethod(String className, String methodName, Collection<
     static boolean isStaticMethodPresent(String className, String methodName, Collection<Class<?>> parameterTypes) {
         try {
             Class<?> clazz = Class.forName(className);
-            Method method = ReflectionUtil.lookupMethod(clazz, methodName, parameterTypes.toArray(new Class<?>[0]));
+            Method method = ReflectionUtil.lookupMethod(true, clazz, methodName, parameterTypes.toArray(new Class<?>[0]));
             return method != null;
         } catch (ReflectiveOperationException e) {
             throw VMError.shouldNotReachHere(e);

substratevm/src/com.oracle.svm.truffle/src/com/oracle/svm/truffle/TruffleFeature.java

@@ -444,12 +444,16 @@ public void beforeAnalysis(BeforeAnalysisAccess access) {
         /* Ensure org.graalvm.polyglot.io.IOHelper.IMPL is initialized. */
         ((BeforeAnalysisAccessImpl) access).ensureInitialized("org.graalvm.polyglot.io.IOHelper");
 
-        /* Support for deprecated bytecode osr frame transfer: GR-38296 */
-        config.registerSubtypeReachabilityHandler((acc, klass) -> {
-            /* Pass known reachable classes to the initializer: it will decide there what to do. */
-            TruffleBaseFeature.invokeStaticMethod("com.oracle.truffle.runtime.BytecodeOSRRootNode", "initializeClassUsingDeprecatedFrameTransfer",
-                            Collections.singleton(Class.class), klass);
-        }, BytecodeOSRNode.class);
+        /* Support for deprecated bytecode osr frame transfer: GR-65788 */
+        if (TruffleBaseFeature.isStaticMethodPresent("com.oracle.truffle.runtime.BytecodeOSRRootNode", "initializeClassUsingDeprecatedFrameTransfer", Collections.singleton(Class.class))) {
+            config.registerSubtypeReachabilityHandler((acc, klass) -> {
+                /*
+                 * Pass known reachable classes to the initializer: it will decide there what to do.
+                 */
+                TruffleBaseFeature.invokeStaticMethod("com.oracle.truffle.runtime.BytecodeOSRRootNode", "initializeClassUsingDeprecatedFrameTransfer",
+                                Collections.singleton(Class.class), klass);
+            }, BytecodeOSRNode.class);
+        }
     }
 
     static class TruffleAllowInliningPredicate {

truffle/CHANGELOG.md

@@ -29,6 +29,8 @@ This changelog summarizes major changes between Truffle versions relevant to lan
 * GR-64594 Bytecode DSL: Builder instances now have a `toString()` implementation that prints current operations as well as the instructions that have been already emitted. This should make it easier to debug problems with builder usage.
 * GR-64594 Bytecode DSL: Added `@GenerateBytecode(..., additionalAssertions=true)` to enable additional assertions for Bytecode DSL implementation bugs. This feature can also be enabled with `-A.truffle.dsl.AdditionalAssertions=true` at Java source compile time. These assertions are intentionally disabled by default, as they can lead to slow-downs even when assertions are disabled.
 * GR-65428 JDK specific optimizations when building Truffle languages with native-image are now enabled by default, even if the truffle-enterprise.jar is not provided on the class or module-path.
+* GR-38296 Removed the deprecated `BytecodeOSRNode.copyIntoOSRFrame` hook that does not declare a `targetMetadata` parameter.
+
 
 ## Version 24.2.0
 * GR-60636 Truffle now stops compiling when the code cache fills up on HotSpot. A warning is printed when that happens.

truffle/src/com.oracle.truffle.api/snapshot.sigtest

@@ -1166,8 +1166,6 @@ meth public static boolean pollOSRBackEdge(com.oracle.truffle.api.nodes.Bytecode
 meth public static boolean pollOSRBackEdge(com.oracle.truffle.api.nodes.BytecodeOSRNode,int)
 meth public static java.lang.Object tryOSR(com.oracle.truffle.api.nodes.BytecodeOSRNode,int,java.lang.Object,java.lang.Runnable,com.oracle.truffle.api.frame.VirtualFrame)
 meth public static java.lang.Object tryOSR(com.oracle.truffle.api.nodes.BytecodeOSRNode,long,java.lang.Object,java.lang.Runnable,com.oracle.truffle.api.frame.VirtualFrame)
-meth public void copyIntoOSRFrame(com.oracle.truffle.api.frame.VirtualFrame,com.oracle.truffle.api.frame.VirtualFrame,int)
- anno 0 java.lang.Deprecated(boolean forRemoval=false, java.lang.String since="22.2")
 meth public void copyIntoOSRFrame(com.oracle.truffle.api.frame.VirtualFrame,com.oracle.truffle.api.frame.VirtualFrame,int,java.lang.Object)
 meth public void copyIntoOSRFrame(com.oracle.truffle.api.frame.VirtualFrame,com.oracle.truffle.api.frame.VirtualFrame,long,java.lang.Object)
 meth public void prepareOSR(int)

truffle/src/com.oracle.truffle.api/src/com/oracle/truffle/api/impl/Accessor.java

@@ -1227,13 +1227,6 @@ public final FrameExtensions getFrameExtensionsUnsafe() {
          */
         public abstract void onOSRNodeReplaced(BytecodeOSRNode osrNode, Node oldNode, Node newNode, CharSequence reason);
 
-        /**
-         * Same as {@link #transferOSRFrame(BytecodeOSRNode, Frame, Frame, long, Object)}, but
-         * fetches the target metadata.
-         */
-        // Support for deprecated frame transfer: GR-38296
-        public abstract void transferOSRFrame(BytecodeOSRNode osrNode, Frame source, Frame target, long bytecodeTarget);
-
         /**
          * Transfers state from the {@code source} frame into the {@code target} frame. This method
          * should only be used inside OSR code. The frames must have the same layout as the frame
@@ -1243,6 +1236,8 @@ public final FrameExtensions getFrameExtensionsUnsafe() {
          * @param source the frame to transfer state from
          * @param target the frame to transfer state into
          * @param bytecodeTarget the target location OSR executes from (e.g., bytecode index).
+         * @param targetMetadata Additional metadata associated with this {@code target} for the
+         *            default frame transfer behavior.
          */
         public abstract void transferOSRFrame(BytecodeOSRNode osrNode, Frame source, Frame target, long bytecodeTarget, Object targetMetadata);
 

truffle/src/com.oracle.truffle.api/src/com/oracle/truffle/api/impl/DefaultRuntimeAccessor.java

@@ -154,12 +154,6 @@ public void onOSRNodeReplaced(BytecodeOSRNode osrNode, Node oldNode, Node newNod
             // do nothing
         }
 
-        @Override
-        // Support for deprecated frame transfer: GR-38296
-        public void transferOSRFrame(BytecodeOSRNode osrNode, Frame source, Frame target, long bytecodeTarget) {
-            throw new UnsupportedOperationException();
-        }
-
         @Override
         public void transferOSRFrame(BytecodeOSRNode osrNode, Frame source, Frame target, long bytecodeTarget, Object targetMetadata) {
             throw new UnsupportedOperationException();

truffle/src/com.oracle.truffle.api/src/com/oracle/truffle/api/nodes/BytecodeOSRNode.java

@@ -171,20 +171,6 @@ default Object executeOSR(VirtualFrame osrFrame, long target, Object interpreter
      */
     void setOSRMetadata(Object osrMetadata);
 
-    /**
-     * Note that if this method is implemented, the
-     * {@link #copyIntoOSRFrame(VirtualFrame, VirtualFrame, int, Object) preferred one} will not be
-     * used.
-     *
-     * @since 21.3
-     * @deprecated since 22.2
-     * @see #copyIntoOSRFrame(VirtualFrame, VirtualFrame, int, Object)
-     */
-    @Deprecated(since = "22.2")
-    default void copyIntoOSRFrame(VirtualFrame osrFrame, VirtualFrame parentFrame, int target) {
-        NodeAccessor.RUNTIME.transferOSRFrame(this, parentFrame, osrFrame, target);
-    }
-
     /**
      * Copies the contents of the {@code parentFrame} into the {@code osrFrame} used to execute OSR.
      * By default, performs a slot-wise copy of the frame.

truffle/src/com.oracle.truffle.runtime/src/com/oracle/truffle/runtime/BytecodeOSRMetadata.java

@@ -67,7 +67,7 @@
  * <p>
  * Performance note: We do not require the metadata field to be {@code volatile}. As long as the
  * field is initialized under double-checked locking (as is done in
- * {@link OptimizedRuntimeSupport#pollBytecodeOSRBackEdge}, all threads will observe the same
+ * {@link OptimizedRuntimeSupport#pollBytecodeOSRBackEdge}), all threads will observe the same
  * metadata instance. The JMM guarantees that the instance's final fields will be safely initialized
  * before it is published; the non-final + non-volatile fields (e.g., the back edge counter) may not
  * be, but we tolerate this inaccuracy in order to avoid volatile accesses in the hot path.
@@ -139,9 +139,7 @@ private OptimizedCallTarget getCurrentlyCompiling() {
 
     // Lazily initialized state. Most nodes with back-edges will not trigger compilation, so we
     // defer initialization of some fields until they're actually used.
-    static final class LazyState //
-                    // Support for deprecated frame transfer: GR-38296
-                    extends FinalCompilationListMap {
+    static final class LazyState {
 
         private final Map<Long, OptimizedCallTarget> compilationMap;
         @CompilationFinal private FrameDescriptor frameDescriptor;
@@ -153,19 +151,14 @@ static final class LazyState //
             this.frameDescriptor = null;
         }
 
-        private void push(long target, OptimizedCallTarget callTarget, OsrEntryDescription entry) {
+        private void push(long target, OptimizedCallTarget callTarget) {
             compilationMap.put(target, callTarget);
-            // Support for deprecated frame transfer: GR-38296
-            put(target, entry);
         }
 
         private void doClear() {
             compilationMap.clear();
-            // We might be disabling OSR while doing an OSR call. Keep around the data necessary to
-            // transfer from and restore the parent frame.
-            // In particular, we must keep alive:
-            // - The frame descriptor
-            // - (GR-38296) The map from target to entry description.
+            // We might be disabling OSR while doing an OSR call. Do not clear the frame descriptor,
+            // in case we need it to transfer from and restore the parent frame.
         }
     }
 
@@ -195,8 +188,7 @@ private void updateFrameSlots(FrameWithoutBoxing frame, OsrEntryDescription osrE
         LazyState state = getLazyState();
         ((Node) osrNode).atomic(() -> {
             if (state.frameDescriptor == null) {
-                FrameDescriptor frameDescriptor = frame.getFrameDescriptor();
-                state.frameDescriptor = frameDescriptor;
+                state.frameDescriptor = frame.getFrameDescriptor();
             }
             if (osrEntry != null) {
                 // The concrete frame can have different tags from the descriptor (e.g., when a slot
@@ -259,9 +251,8 @@ Object tryOSR(long target, Object interpreterState, Runnable beforeTransfer, Vir
             callTarget = ((Node) osrNode).atomic(() -> {
                 OptimizedCallTarget lockedTarget = state.compilationMap.get(target);
                 if (lockedTarget == null) {
-                    OsrEntryDescription entryDescription = new OsrEntryDescription();
-                    lockedTarget = createOSRTarget(target, interpreterState, parentFrame.getFrameDescriptor(), entryDescription);
-                    state.push(target, lockedTarget, entryDescription);
+                    lockedTarget = createOSRTarget(target, interpreterState, parentFrame.getFrameDescriptor());
+                    state.push(target, lockedTarget);
                     if (stage == FRESH_STAGE) {
                         // First attempt at compilation gets a free pass
                         requestOSRCompilation(target, lockedTarget, (FrameWithoutBoxing) parentFrame);
@@ -372,10 +363,9 @@ private void resetCounter() {
      * Creates an OSR call target at the given dispatch target and requests compilation. The node's
      * AST lock should be held when this is invoked.
      */
-    private OptimizedCallTarget createOSRTarget(long target, Object interpreterState, FrameDescriptor frameDescriptor, Object frameEntryState) {
+    private OptimizedCallTarget createOSRTarget(long target, Object interpreterState, FrameDescriptor frameDescriptor) {
         TruffleLanguage<?> language = OptimizedRuntimeAccessor.NODES.getLanguage(((Node) osrNode).getRootNode());
-        return (OptimizedCallTarget) new BytecodeOSRRootNode(language, frameDescriptor, osrNode, target, interpreterState, frameEntryState).getCallTarget();
-
+        return (OptimizedCallTarget) new BytecodeOSRRootNode(language, frameDescriptor, osrNode, target, interpreterState, new OsrEntryDescription()).getCallTarget();
     }
 
     private void requestOSRCompilation(long target, OptimizedCallTarget callTarget, FrameWithoutBoxing frame) {
@@ -431,7 +421,7 @@ private static OsrEntryDescription getEntryCacheFromCallTarget(OptimizedCallTarg
      * Transfer state from {@code source} to {@code target}. Can be used to transfer state into an
      * OSR frame.
      */
-    public void transferFrame(FrameWithoutBoxing source, FrameWithoutBoxing target, long bytecodeTarget, Object targetMetadata) {
+    public void transferFrame(FrameWithoutBoxing source, FrameWithoutBoxing target, Object targetMetadata) {
         LazyState state = getLazyState();
         CompilerAsserts.partialEvaluationConstant(state);
         // The frames should use the same descriptor.
@@ -445,7 +435,6 @@ public void transferFrame(FrameWithoutBoxing source, FrameWithoutBoxing target,
             CompilerDirectives.transferToInterpreterAndInvalidate();
             throw new IllegalArgumentException("Wrong usage of targetMetadata during OSR frame transfer.");
         }
-        assert targetMetadata == state.get(bytecodeTarget); // GR-38296
 
         OsrEntryDescription description = (OsrEntryDescription) targetMetadata;
         CompilerAsserts.partialEvaluationConstant(description);
@@ -460,9 +449,9 @@ public void transferFrame(FrameWithoutBoxing source, FrameWithoutBoxing target,
     /**
      * Transfer state from {@code source} to {@code target}. Can be used to transfer state from an
      * OSR frame to a parent frame. Overall less efficient than its
-     * {@link #transferFrame(FrameWithoutBoxing, FrameWithoutBoxing, long, Object) counterpart},
-     * mainly due to not being able to speculate on the source tags: While entering bytecode OSR is
-     * done through specific entry points (likely back edges), returning could be done from anywhere
+     * {@link #transferFrame(FrameWithoutBoxing, FrameWithoutBoxing, Object) counterpart}, mainly
+     * due to not being able to speculate on the source tags: While entering bytecode OSR is done
+     * through specific entry points (likely back edges), returning could be done from anywhere
      * within a method body (through regular returns, or exception thrown).
      *
      * While we could theoretically have the same mechanism as on entries (caching encountered
@@ -712,49 +701,4 @@ public void clear() {
     static final class OsrEntryDescription {
         @CompilationFinal(dimensions = 1) private byte[] indexedFrameTags;
     }
-
-    // Support for deprecated frame transfer: GR-38296
-    private abstract static class FinalCompilationListMap {
-        private static final class Cell {
-            final Cell next;
-            final long target;
-            final OsrEntryDescription entry;
-
-            Cell(long target, OsrEntryDescription entry, Cell next) {
-                this.next = next;
-                this.target = target;
-                this.entry = entry;
-            }
-        }
-
-        @CompilationFinal //
-        volatile Cell head = null;
-
-        @ExplodeLoop
-        public final OsrEntryDescription get(long target) {
-            Cell cur = head;
-            while (cur != null) {
-                if (cur.target == target) {
-                    return cur.entry;
-                }
-                cur = cur.next;
-            }
-            return null;
-        }
-
-        public final void put(long target, OsrEntryDescription value) {
-            CompilerDirectives.transferToInterpreterAndInvalidate();
-            synchronized (this) {
-                assert get(target) == null;
-                head = new Cell(target, value, head);
-            }
-        }
-
-        public final void clear() {
-            CompilerDirectives.transferToInterpreterAndInvalidate();
-            synchronized (this) {
-                head = null;
-            }
-        }
-    }
 }

truffle/src/com.oracle.truffle.runtime/src/com/oracle/truffle/runtime/BytecodeOSRRootNode.java

@@ -40,12 +40,6 @@
  */
 package com.oracle.truffle.runtime;
 
-import java.lang.reflect.Method;
-import java.util.Map;
-import java.util.concurrent.ConcurrentHashMap;
-
-import org.graalvm.nativeimage.ImageInfo;
-
 import com.oracle.truffle.api.CompilerDirectives;
 import com.oracle.truffle.api.CompilerDirectives.CompilationFinal;
 import com.oracle.truffle.api.Truffle;
@@ -68,9 +62,6 @@ final class BytecodeOSRRootNode extends BaseOSRRootNode {
         this.interpreterState = interpreterState;
         this.seenMaterializedFrame = materializeCalled(frameDescriptor);
         this.entryTagsCache = entryTagsCache;
-
-        // Support for deprecated frame transfer: GR-38296
-        this.usesDeprecatedFrameTransfer = checkUsesDeprecatedFrameTransfer(bytecodeOSRNode.getClass());
     }
 
     private static boolean materializeCalled(FrameDescriptor frameDescriptor) {
@@ -101,15 +92,7 @@ public Object executeOSR(VirtualFrame frame) {
             // required to prevent the materialized frame from getting out of sync during OSR.
             return osrNode.executeOSR(parentFrame, target, interpreterState);
         } else {
-            if (usesDeprecatedFrameTransfer) { // Support for deprecated frame transfer: GR-38296
-                int intTarget = (int) target;
-                if (intTarget != target) {
-                    throw CompilerDirectives.shouldNotReachHere("long target cannot be used with deprecated frame transfer");
-                }
-                osrNode.copyIntoOSRFrame(frame, parentFrame, intTarget);
-            } else {
-                osrNode.copyIntoOSRFrame(frame, parentFrame, target, entryTagsCache);
-            }
+            osrNode.copyIntoOSRFrame(frame, parentFrame, target, entryTagsCache);
             try {
                 return osrNode.executeOSR(frame, target, interpreterState);
             } finally {
@@ -128,47 +111,10 @@ public String toString() {
         return ((Node) loopNode).getRootNode().toString() + "<OSR@" + target + ">";
     }
 
-    // GR-38296
-    /* Deprecated frame transfer handling below */
-
-    private static final Map<Class<?>, Boolean> usesDeprecatedTransferClasses = new ConcurrentHashMap<>();
-
-    private final boolean usesDeprecatedFrameTransfer;
-
-    /**
-     * Detects usage of deprecated frame transfer, and directs the frame transfer path accordingly
-     * later. When removing the support for this deprecation, constructs used and paths related are
-     * marked with the comment "Support for deprecated frame transfer" and a reference to GR-38296.
-     */
-    private static boolean usesDeprecatedFrameTransfer(Class<?> osrNodeClass) {
-        try {
-            Method m = osrNodeClass.getMethod("copyIntoOSRFrame", VirtualFrame.class, VirtualFrame.class, int.class);
-            return m.getDeclaringClass() != BytecodeOSRNode.class;
-        } catch (NoSuchMethodException e) {
-            return false;
-        }
-    }
-
-    private static boolean checkUsesDeprecatedFrameTransfer(Class<?> osrNodeClass) {
-        if (ImageInfo.inImageRuntimeCode()) {
-            // this must have been pre-computed
-            return usesDeprecatedTransferClasses.get(osrNodeClass);
-        } else {
-            return usesDeprecatedTransferClasses.computeIfAbsent(osrNodeClass, BytecodeOSRRootNode::usesDeprecatedFrameTransfer);
-        }
-    }
-
     // Called by truffle feature to initialize the map at build time.
     @SuppressWarnings("unused")
     private static boolean initializeClassUsingDeprecatedFrameTransfer(Class<?> subType) {
-        if (subType.isInterface()) {
-            return false;
-        }
-        if (usesDeprecatedTransferClasses.containsKey(subType)) {
-            return false;
-        }
-        // Eagerly initialize result.
-        usesDeprecatedTransferClasses.put(subType, usesDeprecatedFrameTransfer(subType));
+        // empty method implementation for backward-compatibility with SVM: GR-65788
         return true;
     }