bpo-40512: Store pointer to interpreter state in a thread local variable

[markshannon]

A more future proof way to handle interpreter state. 1% faster as well.

https://bugs.python.org/issue40512

[bedevere-bot]

🤖 New build scheduled with the buildbot fleet by @markshannon for commit ada883d6b97bb072ef8465b71671ef9db101c7ae 🤖

If you want to schedule another build, you need to add the ":hammer: test-with-buildbots" label again.

[markshannon]

Build failures look like the usual suspects.

[pablogsal]

This looks goot to me in general but it would be good if @vstinner could take a look since he has been modifying these APIs recently a lot.

[erlend-aasland]

Suggested change

	if (PyInterpreterState_Get() != PyInterpreterState_Main()) {
	PyInterpreterState *interp = PyInterpreterState_Get();
	if (!_Py_IsMainInterpreter(interp)) {

[markshannon]

It is too easy to overlook the ! when making this change. Using the comparison operator makes it clearer that we are checking that interp is not the main interpreter.
Both @ericsnowcurrently and I have made this mistake recently.

[markshannon]

@vstinner Any comments?

[tiran]

The old approach had a NULL check and Py_FatalError. The check is useful for embedders. They get a sensible error message when they forget to create a thread state for an external thread. Would it be possible to retain the check for debug builds?

[vstinner]

@vstinner Any comments?

Sorry, I don't have the bandwidth to review this change :-(

[ericsnowcurrently]

Why the current interpreter and not the current PyThreadState?

[markshannon]

Two reasons.

1. I though this would add the most value, as we need the interpreter state to get to freelists which are performance critical.
2. We already pass the threadstate as an argument to a lot of functions, so I wanted to verify this approach before rewriting all that code.

[ericsnowcurrently]

Would it make sense to do both? (They are both hot in different situations.) What is the cost to the number of thread-local variables?

[ericsnowcurrently]

Why the switch to PyInterpreterState_Get()?

[markshannon]

Modules cannot access a thread-local directly because they are dynamically loaded. It is a loader, rather a language, limitation, I think.

[ericsnowcurrently]

That's good to know. Thanks for explaining. This might not be obvious to maintainers. Would you mind adding a short comment next to the declaration of _py_current_interpreter noting the constraint?

Also, this doesn't affect builtin modules, right? Aren't some of the modules updated here builtin modules?

[markshannon]

I'm dropping this as there are a few reasons why passing the interpreter state explicitly make sense.

• Accessing thread locals for DLL can be slow
• Passing the interpreter state explicitly make migrating to a new API easier
• We can pass around different aspects of the interpreter, e.g. the GIL, clarifying what parts of the system code is allowed to access.