Repository navigation
Conversation
| /// placeholder exception after an element conversion error. | ||
| type ItemProducer = dyn FnMut() -> *mut ffi::PyObject; | ||
|
|
||
| crate::platform::thread_local! { |
There was a problem hiding this comment.
I looked at storing this in type data instead and it ended up slower because it adds function call overhead
|
|
||
| // TODO(subinterpreters): Cache the `ListBuildIterator` heap type per interpreter once PyO3 | ||
| // supports subinterpreters. | ||
| fn get_iterator_type(py: Python<'_>) -> &Py<PyType> { |
There was a problem hiding this comment.
Is it possible to create this type using our marco machinery? This will reduce code complexity significantly.
|
Nice work! Should we have a benchmark for this that shows the improvement and future performance? |
| /// `PyType_FromSpec` creates the `pyo3.ListBuildIterator` type without the `macros` feature. | ||
| /// A `#[pymethods]` `__next__` trampoline costs more per item than the critical section. |
There was a problem hiding this comment.
Sorry, I've missed this explanation. Can we improve the trampoline to reduce the cost in any way?
There was a problem hiding this comment.
The other issue is that pyclass needs the macros feature. It would be nice if this optimization didn't rely on that feature.
We could but right now we also don't have any limited API benchmark runs in CI. I think if we had an abi3t run the current benchmarks would show a significant improvement in relevant benchmarks. I'll need to double-check that. |
On free-threaded CPython,
abi3tPyList::newcallsPyList_SetItemfor every item, entering the new list's critical section each time (CPython #158660). For size hints of at least 32,PyList::newinstead uses a small C-API iterator withPySequence_Listso CPython can preallocate and fill the list under one critical section. Short lists and GIL-enabled interpreters keep the existing path.At 1,024 existing objects and above, the iterator path is 62–75% faster across macOS, arm64 Linux, Windows, and WSL2. The 32 item cutoff is conservative: iterator setup can offset the savings for short lists.
I had to add a small manually defined static heap type since
PyList::newneeds to work withoutpyclassand the macros feature. As far as I can tell this is the first spot that defines a type like this, although we define exceptions and functions manually like this in a few spots. I added a TODO to point to the fact that this is incompatible with subinterpreters.Here's a plot that summarizes the choices in this PR:
The dashed blue points are using the iterator approach below the 32 item cutoff to show that this approach performs badly for small lists and to show why I picked 32. The solid blue curve measures the performance improvement from this patch from 32 items onward. The pink line shows the outer-critical-section proposal in #6478. The yellow line shows an alternate idea based on building a tuple and calling
PyList_FromSequenceon it. The greenPyList_SET_ITEMcurve shows the theoretical maximum speedup, if we were able to use APIs outside the stable ABI. All curves use existing objects and are relative to the currentabi3tPyList_SetItempath in the same environment.