Skip to content

Commit 9d47fd1

Browse files
msimacekmsimacek
committed
Improve handling generator exception state
1 parent 80d45ed commit 9d47fd1

File tree

3 files changed

+53
-55
lines changed

3 files changed

+53
-55
lines changed

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/bytecode/PBytecodeGeneratorRootNode.java

Lines changed: 0 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,6 @@
4545
import com.oracle.graal.python.builtins.objects.function.Signature;
4646
import com.oracle.graal.python.nodes.PRootNode;
4747
import com.oracle.graal.python.runtime.ExecutionContext;
48-
import com.oracle.graal.python.runtime.exception.PException;
4948
import com.oracle.truffle.api.CompilerAsserts;
5049
import com.oracle.truffle.api.CompilerDirectives;
5150
import com.oracle.truffle.api.CompilerDirectives.CompilationFinal;
@@ -188,17 +187,6 @@ private void copyOSRStackRemainderIntoVirtualFrame(MaterializedFrame generatorFr
188187
public Object execute(VirtualFrame frame) {
189188
calleeContext.enter(frame);
190189
MaterializedFrame generatorFrame = PArguments.getGeneratorFrame(frame);
191-
/*
192-
* This copying of exceptions is necessary because we need to remember the exception state
193-
* in the generator, but we don't want to remember the state that is "inherited" from the
194-
* outer frame as that can change with each invocation. The values are copied back in
195-
* YIELD_VALUE (because the stack pointer is still PE-constant there, so we can explode the
196-
* loop). In interpreter, we use the materialized frame directly, but we still profile the
197-
* incoming types.
198-
*/
199-
PException localException = PArguments.getException(generatorFrame);
200-
PException outerException = PArguments.getException(frame);
201-
PArguments.setException(frame, localException == null ? outerException : localException);
202190
Frame stackFrame;
203191
/*
204192
* Using the materialized frame as stack would be bad for compiled performance, so we copy
@@ -220,10 +208,6 @@ public Object execute(VirtualFrame frame) {
220208
return rootNode.executeFromBci(frame, generatorFrame, stackFrame, this, resumeBci, resumeStackTop);
221209
} finally {
222210
calleeContext.exit(frame, this);
223-
PException exception = PArguments.getException(frame);
224-
if (exception != outerException && exception != PException.NO_EXCEPTION) {
225-
PArguments.setException(generatorFrame, exception);
226-
}
227211
}
228212
}
229213

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/bytecode/PBytecodeRootNode.java

Lines changed: 51 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -754,6 +754,18 @@ Object executeFromBci(VirtualFrame virtualFrame, Frame localFrame, Frame stackFr
754754
String[] localNames = names;
755755
Node[] localNodes = adoptedNodes;
756756

757+
/*
758+
* This separate tracking of local exception is necessary to make exception state saving
759+
* work in generators. On one hand we need to retain the exception that was caught in the
760+
* generator, on the other hand we don't want to retain the exception state that was passed
761+
* from the outer frame because that changes with every resume.
762+
*/
763+
PException localException = null;
764+
765+
// We initialize this lazily when pushing exception state
766+
boolean fetchedException = false;
767+
PException outerException = null;
768+
757769
CompilerAsserts.partialEvaluationConstant(localBC);
758770
CompilerAsserts.partialEvaluationConstant(bci);
759771
CompilerAsserts.partialEvaluationConstant(stackTop);
@@ -1261,34 +1273,32 @@ Object executeFromBci(VirtualFrame virtualFrame, Frame localFrame, Frame stackFr
12611273
if (!(exception instanceof PException)) {
12621274
throw CompilerDirectives.shouldNotReachHere("interop exception state not implemented");
12631275
}
1264-
stackFrame.setObject(stackTop++, PArguments.getException(virtualFrame));
1265-
PArguments.setException(virtualFrame, (PException) exception);
1276+
if (!fetchedException) {
1277+
outerException = PArguments.getException(virtualFrame);
1278+
fetchedException = true;
1279+
}
1280+
stackFrame.setObject(stackTop++, localException);
1281+
localException = (PException) exception;
1282+
PArguments.setException(virtualFrame, localException);
12661283
stackFrame.setObject(stackTop, exception);
12671284
break;
12681285
}
12691286
case OpCodesConstants.POP_EXCEPT: {
1270-
Object savedException = stackFrame.getObject(stackTop);
1271-
if (savedException == null) {
1272-
stackTop--;
1273-
PArguments.setException(virtualFrame, null);
1274-
} else {
1275-
if (!(savedException instanceof PException)) {
1276-
throw CompilerDirectives.shouldNotReachHere("interop exception state not implemented");
1277-
}
1278-
stackFrame.setObject(stackTop--, null);
1279-
PArguments.setException(virtualFrame, (PException) savedException);
1280-
}
1287+
localException = popExceptionState(virtualFrame, stackFrame.getObject(stackTop), outerException);
1288+
stackFrame.setObject(stackTop--, null);
12811289
break;
12821290
}
12831291
case OpCodesConstants.END_EXC_HANDLER: {
1284-
throw bytecodeEndExcHandler(virtualFrame, stackFrame, stackTop);
1292+
localException = popExceptionState(virtualFrame, stackFrame.getObject(stackTop - 1), outerException);
1293+
throw bytecodeEndExcHandler(stackFrame, stackTop);
12851294
}
12861295
case OpCodesConstants.YIELD_VALUE: {
12871296
if (CompilerDirectives.inInterpreter() && loopCount > 0) {
12881297
LoopNode.reportLoopCount(this, loopCount);
12891298
}
12901299
Object value = stackFrame.getObject(stackTop);
12911300
stackFrame.setObject(stackTop--, null);
1301+
PArguments.setException(PArguments.getGeneratorFrame(virtualFrame), localException);
12921302
// See PBytecodeGeneratorRootNode#execute
12931303
if (localFrame != stackFrame) {
12941304
copyStackSlotsToGeneratorFrame(stackFrame, localFrame, stackTop);
@@ -1305,6 +1315,10 @@ Object executeFromBci(VirtualFrame virtualFrame, Frame localFrame, Frame stackFr
13051315
ThrowData throwData = (ThrowData) sendValue;
13061316
throw PException.fromObject(throwData.pythonException, this, throwData.withJavaStacktrace);
13071317
}
1318+
localException = PArguments.getException(PArguments.getGeneratorFrame(virtualFrame));
1319+
if (localException != null) {
1320+
PArguments.setException(virtualFrame, localException);
1321+
}
13081322
stackFrame.setObject(++stackTop, sendValue);
13091323
break;
13101324
}
@@ -1375,13 +1389,17 @@ Object executeFromBci(VirtualFrame virtualFrame, Frame localFrame, Frame stackFr
13751389
}
13761390
long newTarget = findHandler(bci);
13771391
CompilerAsserts.partialEvaluationConstant(newTarget);
1378-
if (getCaughtExceptionNode == null) {
1379-
CompilerDirectives.transferToInterpreterAndInvalidate();
1380-
getCaughtExceptionNode = ExceptionStateNodes.GetCaughtExceptionNode.create();
1381-
}
1382-
PException exceptionState = getCaughtExceptionNode.execute(virtualFrame);
1383-
if (exceptionState != null) {
1384-
ExceptionHandlingStatementNode.chainExceptions(pe.getUnreifiedException(), exceptionState, exceptionChainProfile1, exceptionChainProfile2);
1392+
if (localException != null) {
1393+
ExceptionHandlingStatementNode.chainExceptions(pe.getUnreifiedException(), localException, exceptionChainProfile1, exceptionChainProfile2);
1394+
} else {
1395+
if (getCaughtExceptionNode == null) {
1396+
CompilerDirectives.transferToInterpreterAndInvalidate();
1397+
getCaughtExceptionNode = ExceptionStateNodes.GetCaughtExceptionNode.create();
1398+
}
1399+
PException exceptionState = getCaughtExceptionNode.execute(virtualFrame);
1400+
if (exceptionState != null) {
1401+
ExceptionHandlingStatementNode.chainExceptions(pe.getUnreifiedException(), exceptionState, exceptionChainProfile1, exceptionChainProfile2);
1402+
}
13851403
}
13861404
if (newTarget == -1) {
13871405
// For tracebacks
@@ -1527,18 +1545,20 @@ private int bytecodeClosureFromStack(Frame stackFrame, int stackTop, int oparg)
15271545
return stackTop;
15281546
}
15291547

1530-
private PException bytecodeEndExcHandler(VirtualFrame virtualFrame, Frame stackFrame, int stackTop) {
1531-
Object exception = stackFrame.getObject(stackTop);
1532-
Object savedException = stackFrame.getObject(stackTop - 1);
1548+
private PException popExceptionState(VirtualFrame virtualFrame, Object savedException, PException outerException) {
1549+
PException localException = null;
1550+
if (savedException instanceof PException) {
1551+
localException = (PException) savedException;
1552+
}
15331553
if (savedException == null) {
1534-
PArguments.setException(virtualFrame, null);
1535-
} else {
1536-
if (!(savedException instanceof PException)) {
1537-
throw CompilerDirectives.shouldNotReachHere("interop exception state not implemented");
1538-
}
1539-
stackFrame.setObject(stackTop, null);
1540-
PArguments.setException(virtualFrame, (PException) savedException);
1554+
savedException = outerException;
15411555
}
1556+
PArguments.setException(virtualFrame, (PException) savedException);
1557+
return localException;
1558+
}
1559+
1560+
private PException bytecodeEndExcHandler(Frame stackFrame, int stackTop) {
1561+
Object exception = stackFrame.getObject(stackTop);
15421562
if (exception instanceof PException) {
15431563
throw ((PException) exception).getExceptionForReraise();
15441564
} else if (exception instanceof AbstractTruffleException) {

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/util/ExceptionStateNodes.java

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -109,14 +109,8 @@ public PException execute(VirtualFrame frame) {
109109
if (e == null) {
110110
e = PException.NO_EXCEPTION;
111111
}
112-
/*
113-
* Set into frame to avoid doing the stack walk again. We cannot do this for
114-
* generators because exception state inherited from outer frame needs to be treated
115-
* differently from local exception state.
116-
*/
117-
if (PArguments.getGeneratorFrame(frame) == null) {
118-
PArguments.setException(frame, e);
119-
}
112+
// Set into frame to avoid doing the stack walk again
113+
PArguments.setException(frame, e);
120114
}
121115
return ensure(e);
122116
}

0 commit comments

Comments
 (0)