Skip to content

Commit 6ac549e

Browse files
fangererfangerer
committed
[GR-75959] Optimize C extension fastcall keyword-name handling.
PullRequest: graalpython/4575
2 parents 3fb8ba9 + 65f11d9 commit 6ac549e

27 files changed

Lines changed: 407 additions & 243 deletions

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

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -554,6 +554,13 @@ visit_weak_reachable(PyObject *op, PyGC_Head *reachable)
554554
return 0;
555555
}
556556

557+
/* Untracked objects cannot be weak candidates, even if they are reached
558+
* from a container traversed in this phase.
559+
*/
560+
if (!_PyObject_GC_IS_TRACKED(op)) {
561+
return 0;
562+
}
563+
557564
PyGC_Head *gc = AS_GC(op);
558565

559566
/* 'visit_reachable' tests at this point for 'gc_is_collecting(gc)'.
@@ -563,10 +570,6 @@ visit_weak_reachable(PyObject *op, PyGC_Head *reachable)
563570
* 'NEXT_MASK_UNREACHABLE' is set.
564571
*/
565572

566-
// It would be a logic error elsewhere if the collecting flag were set on
567-
// an untracked object.
568-
assert(UNTAG(gc)->_gc_next != 0);
569-
570573
/* Note: one could expect that we need to test
571574
* 'gc_get_refs(gc) == MANAGED_REFCNT' but that's no longer true because in
572575
* the previous phase (i.e. 'move_unreachable') when we move the object into

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

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,66 @@ PyTuple_Size(PyObject *op)
105105
}
106106
#endif // GraalPy change
107107

108+
/* Allocate an uninitialized tuple object. Before making it public, following
109+
steps must be done:
110+
111+
- Initialize its items.
112+
- Call _PyObject_GC_TRACK() on it.
113+
114+
GraalPy change: unlike CPython, this does not use the tuple freelist.
115+
*/
116+
static PyTupleObject *
117+
tuple_alloc(Py_ssize_t size)
118+
{
119+
if (size < 0) {
120+
PyErr_BadInternalCall();
121+
return NULL;
122+
}
123+
/* Check for overflow */
124+
if ((size_t)size > ((size_t)PY_SSIZE_T_MAX - (sizeof(PyTupleObject) -
125+
sizeof(PyObject *))) / sizeof(PyObject *)) {
126+
return (PyTupleObject *)PyErr_NoMemory();
127+
}
128+
PyTupleObject *op = PyObject_GC_NewVar(PyTupleObject, &PyTuple_Type, size);
129+
if (op == NULL) {
130+
return NULL;
131+
}
132+
return op;
133+
}
134+
135+
static inline PyObject *
136+
tuple_get_empty(void)
137+
{
138+
/*
139+
* GraalPy change: allocate a native empty tuple instead of returning CPython's static
140+
* singleton, so PyTuple_New consistently returns native tuples.
141+
*/
142+
PyTupleObject *op = tuple_alloc(0);
143+
if (op == NULL) {
144+
return NULL;
145+
}
146+
_PyObject_GC_TRACK(op);
147+
return (PyObject *)op;
148+
}
149+
150+
PyObject *
151+
PyTuple_New(Py_ssize_t size)
152+
{
153+
PyTupleObject *op;
154+
if (size == 0) {
155+
return tuple_get_empty();
156+
}
157+
op = tuple_alloc(size);
158+
if (op == NULL) {
159+
return NULL;
160+
}
161+
for (Py_ssize_t i = 0; i < size; i++) {
162+
op->ob_item[i] = NULL;
163+
}
164+
_PyObject_GC_TRACK(op);
165+
return (PyObject *)op;
166+
}
167+
108168
PyObject *
109169
PyTuple_GetItem(PyObject *op, Py_ssize_t i) {
110170
// GraalPy change: different implementation

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7321,6 +7321,9 @@ type_ready_mro(PyTypeObject *type)
73217321
}
73227322
#else // GraalPy change
73237323
PyObject* mro = GraalPyPrivate_Compute_Mro(type, type->tp_name);
7324+
if (mro == NULL) {
7325+
return -1;
7326+
}
73247327
type->tp_mro = mro;
73257328
#endif // GraalPy change
73267329
return 0;

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,8 @@ class TestMethodsSubclass(TestMethods):
138138
assert obj.meth_varargs_keywords(1, 2, 3, a=1, b=2) == (obj, (1, 2, 3), {'a': 1, 'b': 2})
139139
assert obj.meth_fastcall(1, 2, 3) == (obj, (1, 2, 3))
140140
assert obj.meth_fastcall_keywords(1, 2, 3, a=1, b=2) == (obj, (1, 2, 3), {'a': 1, 'b': 2})
141+
# Repeat to exercise the eager native kwnames tuple path after storage profiling.
142+
assert obj.meth_fastcall_keywords(1, 2, 3, a=1, b=2) == (obj, (1, 2, 3), {'a': 1, 'b': 2})
141143
assert obj.meth_method(1, 2, 3, a=1, b=2) == (obj, TestMethods, (1, 2, 3), {'a': 1, 'b': 2})
142144
# class methods
143145
assert obj.meth_class_o(1) == (TestMethodsSubclass, 1)

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

