GeezerVison is showing github.com — Add a `PyThreadState *` parameter (almost) everywhere · Issue #132312 · python/cpython
Add a PyThreadState * parameter (almost) everywhere #132312DescriptionThis is part feature request, part performance issue and part a general appeal for assistance.
We should add a new variant taking a PyThreadState * parameter for most C API and internal functions.
There are two motivations for this, performance and a future, consistent C API.
Performance
The PyThreadState struct is ubiquitous in the VM, it controls stack usage, holds the freelists, holds the current exception, etc, etc.
Consequently many C functions, both API and internal, take a PyThreadState *tstate parameter.
However, for historical reasons, many C functions, both API and internal, do not such a parameter.
This leads to some fairly easy to fix inefficiencies, where spam() and eggs() take a thread state, but ham() does not, then spam() calls ham() which calls eggs(), forcing ham() to load the thread state from thread local storage, in order to pass it to eggs().
Adding a PyThreadState *tstate to ham() avoids need to access thread local storage.
Consistent, portable, future looking C API
In order to support things like tagged integers, we are going to need to a new C API.
It is out of scope to discuss what such an API would look like, but all realistic proposal so far take a "context" parameter to all, or almost all, API functions. Using a PyThreadState * parameter everywhere would make a mechanical transformation from old to new API much simpler.
Unfortunately the C API is large and cannot be changed, so we will need many new functions, like ham_tstate() which replicates ham() but with a thread state parameter.
This work is largely mechanical, and can be done by inexperienced contributors. Hence the appeal for assistance.
Linked PRs
Reacted by Sergey Miryanov and Taegyun Kim Reacted by Tim Felgentreff Reacted by Bartosz Sławecki Activitylarryhastings commented on Apr 9, 2025 I trust that as part of this work we'd also excise "borrowed references" from the API. It'd be slightly perverse to add a thread state data structure to the parameters and preserve something so un-thread-safe.
As long as you've got the eggs out, might as well make a really big omelete. neonene commented on Apr 10, 2025 The thread state is currently heavily loaded by:
I hope their overhead will be reduced first. ericvsmith commented on Apr 10, 2025
The thread state is currently heavily loaded by:
Can you elaborate on what "heavily loaded" means? taegyunkim commented on Apr 10, 2025 I'm happy to take part in this effort. I would appreciate some mentoring since I've never contributed to CPython before. neonene commented on Apr 10, 2025
Can you elaborate on what "heavily loaded" means?
The following are the frequencies of loading the tstate I observed half a year ago on main.
Running 44 PGO test suites:
entry cnt
230316309 get_state
138890858 _Py_freelists_GET
59591679 _PyObject_GC_TRACK
35143788 _PyObject_GC_Link
35114229 get_gc_state
33799583 PyObject_RichCompare
33433242 _PyType_LookupRef
33433242 get_type_cache
27214189 gc_alloc
16712566 PyErr_CheckSignals
15785715 lookup_tp_dict
14375406 _PyFunction_Vectorcall
13256280 PyObject_Vectorcall
10946665 PyErr_Occurred
8802005 gen_send_ex2
...
Running 47 pyperformance benchmarks:
entry cnt
209530845 get_state
190762133 _Py_freelists_GET
60323197 _PyObject_GC_TRACK
28580018 _PyType_LookupRef
28580018 get_type_cache
21723677 _PyObject_GC_Link
21709242 get_gc_state
20950708 _PyEval_SliceIndex
16510221 PyObject_RichCompare
15767698 gc_alloc
...
IIUC, _Py_freelists_GET() should be now more frequently used by recent commits.
When running only main-interpreter, _Py_freelists_GET() can revert the TLS access back to the previous way, where I saw some performance improvement on the macro benchmark. However, I saw little performance change when I omitted _PyThreadState_GET() from gc_alloc(). I guess improving 1% on average would not be so easy without fixing the top 2. markshannon commented on Apr 11, 2025
I trust that as part of this work we'd also excise "borrowed references" from the API. It'd be slightly perverse to add a thread state data structure to the parameters and preserve something so un-thread-safe.
We won't be changing any functions, just adding new ones. No new functions should return a borrowed reference.
Hopefully the old functions that return borrowed references will just fall out of use and can be removed. vstinner commented on Apr 23, 2025
This leads to some fairly easy to fix inefficiencies, where spam() and eggs() take a thread state, but ham() does not, then spam() calls ham() which calls eggs(), forcing ham() to load the thread state from thread local storage, in order to pass it to eggs().
So far, no calling convention for Python C API function pass tstate to the function. I'm talking about METH_O, METH_VARARGS, METH_FASTCALL, etc. If we need to pass tstate everywhere, maybe we should consider adding new calling conventions passing tstate as well.
Unfortunately the C API is large and cannot be changed, so we will need many new functions, like ham_tstate() which replicates ham() but with a thread state parameter.
Right, the C API is big, it has now more than 1,000 public functions. Would you mind to elaborate which functions should be added first to get a new tstate parameter?
There are two motivations for this, performance and a future, consistent C API.
The proposed new C API sounds like an optimization. Do you have an idea on the impact on performance of passing tstate? For example, do you expect code modified to use to run 10% faster? zooba commented on Apr 23, 2025
Do you have an idea on the impact on performance of passing tstate?
Probably not, but we do have an idea of the impact of reading it from reliable and portable thread-local storage - it's high. The more we can avoid using TLS/TSS for things that can be passed directly (on the stack/in registers), the better. vstinner commented on Apr 23, 2025
The more we can avoid using TLS/TSS for things that can be passed directly (on the stack/in registers), the better.
Passing tstate as a parameter may can also consume one register in the calling convention ABI, whereas it may be unused in some cases. zooba commented on Apr 23, 2025
Passing tstate as a parameter may can also consume one register in the calling convention ABI
Which is unpredictable enough that no benchmark is going to tell you the impact anyway. The compiler might inline it and allocate registers around it, which makes it free, or it might cause a spill to stack in some cases, which could be highly impactful in a micro sense, but likely still irrelevant compared to the TLS read.
Mark's team (and others) are watching the impact on the innermost loops really closely, so I'd expect he'll be one of the first to pull it out if it's massively degrading. Reacted by Gregory P. Smith encukou commented on May 1, 2025 If we add a whole new API layer and ask everyone to rewrite their extensions to get best speed, should we also switch all PyObject* arguments/returns to opaque typed linear references?
Otherwise we'll be adding a new layer if/when something like tagged pointers turns out to be fast. Or if type-checking arguments turns out to be slow.
This would of course also mean implementations with a moving GC could start providing CPython-compatible C API. Reacted by Benoit Daloze and Eric V. Smith zooba commented on May 5, 2025
If we add a whole new API layer and ask everyone to rewrite their extensions to get best speed ...
I don't think we're doing that yet, but if this "if" turns out to be true, then yes. I see this more as a change to internal APIs than anything like deprecating existing public APIs (neither of which you suggested, but on that spectrum "ask everyone to rewrite..." is much nearer one end than this proposal). Changing the public API to always take thread state is pointless if the internal APIs don't.
So I'd say that new public APIs get reviewed as usual, and this proposal doesn't override that.
(FTR, I think every other word in your original sentence also deserves discussion, but that can happen once we agree to invent a new public API.) ZeroIntensity commented on Jul 15, 2025 Following some of the discussion at the C API summit yesterday, it seems like we do plan to add a whole new API layer. But that's beside the point here, anyway. Has anybody started working on this for the internal C API? If not, I'm happy to get the ball rolling. zooba commented on Jul 28, 2025 @vstinner has done a decent amount of work already, pretty much all internal API, so I'd say it's worth syncing up with him (maybe in Discord chat) to see what approach he'd recommend.
MetadataMetadataDevelopmentNo branches or pull requestsIssue actions
|