Skip to content

Commit ae29eaf

Browse files
fangererfangerer
committed
[GR-16437] Properly call async handler root node.
PullRequest: graalpython/540
2 parents bb48d46 + 2709182 commit ae29eaf

File tree

7 files changed

+160
-37
lines changed

7 files changed

+160
-37
lines changed

‎graalpython/com.oracle.graal.python.test/src/tests/test_weakref.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@
3737
# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
3838
# SOFTWARE.
3939

40-
MAX_WAIT_COUNT = 100
40+
MAX_WAIT_COUNT = 500
4141

4242
import sys
4343

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/function/PArguments.java

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -309,10 +309,6 @@ public static void setGeneratorFrame(Object[] arguments, MaterializedFrame gener
309309
arguments[INDEX_GENERATOR_FRAME] = generatorFrame;
310310
}
311311

312-
public static void setCallerFrame(Object[] arguments, Frame callerFrame) {
313-
arguments[INDEX_CALLER_FRAME_INFO] = callerFrame;
314-
}
315-
316312
public static void setControlData(Object[] arguments, GeneratorControlData generatorArguments) {
317313
MaterializedFrame generatorFrame = (MaterializedFrame) arguments[INDEX_GENERATOR_FRAME];
318314
generatorFrame.getArguments()[INDEX_GENERATOR_FRAME] = generatorArguments;

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/call/CallNode.java

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -226,9 +226,6 @@ public Object execute(VirtualFrame frame, Object callableObject, Object[] args,
226226
}
227227
return CallVarargsMethodNode.getUncached().execute(frame, attrCall, PositionalArgumentsNode.prependArgument(callableObject, args), keywords);
228228
} else {
229-
if (frame != null) {
230-
PArguments.setCallerFrame(arguments, frame.materialize());
231-
}
232229
if (ct.getRootNode() instanceof ClassBodyRootNode) {
233230
PArguments.setSpecialArgument(arguments, ct.getRootNode());
234231
}

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/call/GenericInvokeNode.java

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,6 @@
4848
import com.oracle.graal.python.runtime.ExecutionContext.IndirectCalleeContext;
4949
import com.oracle.graal.python.runtime.PythonContext;
5050
import com.oracle.truffle.api.RootCallTarget;
51-
import com.oracle.truffle.api.Truffle;
5251
import com.oracle.truffle.api.frame.VirtualFrame;
5352
import com.oracle.truffle.api.nodes.IndirectCallNode;
5453
import com.oracle.truffle.api.nodes.NodeCost;
@@ -57,8 +56,8 @@
5756
public abstract class GenericInvokeNode extends AbstractInvokeNode {
5857
private static final GenericInvokeUncachedNode UNCACHED = new GenericInvokeUncachedNode();
5958

60-
@Child private IndirectCallNode callNode = Truffle.getRuntime().createIndirectCallNode();
61-
@Child private CallContext callContext = CallContext.create();
59+
@Child private IndirectCallNode callNode;
60+
@Child private CallContext callContext;
6261

6362
private final ConditionProfile isNullFrameProfile;
6463

@@ -70,7 +69,9 @@ public static GenericInvokeNode getUncached() {
7069
return UNCACHED;
7170
}
7271

73-
public GenericInvokeNode(ConditionProfile isNullFrameProfile) {
72+
public GenericInvokeNode(IndirectCallNode callNode, CallContext callContext, ConditionProfile isNullFrameProfile) {
73+
this.callNode = callNode;
74+
this.callContext = callContext;
7475
this.isNullFrameProfile = isNullFrameProfile;
7576
}
7677

@@ -109,14 +110,14 @@ public Object execute(VirtualFrame frame, RootCallTarget callTarget, Object[] ar
109110
private static final class GenericInvokeCachedNode extends GenericInvokeNode {
110111

111112
public GenericInvokeCachedNode() {
112-
super(ConditionProfile.createBinaryProfile());
113+
super(IndirectCallNode.create(), CallContext.create(), ConditionProfile.createBinaryProfile());
113114
}
114115

115116
}
116117

117118
private static final class GenericInvokeUncachedNode extends GenericInvokeNode {
118119
public GenericInvokeUncachedNode() {
119-
super(ConditionProfile.getUncached());
120+
super(IndirectCallNode.getUncached(), CallContext.getUncached(), ConditionProfile.getUncached());
120121
}
121122

122123
@Override

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/frame/MaterializeFrameNode.java

Lines changed: 88 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,9 @@
5151
import com.oracle.graal.python.builtins.objects.function.PArguments;
5252
import com.oracle.graal.python.nodes.ModuleRootNode;
5353
import com.oracle.graal.python.nodes.SpecialMethodNames;
54+
import com.oracle.graal.python.nodes.attributes.LookupInheritedAttributeNode;
55+
import com.oracle.graal.python.nodes.call.CallNode;
56+
import com.oracle.graal.python.nodes.frame.MaterializeFrameNodeGen.SyncFrameValuesNodeGen;
5457
import com.oracle.graal.python.nodes.function.ClassBodyRootNode;
5558
import com.oracle.graal.python.runtime.object.PythonObjectFactory;
5659
import com.oracle.truffle.api.dsl.Cached;
@@ -73,6 +76,17 @@
7376
**/
7477
public abstract class MaterializeFrameNode extends Node {
7578

79+
private final boolean adoptable;
80+
private static final MaterializeFrameNode INSTANCE = MaterializeFrameNodeGen.create(false);
81+
82+
public MaterializeFrameNode() {
83+
this.adoptable = true;
84+
}
85+
86+
protected MaterializeFrameNode(boolean adoptable) {
87+
this.adoptable = adoptable;
88+
}
89+
7690
public final PFrame execute(VirtualFrame frame, boolean markAsEscaped, Frame frameToMaterialize) {
7791
return execute(frame, markAsEscaped, false, frameToMaterialize);
7892
}
@@ -96,7 +110,7 @@ public final PFrame execute(VirtualFrame frame, Node location, boolean markAsEsc
96110

97111
@Specialization(guards = {"getPFrame(frameToMaterialize) == null", "isGeneratorFrame(frameToMaterialize)"})
98112
static PFrame freshPFrameForGenerator(Node location, @SuppressWarnings("unused") boolean markAsEscaped, @SuppressWarnings("unused") boolean forceSync, Frame frameToMaterialize,
99-
@Shared("factory") @Cached PythonObjectFactory factory) {
113+
@Shared("factory") @Cached("createFactory()") PythonObjectFactory factory) {
100114
PFrame escapedFrame = factory.createPFrame(PArguments.getCurrentFrameInfo(frameToMaterialize), location, PArguments.getGeneratorFrameLocals(frameToMaterialize), false);
101115
syncArgs(frameToMaterialize, escapedFrame);
102116
PFrame.Reference topFrameRef = PArguments.getCurrentFrameInfo(frameToMaterialize);
@@ -106,17 +120,17 @@ static PFrame freshPFrameForGenerator(Node location, @SuppressWarnings("unused")
106120

107121
@Specialization(guards = {"getPFrame(frameToMaterialize) == null", "!inClassBody(frameToMaterialize)", "!isGeneratorFrame(frameToMaterialize)"})
108122
static PFrame freshPFrame(VirtualFrame frame, Node location, boolean markAsEscaped, @SuppressWarnings("unused") boolean forceSync, Frame frameToMaterialize,
109-
@Shared("factory") @Cached PythonObjectFactory factory,
110-
@Shared("syncValuesNode") @Cached SyncFrameValuesNode syncValuesNode) {
123+
@Shared("factory") @Cached("createFactory()") PythonObjectFactory factory,
124+
@Shared("syncValuesNode") @Cached("createSyncNode()") SyncFrameValuesNode syncValuesNode) {
111125
PDict locals = factory.createDictLocals(frameToMaterialize.getFrameDescriptor());
112126
PFrame escapedFrame = factory.createPFrame(PArguments.getCurrentFrameInfo(frameToMaterialize), location, locals, false);
113127
return doEscapeFrame(frame, frameToMaterialize, escapedFrame, markAsEscaped, forceSync && !inModuleRoot(location), syncValuesNode);
114128
}
115129

116130
@Specialization(guards = {"getPFrame(frameToMaterialize) == null", "inClassBody(frameToMaterialize)"})
117131
static PFrame freshPFrameInClassBody(VirtualFrame frame, Node location, boolean markAsEscaped, @SuppressWarnings("unused") boolean forceSync, Frame frameToMaterialize,
118-
@Shared("factory") @Cached PythonObjectFactory factory,
119-
@Shared("syncValuesNode") @Cached SyncFrameValuesNode syncValuesNode) {
132+
@Shared("factory") @Cached("createFactory()") PythonObjectFactory factory,
133+
@Shared("syncValuesNode") @Cached("createSyncNode()") SyncFrameValuesNode syncValuesNode) {
120134
// the namespace argument stores the locals
121135
PFrame escapedFrame = factory.createPFrame(PArguments.getCurrentFrameInfo(frameToMaterialize), location, PArguments.getArgument(frameToMaterialize, 0), true);
122136
// The locals dict in a class body is always custom; we must not write the values from the
@@ -133,16 +147,16 @@ static PFrame freshPFrameInClassBody(VirtualFrame frame, Node location, boolean
133147
**/
134148
@Specialization(guards = {"getPFrame(frameToMaterialize) != null", "!getPFrame(frameToMaterialize).hasFrame()"})
135149
static PFrame incompleteFrame(VirtualFrame frame, Node location, boolean markAsEscaped, boolean forceSync, Frame frameToMaterialize,
136-
@Shared("factory") @Cached PythonObjectFactory factory,
137-
@Shared("syncValuesNode") @Cached SyncFrameValuesNode syncValuesNode) {
150+
@Shared("factory") @Cached("createFactory()") PythonObjectFactory factory,
151+
@Shared("syncValuesNode") @Cached("createSyncNode()") SyncFrameValuesNode syncValuesNode) {
138152
Object locals = getPFrame(frameToMaterialize).getLocalsDict();
139153
PFrame escapedFrame = factory.createPFrame(PArguments.getCurrentFrameInfo(frameToMaterialize), location, locals, inClassBody(frameToMaterialize));
140154
return doEscapeFrame(frame, frameToMaterialize, escapedFrame, markAsEscaped, forceSync && !inModuleRoot(location), syncValuesNode);
141155
}
142156

143157
@Specialization(guards = {"getPFrame(frameToMaterialize) != null", "getPFrame(frameToMaterialize).hasFrame()"}, replaces = "freshPFrame")
144158
static PFrame alreadyEscapedFrame(VirtualFrame frame, Node location, boolean markAsEscaped, boolean forceSync, Frame frameToMaterialize,
145-
@Shared("syncValuesNode") @Cached SyncFrameValuesNode syncValuesNode,
159+
@Shared("syncValuesNode") @Cached("createSyncNode()") SyncFrameValuesNode syncValuesNode,
146160
@Cached("createBinaryProfile()") ConditionProfile syncProfile) {
147161
PFrame pyFrame = getPFrame(frameToMaterialize);
148162
if (syncProfile.profile(forceSync && !inClassBody(frameToMaterialize) && !inModuleRoot(location))) {
@@ -158,8 +172,8 @@ static PFrame alreadyEscapedFrame(VirtualFrame frame, Node location, boolean mar
158172

159173
@Specialization(replaces = {"freshPFrame", "alreadyEscapedFrame"})
160174
static PFrame notInClassBody(VirtualFrame frame, Node location, boolean markAsEscaped, boolean forceSync, Frame frameToMaterialize,
161-
@Shared("factory") @Cached PythonObjectFactory factory,
162-
@Shared("syncValuesNode") @Cached SyncFrameValuesNode syncValuesNode,
175+
@Shared("factory") @Cached("createFactory()") PythonObjectFactory factory,
176+
@Shared("syncValuesNode") @Cached("createSyncNode()") SyncFrameValuesNode syncValuesNode,
163177
@Cached("createBinaryProfile()") ConditionProfile syncProfile) {
164178
if (getPFrame(frameToMaterialize) != null) {
165179
return alreadyEscapedFrame(frame, location, markAsEscaped, forceSync, frameToMaterialize, syncValuesNode, syncProfile);
@@ -220,6 +234,26 @@ protected static boolean inModuleRoot(Node location) {
220234
return location.getRootNode() instanceof ModuleRootNode;
221235
}
222236

237+
protected final SyncFrameValuesNode createSyncNode() {
238+
return SyncFrameValuesNodeGen.create(isAdoptable());
239+
}
240+
241+
protected final PythonObjectFactory createFactory() {
242+
if (isAdoptable()) {
243+
return PythonObjectFactory.create();
244+
}
245+
return PythonObjectFactory.getUncached();
246+
}
247+
248+
@Override
249+
public boolean isAdoptable() {
250+
return adoptable;
251+
}
252+
253+
public static MaterializeFrameNode getUnadoptable() {
254+
return INSTANCE;
255+
}
256+
223257
/**
224258
* When refreshing the frame values in the locals dict, there are 4 cases:
225259
* <ol>
@@ -241,6 +275,12 @@ protected static boolean inModuleRoot(Node location) {
241275
@ImportStatic(SpecialMethodNames.class)
242276
public abstract static class SyncFrameValuesNode extends Node {
243277

278+
private final boolean adoptable;
279+
280+
public SyncFrameValuesNode(boolean adoptable) {
281+
this.adoptable = adoptable;
282+
}
283+
244284
public abstract void execute(VirtualFrame frame, PFrame pyframe, Frame frameToSync);
245285

246286
@Specialization(guards = {"hasLocalsStorage(pyFrame, frameToSync)", "frameToSync.getFrameDescriptor() == cachedFd"}, //
@@ -315,11 +355,11 @@ static void doLocalsStorageUncached(PFrame pyFrame, Frame frameToSync) {
315355
}
316356
}
317357

318-
@Specialization(guards = {"isDictWithCustomStorage(pyFrame)", "frameToSync.getFrameDescriptor() == cachedFd"}, //
358+
@Specialization(guards = {"isDictWithCustomStorage(pyFrame)", "frameToSync.getFrameDescriptor() == cachedFd", "isAdoptable()"}, //
319359
assumptions = "cachedFd.getVersion()", //
320360
limit = "1")
321361
@ExplodeLoop
322-
static void doGenericDictCached(VirtualFrame frame, PFrame pyFrame, Frame frameToSync,
362+
static void doGenericDictAdoptableCached(VirtualFrame frame, PFrame pyFrame, Frame frameToSync,
323363
@Cached("frameToSync.getFrameDescriptor()") @SuppressWarnings("unused") FrameDescriptor cachedFd,
324364
@Cached(value = "getSlots(cachedFd)", dimensions = 1) FrameSlot[] cachedSlots,
325365
@Cached(value = "getProfiles(cachedSlots.length)", dimensions = 1) ConditionProfile[] profiles,
@@ -346,8 +386,8 @@ static void doGenericDictCached(VirtualFrame frame, PFrame pyFrame, Frame frameT
346386
}
347387
}
348388

349-
@Specialization(guards = "isDictWithCustomStorage(pyFrame)", replaces = "doGenericDictCached")
350-
static void doGenericDict(VirtualFrame frame, PFrame pyFrame, Frame frameToSync,
389+
@Specialization(guards = {"isDictWithCustomStorage(pyFrame)", "isAdoptable()"}, replaces = "doGenericDictAdoptableCached")
390+
static void doGenericDictAdoptable(VirtualFrame frame, PFrame pyFrame, Frame frameToSync,
351391
@Cached HashingCollectionNodes.SetItemNode setItemNode,
352392
@Cached HashingStorageNodes.DelItemNode deleteItemNode) {
353393
// This can happen if someone received the locals dict using 'locals()' or similar and
@@ -373,6 +413,34 @@ static void doGenericDict(VirtualFrame frame, PFrame pyFrame, Frame frameToSync,
373413
}
374414
}
375415

416+
@Specialization(guards = {"isDictWithCustomStorage(pyFrame)", "!isAdoptable()"})
417+
static void doGenericDict(VirtualFrame frame, PFrame pyFrame, Frame frameToSync) {
418+
// Same as 'doGenericDictAdoptable' but uses a full call node to call '__setitem__' and
419+
// '__delitem__' since this node is not adoptable.
420+
421+
FrameDescriptor fd = frameToSync.getFrameDescriptor();
422+
FrameSlot[] slots = getSlots(fd);
423+
// The cast is guaranteed by the guard.
424+
PDict localsDict = (PDict) pyFrame.getLocalsDict();
425+
426+
// we need to use nodes where we are sure that they may not be adopted
427+
Object setItemMethod = LookupInheritedAttributeNode.Dynamic.getUncached().execute(localsDict, SpecialMethodNames.__SETITEM__);
428+
Object deleteItemMethod = LookupInheritedAttributeNode.Dynamic.getUncached().execute(localsDict, SpecialMethodNames.__DELITEM__);
429+
430+
for (int i = 0; i < slots.length; i++) {
431+
FrameSlot slot = slots[i];
432+
if (FrameSlotIDs.isUserFrameSlot(slot.getIdentifier())) {
433+
Object value = frameToSync.getValue(slot);
434+
if (value != null) {
435+
CallNode.getUncached().execute(frame, setItemMethod, localsDict, slot.getIdentifier(), resolveCellValue(ConditionProfile.getUncached(), value));
436+
} else {
437+
// delete variable
438+
CallNode.getUncached().execute(frame, deleteItemMethod, localsDict, slot.getIdentifier());
439+
}
440+
}
441+
}
442+
}
443+
376444
@Specialization(guards = "isCustomLocalsObject(pyFrame, frameToSync)")
377445
@SuppressWarnings("unused")
378446
static void doCustomLocalsObject(PFrame pyFrame, Frame frameToSync) {
@@ -428,5 +496,11 @@ private static Object resolveCellValue(ConditionProfile profile, Object value) {
428496
}
429497
return value;
430498
}
499+
500+
@Override
501+
public boolean isAdoptable() {
502+
return adoptable;
503+
}
431504
}
505+
432506
}

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

Lines changed: 36 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -52,17 +52,22 @@
5252

5353
import com.oracle.graal.python.PythonLanguage;
5454
import com.oracle.graal.python.builtins.objects.function.PArguments;
55+
import com.oracle.graal.python.builtins.objects.function.Signature;
56+
import com.oracle.graal.python.nodes.PRootNode;
5557
import com.oracle.graal.python.nodes.call.CallNode;
58+
import com.oracle.graal.python.nodes.call.GenericInvokeNode;
5659
import com.oracle.graal.python.nodes.frame.MaterializeFrameNode;
5760
import com.oracle.graal.python.nodes.frame.MaterializeFrameNodeGen;
61+
import com.oracle.graal.python.nodes.frame.ReadCallerFrameNode;
62+
import com.oracle.graal.python.runtime.ExecutionContext.CalleeContext;
5863
import com.oracle.truffle.api.CompilerDirectives;
5964
import com.oracle.truffle.api.RootCallTarget;
6065
import com.oracle.truffle.api.Truffle;
6166
import com.oracle.truffle.api.TruffleLanguage;
6267
import com.oracle.truffle.api.frame.VirtualFrame;
6368
import com.oracle.truffle.api.nodes.Node;
6469
import com.oracle.truffle.api.nodes.Node.Child;
65-
import com.oracle.truffle.api.nodes.RootNode;
70+
import com.oracle.truffle.api.profiles.BranchProfile;
6671

6772
/**
6873
* A handler for asynchronous actions events that need to be handled on a main thread of execution,
@@ -129,28 +134,51 @@ public void run() {
129134
}
130135
}
131136

132-
private static class CallRootNode extends RootNode {
137+
private static class CallRootNode extends PRootNode {
133138
static final int ASYNC_ARGS = 4;
134139

135140
@Child private CallNode callNode = CallNode.create();
136141
@Child private MaterializeFrameNode materializeNode = MaterializeFrameNodeGen.create();
142+
@Child private ReadCallerFrameNode readCallerFrameNode = ReadCallerFrameNode.create();
143+
@Child private CalleeContext calleeContext = CalleeContext.create();
144+
145+
private final BranchProfile profile = BranchProfile.create();
137146

138147
protected CallRootNode(TruffleLanguage<?> language) {
139148
super(language);
140149
}
141150

142151
@Override
143152
public Object execute(VirtualFrame frame) {
153+
CalleeContext.enter(frame, profile);
144154
Object[] frameArguments = frame.getArguments();
145155
Object callable = PArguments.getArgument(frameArguments, 0);
146156
int frameIndex = (int) PArguments.getArgument(frameArguments, 1);
147-
VirtualFrame callerFrame = (VirtualFrame) PArguments.getArgument(frameArguments, 3);
148157
Object[] arguments = Arrays.copyOfRange(frameArguments, PArguments.USER_ARGUMENTS_OFFSET + ASYNC_ARGS, frameArguments.length);
158+
149159
if (frameIndex >= 0) {
150-
Node location = (Node) PArguments.getArgument(frameArguments, 2);
151-
arguments[frameIndex] = materializeNode.execute(frame, location, true, false, callerFrame);
160+
arguments[frameIndex] = readCallerFrameNode.executeWith(frame, 0);
161+
}
162+
try {
163+
return callNode.execute(frame, callable, arguments);
164+
} finally {
165+
calleeContext.exit(frame, this);
152166
}
153-
return callNode.execute(callerFrame, callable, arguments);
167+
}
168+
169+
@Override
170+
public Signature getSignature() {
171+
return Signature.EMPTY;
172+
}
173+
174+
@Override
175+
public boolean isPythonInternal() {
176+
return true;
177+
}
178+
179+
@Override
180+
public boolean isInternal() {
181+
return true;
154182
}
155183
}
156184

@@ -222,8 +250,9 @@ private void processAsyncActions(VirtualFrame frame, Node location) {
222250
PArguments.setArgument(args, 1, action.frameIndex());
223251
PArguments.setArgument(args, 2, location);
224252
PArguments.setArgument(args, 3, frame);
253+
225254
try {
226-
callTarget.call(args);
255+
GenericInvokeNode.getUncached().execute(frame, callTarget, args);
227256
} catch (RuntimeException e) {
228257
// we cannot raise the exception here (well, we could, but CPython
229258
// doesn't), so we do what they do and just print it

0 commit comments

Comments
 (0)