Skip to content

Commit 9758158

Browse files
msimacekmsimacek
committed
Avoid intermediate tuples in _PyDict_Next
1 parent 85ab9cb commit 9758158

File tree

5 files changed

+103
-41
lines changed

5 files changed

+103
-41
lines changed

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

Lines changed: 57 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -2074,7 +2074,6 @@ PyDict_Clear(PyObject *op)
20742074
}
20752075
ASSERT_CONSISTENT(mp);
20762076
}
2077-
#endif // GraalPy change
20782077

20792078
/* Internal version of PyDict_Next that returns a hash value in addition
20802079
* to the key and value.
@@ -2085,30 +2084,65 @@ int
20852084
_PyDict_Next(PyObject *op, Py_ssize_t *ppos, PyObject **pkey,
20862085
PyObject **pvalue, Py_hash_t *phash)
20872086
{
2088-
// GraalPy change: different implementation
2089-
PyObject *tresult = GraalPyTruffleDict_Next(op, *ppos);
2090-
if (tresult == NULL) {
2091-
if(pkey != NULL) {
2092-
*pkey = NULL;
2093-
}
2094-
if(pvalue != NULL) {
2095-
*pvalue = NULL;
2096-
}
2097-
return 0;
2098-
}
2099-
if (pkey != NULL) {
2100-
*pkey = PyTuple_GetItem(tresult, 0);
2101-
}
2102-
if (pvalue != NULL) {
2103-
*pvalue = PyTuple_GetItem(tresult, 1);
2104-
}
2105-
if (phash != NULL) {
2106-
*phash = PyLong_AsSsize_t(PyTuple_GetItem(tresult, 2));
2107-
}
2108-
*ppos = PyLong_AsSsize_t(PyTuple_GetItem(tresult, 3));
2109-
Py_DECREF(tresult);
2087+
Py_ssize_t i;
2088+
PyDictObject *mp;
2089+
PyObject *key, *value;
2090+
Py_hash_t hash;
2091+
2092+
if (!PyDict_Check(op))
2093+
return 0;
2094+
mp = (PyDictObject *)op;
2095+
i = *ppos;
2096+
if (mp->ma_values) {
2097+
assert(mp->ma_used <= SHARED_KEYS_MAX_SIZE);
2098+
if (i < 0 || i >= mp->ma_used)
2099+
return 0;
2100+
int index = get_index_from_order(mp, i);
2101+
value = mp->ma_values->values[index];
2102+
2103+
key = DK_UNICODE_ENTRIES(mp->ma_keys)[index].me_key;
2104+
hash = unicode_get_hash(key);
2105+
assert(value != NULL);
2106+
}
2107+
else {
2108+
Py_ssize_t n = mp->ma_keys->dk_nentries;
2109+
if (i < 0 || i >= n)
2110+
return 0;
2111+
if (DK_IS_UNICODE(mp->ma_keys)) {
2112+
PyDictUnicodeEntry *entry_ptr = &DK_UNICODE_ENTRIES(mp->ma_keys)[i];
2113+
while (i < n && entry_ptr->me_value == NULL) {
2114+
entry_ptr++;
2115+
i++;
2116+
}
2117+
if (i >= n)
2118+
return 0;
2119+
key = entry_ptr->me_key;
2120+
hash = unicode_get_hash(entry_ptr->me_key);
2121+
value = entry_ptr->me_value;
2122+
}
2123+
else {
2124+
PyDictKeyEntry *entry_ptr = &DK_ENTRIES(mp->ma_keys)[i];
2125+
while (i < n && entry_ptr->me_value == NULL) {
2126+
entry_ptr++;
2127+
i++;
2128+
}
2129+
if (i >= n)
2130+
return 0;
2131+
key = entry_ptr->me_key;
2132+
hash = entry_ptr->me_hash;
2133+
value = entry_ptr->me_value;
2134+
}
2135+
}
2136+
*ppos = i+1;
2137+
if (pkey)
2138+
*pkey = key;
2139+
if (pvalue)
2140+
*pvalue = value;
2141+
if (phash)
2142+
*phash = hash;
21102143
return 1;
21112144
}
2145+
#endif // GraalPy change
21122146

21132147
/*
21142148
* Iterate over a dict. Use like so:

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

Lines changed: 41 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -43,10 +43,12 @@
4343
import static com.oracle.graal.python.builtins.PythonBuiltinClassType.AttributeError;
4444
import static com.oracle.graal.python.builtins.PythonBuiltinClassType.SystemError;
4545
import static com.oracle.graal.python.builtins.modules.cext.PythonCextBuiltins.CApiCallPath.Direct;
46-
import static com.oracle.graal.python.builtins.modules.cext.PythonCextBuiltins.CApiCallPath.Ignored;
4746
import static com.oracle.graal.python.builtins.objects.cext.capi.transitions.ArgDescriptor.Int;
47+
import static com.oracle.graal.python.builtins.objects.cext.capi.transitions.ArgDescriptor.PY_HASH_T_PTR;
48+
import static com.oracle.graal.python.builtins.objects.cext.capi.transitions.ArgDescriptor.PY_SSIZE_T_PTR;
4849
import static com.oracle.graal.python.builtins.objects.cext.capi.transitions.ArgDescriptor.PyObject;
4950
import static com.oracle.graal.python.builtins.objects.cext.capi.transitions.ArgDescriptor.PyObjectBorrowed;
51+
import static com.oracle.graal.python.builtins.objects.cext.capi.transitions.ArgDescriptor.PyObjectPtr;
5052
import static com.oracle.graal.python.builtins.objects.cext.capi.transitions.ArgDescriptor.PyObjectTransfer;
5153
import static com.oracle.graal.python.builtins.objects.cext.capi.transitions.ArgDescriptor.Py_hash_t;
5254
import static com.oracle.graal.python.builtins.objects.cext.capi.transitions.ArgDescriptor.Py_ssize_t;
@@ -59,6 +61,7 @@
5961

6062
import com.oracle.graal.python.builtins.PythonBuiltinClassType;
6163
import com.oracle.graal.python.builtins.modules.BuiltinConstructors.StrNode;
64+
import com.oracle.graal.python.builtins.modules.cext.PythonCextBuiltins.CApi5BuiltinNode;
6265
import com.oracle.graal.python.builtins.modules.cext.PythonCextBuiltins.CApiBinaryBuiltinNode;
6366
import com.oracle.graal.python.builtins.modules.cext.PythonCextBuiltins.CApiBuiltin;
6467
import com.oracle.graal.python.builtins.modules.cext.PythonCextBuiltins.CApiNullaryBuiltinNode;
@@ -67,6 +70,8 @@
6770
import com.oracle.graal.python.builtins.modules.cext.PythonCextBuiltins.CApiUnaryBuiltinNode;
6871
import com.oracle.graal.python.builtins.modules.cext.PythonCextBuiltins.PromoteBorrowedValue;
6972
import com.oracle.graal.python.builtins.objects.PNone;
73+
import com.oracle.graal.python.builtins.objects.cext.capi.transitions.CApiTransitions;
74+
import com.oracle.graal.python.builtins.objects.cext.structs.CStructAccess;
7075
import com.oracle.graal.python.builtins.objects.common.EconomicMapStorage;
7176
import com.oracle.graal.python.builtins.objects.common.HashingCollectionNodes.SetItemNode;
7277
import com.oracle.graal.python.builtins.objects.common.HashingStorage;
@@ -107,6 +112,8 @@
107112
import com.oracle.truffle.api.dsl.Cached.Shared;
108113
import com.oracle.truffle.api.dsl.Fallback;
109114
import com.oracle.truffle.api.dsl.Specialization;
115+
import com.oracle.truffle.api.interop.InteropLibrary;
116+
import com.oracle.truffle.api.library.CachedLibrary;
110117
import com.oracle.truffle.api.nodes.Node;
111118
import com.oracle.truffle.api.profiles.InlinedBranchProfile;
112119
import com.oracle.truffle.api.profiles.InlinedLoopConditionProfile;
@@ -122,12 +129,17 @@ static Object run(@Cached PythonObjectFactory factory) {
122129
}
123130
}
124131

125-
@CApiBuiltin(ret = PyObjectTransfer, args = {PyObject, Py_ssize_t}, call = Ignored)
126-
abstract static class PyTruffleDict_Next extends CApiBinaryBuiltinNode {
132+
@CApiBuiltin(ret = Int, args = {PyObject, PY_SSIZE_T_PTR, PyObjectPtr, PyObjectPtr, PY_HASH_T_PTR}, call = Direct)
133+
abstract static class _PyDict_Next extends CApi5BuiltinNode {
127134

128135
@Specialization
129-
static Object run(PDict dict, long pos,
136+
static int next(PDict dict, Object posPtr, Object keyPtr, Object valuePtr, Object hashPtr,
130137
@Bind("this") Node inliningTarget,
138+
@CachedLibrary(limit = "2") InteropLibrary lib,
139+
@Cached CStructAccess.ReadI64Node readI64Node,
140+
@Cached CStructAccess.WriteLongNode writeLongNode,
141+
@Cached CStructAccess.WritePointerNode writePointerNode,
142+
@Cached CApiTransitions.PythonToNativeNode toNativeNode,
131143
@Cached InlinedBranchProfile needsRewriteProfile,
132144
@Cached InlinedBranchProfile economicMapProfile,
133145
@Cached HashingStorageLen lenNode,
@@ -138,14 +150,14 @@ static Object run(PDict dict, long pos,
138150
@Cached HashingStorageIteratorKeyHash itKeyHash,
139151
@Cached PromoteBorrowedValue promoteKeyNode,
140152
@Cached PromoteBorrowedValue promoteValueNode,
141-
@Cached HashingStorageSetItem setItem,
142-
@Cached PythonObjectFactory factory) {
153+
@Cached HashingStorageSetItem setItem) {
143154
/*
144155
* We need to promote primitive values and strings to object types for borrowing to work
145156
* correctly. This is very hard to do mid-iteration, so we do all the promotion for the
146157
* whole dict at once in the first call (which is required to start with position 0). In
147158
* order to not violate the ordering, we construct a completely new storage.
148159
*/
160+
long pos = readI64Node.read(posPtr);
149161
if (pos == 0) {
150162
HashingStorage storage = dict.getDictStorage();
151163
int len = lenNode.execute(inliningTarget, storage);
@@ -200,20 +212,33 @@ static Object run(PDict dict, long pos,
200212
it.setState((int) pos - 1);
201213
boolean hasNext = itNext.execute(inliningTarget, storage, it);
202214
if (!hasNext) {
203-
return getNativeNull(inliningTarget);
215+
return 0;
216+
}
217+
long newPos = it.getState() + 1;
218+
writeLongNode.write(posPtr, newPos);
219+
if (!lib.isNull(keyPtr)) {
220+
Object key = itKey.execute(inliningTarget, storage, it);
221+
assert promoteKeyNode.execute(inliningTarget, key) == null;
222+
// Borrowed reference
223+
writePointerNode.write(keyPtr, toNativeNode.execute(key));
224+
}
225+
if (!lib.isNull(valuePtr)) {
226+
Object value = itValue.execute(inliningTarget, storage, it);
227+
assert promoteValueNode.execute(inliningTarget, value) == null;
228+
// Borrowed reference
229+
writePointerNode.write(valuePtr, toNativeNode.execute(value));
204230
}
205-
Object key = itKey.execute(inliningTarget, storage, it);
206-
Object value = itValue.execute(inliningTarget, storage, it);
207-
assert promoteKeyNode.execute(inliningTarget, key) == null;
208-
assert promoteValueNode.execute(inliningTarget, value) == null;
209-
long hash = itKeyHash.execute(inliningTarget, storage, it);
210-
int newPos = it.getState() + 1;
211-
return factory.createTuple(new Object[]{key, value, hash, newPos});
231+
if (!lib.isNull(hashPtr)) {
232+
long hash = itKeyHash.execute(inliningTarget, storage, it);
233+
writeLongNode.write(hashPtr, hash);
234+
}
235+
return 1;
212236
}
213237

214238
@Fallback
215-
Object run(@SuppressWarnings("unused") Object dict, @SuppressWarnings("unused") Object pos) {
216-
return getNativeNull();
239+
@SuppressWarnings("unused")
240+
static int run(Object dict, Object posPtr, Object keyPtr, Object valuePtr, Object hashPtr) {
241+
return 0;
217242
}
218243
}
219244

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -558,7 +558,6 @@ public final class CApiFunction {
558558
@CApiBuiltin(name = "_PyDict_GetItemStringWithError", ret = PyObject, args = {PyObject, ConstCharPtrAsTruffleString}, call = CImpl)
559559
@CApiBuiltin(name = "_PyDict_GetItem_KnownHash", ret = PyObject, args = {PyObject, PyObject, Py_hash_t}, call = CImpl)
560560
@CApiBuiltin(name = "_PyDict_NewPresized", ret = PyObject, args = {Py_ssize_t}, call = CImpl)
561-
@CApiBuiltin(name = "_PyDict_Next", ret = Int, args = {PyObject, PY_SSIZE_T_PTR, PyObjectPtr, PyObjectPtr, PY_HASH_T_PTR}, call = CImpl)
562561
@CApiBuiltin(name = "_PyDict_SetItemId", ret = Int, args = {PyObject, _PY_IDENTIFIER_PTR, PyObject}, call = CImpl)
563562
@CApiBuiltin(name = "_PyErr_BadInternalCall", ret = Void, args = {ConstCharPtr, PrimitiveResult32}, call = CImpl)
564563
@CApiBuiltin(name = "_PyErr_FormatFromCause", ret = PyObject, args = {PyObject, ConstCharPtrAsTruffleString, VARARGS}, call = CImpl)

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -221,7 +221,7 @@ public enum ArgDescriptor {
221221
PY_GEN_OBJECT(ArgBehavior.PyObject, "PyGenObject*"),
222222
PyGetSetDef(ArgBehavior.Pointer, "PyGetSetDef*"),
223223
PY_GIL_STATE_STATE(ArgBehavior.Int32, "PyGILState_STATE"),
224-
PY_HASH_T_PTR("Py_hash_t*"),
224+
PY_HASH_T_PTR(ArgBehavior.Pointer, "Py_hash_t*"),
225225
PY_IDENTIFIER("_Py_Identifier*"),
226226
PyInterpreterState(ArgBehavior.Pointer, "PyInterpreterState*"),
227227
PY_LOCK_STATUS("PyLockStatus"),

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/cext/structs/CStructAccess.java

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -455,6 +455,10 @@ public abstract static class ReadI64Node extends ReadBaseNode {
455455

456456
abstract long execute(Object pointer, long offset);
457457

458+
public final long read(Object pointer) {
459+
return execute(pointer, 0);
460+
}
461+
458462
public final long read(Object pointer, CFields field) {
459463
assert accepts(field);
460464
return execute(pointer, field.offset());

0 commit comments

Comments
 (0)