GH-132554: "Virtual" iterators #132555
GH-132554: "Virtual" iterators #132555
Conversation
417cd59 to
6c955e0
Compare
|
Performance is good. Nothing amazing, but a small speedup. Stats show no significant changes |
a4b740d to
025049d
Compare
| @@ -233,6 +233,9 @@ extern intptr_t PyStackRef_UntagInt(_PyStackRef ref); | |||
|
|
|||
| extern _PyStackRef PyStackRef_TagInt(intptr_t i); | |||
|
|
|||
| /* Increments a tagged int, but does not check for overflow */ | |||
| extern _PyStackRef PyStackRef_IncrementTaggedIntNoOverflow(_PyStackRef ref); | |||
|
|
|||
There was a problem hiding this comment.
I don't think you need this declaration since the definition is here.
There was a problem hiding this comment.
This is the Py_STACKREF_DEBUG declaration. The definition below is the non-debug version.
Python/specialize.c
Outdated
| if (_PyInterpreterState_GET()->eval_frame) | ||
| goto failure; |
There was a problem hiding this comment.
| if (_PyInterpreterState_GET()->eval_frame) | |
| goto failure; | |
| if (_PyInterpreterState_GET()->eval_frame) { | |
| goto failure; | |
| } |
| PyStackRef_IncrementTaggedIntNoOverflow(_PyStackRef ref) | ||
| { | ||
| assert(ref.index != (uintptr_t)-1); // Overflow | ||
| return (_PyStackRef){ .index = ref.index + 2 }; |
There was a problem hiding this comment.
It's a bit weird that "increment" does += 2. Maybe "Advance"?
There was a problem hiding this comment.
It increases the value by one. The low bit is the tag bit.
Python/ceval.c
Outdated
| _PyForIter_NextWithIndex(PyObject *seq, _PyStackRef index) | ||
| { | ||
| assert(PyStackRef_IsTaggedInt(index)); | ||
| assert(Py_TYPE(seq) == &PyTuple_Type || Py_TYPE(seq) == &PyList_Type); |
Python/ceval.c
Outdated
| if ((size_t)i >= size) { | ||
| return PyStackRef_NULL; | ||
| } | ||
| PyObject *next_o = PySequence_Fast_GET_ITEM(seq, i); |
Python/ceval.c
Outdated
| if ((size_t)i >= size) { | ||
| return PyStackRef_NULL; | ||
| } | ||
| PyObject *next_o = PySequence_Fast_GET_ITEM(seq, i); |
There was a problem hiding this comment.
This isn't thread safe for FT. Check out how to do this in the function _PyList_GetItemRefNoLock.
| size_t size = PyList_GET_SIZE(seq); | ||
| if ((size_t)i >= size) { | ||
| return PyStackRef_NULL; | ||
| } | ||
| #ifdef Py_GIL_DISABLED | ||
| PyObject *item = _PyList_GetItemRef((PyListObject *)seq, i); | ||
| if (item == NULL) { | ||
| return PyStackRef_NULL; | ||
| } | ||
| return PyStackRef_FromPyObjectSteal(item); | ||
| #else | ||
| return PyStackRef_FromPyObjectNew(PyList_GET_ITEM(seq, i)); | ||
| #endif |
There was a problem hiding this comment.
_PyList_GetItemRef does the same thing as this entire block (check index and get the item). It also behaves differently on FT and default builds, so you can just use that here, and replace all this code.
Just a draft PR until I have performance numbers.