Skip to content

Commit db14f75

Browse files
fangererfangerer
committed
Improve PyTruffleErr_Fetch; write to out vars
1 parent 1132b35 commit db14f75

File tree

3 files changed

+92
-44
lines changed

3 files changed

+92
-44
lines changed

‎graalpython/com.oracle.graal.python.cext/src/errors.c

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -253,19 +253,13 @@ _PyErr_Fetch(PyThreadState *tstate, PyObject **p_type, PyObject **p_value,
253253
PyObject **p_traceback)
254254
{
255255
// GraalPy change: different implementation
256-
PyObject* result = GraalPyTruffleErr_Fetch();
257-
if(result == NULL) {
256+
if (_PyErr_Occurred(tstate)) {
257+
// avoid the upcall if there is no current exception
258+
GraalPyTruffleErr_Fetch(p_type, p_value, p_traceback);
259+
} else {
258260
*p_type = NULL;
259261
*p_value = NULL;
260262
*p_traceback = NULL;
261-
} else {
262-
*p_type = PyTuple_GetItem(result, 0);
263-
*p_value = PyTuple_GetItem(result, 1);
264-
*p_traceback = PyTuple_GetItem(result, 2);
265-
Py_XINCREF(*p_type);
266-
Py_XINCREF(*p_value);
267-
Py_XINCREF(*p_traceback);
268-
Py_DecRef(result);
269263
}
270264
}
271265

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/modules/cext/PythonCextErrBuiltins.java

Lines changed: 52 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@
4949
import static com.oracle.graal.python.builtins.objects.cext.capi.transitions.ArgDescriptor.Int;
5050
import static com.oracle.graal.python.builtins.objects.cext.capi.transitions.ArgDescriptor.PyObject;
5151
import static com.oracle.graal.python.builtins.objects.cext.capi.transitions.ArgDescriptor.PyObjectBorrowed;
52+
import static com.oracle.graal.python.builtins.objects.cext.capi.transitions.ArgDescriptor.PyObjectPtr;
5253
import static com.oracle.graal.python.builtins.objects.cext.capi.transitions.ArgDescriptor.PyObjectTransfer;
5354
import static com.oracle.graal.python.builtins.objects.cext.capi.transitions.ArgDescriptor.PyThreadState;
5455
import static com.oracle.graal.python.builtins.objects.cext.capi.transitions.ArgDescriptor.Void;
@@ -83,10 +84,13 @@
8384
import com.oracle.graal.python.builtins.modules.cext.PythonCextFileBuiltins.PyFile_WriteObject;
8485
import com.oracle.graal.python.builtins.objects.PNone;
8586
import com.oracle.graal.python.builtins.objects.cext.capi.CExtNodes.ClearCurrentExceptionNode;
87+
import com.oracle.graal.python.builtins.objects.cext.capi.CExtNodes.PyErrFetchNode;
8688
import com.oracle.graal.python.builtins.objects.cext.capi.CExtNodes.PyErrOccurredNode;
8789
import com.oracle.graal.python.builtins.objects.cext.capi.CExtNodes.TransformExceptionToNativeNode;
8890
import com.oracle.graal.python.builtins.objects.cext.capi.PThreadState;
91+
import com.oracle.graal.python.builtins.objects.cext.capi.transitions.CApiTransitions.PythonToNativeNewRefNode;
8992
import com.oracle.graal.python.builtins.objects.cext.common.NativePointer;
93+
import com.oracle.graal.python.builtins.objects.cext.structs.CStructAccess;
9094
import com.oracle.graal.python.builtins.objects.common.HashingStorageNodes.HashingStorageGetItem;
9195
import com.oracle.graal.python.builtins.objects.dict.DictBuiltins.SetItemNode;
9296
import com.oracle.graal.python.builtins.objects.dict.PDict;
@@ -96,7 +100,6 @@
96100
import com.oracle.graal.python.builtins.objects.function.PKeyword;
97101
import com.oracle.graal.python.builtins.objects.module.PythonModule;
98102
import com.oracle.graal.python.builtins.objects.traceback.LazyTraceback;
99-
import com.oracle.graal.python.builtins.objects.traceback.MaterializeLazyTracebackNode;
100103
import com.oracle.graal.python.builtins.objects.traceback.PTraceback;
101104
import com.oracle.graal.python.builtins.objects.tuple.PTuple;
102105
import com.oracle.graal.python.builtins.objects.tuple.TupleBuiltins;
@@ -116,6 +119,7 @@
116119
import com.oracle.graal.python.nodes.util.ExceptionStateNodes.GetCaughtExceptionNode;
117120
import com.oracle.graal.python.runtime.PythonContext;
118121
import com.oracle.graal.python.runtime.PythonContext.GetThreadStateNode;
122+
import com.oracle.graal.python.runtime.PythonContext.PythonThreadState;
119123
import com.oracle.graal.python.runtime.PythonOptions;
120124
import com.oracle.graal.python.runtime.exception.ExceptionUtils;
121125
import com.oracle.graal.python.runtime.exception.PException;
@@ -188,43 +192,56 @@ static Object run(Object typ, Object val, Object tb,
188192
}
189193
}
190194