Lines changed: 0 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -160,9 +160,7 @@
160160
import com.oracle.graal.python.builtins.objects.memoryview.PMemoryView;
161161
import com.oracle.graal.python.builtins.objects.mmap.PMMap;
162162
import com.oracle.graal.python.builtins.objects.module.PythonModule;
163-
import com.oracle.graal.python.builtins.objects.object.PythonBuiltinObject;
164163
import com.oracle.graal.python.builtins.objects.object.PythonObject;
165-
import com.oracle.graal.python.builtins.objects.str.PString;
166164
import com.oracle.graal.python.builtins.objects.tuple.PTuple;
167165
import com.oracle.graal.python.builtins.objects.type.PythonAbstractClass;
168166
import com.oracle.graal.python.builtins.objects.type.PythonBuiltinClass;
@@ -207,10 +205,8 @@
207205
import com.oracle.truffle.api.RootCallTarget;
208206
import com.oracle.truffle.api.Truffle;
209207
import com.oracle.truffle.api.TruffleLogger;
210-
import com.oracle.truffle.api.dsl.Bind;
211208
import com.oracle.truffle.api.dsl.Cached;
212209
import com.oracle.truffle.api.dsl.Cached.Exclusive;
213-
import com.oracle.truffle.api.dsl.Fallback;
214210
import com.oracle.truffle.api.dsl.GenerateCached;
215211
import com.oracle.truffle.api.dsl.GenerateInline;
216212
import com.oracle.truffle.api.dsl.GenerateUncached;
@@ -231,59 +227,6 @@ public final class PythonCextBuiltins {
231227

232228
private static final TruffleLogger LOGGER = CApiContext.getLogger(PythonCextBuiltins.class);
233229

234-
/**
235-
* Certain builtin types like {@code int} cannot handle refcounts. They cannot be handed out to
236-
* the native side as borrowed references, since the handle would be collected immediately. (the
237-
* boxed int, for example, is not referenced from anyhwere). This node promotes these types to
238-
* full types like {@link PInt} and {@link PString}.
239-
*/
240-
@GenerateInline
241-
@GenerateCached(false)
242-
@GenerateUncached
243-
public abstract static class PromoteBorrowedValue extends Node {
244-
245-
public abstract Object execute(Node inliningTarget, Object value);
246-
247-
@Specialization
248-
static PString doString(TruffleString str,
249-
@Bind PythonLanguage language) {
250-
return PFactory.createString(language, str);
251-
}
252-
253-
@Specialization
254-
static PythonBuiltinObject doInteger(int i,
255-
@Bind PythonLanguage language) {
256-
return PFactory.createInt(language, i);
257-
}
258-
259-
@Specialization
260-
static PythonBuiltinObject doLong(long i,
261-
@Bind PythonLanguage language) {
262-
return PFactory.createInt(language, i);
263-
}
264-
265-
@Specialization(guards = "!isNaN(d)")
266-
static PythonBuiltinObject doDouble(double d,
267-
@Bind PythonLanguage language) {
268-
return PFactory.createFloat(language, d);
269-
}
270-
271-
@Specialization
272-
static PythonBuiltinObject doBoolean(boolean b,
273-
@Bind PythonContext context) {
274-
return b ? context.getTrue() : context.getFalse();
275-
}
276-
277-
static boolean isNaN(double d) {
278-
return Double.isNaN(d);
279-
}
280-
281-
@Fallback
282-
static PythonBuiltinObject doOther(@SuppressWarnings("unused") Object value) {
283-
return null;
284-
}
285-
}
286-
287230
public static PException checkThrowableBeforeNative(Throwable t, String where1, Object where2) {
288231
if (t instanceof PException pe) {
289232
// this is ok, and will be handled correctly

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

Lines changed: 42 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -54,15 +54,15 @@
5454
import static com.oracle.graal.python.builtins.objects.cext.capi.transitions.ArgDescriptor.PyObjectTransfer;
5555
import static com.oracle.graal.python.builtins.objects.cext.capi.transitions.ArgDescriptor.Py_hash_t;
5656
import static com.oracle.graal.python.builtins.objects.cext.capi.transitions.ArgDescriptor.Void;
57-
import static com.oracle.graal.python.runtime.nativeaccess.NativeMemory.NULLPTR;
58-
import static com.oracle.graal.python.runtime.nativeaccess.NativeMemory.readLong;
59-
import static com.oracle.graal.python.runtime.nativeaccess.NativeMemory.writeLong;
6057
import static com.oracle.graal.python.nodes.ErrorMessages.BAD_ARG_TO_INTERNAL_FUNC_WAS_S_P;
6158
import static com.oracle.graal.python.nodes.ErrorMessages.HASH_MISMATCH;
6259
import static com.oracle.graal.python.nodes.ErrorMessages.OBJ_P_HAS_NO_ATTR_S;
6360
import static com.oracle.graal.python.nodes.SpecialMethodNames.T_KEYS;
6461
import static com.oracle.graal.python.nodes.SpecialMethodNames.T_UPDATE;
6562
import static com.oracle.graal.python.runtime.PythonContext.NATIVE_NULL;
63+
import static com.oracle.graal.python.runtime.nativeaccess.NativeMemory.NULLPTR;
64+
import static com.oracle.graal.python.runtime.nativeaccess.NativeMemory.readLong;
65+
import static com.oracle.graal.python.runtime.nativeaccess.NativeMemory.writeLong;
6666

6767
import java.util.logging.Level;
6868

@@ -71,16 +71,17 @@
7171
import com.oracle.graal.python.builtins.modules.cext.PythonCextBuiltins.CApi5BuiltinNode;
7272
import com.oracle.graal.python.builtins.modules.cext.PythonCextBuiltins.CApiBinaryBuiltinNode;
7373
import com.oracle.graal.python.builtins.modules.cext.PythonCextBuiltins.CApiBuiltin;
74-
import com.oracle.graal.python.builtins.modules.cext.PythonCextBuiltins.CApiNullaryBuiltinNode;
7574
import com.oracle.graal.python.builtins.modules.cext.PythonCextBuiltins.CApiQuaternaryBuiltinNode;
7675
import com.oracle.graal.python.builtins.modules.cext.PythonCextBuiltins.CApiTernaryBuiltinNode;
7776
import com.oracle.graal.python.builtins.modules.cext.PythonCextBuiltins.CApiUnaryBuiltinNode;
78-
import com.oracle.graal.python.builtins.modules.cext.PythonCextBuiltins.PromoteBorrowedValue;
7977
import com.oracle.graal.python.builtins.objects.PNone;
8078
import com.oracle.graal.python.builtins.objects.cext.capi.CApiContext;
79+
import com.oracle.graal.python.builtins.objects.cext.capi.CExtNodes.EnsurePythonObjectNode;
80+
import com.oracle.graal.python.builtins.objects.cext.capi.transitions.CApiTiming;
8181
import com.oracle.graal.python.builtins.objects.cext.capi.transitions.CApiTransitions;
8282
import com.oracle.graal.python.builtins.objects.cext.capi.transitions.CApiTransitions.GcNativePtrToPythonNode;
8383
import com.oracle.graal.python.builtins.objects.cext.capi.transitions.CApiTransitions.HandlePointerConverter;
84+
import com.oracle.graal.python.builtins.objects.cext.capi.transitions.CApiTransitions.PythonToNativeInternalNode;
8485
import com.oracle.graal.python.builtins.objects.common.DynamicObjectStorage;
8586
import com.oracle.graal.python.builtins.objects.common.EconomicMapStorage;
8687
import com.oracle.graal.python.builtins.objects.common.HashingCollectionNodes.SetItemNode;
@@ -114,12 +115,13 @@
114115
import com.oracle.graal.python.lib.PyObjectGetAttr;
115116
import com.oracle.graal.python.lib.PyObjectHashNode;
116117
import com.oracle.graal.python.lib.PyUnicodeCheckNode;
117-
import com.oracle.graal.python.runtime.nativeaccess.NativeMemory;
118118
import com.oracle.graal.python.nodes.PRaiseNode;
119119
import com.oracle.graal.python.nodes.builtins.ListNodes.ConstructListNode;
120120
import com.oracle.graal.python.nodes.call.CallNode;
121121
import com.oracle.graal.python.nodes.util.CastToJavaLongExactNode;
122+
import com.oracle.graal.python.runtime.PythonContext;
122123
import com.oracle.graal.python.runtime.exception.PException;
124+
import com.oracle.graal.python.runtime.nativeaccess.NativeMemory;
123125
import com.oracle.graal.python.runtime.object.PFactory;
124126
import com.oracle.graal.python.runtime.sequence.storage.SequenceStorage;
125127
import com.oracle.graal.python.util.PythonUtils;
@@ -143,13 +145,16 @@
143145
public final class PythonCextDictBuiltins {
144146
private static final TruffleLogger LOGGER = CApiContext.getLogger(PythonCextDictBuiltins.class);
145147

146-
@CApiBuiltin(ret = PyObjectTransfer, args = {}, call = Direct)
147-
abstract static class PyDict_New extends CApiNullaryBuiltinNode {
148+
private static final CApiTiming TIMING_PYDICT_NEW = CApiTiming.create(false, "PyDict_New");
148149

149-
@Specialization
150-
static Object run(
151-
@Bind PythonLanguage language) {
152-
return PFactory.createDict(language);
150+
@CApiBuiltin(ret = PyObjectTransfer, args = {}, call = Direct, acquireGil = false)
151+
static long PyDict_New() {
152+
CApiTiming.enter();
153+
try {
154+
PDict dict = PFactory.createDict(PythonLanguage.get(null));
155+
return PythonToNativeInternalNode.executeUncached(dict, true);
156+
} finally {
157+
CApiTiming.exit(TIMING_PYDICT_NEW);
153158
}
154159
}
155160

@@ -159,6 +164,7 @@ abstract static class _PyDict_Next extends CApi5BuiltinNode {
159164
@Specialization
160165
static int next(PDict dict, long posPtr, long keyPtr, long valuePtr, long hashPtr,
161166
@Bind Node inliningTarget,
167+
@Bind PythonContext context,
162168
@Cached CApiTransitions.PythonToNativeNode toNativeNode,
163169
@Cached InlinedBranchProfile needsRewriteProfile,
164170
@Cached InlinedBranchProfile economicMapProfile,
@@ -168,8 +174,8 @@ static int next(PDict dict, long posPtr, long keyPtr, long valuePtr, long hashPt
168174
@Cached HashingStorageIteratorKey itKey,
169175
@Cached HashingStorageIteratorValue itValue,
170176
@Cached HashingStorageIteratorKeyHash itKeyHash,
171-
@Cached PromoteBorrowedValue promoteKeyNode,
172-
@Cached PromoteBorrowedValue promoteValueNode,
177+
@Cached EnsurePythonObjectNode ensureKeyNode,
178+
@Cached EnsurePythonObjectNode ensureValueNode,
173179
@Cached HashingStorageSetItem setItem,
174180
@Cached DynamicObject.SetShapeFlagsNode setShapeFlagsNode) {
175181
/*
@@ -188,8 +194,10 @@ static int next(PDict dict, long posPtr, long keyPtr, long valuePtr, long hashPt
188194
economicMapProfile.enter(inliningTarget);
189195
HashingStorageIterator it = getIterator.execute(inliningTarget, storage);
190196
while (itNext.execute(inliningTarget, storage, it)) {
191-
if (promoteKeyNode.execute(inliningTarget, itKey.execute(inliningTarget, storage, it)) != null ||
192-
promoteValueNode.execute(inliningTarget, itValue.execute(inliningTarget, storage, it)) != null) {
197+
Object key = itKey.execute(inliningTarget, storage, it);
198+
Object value = itValue.execute(inliningTarget, storage, it);
199+
if (ensureKeyNode.execute(context, key, false) != key ||
200+
ensureValueNode.execute(context, value, false) != value) {
193201
needsRewrite = true;
194202
break;
195203
}
@@ -208,12 +216,12 @@ static int next(PDict dict, long posPtr, long keyPtr, long valuePtr, long hashPt
208216
while (itNext.execute(inliningTarget, storage, it)) {
209217
Object key = itKey.execute(inliningTarget, storage, it);
210218
Object value = itValue.execute(inliningTarget, storage, it);
211-
Object promotedKey = promoteKeyNode.execute(inliningTarget, key);
212-
if (promotedKey != null) {
219+
Object promotedKey = ensureKeyNode.execute(context, key, false);
220+
if (promotedKey != key) {
213221
key = promotedKey;
214222
}
215-
Object promotedValue = promoteValueNode.execute(inliningTarget, value);
216-
if (promotedValue != null) {
223+
Object promotedValue = ensureValueNode.execute(context, value, false);
224+
if (promotedValue != value) {
217225
value = promotedValue;
218226
}
219227
// promoted key will never have side-effecting __hash__/__eq__
@@ -243,13 +251,13 @@ static int next(PDict dict, long posPtr, long keyPtr, long valuePtr, long hashPt
243251
writeLong(posPtr, newPos);
244252
if (keyPtr != NULLPTR) {
245253
Object key = itKey.execute(inliningTarget, storage, it);
246-
assert promoteKeyNode.execute(inliningTarget, key) == null;
254+
assert ensureKeyNode.execute(context, key, false) == key;
247255
// Borrowed reference
248256
NativeMemory.writePtr(keyPtr, toNativeNode.executeLong(key));
249257
}
250258
if (valuePtr != NULLPTR) {
251259
Object value = itValue.execute(inliningTarget, storage, it);
252-
assert promoteValueNode.execute(inliningTarget, value) == null;
260+
assert ensureValueNode.execute(context, value, false) == value;
253261
// Borrowed reference
254262
NativeMemory.writePtr(valuePtr, toNativeNode.executeLong(value));
255263
}
@@ -303,8 +311,9 @@ public abstract static class PyDict_GetItem extends CApiBinaryBuiltinNode {
303311
@Specialization
304312
static Object getItem(PDict dict, Object key,
305313
@Bind Node inliningTarget,
314+
@Bind PythonContext context,
306315
@Cached HashingStorageGetItem getItem,
307-
@Cached PromoteBorrowedValue promoteNode,
316+
@Cached EnsurePythonObjectNode ensureNode,
308317
@Cached SetItemNode setItemNode,
309318
@Cached InlinedBranchProfile noResultProfile) {
310319
try {
@@ -313,8 +322,8 @@ static Object getItem(PDict dict, Object key,
313322
noResultProfile.enter(inliningTarget);
314323
return NATIVE_NULL;
315324
}
316-
Object promotedValue = promoteNode.execute(inliningTarget, res);
317-
if (promotedValue != null) {
325+
Object promotedValue = ensureNode.execute(context, res, false);
326+
if (promotedValue != res) {
318327
setItemNode.execute(null, inliningTarget, dict, key, promotedValue);
319328
return promotedValue;
320329
}
@@ -342,17 +351,18 @@ abstract static class PyDict_GetItemWithError extends CApiBinaryBuiltinNode {
342351
@Specialization
343352
static Object getItem(PDict dict, Object key,
344353
@Bind Node inliningTarget,
354+
@Bind PythonContext context,
345355
@Cached HashingStorageGetItem getItem,
346-
@Cached PromoteBorrowedValue promoteNode,
356+
@Cached EnsurePythonObjectNode ensureNode,
347357
@Cached SetItemNode setItemNode,
348358
@Cached InlinedBranchProfile noResultProfile) {
349359
Object res = getItem.execute(null, inliningTarget, dict.getDictStorage(), key);
350360
if (res == null) {
351361
noResultProfile.enter(inliningTarget);
352362
return NATIVE_NULL;
353363
}
354-
Object promotedValue = promoteNode.execute(inliningTarget, res);
355-
if (promotedValue != null) {
364+
Object promotedValue = ensureNode.execute(context, res, false);
365+
if (promotedValue != res) {
356366
setItemNode.execute(null, inliningTarget, dict, key, promotedValue);
357367
return promotedValue;
358368
}
@@ -412,12 +422,13 @@ abstract static class PyDict_SetDefault extends CApiTernaryBuiltinNode {
412422
@Specialization
413423
static Object setItem(PDict dict, Object key, Object value,
414424
@Bind Node inliningTarget,
425+
@Bind PythonContext context,
415426
@Cached PyDictSetDefault setDefault,
416-
@Cached PromoteBorrowedValue promoteNode,
427+
@Cached EnsurePythonObjectNode ensureNode,
417428
@Cached SetItemNode setItemNode) {
418429
Object result = setDefault.execute(null, inliningTarget, dict, key, value);
419-
Object promotedValue = promoteNode.execute(inliningTarget, result);
420-
if (promotedValue != null) {
430+
Object promotedValue = ensureNode.execute(context, result, false);
431+
if (promotedValue != result) {
421432
setItemNode.execute(null, inliningTarget, dict, key, promotedValue);
422433
return promotedValue;
423434
}

0 commit comments

Comments
 (0)