Replace PFrame.stale with PFrame.lastCallerFlags

msimacek · msimacek · commit 4956ef33aeb2 · 2025-11-20T17:51:33.000+01:00
diff --git a/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/frame/PFrame.java b/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/frame/PFrame.java
@@ -103,8 +103,11 @@ public final class PFrame extends PythonBuiltinObject {
 
     private PFrame.Reference backref = null;
 
-    // Whether lasti and locals are stale, see isStale
-    private boolean stale;
+    /**
+     * The last {@link CallerFlags} that were used the last time the frame was synced or passed down
+     * to a callee. See {@link #isStale(VirtualFrame, int)} for more details.
+     */
+    private int lastCallerFlags;
 
     public Object getLocalTraceFun() {
         return localTraceFun;
@@ -200,6 +203,9 @@ public PFrame(PythonLanguage lang, Reference virtualFrameInfo, Node location, bo
         this.virtualFrameInfo = virtualFrameInfo;
         this.location = location;
         this.hasCustomLocals = hasCustomLocals;
+        // Mark everything as current for now. MaterializeFrameNode will set lastCallerFlags to a
+        // narrower value if needed
+        this.lastCallerFlags = CallerFlags.ALL_FRAME_FLAGS;
     }
 
     public PFrame(PythonLanguage lang, @SuppressWarnings("unused") Object threadState, PCode code, PythonObject globals, Object localsDict) {
@@ -216,18 +222,21 @@ public PFrame(PythonLanguage lang, @SuppressWarnings("unused") Object threadStat
         this.arguments = frameArgs;
         this.hasCustomLocals = true;
         this.localsDict = localsDict;
+        // This is a synthetic frame, there will be no sync, mark everything as current
+        this.lastCallerFlags = CallerFlags.ALL_FRAME_FLAGS;
     }
 
     /**
      * Get the locals synced by {@link MaterializeFrameNode}. May be null when using custom locals.
      * In most cases, you should use {@link GetFrameLocalsNode}.
      */
     public MaterializedFrame getLocals() {
-        assert !stale;
+        assert CallerFlags.needsLocals(lastCallerFlags) : "Missing frame locals sync";
         return locals;
     }
 
     public void setLocals(MaterializedFrame locals) {
+        lastCallerFlags |= CallerFlags.NEEDS_LOCALS;
         this.locals = locals;
     }
 
@@ -245,15 +254,23 @@ public void setLocals(MaterializedFrame locals) {
      *            flags from {@link CallerFlags}.
      */
     public boolean isStale(VirtualFrame frame, int callerFlags) {
-        boolean needsLocals = CallerFlags.needsLocals(callerFlags) && !hasCustomLocals;
-        if (needsLocals && locals == null) {
+        if (hasCustomLocals) {
+            // Custom locals don't need locals sync
+            callerFlags &= ~CallerFlags.NEEDS_LOCALS;
+        }
+        if (CallerFlags.needsLocals(callerFlags) && locals == null) {
             return true;
         }
-        return (CallerFlags.needsLasti(callerFlags) || needsLocals) && (stale || (frame != null && PArguments.getCurrentFrameInfo(frame) == getRef()));
+        if (CallerFlags.needsLocals(callerFlags) || CallerFlags.needsLasti(callerFlags)) {
+            if (frame != null && PArguments.getCurrentFrameInfo(frame) == getRef()) {
+                return true;
+            }
+        }
+        return (callerFlags & lastCallerFlags) != callerFlags;
     }
 
-    public void setStale(boolean stale) {
-        this.stale = stale;
+    public void setLastCallerFlags(int lastCallerFlags) {
+        this.lastCallerFlags = lastCallerFlags;
     }
 
     /**
@@ -309,17 +326,16 @@ public boolean didJump() {
 
     @TruffleBoundary
     public int getLine() {
-        assert !stale;
         if (line == UNINITIALIZED_LINE) {
             if (location == null) {
                 line = -1;
             } else if (PythonOptions.ENABLE_BYTECODE_DSL_INTERPRETER) {
                 if (location instanceof BytecodeNode bytecodeNode) {
                     PBytecodeDSLRootNode rootNode = (PBytecodeDSLRootNode) bytecodeNode.getRootNode();
-                    return rootNode.bciToLine(bci, bytecodeNode);
+                    return rootNode.bciToLine(getBci(), bytecodeNode);
                 }
             } else if (location instanceof PBytecodeRootNode bytecodeRootNode) {
-                return bytecodeRootNode.bciToLine(bci);
+                return bytecodeRootNode.bciToLine(getBci());
             }
         }
         return line;
@@ -374,10 +390,12 @@ public int getBci() {
     }
 
     public void setBci(int bci) {
+        this.lastCallerFlags |= CallerFlags.NEEDS_LASTI;
         this.bci = bci;
     }
 
     public int getLasti() {
+        assert CallerFlags.needsLasti(lastCallerFlags) : "Missing frame location sync";
         return bciToLasti(bci, location);
     }
 
diff --git a/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/frame/GetFrameLocalsNode.java b/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/frame/GetFrameLocalsNode.java
@@ -107,6 +107,7 @@ static Object doLoop(VirtualFrame frame, Node inliningTarget, PFrame pyFrame, bo
         if (!freshFrame && pyFrame.isStale(frame, CallerFlags.NEEDS_LOCALS)) {
             pyFrame = readFrameNode.refreshFrame(frame, pyFrame.getRef(), CallerFlags.NEEDS_LOCALS);
         }
+        assert !pyFrame.isStale(null, CallerFlags.NEEDS_LOCALS);
         MaterializedFrame locals = pyFrame.getLocals();
         // It doesn't have custom locals, so it has to be a builtin dict or null
         PDict localsDict = (PDict) pyFrame.getLocalsDict();
diff --git a/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/frame/MaterializeFrameNode.java b/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/frame/MaterializeFrameNode.java
@@ -50,6 +50,7 @@
 import com.oracle.graal.python.nodes.bytecode.BytecodeFrameInfo;
 import com.oracle.graal.python.nodes.bytecode.FrameInfo;
 import com.oracle.graal.python.nodes.bytecode_dsl.PBytecodeDSLRootNode;
+import com.oracle.graal.python.runtime.CallerFlags;
 import com.oracle.graal.python.runtime.PythonOptions;
 import com.oracle.graal.python.runtime.object.PFactory;
 import com.oracle.truffle.api.CompilerAsserts;
@@ -190,12 +191,9 @@ static PFrame freshPFrameForGenerator(Node location, @SuppressWarnings("unused")
     static PFrame alreadyEscapedFrame(@SuppressWarnings("unused") Node location, boolean markAsEscaped, boolean forceSync, Frame frameToMaterialize,
                     @Shared("syncValuesNode") @Cached SyncFrameValuesNode syncValuesNode) {
         PFrame pyFrame = getPFrame(frameToMaterialize);
-        pyFrame.setStale(false);
+        pyFrame.setLastCallerFlags(getCallerFlags(forceSync));
         if (forceSync) {
             syncValuesNode.execute(pyFrame, frameToMaterialize, location);
-        } else {
-            // Clear stale locals
-            pyFrame.setLocals(null);
         }
         if (markAsEscaped) {
             pyFrame.getRef().markAsEscaped();
@@ -244,12 +242,9 @@ private static PFrame doEscapeFrame(Frame frameToMaterialize, PFrame escapedFram
 
         // on a freshly created PFrame, we do always sync the arguments
         PArguments.synchronizeArgs(frameToMaterialize, escapedFrame);
-        escapedFrame.setStale(false);
+        escapedFrame.setLastCallerFlags(getCallerFlags(forceSync));
         if (forceSync) {
             syncValuesNode.execute(escapedFrame, frameToMaterialize, location);
-        } else {
-            // Clear stale locals
-            escapedFrame.setLocals(null);
         }
         if (markAsEscaped) {
             topFrameRef.markAsEscaped();
@@ -258,6 +253,10 @@ private static PFrame doEscapeFrame(Frame frameToMaterialize, PFrame escapedFram
         return escapedFrame;
     }
 
+    private static int getCallerFlags(boolean forceSync) {
+        return CallerFlags.NEEDS_FRAME_REFERENCE | CallerFlags.NEEDS_PFRAME | CallerFlags.NEEDS_LASTI | (forceSync ? CallerFlags.NEEDS_LOCALS : 0);
+    }
+
     protected static boolean hasCustomLocals(Frame frame) {
         return PArguments.getSpecialArgument(frame) != null;
     }
diff --git a/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/runtime/CallerFlags.java b/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/runtime/CallerFlags.java
@@ -58,6 +58,8 @@ public abstract class CallerFlags {
      */
     public static final int NEEDS_LASTI = 1 << 4;
 
+    public static final int ALL_FRAME_FLAGS = NEEDS_FRAME_REFERENCE | NEEDS_PFRAME | NEEDS_LOCALS | NEEDS_LASTI;
+
     public static boolean needsExceptionState(int callerFlags) {
         return (callerFlags & NEEDS_EXCEPTION_STATE) != 0;
     }
diff --git a/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/runtime/ExecutionContext.java b/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/runtime/ExecutionContext.java
@@ -228,14 +228,14 @@ protected static void passCallerFrame(VirtualFrame frame, Object[] callArguments
                         assert materialize.isAdoptable();
                     }
                     if (thisInfo.getPyFrame() != null && !CallerFlags.needsLocals(callerFlags) && !CallerFlags.needsLasti(callerFlags)) {
-                        thisInfo.getPyFrame().setStale(true);
+                        thisInfo.getPyFrame().setLastCallerFlags(callerFlags);
                     } else {
                         PFrame pyFrame = materialize.executeOnStack(false, CallerFlags.needsLocals(callerFlags), frame);
                         assert thisInfo.getPyFrame() == pyFrame;
                         assert pyFrame.getRef() == thisInfo;
                     }
                 } else if (thisInfo.getPyFrame() != null) {
-                    thisInfo.getPyFrame().setStale(true);
+                    thisInfo.getPyFrame().setLastCallerFlags(callerFlags);
                 }
                 PArguments.setCallerFrameInfo(callArguments, thisInfo);
             }
@@ -712,8 +712,7 @@ private static Object enterSlowPath(VirtualFrame frame, IndirectCallDataBase cal
                 if (CallerFlags.needsPFrame(callerFlags)) {
                     callData.getMaterializeFrameNode().executeOnStack(false, CallerFlags.needsLocals(callerFlags), frame);
                 } else if (info.getPyFrame() != null) {
-                    // Avoid passing stale locals
-                    info.getPyFrame().setStale(true);
+                    info.getPyFrame().setLastCallerFlags(callerFlags);
                 }
             }
             AbstractTruffleException curExc = pythonThreadState.getCaughtException();
diff --git a/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/runtime/IndirectCallData.java b/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/runtime/IndirectCallData.java
@@ -132,7 +132,7 @@ public int getCallerFlags() {
 
         @TruffleBoundary
         public void updateCallerFlags(int newFlags) {
-            if ((newFlags & CallerFlags.NEEDS_LOCALS) != 0) {
+            if ((newFlags & (CallerFlags.NEEDS_LOCALS | CallerFlags.NEEDS_LASTI)) != 0) {
                 newFlags |= CallerFlags.NEEDS_PFRAME;
             }
             if ((newFlags & CallerFlags.NEEDS_PFRAME) != 0) {

Original file line number	Diff line number	Diff line change
`@@ -107,6 +107,7 @@ static Object doLoop(VirtualFrame frame, Node inliningTarget, PFrame pyFrame, bo`
`107`	`107`	`if (!freshFrame && pyFrame.isStale(frame, CallerFlags.NEEDS_LOCALS)) {`
`108`	`108`	`pyFrame = readFrameNode.refreshFrame(frame, pyFrame.getRef(), CallerFlags.NEEDS_LOCALS);`
`109`	`109`	`}`
	`110`	`+ assert !pyFrame.isStale(null, CallerFlags.NEEDS_LOCALS);`
`110`	`111`	`MaterializedFrame locals = pyFrame.getLocals();`
`111`	`112`	`// It doesn't have custom locals, so it has to be a builtin dict or null`
`112`	`113`	`PDict localsDict = (PDict) pyFrame.getLocalsDict();`
Original file line number	Diff line number	Diff line change
`@@ -58,6 +58,8 @@ public abstract class CallerFlags {`
`58`	`58`	`*/`
`59`	`59`	`public static final int NEEDS_LASTI = 1 << 4;`
`60`	`60`
	`61`	`+ public static final int ALL_FRAME_FLAGS = NEEDS_FRAME_REFERENCE \| NEEDS_PFRAME \| NEEDS_LOCALS \| NEEDS_LASTI;`
	`62`	`+`
`61`	`63`	`public static boolean needsExceptionState(int callerFlags) {`
`62`	`64`	`return (callerFlags & NEEDS_EXCEPTION_STATE) != 0;`
`63`	`65`	`}`
Original file line number	Diff line number	Diff line change
`@@ -228,14 +228,14 @@ protected static void passCallerFrame(VirtualFrame frame, Object[] callArguments`
`228`	`228`	`assert materialize.isAdoptable();`
`229`	`229`	`}`
`230`	`230`	`if (thisInfo.getPyFrame() != null && !CallerFlags.needsLocals(callerFlags) && !CallerFlags.needsLasti(callerFlags)) {`
`231`		`- thisInfo.getPyFrame().setStale(true);`
	`231`	`+ thisInfo.getPyFrame().setLastCallerFlags(callerFlags);`
`232`	`232`	`} else {`
`233`	`233`	`PFrame pyFrame = materialize.executeOnStack(false, CallerFlags.needsLocals(callerFlags), frame);`
`234`	`234`	`assert thisInfo.getPyFrame() == pyFrame;`
`235`	`235`	`assert pyFrame.getRef() == thisInfo;`
`236`	`236`	`}`
`237`	`237`	`} else if (thisInfo.getPyFrame() != null) {`
`238`		`- thisInfo.getPyFrame().setStale(true);`
	`238`	`+ thisInfo.getPyFrame().setLastCallerFlags(callerFlags);`
`239`	`239`	`}`
`240`	`240`	`PArguments.setCallerFrameInfo(callArguments, thisInfo);`
`241`	`241`	`}`
`@@ -712,8 +712,7 @@ private static Object enterSlowPath(VirtualFrame frame, IndirectCallDataBase cal`
`712`	`712`	`if (CallerFlags.needsPFrame(callerFlags)) {`
`713`	`713`	`callData.getMaterializeFrameNode().executeOnStack(false, CallerFlags.needsLocals(callerFlags), frame);`
`714`	`714`	`} else if (info.getPyFrame() != null) {`
`715`		`- // Avoid passing stale locals`
`716`		`- info.getPyFrame().setStale(true);`
	`715`	`+ info.getPyFrame().setLastCallerFlags(callerFlags);`
`717`	`716`	`}`
`718`	`717`	`}`
`719`	`718`	`AbstractTruffleException curExc = pythonThreadState.getCaughtException();`
Original file line number	Diff line number	Diff line change
`@@ -132,7 +132,7 @@ public int getCallerFlags() {`
`132`	`132`
`133`	`133`	`@TruffleBoundary`
`134`	`134`	`public void updateCallerFlags(int newFlags) {`
`135`		`- if ((newFlags & CallerFlags.NEEDS_LOCALS) != 0) {`
	`135`	`+ if ((newFlags & (CallerFlags.NEEDS_LOCALS \| CallerFlags.NEEDS_LASTI)) != 0) {`
`136`	`136`	`newFlags \|= CallerFlags.NEEDS_PFRAME;`
`137`	`137`	`}`
`138`	`138`	`if ((newFlags & CallerFlags.NEEDS_PFRAME) != 0) {`