Add a PyThreadState * parameter (almost) everywhere

[markshannon]

This 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.

[larryhastings]

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]

The thread state is currently heavily loaded by:

• get_state() in obmalloc.c

• _Py_freelists_GET() in pycore_freelist.h

I hope their overhead will be reduced first.

[ericvsmith]

The thread state is currently heavily loaded by:

Can you elaborate on what "heavily loaded" means?

[taegyunkim]

I'm happy to take part in this effort. I would appreciate some mentoring since I've never contributed to CPython before.

[neonene]

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]

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]

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]

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]

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]

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.

[encukou]

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.

[zooba]

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.)