On 08Jan2020 0746, Victor Stinner wrote:

The question is now if we should "propagate" tstate to function calls in the latest VECTORCALL calling convention (which is currently private). +1, for all the same reasons I'm +1 on passing it to C functions explicitly. (In short, embedding can be very painful when you have to manipulate ambient thread state to make Python code work, and this is a good step towards getting rid of that requirement.)

Cheers, Steve

Le mer. 8 janv. 2020 à 22:15, Nick Coghlan a écrit :

The vectorcall convention places a significant emphasis on speed, so being able to do a single PyThreadState_Get() call on the initial transition from Python to C, and then pass the thread state explicitly after that would make a lot of sense.

Honestly, I'm not 100% sure that performance is a good argument in favor of adding tstate to the calling convention, since we cannot measure yet the overhead in term of performance of calling frequently _PyThreadState_GET(), especially with hypothetical changes on the GIL state API. I can only "feel" that it might be a performance issue tomorrow. For me a stronger argument is that it should help to make Python more "correct" in the case of subinterpreters. Help to get the right state, since today too many things are "implicit" (rely on "globals").

So I think this is a good idea, but we might need to place a caveat on Python 3.9 that passing in a thread state that is NOT the currently active thread state may not work correctly yet (it's going to take a while to make sure that we never inadvertently call back into "implicit thread state" APIs from the implementation of APIs that accept an explicit thread state, and even after CPython itself is fixed, there's no telling what other extension modules might be doing).

In term of API, we may keep the current *API* unchanged: pass implicitly the current Python thread state. For example, _PyObject_Vectorcall() doesn't take a tstate argument, but gets the current Python thread state. static inline PyObject * _PyObject_Vectorcall(PyObject *callable, PyObject *const *args, size_t nargsf, PyObject *kwnames) { PyThreadState *tstate = PyThreadState_GET(); return _PyObject_VectorcallTstate(tstate, callable, args, nargsf, kwnames); } (I added _PyObject_VectorcallTstate() recently :-)) We can keep this prototype for the public API, but have a *private* API which allows to pass tstate. My idea is that internally, tstate would be passed *explicitly*. For example: res = PyObject_Vectorcall(func, ...); // public API, 3rd party code vs res = _PyObject_Vectorcall(tstate, func, ...); // private API, Python internals My first question is about the calling convention, not the API ;-) Victor -- Night gathers, and now my watch begins. It shall not end until my death.

Le jeu. 9 janv. 2020 à 13:35, Mark Shannon a écrit :

Passing the thread state explicitly creates a new class of errors that was not there before. What happens if the tstate argument is NULL,

You will likely get a crash.

or points to a different thread?

No idea what will happen. If it's a different subinterpreter, bad things can happen I guess :-) Well, I don't see this class of bug as a blocker issuer. If you pass invalid data, you get a crash. I'm not surprised by that. In my previous email, I proposed to continue to pass implicitly tstate when you call a function using the public C API. Only Python internals would pass explicitly tstate and so only internals should be carefully reviewed.

There is a one-to-one correspondence between Python threads and O/S threads.

I'm not convinced that this :-) So far, it's unclear to me how the transition from one interpreter to another happens with Python thread states. Let's say that the main thead spawns a thread A. The main thread and the thread A are running in the main interpreter. Then thread A calls _testcapi.run_in_subinterp() or _xxsubinterpreter.run_string() (or something else to run code in a subinterpreter). A subinterpreter is created and a new and different Python thread state is "attached" to thread A. The thread A gets 2 Python thread states, one per interpreter. It gets one or the other depending which in which interpreter the thread is running... We have to be careful to pass the proper thread state to internal C functions while doing this dance between two interpreters in the same thread. PyThreadState_Swap(tstate) must be called to set the current Python thread state. Full implementation of _testcapi.run_in_subinterp(), notice the PyThreadState_Swap() dance: /* To run some code in a sub-interpreter. */ static PyObject * run_in_subinterp(PyObject *self, PyObject *args) { const char *code; int r; PyThreadState *substate, *mainstate; if (!PyArg_ParseTuple(args, "s:run_in_subinterp", &code)) return NULL; mainstate = PyThreadState_Get(); PyThreadState_Swap(NULL); substate = Py_NewInterpreter(); if (substate == NULL) { /* Since no new thread state was created, there is no exception to propagate; raise a fresh one after swapping in the old thread state. */ PyThreadState_Swap(mainstate); PyErr_SetString(PyExc_RuntimeError, "sub-interpreter creation failed"); return NULL; } r = PyRun_SimpleString(code); Py_EndInterpreter(substate); PyThreadState_Swap(mainstate); return PyLong_FromLong(r); }

