Skip to content

Commit 30e443d

Browse files
fangererfangerer
committed
[GR-79957] Convert PyList_(New|Append|Size|GetItemRef) to static upcalls.
PullRequest: graalpython/4891
2 parents 178ea26 + 0828994 commit 30e443d

4 files changed

Lines changed: 127 additions & 101 deletions

File tree

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

Lines changed: 72 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@
4141
package com.oracle.graal.python.builtins.modules.cext;
4242

4343
import static com.oracle.graal.python.builtins.PythonBuiltinClassType.IndexError;
44+
import static com.oracle.graal.python.builtins.PythonBuiltinClassType.MemoryError;
4445
import static com.oracle.graal.python.builtins.PythonBuiltinClassType.SystemError;
4546
import static com.oracle.graal.python.builtins.PythonBuiltinClassType.TypeError;
4647
import static com.oracle.graal.python.builtins.modules.cext.PythonCextBuiltins.CApiCallPath.Direct;
@@ -68,9 +69,13 @@
6869
import com.oracle.graal.python.builtins.objects.PNone;
6970
import com.oracle.graal.python.builtins.objects.cext.capi.CExtNodes.EnsurePythonObjectNode;
7071
import com.oracle.graal.python.builtins.objects.cext.capi.PySequenceArrayWrapper;
72+
import com.oracle.graal.python.builtins.objects.cext.capi.transitions.CApiTiming;
73+
import com.oracle.graal.python.builtins.objects.cext.capi.transitions.CApiTransitions.NativeToPythonInternalNode;
74+
import com.oracle.graal.python.builtins.objects.cext.capi.transitions.CApiTransitions.PythonToNativeInternalNode;
7175
import com.oracle.graal.python.builtins.objects.common.SequenceStorageNodes.GetItemScalarNode;
7276
import com.oracle.graal.python.builtins.objects.common.SequenceStorageNodes.ListGeneralizationNode;
7377
import com.oracle.graal.python.builtins.objects.common.SequenceStorageNodes.SetItemScalarNode;
78+
import com.oracle.graal.python.builtins.objects.ints.PInt;
7479
import com.oracle.graal.python.builtins.objects.list.ListBuiltins;
7580
import com.oracle.graal.python.builtins.objects.list.ListBuiltins.ListExtendNode;
7681
import com.oracle.graal.python.builtins.objects.list.ListBuiltins.ListInsertNode;
@@ -79,7 +84,7 @@
7984
import com.oracle.graal.python.lib.PySliceNew;
8085
import com.oracle.graal.python.nodes.ErrorMessages;
8186
import com.oracle.graal.python.nodes.PRaiseNode;
82-
import com.oracle.graal.python.nodes.builtins.ListNodes.AppendNode;
87+
import com.oracle.graal.python.nodes.builtins.ListNodes.PListAppendNode;
8388
import com.oracle.graal.python.nodes.builtins.TupleNodes.ConstructTupleNode;
8489
import com.oracle.graal.python.runtime.PythonContext;
8590
import com.oracle.graal.python.runtime.object.PFactory;
@@ -95,31 +100,31 @@
95100