191-
@CApiBuiltin(ret = PyObjectTransfer, call = Ignored)
192-
abstract static class PyTruffleErr_Fetch extends CApiNullaryBuiltinNode {
195+
@CApiBuiltin(ret = Void, args = {PyObjectPtr, PyObjectPtr, PyObjectPtr}, call = Ignored)
196+
abstract static class PyTruffleErr_Fetch extends CApiTernaryBuiltinNode {
193197
@Specialization
194-
Object run(
198+
static Object doGeneric(Object pType, Object pValue, Object pTraceback,
195199
@Bind("this") Node inliningTarget,
196200
@Cached GetThreadStateNode getThreadStateNode,
197-
@Cached GetClassNode getClassNode,
198-
@Cached MaterializeLazyTracebackNode materializeTraceback,
199-
@Cached PythonObjectFactory factory,
200-
@Cached ClearCurrentExceptionNode clearCurrentExceptionNode) {
201+
@Cached PyErrFetchNode pyErrFetchNode,
202+
@Cached PythonToNativeNewRefNode toNativeNewRefNode,
203+
@Cached CStructAccess.WritePointerNode writePointerNode) {
201204
PythonContext.PythonThreadState threadState = getThreadStateNode.execute(inliningTarget);
202-
PException currentException = threadState.getCurrentException();
203-
Object result;
204-
if (currentException == null) {
205-
result = getNativeNull();
205+
Object[] exceptionState = pyErrFetchNode.execute(inliningTarget, threadState);
206+
// null or [type, value, traceback]
207+
assert exceptionState == null || exceptionState.length == 3;
208+
if (exceptionState == null) {
209+
/*
210+
* This should be caught in native by checking 'PyErr_Occurred' and avoiding the
211+
* upcall. But let's be defensive and treat that case on a slow path.
212+
*/
213+
doNoException(pType, pValue, pTraceback);
206214
} else {
207-
Object exception = currentException.getEscapedException();
208-
Object traceback = null;
209-
if (threadState.getCurrentTraceback() != null) {
210-
traceback = materializeTraceback.execute(inliningTarget, threadState.getCurrentTraceback());
211-
}
212-
if (traceback == null) {
213-
traceback = getNativeNull();
214-
}
215-
result = factory.createTuple(new Object[]{getClassNode.execute(inliningTarget, exception), exception, traceback});
216-
clearCurrentExceptionNode.execute(inliningTarget, threadState);
215+
assert exceptionState[0] != null;
216+
assert exceptionState[1] != null;
217+
/*
218+
* NOTE: We need cannot use 'WriteObjectNewRefNode' because we are writing to out
219+
* variables (C type 'PyObject **out') where the previous value (i.e. '*out') of
220+
* those is unspecified. 'WriteObjectNewRefNode' would try to decref the previous
221+
* object and we MUST NOT do that. Therefore, we use the combination of
222+
* 'WritePointerNode' and 'PythonToNativeNewRefNode'.
223+
*/
224+
writePointerNode.write(pType, toNativeNewRefNode.execute(exceptionState[0]));
225+
writePointerNode.write(pValue, toNativeNewRefNode.execute(exceptionState[1]));
226+
writePointerNode.write(pTraceback, toNativeNewRefNode.execute(exceptionState[2] != null ? exceptionState[2] : PNone.NO_VALUE));
217227
}
218-
return result;
228+
return PNone.NO_VALUE;
229+
}
230+
231+
@TruffleBoundary
232+
private static void doNoException(Object pType, Object pValue, Object pTraceback) {
233+
CStructAccess.WritePointerNode.writeUncached(pType, 0, 0L);
234+
CStructAccess.WritePointerNode.writeUncached(pValue, 0, 0L);
235+
CStructAccess.WritePointerNode.writeUncached(pTraceback, 0, 0L);
219236
}
220237
}
221238

222239
@CApiBuiltin(ret = PyObjectBorrowed, args = {PyThreadState}, call = Ignored)
223240
abstract static class _PyTruffleErr_Occurred extends CApiUnaryBuiltinNode {
224241
@Specialization
225242
Object run(PThreadState state,
226-
@Bind("this") Node inliningTarget,
227-
@Cached PyErrOccurredNode pyErrOccurredNode) {
243+
@Bind("this") Node inliningTarget,
244+
@Cached PyErrOccurredNode pyErrOccurredNode) {
228245
Object excType = pyErrOccurredNode.execute(inliningTarget, state.getThreadState());
229246
return excType != null ? excType : getNativeNull();
230247
}
@@ -426,31 +443,32 @@ static Object raise(int set_sys_last_vars,
426443
@Cached PyErr_Restore restoreNode,
427444
@Cached PyFile_WriteObject writeFileNode,
428445
@Cached ExitNode exitNode,
429-
@Cached PyTruffleErr_Fetch fetchNode,
430446
@Cached PyErr_Display errDisplayNode) {
431447
PythonContext context = PythonContext.get(null);
432448
NativePointer nativeNull = context.getNativeNull();
433449

434-
Object err = PyErrOccurredNode.executeUncached(context.getThreadState(PythonLanguage.get(null)));
450+
PythonThreadState threadState = context.getThreadState(PythonLanguage.get(null));
451+
Object err = PyErrOccurredNode.executeUncached(threadState);
435452
PythonModule sys = context.getSysModule();
436453
if (err != nativeNull && IsBuiltinObjectProfile.profileObjectUncached(err, PythonBuiltinClassType.SystemExit)) {
437454
handleSystemExit(excInfoNode, getItemNode, isInstanceNode, restoreNode, (SysModuleBuiltins) sys.getBuiltins(), writeFileNode, exitNode);
438455
}
439-
Object fetched = fetchNode.execute(null);
456+
Object[] fetched = PyErrFetchNode.executeUncached(threadState);
457+
// null or [type, value, traceback]
458+
assert fetched == null || fetched.length == 3;
440459
Object type = null;
441460
Object val = null;
442461
Object tb = null;
443462

444-
if (fetched != nativeNull) {
445-
PTuple fetchedTuple = (PTuple) fetched;
446-
type = getItemNode.execute(null, fetchedTuple, 0);
447-
val = getItemNode.execute(null, fetchedTuple, 1);
448-
tb = getItemNode.execute(null, fetchedTuple, 2);
463+
if (fetched != null) {
464+
type = fetched[0];
465+
val = fetched[1];
466+
tb = fetched[2];
449467
}
450468
if (type == null || type == PNone.NONE) {
451469
return PNone.NONE;
452470
}
453-
if (tb == nativeNull) {
471+
if (tb == null) {
454472
tb = PNone.NONE;
455473
}
456474
if (PyObjectLookupAttr.executeUncached(val, T___TRACEBACK__) == PNone.NONE) {

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

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,7 @@
8787
import com.oracle.graal.python.builtins.objects.cext.capi.CExtNodesFactory.AsCharPointerNodeGen;
8888
import com.oracle.graal.python.builtins.objects.cext.capi.CExtNodesFactory.CreateFunctionNodeGen;
8989
import com.oracle.graal.python.builtins.objects.cext.capi.CExtNodesFactory.FromCharPointerNodeGen;
90+
import com.oracle.graal.python.builtins.objects.cext.capi.CExtNodesFactory.PyErrFetchNodeGen;
9091
import com.oracle.graal.python.builtins.objects.cext.capi.CExtNodesFactory.PyErrOccurredNodeGen;
9192
import com.oracle.graal.python.builtins.objects.cext.capi.CExtNodesFactory.ResolvePointerNodeGen;
9293
import com.oracle.graal.python.builtins.objects.cext.capi.ExternalFunctionNodes.DefaultCheckFunctionResultNode;
@@ -125,6 +126,7 @@
125126
import com.oracle.graal.python.builtins.objects.object.PythonObject;
126127
import com.oracle.graal.python.builtins.objects.str.PString;
127128
import com.oracle.graal.python.builtins.objects.traceback.LazyTraceback;
129+
import com.oracle.graal.python.builtins.objects.traceback.MaterializeLazyTracebackNode;
128130
import com.oracle.graal.python.builtins.objects.type.PythonAbstractClass;
129131
import com.oracle.graal.python.builtins.objects.type.PythonBuiltinClass;
130132
import com.oracle.graal.python.builtins.objects.type.PythonClass;
@@ -1103,6 +1105,40 @@ static Object doGeneric(Node inliningTarget, PythonThreadState threadState,
11031105
}
11041106
}
11051107

1108+
@GenerateInline
1109+
@GenerateCached(false)
1110+
@GenerateUncached
1111+
public abstract static class PyErrFetchNode extends Node {
1112+
1113+
public static Object[] executeUncached(PythonThreadState threadState) {
1114+
return PyErrFetchNodeGen.getUncached().execute(null, threadState);
1115+
}
1116+
1117+
public abstract Object[] execute(Node inliningTarget, PythonThreadState threadState);
1118+
1119+
@Specialization
1120+
static Object[] doGeneric(Node inliningTarget, PythonThreadState threadState,
1121+
@Cached GetClassNode getClassNode,
1122+
@Cached MaterializeLazyTracebackNode materializeTraceback,
1123+
@Cached ClearCurrentExceptionNode clearCurrentExceptionNode) {
1124+
PException currentException = threadState.getCurrentException();
1125+
if (currentException == null) {
1126+
/*
1127+
* This should be caught in native by checking 'PyErr_Occurred' and avoiding the
1128+
* upcall. But let's be defensive and treat that case on a slow path.
1129+
*/
1130+
return null;
1131+
}
1132+
Object exception = currentException.getEscapedException();
1133+
Object traceback = null;
1134+
if (threadState.getCurrentTraceback() != null) {
1135+
traceback = materializeTraceback.execute(inliningTarget, threadState.getCurrentTraceback());
1136+
}
1137+
clearCurrentExceptionNode.execute(inliningTarget, threadState);
1138+
return new Object[]{getClassNode.execute(inliningTarget, exception), exception, traceback};
1139+
}
1140+
}
1141+
11061142
@GenerateUncached
11071143
@GenerateCached
11081144
@GenerateInline(false)

0 commit comments

Comments
 (0)