Skip to content

Commit a910a26

Browse files
committed
Refactor 'PBaseException.reifyException' to not do an eager stack walk.
1 parent 372a51f commit a910a26

File tree

9 files changed

+205
-113
lines changed

9 files changed

+205
-113
lines changed

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/modules/PythonCextBuiltins.java

Lines changed: 90 additions & 89 deletions
Large diffs are not rendered by default.

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/cext/CExtNodes.java

Lines changed: 46 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,8 @@
100100
import com.oracle.graal.python.nodes.call.special.LookupAndCallUnaryNode.CallUnaryContextManager;
101101
import com.oracle.graal.python.nodes.call.special.LookupAndCallUnaryNode.LookupAndCallUnaryDynamicNode;
102102
import com.oracle.graal.python.nodes.classes.IsSubtypeNode;
103+
import com.oracle.graal.python.nodes.frame.MaterializeFrameNode;
104+
import com.oracle.graal.python.nodes.frame.MaterializeFrameNodeGen;
103105
import com.oracle.graal.python.nodes.function.PythonBuiltinBaseNode;
104106
import com.oracle.graal.python.nodes.function.PythonBuiltinNode;
105107
import com.oracle.graal.python.nodes.function.builtins.PythonBinaryBuiltinNode;
@@ -1486,6 +1488,8 @@ public List<Class<? extends Node>> getExecutionSignature() {
14861488
public abstract static class MayRaiseUnaryNode extends PythonUnaryBuiltinNode {
14871489
@Child private CreateArgumentsNode createArgsNode;
14881490
@Child private InvokeNode invokeNode;
1491+
@Child private MaterializeFrameNode materializeNode;
1492+
14891493
private final PFunction func;
14901494
private final Object errorResult;
14911495

@@ -1504,17 +1508,27 @@ Object doit(VirtualFrame frame, Object argument) {
15041508
} catch (PException e) {
15051509
// getContext() acts as a branch profile
15061510
getContext().setCurrentException(e);
1507-
e.getExceptionObject().reifyException();
1511+
e.getExceptionObject().reifyException(ensureMaterializeNode().execute(frame, this, true, false), factory());
15081512
return errorResult;
15091513
}
15101514
}
1515+
1516+
private MaterializeFrameNode ensureMaterializeNode() {
1517+
if (materializeNode == null) {
1518+
CompilerDirectives.transferToInterpreterAndInvalidate();
1519+
materializeNode = insert(MaterializeFrameNodeGen.create());
1520+
}
1521+
return materializeNode;
1522+
}
15111523
}
15121524

15131525
// -----------------------------------------------------------------------------------------------------------------
15141526
@Builtin(minNumOfPositionalArgs = 2)
15151527
public abstract static class MayRaiseBinaryNode extends PythonBinaryBuiltinNode {
15161528
@Child private CreateArgumentsNode createArgsNode;
15171529
@Child private InvokeNode invokeNode;
1530+
@Child private MaterializeFrameNode materializeNode;
1531+
15181532
private final PFunction func;
15191533
private final Object errorResult;
15201534

@@ -1533,17 +1547,27 @@ Object doit(VirtualFrame frame, Object arg1, Object arg2) {
15331547
} catch (PException e) {
15341548
// getContext() acts as a branch profile
15351549
getContext().setCurrentException(e);
1536-
e.getExceptionObject().reifyException();
1550+
e.getExceptionObject().reifyException(ensureMaterializeNode().execute(frame, this, true, false), factory());
15371551
return errorResult;
15381552
}
15391553
}
1554+
1555+
private MaterializeFrameNode ensureMaterializeNode() {
1556+
if (materializeNode == null) {
1557+
CompilerDirectives.transferToInterpreterAndInvalidate();
1558+
materializeNode = insert(MaterializeFrameNodeGen.create());
1559+
}
1560+
return materializeNode;
1561+
}
15401562
}
15411563