96101
public final class PythonCextListBuiltins {
97102

98-
@CApiBuiltin(ret = PyObjectTransfer, args = {Py_ssize_t}, call = Direct)
99-
abstract static class PyList_New extends CApiUnaryBuiltinNode {
100-
@Specialization(guards = "size < 0")
101-
static Object newListError(long size,
102-
@Bind Node inliningTarget) {
103-
throw PRaiseNode.raiseStatic(inliningTarget, SystemError, BAD_ARG_TO_INTERNAL_FUNC_S, size);
104-
}
105-
106-
@SuppressWarnings("unused")
107-
@Specialization(guards = "size == 0")
108-
static Object newEmptyList(long size,
109-
@Bind PythonLanguage language) {
110-
return PFactory.createList(language);
111-
}
103+
private static final CApiTiming TIMING_PYLIST_NEW = CApiTiming.create(false, "PyList_New");
112104

113-
@Specialization(guards = "size > 0")
114-
static Object newList(long size,
115-
@Bind PythonLanguage language) {
116-
return PFactory.createList(language, array(size));
117-
}
105+
@CApiBuiltin(ret = PyObjectTransfer, args = {Py_ssize_t}, call = Direct, acquireGil = false)
106+
static long PyList_New(long size) {
107+
CApiTiming.enter();
108+
try {
109+
if (size < 0) {
110+
throw PRaiseNode.raiseStatic(null, SystemError, BAD_ARG_TO_INTERNAL_FUNC_S, size);
111+
}
118112

119-
private static Object[] array(long size) {
120-
Object[] a = new Object[(int) size];
121-
Arrays.fill(a, PNone.NO_VALUE);
122-
return a;
113+
PythonLanguage language = PythonLanguage.get(null);
114+
PList result;
115+
if (size == 0) {
116+
result = PFactory.createList(language);
117+
} else {
118+
if (!PInt.fitsInInt(size)) {
119+
throw PRaiseNode.raiseStatic(null, MemoryError);
120+
}
121+
Object[] a = new Object[(int) size];
122+
Arrays.fill(a, PNone.NO_VALUE);
123+
result = PFactory.createList(language, a);
124+
}
125+
return PythonToNativeInternalNode.executeNewRefUncached(result);
126+
} finally {
127+
CApiTiming.exit(TIMING_PYLIST_NEW);
123128
}
124129
}
125130

@@ -157,58 +162,50 @@ Object fallback(Object list, @SuppressWarnings("unused") Object pos) {
157162
}
158163
}
159164

160-
@CApiBuiltin(ret = PyObjectTransfer, args = {PyObject, Py_ssize_t}, call = Direct)
161-
abstract static class PyList_GetItemRef extends CApiBinaryBuiltinNode {
165+
private static final CApiTiming TIMING_PYLIST_GETITEMREF = CApiTiming.create(false, "PyList_GetItemRef");
162166

163-
@Specialization
164-
static Object doPList(PList list, long key,
165-
@Bind Node inliningTarget,
166-
@Bind PythonContext context,
167-
@Cached EnsurePythonObjectNode ensureNode,
168-
@Cached ListGeneralizationNode generalizationNode,
169-
@Cached SetItemScalarNode setItemNode,
170-
@Cached GetItemScalarNode getItemNode,
171-
@Cached PRaiseNode raiseNode) {
167+
@CApiBuiltin(ret = PyObjectTransfer, args = {PyObject, Py_ssize_t}, call = Direct)
168+
static long PyList_GetItemRef(long opPtr, long key) {
169+
CApiTiming.enter();
170+
try {
171+
Object op = NativeToPythonInternalNode.executeUncached(opPtr, false);
172+
if (!(op instanceof PList list)) {
173+
throw PRaiseNode.raiseStatic(null, TypeError, ErrorMessages.EXPECTED_A_LIST);
174+
}
172175
SequenceStorage sequenceStorage = list.getSequenceStorage();
173176
// we must do a bounds-check but we must not normalize the index
174177
if (key < 0 || key >= sequenceStorage.length()) {
175-
throw raiseNode.raise(inliningTarget, IndexError, ErrorMessages.LIST_INDEX_OUT_OF_RANGE);
178+
throw PRaiseNode.raiseStatic(null, IndexError, ErrorMessages.LIST_INDEX_OUT_OF_RANGE);
176179
}
177-
Object result = getItemNode.execute(inliningTarget, sequenceStorage, (int) key);
180+
Object result = GetItemScalarNode.executeUncached(sequenceStorage, (int) key);
178181
// See the note in PyDict_GetItemRef
179-
Object promotedValue = ensureNode.execute(context, result, false);
182+
Object promotedValue = EnsurePythonObjectNode.executeUncached(PythonContext.get(null), result, false);
180183
if (promotedValue != result) {
181-
sequenceStorage = generalizationNode.execute(inliningTarget, sequenceStorage, promotedValue);
184+
sequenceStorage = ListGeneralizationNode.executeUncached(sequenceStorage, promotedValue);
182185
list.setSequenceStorage(sequenceStorage);
183-
setItemNode.execute(inliningTarget, sequenceStorage, (int) key, promotedValue);
184-
return promotedValue;
186+
SetItemScalarNode.executeUncached(sequenceStorage, (int) key, promotedValue);
185187
}
186-
return result;
187-
}
188-
189-
@Fallback
190-
static Object fallback(@SuppressWarnings("unused") Object list, @SuppressWarnings("unused") Object pos,
191-
@Bind Node inliningTarget) {
192-
throw PRaiseNode.raiseStatic(inliningTarget, TypeError, ErrorMessages.EXPECTED_A_LIST);
188+
return PythonToNativeInternalNode.executeNewRefUncached(promotedValue);
189+
} finally {
190+
CApiTiming.exit(TIMING_PYLIST_GETITEMREF);
193191
}
194192
}
195193

196-
@CApiBuiltin(ret = Int, args = {PyObject, PyObject}, call = Direct)
197-
abstract static class PyList_Append extends CApiBinaryBuiltinNode {
194+
private static final CApiTiming TIMING_PYLIST_APPEND = CApiTiming.create(false, "PyList_Append");
198195

199-
@Specialization
200-
int append(PList list, Object newItem,
201-
@Cached AppendNode appendNode) {
202-
if (newItem == PNone.NO_VALUE) {
203-
throw badInternalCall("newitem");
196+
@CApiBuiltin(ret = Int, args = {PyObject, PyObject}, call = Direct)
197+
static int PyList_Append(long opPtr, long itemPtr) {
198+
CApiTiming.enter();
199+
try {
200+
Object op = NativeToPythonInternalNode.executeUncached(opPtr, false);
201+
Object item = NativeToPythonInternalNode.executeUncached(itemPtr, false);
202+
if (op instanceof PList list && item != PNone.NO_VALUE) {
203+
PListAppendNode.executeUncached(list, item);
204+
return 0;
204205
}
205-
appendNode.execute(list, newItem);
206-
return 0;
207-
}
208-
209-
@Fallback
210-
int fallback(Object list, @SuppressWarnings("unused") Object newItem) {
211-
throw raiseFallback(list, PythonBuiltinClassType.PList);
206+
throw PythonCextBuiltins.badInternalCall("PyList_Append", "op");
207+
} finally {
208+
CApiTiming.exit(TIMING_PYLIST_APPEND);
212209
}
213210
}
214211

@@ -293,21 +290,23 @@ Object fallback(Object list, @SuppressWarnings("unused") Object iterable) {
293290
}
294291
}
295292

293+
private static final CApiTiming TIMING_PYLIST_SIZE = CApiTiming.create(false, "PyList_Size");
294+
296295
/*
297296
* A pure-C Py_SIZE implementation regressed mixed managed/native/list-subclass workload by
298297
* about 1.26x.
299298
*/
300-
@CApiBuiltin(ret = Py_ssize_t, args = {PyObject}, call = Direct)
301-
abstract static class PyList_Size extends CApiUnaryBuiltinNode {
302-
303-
@Specialization
304-
static long size(PList list) {
305-
return list.getSequenceStorage().length();
306-
}
307-
308-
@Fallback
309-
long fallback(Object list) {
310-
throw raiseFallback(list, PythonBuiltinClassType.PList);
299+
@CApiBuiltin(ret = Py_ssize_t, args = {PyObject}, call = Direct, acquireGil = false)
300+
static long PyList_Size(long opPtr) {
301+
CApiTiming.enter();
302+
try {
303+
Object op = NativeToPythonInternalNode.executeUncached(opPtr, false);
304+
if (op instanceof PList list) {
305+
return list.getSequenceStorage().length();
306+
}
307+
throw PythonCextBuiltins.badInternalCall("PyList_Size", "op");
308+
} finally {
309+
CApiTiming.exit(TIMING_PYLIST_SIZE);
311310
}
312311
}
313312

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/common/SequenceStorageNodes.java‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,7 @@
8080
import com.oracle.graal.python.builtins.objects.common.SequenceStorageNodesFactory.RepeatNodeGen;
8181
import com.oracle.graal.python.builtins.objects.common.SequenceStorageNodesFactory.SetItemDynamicNodeGen;
8282
import com.oracle.graal.python.builtins.objects.common.SequenceStorageNodesFactory.SetItemNodeGen;
83+
import com.oracle.graal.python.builtins.objects.common.SequenceStorageNodesFactory.SetItemScalarNodeGen;
8384
import com.oracle.graal.python.builtins.objects.common.SequenceStorageNodesFactory.StorageToNativeNodeGen;
8485
import com.oracle.graal.python.builtins.objects.common.SequenceStorageNodesFactory.ToArrayNodeGen;
8586
import com.oracle.graal.python.builtins.objects.common.SequenceStorageNodesFactory.ToByteArrayNodeGen;
@@ -1324,6 +1325,11 @@ static void doError(@SuppressWarnings("unused") Node inliningTarget, SequenceSto
13241325
@GenerateCached(false)
13251326
public abstract static class SetItemScalarNode extends AbstractSetItemScalarNode {
13261327

1328+
@TruffleBoundary
1329+
public static void executeUncached(SequenceStorage storage, int idx, Object value) {
1330+
SetItemScalarNodeGen.getUncached().execute(null, storage, idx, value);
1331+
}
1332+
13271333
@InliningCutoff
13281334
@Specialization
13291335
protected static void doNative(NativeSequenceStorage storage, int idx, Object value,
@@ -2830,6 +2836,11 @@ public GeneralizationNode create() {
28302836
}
28312837
};
28322838

2839+
@TruffleBoundary
2840+
public static SequenceStorage executeUncached(SequenceStorage toGeneralize, Object indicationValue) {
2841+
return ListGeneralizationNodeGen.getUncached().execute(null, toGeneralize, indicationValue);
2842+
}
2843+
28332844
@Override
28342845
public final SequenceStorage executeCached(SequenceStorage toGeneralize, Object indicationValue) {
28352846
return execute(this, toGeneralize, indicationValue);

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/builtins/ListNodes.java‎

Lines changed: 43 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,7 @@
6161
import com.oracle.graal.python.nodes.PRaiseNode;
6262
import com.oracle.graal.python.nodes.builtins.ListNodesFactory.AppendNodeGen;
6363
import com.oracle.graal.python.nodes.builtins.ListNodesFactory.ConstructListNodeGen;
64+
import com.oracle.graal.python.nodes.builtins.ListNodesFactory.PListAppendNodeGen;
6465
import com.oracle.graal.python.nodes.classes.IsSubtypeNode;
6566
import com.oracle.graal.python.nodes.object.GetClassNode;
6667
import com.oracle.graal.python.nodes.object.IsForeignObjectNode;
@@ -74,7 +75,6 @@
7475
import com.oracle.truffle.api.CompilerDirectives;
7576
import com.oracle.truffle.api.HostCompilerDirectives.InliningCutoff;
7677
import com.oracle.truffle.api.bytecode.OperationProxy;
77-
import com.oracle.truffle.api.bytecode.StoreBytecodeIndex;
7878
import com.oracle.truffle.api.dsl.Bind;
7979
import com.oracle.truffle.api.dsl.Cached;
8080
import com.oracle.truffle.api.dsl.Cached.Exclusive;
@@ -260,6 +260,39 @@ protected PList doGeneric(VirtualFrame frame, Object value,
260260

261261
}
262262

263+
@GenerateUncached
264+
@GenerateInline(false) // footprint reduction 36 -> 17
265+
public abstract static class AppendNode extends PNodeWithContext {
266+
267+
public abstract void execute(Object list, Object value);
268+
269+
@Specialization
270+
static void appendObjectGeneric(PList list, Object value,
271+
@Bind Node inliningTarget,
272+
@Cached PListAppendNode appendNode) {
273+
appendNode.execute(list, value);
274+
}
275+
276+
@Fallback
277+
static void appendObjectForeign(Object list, Object value,
278+
@Bind Node inliningTarget,
279+
@Cached GetListStorageNode getStorageNode,
280+
@Cached SequenceStorageNodes.AppendNode appendNode) {
281+
var storage = getStorageNode.execute(inliningTarget, list);
282+
SequenceStorage newStore = appendNode.execute(inliningTarget, storage, value, ListGeneralizationNode.SUPPLIER);
283+
assert newStore == storage;
284+
}
285+
286+
@NeverDefault
287+
public static AppendNode create() {
288+
return AppendNodeGen.create();
289+
}
290+
291+
public static AppendNode getUncached() {
292+
return AppendNodeGen.getUncached();
293+
}
294+
}
295+
263296
/**
264297
* This node takes a bit of care to avoid compiling code for switching storages. In the
265298
* interpreter, it will use a different {@link AppendNode} than in the compiled code, so the
@@ -269,12 +302,16 @@ protected PList doGeneric(VirtualFrame frame, Object value,
269302
* code will only see lists of the correct size and storage type.
270303
*/
271304
@GenerateUncached
272-
@GenerateInline(false) // footprint reduction 36 -> 17
305+
@GenerateInline(false)
273306
@OperationProxy.Proxyable(allowUncached = true, storeBytecodeIndex = false)
274-
public abstract static class AppendNode extends PNodeWithContext {
307+
public abstract static class PListAppendNode extends PNodeWithContext {
275308
private static final BranchProfile[] DISABLED = new BranchProfile[]{BranchProfile.getUncached()};
276309

277-
public abstract void execute(Object list, Object value);
310+
public static void executeUncached(PList list, Object value) {
311+
PListAppendNodeGen.getUncached().execute(list, value);
312+
}
313+
314+
public abstract void execute(PList list, Object value);
278315

279316
@NeverDefault
280317
public static BranchProfile[] getUpdateStoreProfile() {
@@ -286,10 +323,9 @@ public static BranchProfile[] getUpdateStoreProfileUncached() {
286323
}
287324

288325
@Specialization
289-
public static void appendObjectGeneric(PList list, Object value,
326+
public static void doPList(PList list, Object value,
290327
@Bind Node inliningTarget,
291-
// @Exclusive for truffle-interpreted-performance
292-
@Exclusive @Cached SequenceStorageNodes.AppendNode appendNode,
328+
@Cached SequenceStorageNodes.AppendNode appendNode,
293329
@Cached(value = "getUpdateStoreProfile()", uncached = "getUpdateStoreProfileUncached()", dimensions = 1) BranchProfile[] updateStoreProfile) {
294330
if (updateStoreProfile[0] == null) {
295331
// Executed for the first time. We don't pollute the AppendNode specializations,
@@ -313,26 +349,6 @@ public static void appendObjectGeneric(PList list, Object value,
313349
}
314350
}
315351
}
316-
317-
@Fallback
318-
@StoreBytecodeIndex
319-
public static void appendObjectForeign(Object list, Object value,
320-
@Bind Node inliningTarget,
321-
@Cached GetListStorageNode getStorageNode,
322-
@Exclusive @Cached SequenceStorageNodes.AppendNode appendNode) {
323-
var storage = getStorageNode.execute(inliningTarget, list);
324-
SequenceStorage newStore = appendNode.execute(inliningTarget, storage, value, ListGeneralizationNode.SUPPLIER);
325-
assert newStore == storage;
326-
}
327-
328-
@NeverDefault
329-
public static AppendNode create() {
330-
return AppendNodeGen.create();
331-
}
332-
333-
public static AppendNode getUncached() {
334-
return AppendNodeGen.getUncached();
335-
}
336352
}
337353

338354
@GenerateInline(false)

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/bytecode_dsl/PBytecodeDSLRootNode.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -402,7 +402,7 @@
402402
@OperationProxy(value = CopyDictWithoutKeysNode.class, name = "CopyDictWithoutKeys")
403403
@OperationProxy(value = PyObjectIsTrueNode.class, name = "Yes")
404404
@OperationProxy(value = PyObjectIsNotTrueNode.class, name = "Not")
405-
@OperationProxy(value = ListNodes.AppendNode.class, name = "ListAppend")
405+
@OperationProxy(value = ListNodes.PListAppendNode.class, name = "ListAppend")
406406
@OperationProxy(value = SetNodes.AddNode.class, name = "SetAdd")
407407
@ShortCircuitOperation(name = "BoolAnd", booleanConverter = PyObjectIsTrueNode.class, operator = Operator.AND_RETURN_VALUE)
408408
@ShortCircuitOperation(name = "BoolOr", booleanConverter = PyObjectIsTrueNode.class, operator = Operator.OR_RETURN_VALUE)

0 commit comments

Comments
 (0)