Skip to content

Commit 9bceb00

Browse files
steve-ssteve-s
committed
[GR-52717] Refactor ExternalFunctionInvokeNode.
PullRequest: graalpython/3250
2 parents 32a6d9e + 3cf22ac commit 9bceb00

File tree

7 files changed

+96
-45
lines changed

7 files changed

+96
-45
lines changed

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

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -230,8 +230,7 @@ long start(VirtualFrame frame, Object cls, Object callable, Object args, Object
230230
// TODO: python thread stack size != java thread stack size
231231
// ignore setting the stack size for the moment
232232
TruffleThreadBuilder threadBuilder = env.newTruffleThreadBuilder(() -> {
233-
GilNode.UncachedAcquire gil = GilNode.uncachedAcquire();
234-
try {
233+
try (GilNode.UncachedAcquire gil = GilNode.uncachedAcquire()) {
235234
// the increment is protected by the gil
236235
int curCount = (int) HiddenAttr.ReadNode.executeUncached(threadModule, THREAD_COUNT, 0);
237236
HiddenAttr.WriteNode.executeUncached(threadModule, THREAD_COUNT, curCount + 1);
@@ -252,8 +251,6 @@ long start(VirtualFrame frame, Object cls, Object callable, Object args, Object
252251
curCount = (int) HiddenAttr.ReadNode.executeUncached(threadModule, THREAD_COUNT, 1);
253252
HiddenAttr.WriteNode.executeUncached(threadModule, THREAD_COUNT, curCount - 1);
254253
}
255-
} finally {
256-
gil.close();
257254
}
258255
}).context(env.getContext()).threadGroup(context.getThreadGroup());
259256

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

Lines changed: 83 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,7 @@
7272
import com.oracle.graal.python.builtins.objects.cext.capi.CExtNodesFactory.ReleaseNativeWrapperNodeGen;
7373
import com.oracle.graal.python.builtins.objects.cext.capi.ExternalFunctionNodesFactory.CreateArgsTupleNodeGen;
7474
import com.oracle.graal.python.builtins.objects.cext.capi.ExternalFunctionNodesFactory.DefaultCheckFunctionResultNodeGen;
75+
import com.oracle.graal.python.builtins.objects.cext.capi.ExternalFunctionNodesFactory.ExternalFunctionInvokeNodeGen;
7576
import com.oracle.graal.python.builtins.objects.cext.capi.ExternalFunctionNodesFactory.MaterializePrimitiveNodeGen;
7677
import com.oracle.graal.python.builtins.objects.cext.capi.ExternalFunctionNodesFactory.ReleaseNativeSequenceStorageNodeGen;
7778
import com.oracle.graal.python.builtins.objects.cext.capi.PythonNativeWrapper.PythonAbstractObjectNativeWrapper;
@@ -143,6 +144,7 @@
143144
import com.oracle.truffle.api.dsl.Cached.Exclusive;
144145
import com.oracle.truffle.api.dsl.Cached.Shared;
145146
import com.oracle.truffle.api.dsl.Fallback;
147+
import com.oracle.truffle.api.dsl.GenerateCached;
146148
import com.oracle.truffle.api.dsl.GenerateInline;
147149
import com.oracle.truffle.api.dsl.GenerateUncached;
148150
import com.oracle.truffle.api.dsl.ImportStatic;
@@ -830,61 +832,46 @@ public static MethDirectRoot create(PythonLanguage lang, TruffleString name, PEx
830832
* Like {@link com.oracle.graal.python.nodes.call.FunctionInvokeNode} but invokes a C function.
831833
*/
832834
@GenerateUncached
833-
@GenerateInline(false)
835+
@GenerateCached(false)
836+
@GenerateInline
834837
public abstract static class ExternalFunctionInvokeNode extends PNodeWithContext {
835-
public abstract Object execute(VirtualFrame frame, PExternalFunctionWrapper provider, CApiTiming timing, TruffleString name, Object callable, Object[] cArguments);
836-
837-
static CheckFunctionResultNode createCheckResultNode(PExternalFunctionWrapper provider) {
838-
CheckFunctionResultNode node = provider.createCheckFunctionResultNode();
839-
return node != null ? node : DefaultCheckFunctionResultNodeGen.create();
840-
}
838+
abstract Object execute(VirtualFrame frame, Node inliningTarget, PythonContext ctx, PythonThreadState threadState, CApiTiming timing, TruffleString name, Object callable, Object[] cArguments);
841839

842-
static CheckFunctionResultNode getUncachedCheckResultNode(PExternalFunctionWrapper provider) {
843-
CheckFunctionResultNode node = provider.getUncachedCheckFunctionResultNode();
844-
return node != null ? node : DefaultCheckFunctionResultNodeGen.getUncached();
840+
public final Object call(VirtualFrame frame, Node inliningTarget, PythonContext ctx, PythonThreadState threadState, CApiTiming timing, TruffleString name, Object callable,
841+
Object... cArguments) {
842+
return execute(frame, inliningTarget, ctx, threadState, timing, name, callable, cArguments);
845843
}
846844

847-
@Specialization(guards = {"provider == cachedProvider"}, limit = "1")
848-
static Object invoke(VirtualFrame frame, PExternalFunctionWrapper provider, CApiTiming timing, TruffleString name, Object callable, Object[] cArguments,
849-
@Bind("this") Node inliningTarget,
850-
@Cached("provider") PExternalFunctionWrapper cachedProvider,
851-
@Cached(value = "createCheckResultNode(provider)", uncached = "getUncachedCheckResultNode(provider)") CheckFunctionResultNode checkResultNode,
852-
@Cached(value = "provider.createConvertRetNode()", uncached = "provider.getUncachedConvertRetNode()") CExtToJavaNode convertReturnValue,
853-
@Cached PForeignToPTypeNode fromForeign,
854-
@Cached GetThreadStateNode getThreadStateNode,
855-
@Cached GilNode gilNode,
845+
@Specialization
846+
static Object invoke(VirtualFrame frame, Node inliningTarget, PythonContext ctx, PythonThreadState threadState, CApiTiming timing, TruffleString name, Object callable, Object[] cArguments,
847+
@Cached(inline = false) GilNode gilNode,
856848
@Cached(value = "createFor(this)", uncached = "getUncached()") IndirectCallData indirectCallData,
857849
@CachedLibrary(limit = "2") InteropLibrary lib) {
858-
PythonContext ctx = PythonContext.get(inliningTarget);
859-
PythonThreadState threadState = getThreadStateNode.execute(inliningTarget, ctx);
860850

861851
// If any code requested the caught exception (i.e. used 'sys.exc_info()'), we store
862852
// it to the context since we cannot propagate it through the native frames.
863853
Object state = IndirectCallContext.enter(frame, threadState, indirectCallData);
864854

865855
CApiTiming.enter();
866856
try {
867-
Object result = lib.execute(callable, cArguments);
868-
result = checkResultNode.execute(threadState, name, result);
869-
if (convertReturnValue != null) {
870-
result = convertReturnValue.execute(result);
871-
}
872-
return fromForeign.executeConvert(result);
857+
return lib.execute(callable, cArguments);
873858
} catch (UnsupportedTypeException | UnsupportedMessageException e) {
874859
CompilerDirectives.transferToInterpreterAndInvalidate();
875860
throw PRaiseNode.raiseUncached(inliningTarget, TypeError, ErrorMessages.CALLING_NATIVE_FUNC_FAILED, name, e);
876861
} catch (ArityException e) {
877862
CompilerDirectives.transferToInterpreterAndInvalidate();
878863
throw PRaiseNode.raiseUncached(inliningTarget, TypeError, ErrorMessages.CALLING_NATIVE_FUNC_EXPECTED_ARGS, name, e.getExpectedMinArity(), e.getActualArity());
879-
} finally {
880-
CApiTiming.exit(timing);
864+
} catch (Throwable exception) {
881865
/*
882866
* Always re-acquire the GIL here. This is necessary because it could happen that C
883867
* extensions are releasing the GIL and if then an LLVM exception occurs, C code
884868
* wouldn't re-acquire it (unexpectedly).
885869
*/
886-
gilNode.acquire();
887-
870+
CompilerDirectives.transferToInterpreterAndInvalidate();
871+
GilNode.uncachedAcquire();
872+
throw exception;
873+
} finally {
874+
CApiTiming.exit(timing);
888875
/*
889876
* Special case after calling a C function: transfer caught exception back to frame
890877
* to simulate the global state semantics.
@@ -895,14 +882,73 @@ static Object invoke(VirtualFrame frame, PExternalFunctionWrapper provider, CApi
895882
IndirectCallContext.exit(frame, threadState, state);
896883
}
897884
}
885+
}
886+
887+
/**
888+
* Wraps {@link ExternalFunctionInvokeNode} with result checking and conversion according to the
889+
* passed {@link PExternalFunctionWrapper}. This node assumes that the provider argument is in
890+
* the cached case a PE constant.
891+
*/
892+
@GenerateInline(false)
893+
public abstract static class ExternalFunctionWrapperInvokeNode extends PNodeWithContext {
894+
public abstract Object execute(VirtualFrame frame, PExternalFunctionWrapper provider, CApiTiming timing, TruffleString name, Object callable, Object[] cArguments);
895+
896+
@NeverDefault
897+
static CheckFunctionResultNode createCheckResultNode(PExternalFunctionWrapper provider) {
898+
CheckFunctionResultNode node = provider.createCheckFunctionResultNode();
899+
return node != null ? node : DefaultCheckFunctionResultNodeGen.create();
900+
}
901+
902+
static CheckFunctionResultNode getUncachedCheckResultNode(PExternalFunctionWrapper provider) {
903+
CheckFunctionResultNode node = provider.getUncachedCheckFunctionResultNode();
904+
return node != null ? node : DefaultCheckFunctionResultNodeGen.getUncached();
905+
}
906+
907+
@Specialization
908+
static Object invokeCached(VirtualFrame frame, PExternalFunctionWrapper provider, CApiTiming timing, TruffleString name, Object callable, Object[] cArguments,
909+
@Bind("this") Node inliningTarget,
910+
@Cached("createCheckResultNode(provider)") CheckFunctionResultNode checkResultNode,
911+
@SuppressWarnings("truffle-neverdefault") @Cached("provider.createConvertRetNode()") CExtToJavaNode convertReturnValue,
912+
@Cached PForeignToPTypeNode fromForeign,
913+
@Cached GetThreadStateNode getThreadStateNode,
914+
@Cached ExternalFunctionInvokeNode invokeNode) {
915+
CompilerAsserts.partialEvaluationConstant(provider);
916+
PythonContext ctx = PythonContext.get(inliningTarget);
917+
return invoke(frame, ctx, timing, name, callable, cArguments, inliningTarget, checkResultNode, convertReturnValue, fromForeign, getThreadStateNode, invokeNode);
918+
}
919+
920+
private static Object invoke(VirtualFrame frame, PythonContext ctx, CApiTiming timing, TruffleString name, Object callable, Object[] cArguments, Node inliningTarget,
921+
CheckFunctionResultNode checkResultNode, CExtToJavaNode convertReturnValue, PForeignToPTypeNode fromForeign, GetThreadStateNode getThreadStateNode,
922+
ExternalFunctionInvokeNode invokeNode) {
923+
PythonThreadState threadState = getThreadStateNode.execute(inliningTarget, ctx);
924+
Object result = invokeNode.execute(frame, inliningTarget, ctx, threadState, timing, name, callable, cArguments);
925+
result = checkResultNode.execute(threadState, name, result);
926+
if (convertReturnValue != null) {
927+
result = convertReturnValue.execute(result);
928+
}
929+
return fromForeign.executeConvert(result);
930+
}
931+
932+
@GenerateCached(false)
933+
private static final class ExternalFunctionWrapperInvokeNodeUncached extends ExternalFunctionWrapperInvokeNode {
934+
private static final ExternalFunctionWrapperInvokeNodeUncached INSTANCE = new ExternalFunctionWrapperInvokeNodeUncached();
935+
936+
@Override
937+
public Object execute(VirtualFrame frame, PExternalFunctionWrapper provider, CApiTiming timing, TruffleString name, Object callable, Object[] cArguments) {
938+
CompilerDirectives.transferToInterpreterAndInvalidate();
939+
PythonContext ctx = PythonContext.get(null);
940+
return invoke(frame, ctx, timing, name, callable, cArguments, null, getUncachedCheckResultNode(provider), provider.getUncachedConvertRetNode(), PForeignToPTypeNode.getUncached(),
941+
GetThreadStateNode.getUncached(), ExternalFunctionInvokeNodeGen.getUncached());
942+
}
943+
}
898944

899945
@NeverDefault
900-
public static ExternalFunctionInvokeNode create() {
901-
return ExternalFunctionNodesFactory.ExternalFunctionInvokeNodeGen.create();
946+
public static ExternalFunctionWrapperInvokeNode create() {
947+
return ExternalFunctionNodesFactory.ExternalFunctionWrapperInvokeNodeGen.create();
902948
}
903949

904-
public static ExternalFunctionInvokeNode getUncached() {
905-
return ExternalFunctionNodesFactory.ExternalFunctionInvokeNodeGen.getUncached();
950+
public static ExternalFunctionWrapperInvokeNode getUncached() {
951+
return ExternalFunctionWrapperInvokeNodeUncached.INSTANCE;
906952
}
907953
}
908954

@@ -911,7 +957,7 @@ abstract static class MethodDescriptorRoot extends PRootNode {
911957
private final CApiTiming timing;
912958
@Child private CalleeContext calleeContext = CalleeContext.create();
913959
@Child private CallVarargsMethodNode invokeNode;
914-
@Child private ExternalFunctionInvokeNode externalInvokeNode;
960+
@Child private ExternalFunctionWrapperInvokeNode externalInvokeNode;
915961
@Child private ReadIndexedArgumentNode readSelfNode;
916962
@Child private ReadIndexedArgumentNode readCallableNode;
917963
@Child private ReleaseNativeWrapperNode releaseNativeWrapperNode;
@@ -930,7 +976,7 @@ abstract static class MethodDescriptorRoot extends PRootNode {
930976
this.timing = CApiTiming.create(true, name);
931977
this.provider = provider;
932978
if (provider != null) {
933-
this.externalInvokeNode = ExternalFunctionInvokeNode.create();
979+
this.externalInvokeNode = ExternalFunctionWrapperInvokeNode.create();
934980
this.convertArgs = provider.createConvertArgNodes();
935981
} else {
936982
this.invokeNode = CallVarargsMethodNode.create();

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/lib/PySequenceDelItemNode.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@
4747
import com.oracle.graal.python.builtins.objects.cext.PythonAbstractNativeObject;
4848
import com.oracle.graal.python.builtins.objects.cext.capi.CApiContext;
4949
import com.oracle.graal.python.builtins.objects.cext.capi.ExternalFunctionNodes;
50+
import com.oracle.graal.python.builtins.objects.cext.capi.ExternalFunctionNodes.ExternalFunctionWrapperInvokeNode;
5051
import com.oracle.graal.python.builtins.objects.cext.capi.NativeCAPISymbol;
5152
import com.oracle.graal.python.builtins.objects.cext.capi.transitions.CApiTiming;
5253
import com.oracle.graal.python.builtins.objects.cext.capi.transitions.CApiTransitions;
@@ -110,7 +111,7 @@ static Object doGenericManaged(VirtualFrame frame, Object object, int index,
110111
static Object doNative(VirtualFrame frame, PythonAbstractNativeObject object, int index,
111112
@Bind("this") Node inliningTarget,
112113
@Cached CApiTransitions.PythonToNativeNode toNativeNode,
113-
@Cached ExternalFunctionNodes.ExternalFunctionInvokeNode invokeNode) {
114+
@Cached ExternalFunctionWrapperInvokeNode invokeNode) {
114115
Object executable = CApiContext.getNativeSymbol(inliningTarget, SYMBOL);
115116
return invokeNode.execute(frame, DELITEM, C_API_TIMING, SYMBOL.getTsName(), executable, new Object[]{toNativeNode.execute(object), index});
116117
}

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/lib/PySequenceGetItemNode.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@
4747
import com.oracle.graal.python.builtins.objects.cext.PythonAbstractNativeObject;
4848
import com.oracle.graal.python.builtins.objects.cext.capi.CApiContext;
4949
import com.oracle.graal.python.builtins.objects.cext.capi.ExternalFunctionNodes;
50+
import com.oracle.graal.python.builtins.objects.cext.capi.ExternalFunctionNodes.ExternalFunctionWrapperInvokeNode;
5051
import com.oracle.graal.python.builtins.objects.cext.capi.NativeCAPISymbol;
5152
import com.oracle.graal.python.builtins.objects.cext.capi.transitions.CApiTiming;
5253
import com.oracle.graal.python.builtins.objects.cext.capi.transitions.CApiTransitions;
@@ -110,7 +111,7 @@ static Object doGenericManaged(VirtualFrame frame, Object object, int index,
110111
static Object doNative(VirtualFrame frame, PythonAbstractNativeObject object, int index,
111112
@Bind("this") Node inliningTarget,
112113
@Cached CApiTransitions.PythonToNativeNode toNativeNode,
113-
@Cached ExternalFunctionNodes.ExternalFunctionInvokeNode invokeNode) {
114+
@Cached ExternalFunctionWrapperInvokeNode invokeNode) {
114115
Object executable = CApiContext.getNativeSymbol(inliningTarget, SYMBOL);
115116
return invokeNode.execute(frame, GETITEM, C_API_TIMING, SYMBOL.getTsName(), executable, new Object[]{toNativeNode.execute(object), index});
116117
}

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/lib/PySequenceSetItemNode.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@
4747
import com.oracle.graal.python.builtins.objects.cext.PythonAbstractNativeObject;
4848
import com.oracle.graal.python.builtins.objects.cext.capi.CApiContext;
4949
import com.oracle.graal.python.builtins.objects.cext.capi.ExternalFunctionNodes;
50+
import com.oracle.graal.python.builtins.objects.cext.capi.ExternalFunctionNodes.ExternalFunctionWrapperInvokeNode;
5051
import com.oracle.graal.python.builtins.objects.cext.capi.NativeCAPISymbol;
5152
import com.oracle.graal.python.builtins.objects.cext.capi.transitions.CApiTiming;
5253
import com.oracle.graal.python.builtins.objects.cext.capi.transitions.CApiTransitions;
@@ -110,7 +111,7 @@ static Object doGenericManaged(VirtualFrame frame, Object object, int index, Obj
110111
static Object doNative(VirtualFrame frame, PythonAbstractNativeObject object, int index, Object value,
111112
@Bind("this") Node inliningTarget,
112113
@Cached CApiTransitions.PythonToNativeNode toNativeNode,
113-
@Cached ExternalFunctionNodes.ExternalFunctionInvokeNode invokeNode) {
114+
@Cached ExternalFunctionWrapperInvokeNode invokeNode) {
114115
Object executable = CApiContext.getNativeSymbol(inliningTarget, SYMBOL);
115116
return invokeNode.execute(frame, SETITEM, C_API_TIMING, SYMBOL.getTsName(), executable, new Object[]{toNativeNode.execute(object), index, toNativeNode.execute(value)});
116117
}

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/lib/PySequenceSizeNode.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,7 @@ static long doGenericManaged(VirtualFrame frame, Object object,
125125
static long doNative(VirtualFrame frame, PythonAbstractNativeObject object,
126126
@Bind("this") Node inliningTarget,
127127
@Cached CApiTransitions.PythonToNativeNode toNativeNode,
128-
@Cached ExternalFunctionNodes.ExternalFunctionInvokeNode invokeNode) {
128+
@Cached ExternalFunctionNodes.ExternalFunctionWrapperInvokeNode invokeNode) {
129129
Object executable = CApiContext.getNativeSymbol(inliningTarget, SYMBOL);
130130
Object size = invokeNode.execute(frame, LENFUNC, C_API_TIMING, SYMBOL.getTsName(), executable, new Object[]{toNativeNode.execute(object)});
131131
assert PGuards.isInteger(size);

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -159,6 +159,7 @@
159159
import com.oracle.graal.python.nodes.statement.AbstractImportNode;
160160
import com.oracle.graal.python.nodes.util.CastToJavaIntLossyNode;
161161
import com.oracle.graal.python.runtime.AsyncHandler.AsyncAction;
162+
import com.oracle.graal.python.runtime.PythonContextFactory.GetThreadStateNodeGen;
162163
import com.oracle.graal.python.runtime.exception.ExceptionUtils;
163164
import com.oracle.graal.python.runtime.exception.PException;
164165
import com.oracle.graal.python.runtime.exception.PythonThreadKillException;
@@ -612,6 +613,10 @@ private static final class AtExitHook {
612613
@GenerateInline(inlineByDefault = true)
613614
public abstract static class GetThreadStateNode extends Node {
614615

616+
public static GetThreadStateNode getUncached() {
617+
return GetThreadStateNodeGen.getUncached();
618+
}
619+
615620
public abstract PythonThreadState execute(Node inliningTarget, PythonContext context);
616621

617622
public final PythonThreadState execute(Node inliningTarget) {

0 commit comments

Comments
 (0)