So the threadstate can, and should, be stored in a thread local variable. Accessing thread local storage is fast. x86/64 uses the fs register to point to it, whereas ARM dedicates R15 (I think).

*If* we managed to design subinterpreters in a way that a native thread is always associated to the same Python interpreter, moving to a thread local variable can make Python more efficient :-) It is likely to be more efficient than the current atomic variable design.

...

I started to move more and more things from "globals" to "per interpreter". For example, the garbage collector is now "per interpreter" (lives in PyThreadState). Small integer singletons are now also "per singleton": int("1") are now different objects in each interpreter, whereas they were shared previously. Later, even "None" singleton (and all other singletons) should be made "per interpreter". Getting a "per interpreter" object requires to state from the Python thread state: call _PyThreadState_GET(). Avoiding _PyThreadState_GET() calls reduces any risk of making Python slower with incoming subinterpreter changes.

Thread locals are not "global". Each sub-interpreter will have its own pool of threads. Each threadstate object should contain a pointer to its sub-interpreter.

Getting the interpreter from a Python thread state is trivial: interp = tstate->interp. The problem is how to get the current Python thread state. *Currently*, it's an atomic variable. But tomorrow with multiple interpreters running in parallel, I expect that it's going to me more expensive like first having the current the interpreter running the current native thread, and then get the Python thread state of this interpreter. Or something like that. We may get more and more indirections... Victor -- Night gathers, and now my watch begins. It shall not end until my death.

Le jeu. 9 janv. 2020 à 19:33, Steve Dower a écrit :

Requiring an _active_ Python thread (~GIL held) to only run on a single OS thread is fine, but tying it permanently to a single OS thread makes things very painful. (Of course, this isn't the only thing that makes it painful, but if we're going to make this a deliberate design of CPython then we are essentially excluding entire interesting classes of embedding.)

Do you have an use case where you reuse the same Python thread state in different native threads? Currently, the Python runtime expects to have a different Python thread state per native thread. For example, PyGILState_Ensure() creates one if there is no Python thread state associated to the current thread. Victor -- Night gathers, and now my watch begins. It shall not end until my death.

Victor Stinner wrote:

I started to modify Python internals to pass explicitly the Python thread state ("tstate") to internal C a functions:

Is work this summarized somewhere like a PEP?

https://vstinner.github.io/cpython-pass-tstate.html

describes the motivation, but not the details on why what is changed. In particular, I am wondering what proportion of functions actually need the tstate, vs. how often it would just be adding some extra pack-pass-unpack work for no purpose.

To me, it sounds like a good practice to pass the current Python thread state to internal C functions.

Only if they need it (see above) and the passed data is at least as reliable as what they would get by using the current process. If the current process always gets the right answer, and the new process can sometimes gets the wrong one (even just through human error), that seems like a high price to pay.

... Later, even "None" singleton (and all other singletons) should be made "per interpreter". [ later no objects should be shared ]

At that point, it makes sense to feed them from different memory pools, and I am not sure how much advantage there is to a separate interpreter as opposed to a separate process.

On Thu, 9 Jan 2020 15:12:41 +0100 Victor Stinner wrote:

Getting the interpreter from a Python thread state is trivial: interp = tstate->interp.

The problem is how to get the current Python thread state. *Currently*, it's an atomic variable. But tomorrow with multiple interpreters running in parallel, I expect that it's going to me more expensive like first having the current the interpreter running the current native thread, and then get the Python thread state of this interpreter. Or something like that. We may get more and more indirections...

It can still be simple. Store the current interpreter in a thread-local variable (there can be only one interpreter running at a given time in a given OS thread, even if it's not always the same interpreter). Then have the interpreter struct contain a pointer to the current running thread *in that interpreter*. So you have one thread-local access to the current interpreter and then a normal struct member access to the current thread state. For example: struct _ts; struct _is { struct _ts* current_thread; }; _Thread_local struct _is *current_interp; inline struct _is *_PyInterpreterState_GET() { return current_interp; } inline struct _ts *_PyThreadState_GET() { return current_interp->current_thread; } Regards Antoine.