Repository navigation
Tuples should be immutable and safe in C, as well as in Python. #127058
Description
Activity
I'm filing this as a bug rather than an enhancement as it does cause bugs. Most notably: https://github.com/python/cpython/blob/main/Lib/test/crashers/gc_inspection.py
PyTuple_SetItemandPyTuple_SET_ITEMneed to go too.This requires a massive change to C extensions, but doesn't actually address the problem that "tuples should be immutable."
Change PyTuple_New to fill the tuple with pointers to None instead of NULL
This would break backwards compatibility with C extensions. See some examples: https://github.com/search?q=%2FPyTuple_GET_ITEM.*%3D%3D.*NULL%2F&type=code
gc.get_referrers has a lot of issues, not just with tuples. As the linked files writes:
Note that this is only an example. There are many ways to crash Python
by using gc.get_referrers(), as well as many extension modules (even
when they are using perfectly documented patterns to build objects).Reacted by Raymond HettingerIt might take a while for people to stop using
PyTuple_New, but that doesn't mean we shouldn't deprecate it.
I suspect we won't be removing it for a long time.I think you need to refine your search for
PyTuple_GET_ITEMandNULL. A lot of the results look likefoo(PyTuple_GET_ITEM(tuple, index)) == NULLwhich is fine.gc.get_referrers has a lot of issues, not just with tuples.
Such as?
Partially created tuples may not be the only culprit, but they are the main one, I think.
It might take a while for people to stop using PyTuple_New, but that doesn't mean we shouldn't deprecate it
Deprecating commonly used APIs imposes a cost to C API extension authors even when we don't remove the existing API. Many extension authors will change their code in order to adopt the new best practices. If we don't have a foreseeable path to actually removing
PyTuple_New()then it's unlikely that we'll ever reap the benefits of the deprecation. We'll have created churn for extension authors without delivering any benefits.I think you need to refine your search...
My point is that it's a breaking change. The search does not capture all the uses that would break either.
gc.get_referrers has a lot of issues, not just with tuples. Such as?
As Armin wrote in #39117: "Expecting an object not to be seen before you first hand it out is extremely common, and get_referrers() breaks that assumption."
PyType_GenericAllocis the most common way to create extension objects, and it returns an object that is already tracked, but not yet initialized (other than zero initialization).
- In addition to deprecating
PyTuple_New(), won't this also require deprecatingPyTuple_SET_ITEMandPyTuple_SetItemfor initializing tuples? - Other than the proposed
PyTuple_MakePair, what are the intended replacements forPyTuple_New()?PyTuple_Pack()? How is someone supposed to create a tuple with a dynamic number of arguments? - Where is the extra complexity that this would improve? There's not a whole lot of tuple-specific code in the GC, and I don't see any GC code that is dedicated to handling tuple mutability.
- In addition to deprecating
Other than the proposed PyTuple_MakePair, what are the intended replacements for PyTuple_New()? PyTuple_Pack()? How is someone supposed to create a tuple with a dynamic number of arguments?
We already have
PyTuple_FromArraywhich is an efficient way to create a tuple. We could exposePyTuple_FromArrayStealwhich is even more efficient if you don't need the references to the values in the array.For creating tuples of unknown size, the best way is to create a list and then convert it to a tuple with
PyList_AsTuple
If the extra overhead of clearing the list is a concern we could addPyList_AsTupleAndClearwhich would clear the list at the same time as creating the tuple, saving the cost of modifying reference counts.Looking at the uses of
PyTuple_Newin CPython, prepending an object to a tuple is surprising common. So we could add a new functionPyTuple_Prepend(PyObject *first_item, PyObject *tuple).PyTupleConcatis another possibility.Where is the extra complexity that this would improve?
Not just the GC, but in the optimizer and in memory management. Knowing that tuples are genuinely immutable allows some useful savings and a few tricks.
Mainly it allows us to make reasoned improvements. For example, when untracking tuples in the GC we should be able to assume that, thanks to immutability, a tuple must be younger than the objects it contains. Therefore a simple oldest-first scan should find all tuples that can be untracked. Sadly, this isn't the case. Likewise, we would like to know that tuples cannot contain themselves.
(sorry I missed the comment on triaged)
With
PyTuple_FromArraynow incoming in 3.15, should I consider switching PyO3 to it fromPyTuple_New+PyTuple_SET_ITEM? A quick test with 3.15.0a7 suggests that crude benchmarks which convert Rust 2- and 12-tuples of integers to Python tuples gets ~25% slower usingPyTuple_FromArray.using
PyTuple_New+PyTuple_SET_ITEM:tuple_into_pyobject_2 time: [7.5477 ns 7.5943 ns 7.6465 ns] tuple_into_pyobject_12 time: [22.022 ns 22.183 ns 22.341 ns]using
PyTuple_FromArray:tuple_into_pyobject_2 time: [9.4254 ns 9.4982 ns 9.5704 ns] change: [+23.217% +24.299% +25.284%] (p = 0.00 < 0.05) Performance has regressed. tuple_into_pyobject_12 time: [27.944 ns 28.062 ns 28.181 ns] change: [+25.246% +26.118% +27.009%] (p = 0.00 < 0.05) Performance has regressed.
I am sad that
PyTuple_FromArrayStealwas voted against by the C API working group, I think particularly at the FFI boundary PyO3 is sensitive to this because there are many cases where Python objects are built from Rust values and immediately placed into Python tuples in a fashion similar to the benchmark above.Not only is the performance of
PyTuple_FromArrayworse than the status quo, but it's also worse for code size because the Rust compiler has to emit calls toPy_DecReffor all these new Python objects being placed directly into the tuples.If the concensus is that PyO3 should switch to
PyTuple_FromArraynow so that we can clean up the C API, I'm happy to eat the cost for the greater good. Hopefully we can regain that performance in the future 🤞Reacted by Bas SchoenmaeckersIn my opinion, no. What do you gain? This is one of the most commonly used C API functions.
PyO3 indeed doesn't gain much from using
PyTuple_FromArray.With the stealing variant it might be the opposite; we could delete some existing code which fills tuples created by
PyTuple_New, and possibly even benefit from the interpreter being able to knowingly skip stages like zeroing the tuple memory before filling it. Now I think about it, we also don't havePyTuple_SET_ITEMin the stable ABI, we make repeated calls toPyTuple_SetIteminstead, so extensions built for the stable ABI might be accidentally suffering here. A single FFI call would be optimal.I keep seeing discussions like this one and capi-workgroup/problems#56 which makes me believe that the long term desire from many is to deprecate
PyTuple_New, provided suitable replacements exist. If that's the case, perhaps we want extensions to stop using it where possible already, as a gentle forcing function towards finding good replacement APIs.From a performance perspective,
PyTuple_FromArrayis likely to be faster only if you need to retain the references to the objects in the array.@davidhewitt
OOI, when wasPyTuple_FromArrayStealvoted against by the C API working group?There was the opinion that it was a micro-optimization not worth having. capi-workgroup/decisions#78
Bug report
Bug description:
[Apologies if this sounds a bit like a rant. I'm not blaming anyone. Just because something is the wrong choice now, doesn't mean it wasn't the right choice historically]
Tuples are immutable in Python, but we play all sorts of games in C with tuples, filling them will
NULLs, mutating them and reusing them.We do this in the mistaken belief that it improves performance.
But it doesn't. It makes the code base more complicated and fragile as we need to work around tuples that misbehave and do strange things. Any local performance gain is overwhelmed by slowdowns caused by the extra complexity in tuple code, the garbage collector and a few other places.
So let's fix this.
We need to:
PyTuple_MakePair(). Pairs are by far the most common type of tuple that we play games with. By providing a fast way to create pairs, we can provide an upgrade path for C code that creates tuples in unsafe ways to do so safely and quickly.PyTuple_New. I don't know when we'll be able to remove it, but we should deprecate it ASAP.PyTuple_Newto fill the tuple with pointers toNoneinstead ofNULL. This doesn't fix the mutability issue, but it at least means the GC will only see valid objects. (This might break too much code, so we might just have to clearly document that tuples should be fully initialized in one go, before the tuple escapes the function it was created in)PyTuple_New()or perform tuple shenanigans. We can't reasonably expect third-party package authors to follow the rules if we don't.CPython versions tested on:
CPython main branch
Operating systems tested on:
No response
Linked PRs
PySequence_Tuplesafer and probably faster. #127758