15421564
// -----------------------------------------------------------------------------------------------------------------
15431565
@Builtin(minNumOfPositionalArgs = 3)
15441566
public abstract static class MayRaiseTernaryNode extends PythonTernaryBuiltinNode {
15451567
@Child private CreateArgumentsNode createArgsNode;
15461568
@Child private InvokeNode invokeNode;
1569+
@Child private MaterializeFrameNode materializeNode;
1570+
15471571
private final PFunction func;
15481572
private final Object errorResult;
15491573

@@ -1562,10 +1586,18 @@ Object doit(VirtualFrame frame, Object arg1, Object arg2, Object arg3) {
15621586
} catch (PException e) {
15631587
// getContext() acts as a branch profile
15641588
getContext().setCurrentException(e);
1565-
e.getExceptionObject().reifyException();
1589+
e.getExceptionObject().reifyException(ensureMaterializeNode().execute(frame, this, true, false), factory());
15661590
return errorResult;
15671591
}
15681592
}
1593+
1594+
private MaterializeFrameNode ensureMaterializeNode() {
1595+
if (materializeNode == null) {
1596+
CompilerDirectives.transferToInterpreterAndInvalidate();
1597+
materializeNode = insert(MaterializeFrameNodeGen.create());
1598+
}
1599+
return materializeNode;
1600+
}
15691601
}
15701602

15711603
// -----------------------------------------------------------------------------------------------------------------
@@ -1574,6 +1606,8 @@ public static class MayRaiseNode extends PythonBuiltinNode {
15741606
@Child private InvokeNode invokeNode;
15751607
@Child private ReadVarArgsNode readVarargsNode;
15761608
@Child private CreateArgumentsNode createArgsNode;
1609+
@Child private MaterializeFrameNode materializeNode;
1610+
15771611
private final PFunction func;
15781612
private final Object errorResult;
15791613

@@ -1594,11 +1628,19 @@ public final Object execute(VirtualFrame frame) {
15941628
} catch (PException e) {
15951629
// getContext() acts as a branch profile
15961630
getContext().setCurrentException(e);
1597-
e.getExceptionObject().reifyException();
1631+
e.getExceptionObject().reifyException(ensureMaterializeNode().execute(frame, this, true, false), factory());
15981632
return errorResult;
15991633
}
16001634
}
16011635

1636+
private MaterializeFrameNode ensureMaterializeNode() {
1637+
if (materializeNode == null) {
1638+
CompilerDirectives.transferToInterpreterAndInvalidate();
1639+
materializeNode = insert(MaterializeFrameNodeGen.create());
1640+
}
1641+
return materializeNode;
1642+
}
1643+
16021644
@Override
16031645
protected ReadArgumentNode[] getArguments() {
16041646
throw new IllegalAccessError();

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/cext/PThreadState.java

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -356,7 +356,6 @@ private static void setCurrentException(PRaiseNode raiseNode, PythonContext cont
356356
try {
357357
throw raiseNode.raise(exceptionObject);
358358
} catch (PException e) {
359-
exceptionObject.reifyException();
360359
context.setCurrentException(e);
361360
}
362361
}
@@ -365,8 +364,7 @@ private static void setCaughtException(PRaiseNode raiseNode, PythonContext conte
365364
try {
366365
throw raiseNode.raise(exceptionObject);
367366
} catch (PException e) {
368-
exceptionObject.reifyException();
369-
context.setCurrentException(e);
367+
context.setCaughtException(e);
370368
}
371369
}
372370

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/cext/PyProcsWrapper.java

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -121,8 +121,15 @@ static Object doGetAttr(GetAttrWrapper object, Object[] arguments,
121121
} catch (PException e) {
122122
// TODO move to node
123123
e.expectAttributeError(errProfile);
124+
125+
// This node cannot have a frame so we cannot neither be sure if the PFrame is
126+
// already available nor we can create it at this point. Furthermore, the last
127+
// Python caller also won't eagerly create the PFrame since this could unnecessarily
128+
// expensive. So, we just need to provide the frame info. This will be enough to
129+
// re-create the current stack at a later point in time.
130+
e.getExceptionObject().reifyException(context.peekTopFrameInfo());
131+
124132
context.setCurrentException(e);
125-
e.getExceptionObject().reifyException();
126133
result = getNativeNullNode.execute();
127134
}
128135
return result;
@@ -165,8 +172,8 @@ static Object doSsize(SsizeargfuncWrapper object, Object[] arguments,
165172
try {
166173
result = toSulongNode.execute(executeNode.execute(object.getDelegate(), converted));
167174
} catch (PException e) {
175+
e.getExceptionObject().reifyException(context.peekTopFrameInfo());
168176
context.setCurrentException(e);
169-
e.getExceptionObject().reifyException();
170177
result = getNativeNullNode.execute();
171178
}
172179
return result;

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/cext/PySequenceMethodsWrapper.java

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@
5050
import com.oracle.graal.python.nodes.attributes.LookupAttributeInMRONode;
5151
import com.oracle.graal.python.runtime.PythonContext;
5252
import com.oracle.graal.python.runtime.exception.PException;
53+
import com.oracle.truffle.api.TruffleLanguage.ContextReference;
5354
import com.oracle.truffle.api.dsl.Cached;
5455
import com.oracle.truffle.api.dsl.CachedContext;
5556
import com.oracle.truffle.api.dsl.ImportStatic;
@@ -104,7 +105,7 @@ protected Object readMember(String member,
104105
@Cached LookupAttributeInMRONode.Dynamic getSqRepeatNode,
105106
@Cached CExtNodes.ToSulongNode toSulongNode,
106107
@Cached BranchProfile errorProfile,
107-
@CachedContext(PythonLanguage.class) PythonContext context,
108+
@CachedContext(PythonLanguage.class) ContextReference<PythonContext> contextRef,
108109
@Cached GetNativeNullNode getNativeNullNode) throws UnknownIdentifierException {
109110
Object result;
110111
try {
@@ -120,8 +121,9 @@ protected Object readMember(String member,
120121
}
121122
} catch (PException e) {
122123
errorProfile.enter();
124+
PythonContext context = contextRef.get();
123125
context.setCurrentException(e);
124-
e.getExceptionObject().reifyException();
126+
e.getExceptionObject().reifyException(context.peekTopFrameInfo());
125127
result = getNativeNullNode.execute(null);
126128
}
127129
return result;

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/exception/PBaseException.java

Lines changed: 48 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@
4040
*/
4141
package com.oracle.graal.python.builtins.objects.exception;
4242

43+
import com.oracle.graal.python.builtins.objects.frame.PFrame;
4344
import com.oracle.graal.python.builtins.objects.object.PythonObject;
4445
import com.oracle.graal.python.builtins.objects.traceback.PTraceback;
4546
import com.oracle.graal.python.builtins.objects.tuple.PTuple;
@@ -48,11 +49,10 @@
4849
import com.oracle.graal.python.nodes.object.GetLazyClassNode;
4950
import com.oracle.graal.python.runtime.exception.PException;
5051
import com.oracle.graal.python.runtime.formatting.ErrorMessageFormatter;
52+
import com.oracle.graal.python.runtime.object.PythonObjectFactory;
5153
import com.oracle.graal.python.runtime.sequence.storage.BasicSequenceStorage;
5254
import com.oracle.graal.python.runtime.sequence.storage.SequenceStorage;
5355
import com.oracle.truffle.api.CompilerAsserts;
54-
import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary;
55-
import com.oracle.truffle.api.TruffleStackTrace;
5656

5757
public final class PBaseException extends PythonObject {
5858

@@ -67,6 +67,9 @@ public final class PBaseException extends PythonObject {
6767
private PException exception;
6868
private PTraceback traceback;
6969

70+
/** The frame info of the Python frame that first caught the exception. */
71+
private PFrame.Reference frameInfo;
72+
7073
public PBaseException(LazyPythonClass cls, PTuple args) {
7174
super(cls);
7275
this.args = args;
@@ -101,6 +104,10 @@ public void clearTraceback() {
101104
this.traceback = null;
102105
}
103106

107+
public PFrame.Reference getFrameInfo() {
108+
return frameInfo;
109+
}
110+
104111
/**
105112
* Can be null in case of lazily formatted arguments.
106113
*/
@@ -146,13 +153,45 @@ public String toString() {
146153
}
147154

148155
/**
149-
* This function must be called before handing out exceptions into the Python value space,
150-
* because otherwise the stack will not be correct if the exception object escapes the current
151-
* function.
156+
* Create the traceback for this exception using the provided {@link PFrame} instance (which
157+
* usually is the frame of the function that caught the exception).
158+
* <p>
159+
* This function (of {@link #reifyException(PFrame.Reference)} must be called before handing out
160+
* exceptions into the Python value space because otherwise the stack will not be correct if the
161+
* exception object escapes the current function.
162+
* </p>
152163
*/
153-
@TruffleBoundary
154-
public void reifyException() {
155-
// TODO: frames: get rid of this entirely
156-
TruffleStackTrace.fillIn(exception);
164+
public void reifyException(PFrame pyFrame, PythonObjectFactory factory) {
165+
traceback = factory.createTraceback(pyFrame, exception);
166+
frameInfo = pyFrame.getRef();
167+
168+
// TODO: frames: provide legacy stack walk method via Python option
169+
// TruffleStackTrace.fillIn(exception);
170+
}
171+
172+
/**
173+
* Associate this exception with a frame info that represents the {@link PFrame} instance that
174+
* caught the exception.<br>
175+
* <p>
176+
* In contrast to {@link #reifyException(PFrame, PythonObjectFactory)}, this method can be used
177+
* if the {@link PFrame} instance isn't already available and if the Truffle frame is also not
178+
* available to create the {@link PFrame} instance using the
179+
* {@link com.oracle.graal.python.nodes.frame.MaterializeFrameNode}.
180+
* </p>
181+
* <p>
182+
* The most common use case for calling this method is when an exception is thrown in some
183+
* Python code but we catch the exception in some interop node (that is certainly adopted by
184+
* some foreign language's root node). In this case, we do not want to eagerly create the
185+
* {@link PFrame} instance when calling from Python to the foreign language since this could be
186+
* expensive. The traceback can then be created lazily from the frame info.
187+
* </p>
188+
*/
189+
public void reifyException(PFrame.Reference curFrameInfo) {
190+
traceback = null;
191+
curFrameInfo.markAsEscaped();
192+
this.frameInfo = curFrameInfo;
193+
194+
// TODO: frames: provide legacy stack walk method via Python option
195+
// TruffleStackTrace.fillIn(exception);
157196
}
158197
}

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/frame/ReadCallerFrameNode.java

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -99,8 +99,7 @@ public PFrame executeWith(VirtualFrame frame, PFrame.Reference startFrameInfo, F
9999
if (callerInfo == null) {
100100
Frame callerFrame = getCallerFrame(startFrameInfo, frameAccess, skipInternal, level);
101101
if (callerFrame != null) {
102-
ensureMaterializeNode().execute(frame, false, true, callerFrame);
103-
return PArguments.getCurrentFrameInfo(callerFrame).getPyFrame();
102+
return ensureMaterializeNode().execute(frame, false, true, callerFrame);
104103
}
105104
return null;
106105
} else if (!(skipInternal && PRootNode.isPythonInternal(callerInfo.getCallNode().getRootNode()))) {

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/statement/ExceptNode.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -188,13 +188,13 @@ private boolean writeResult(VirtualFrame frame, PException e, boolean matches) {
188188
CompilerDirectives.transferToInterpreterAndInvalidate();
189189
materializeFrameNode = insert(MaterializeFrameNodeGen.create());
190190
}
191-
192191
PFrame escapedFrame = materializeFrameNode.execute(frame, this, true, false);
192+
193193
if (factory == null) {
194194
CompilerDirectives.transferToInterpreterAndInvalidate();
195195
factory = insert(PythonObjectFactory.create());
196196
}
197-
exceptionObject.setTraceback(factory.createTraceback(escapedFrame, e));
197+
exceptionObject.reifyException(escapedFrame, factory);
198198
exceptName.doWrite(frame, exceptionObject);
199199
}
200200
return true;

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/runtime/PythonContext.java

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -250,6 +250,10 @@ public PFrame.Reference popTopFrameInfo() {
250250
return ref;
251251
}
252252

253+
public PFrame.Reference peekTopFrameInfo() {
254+
return topframeref;
255+
}
256+
253257
public boolean isInitialized() {
254258
return isInitialized;
255259
}

0 commit comments

Comments
 (0)