Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
9 changes: 5 additions & 4 deletions Include/internal/pycore_long.h
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,14 @@ static inline PyObject* __PyLong_GetSmallInt_internal(int value)

// Return a borrowed reference to the zero singleton.
// The function cannot return NULL.
static inline PyObject* _PyLong_GetZero(void)
{ return __PyLong_GetSmallInt_internal(0); }
PyAPI_FUNC(PyObject*) _PyLong_GetZero(void);

// Return a borrowed reference to the one singleton.
// The function cannot return NULL.
static inline PyObject* _PyLong_GetOne(void)
{ return __PyLong_GetSmallInt_internal(1); }
PyAPI_FUNC(PyObject*) _PyLong_GetOne(void);

#define PY_ZERO() ((PyObject *)_PyInterpreterState_GET()->small_ints[_PY_NSMALLNEGINTS])
#define PY_ONE() ((PyObject *)_PyInterpreterState_GET()->small_ints[_PY_NSMALLNEGINTS+1])

PyObject *_PyLong_Add(PyLongObject *left, PyLongObject *right);
PyObject *_PyLong_Multiply(PyLongObject *left, PyLongObject *right);
Expand Down
29 changes: 15 additions & 14 deletions Include/internal/pycore_pystate.h
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ extern "C" {
# error "this header requires Py_BUILD_CORE define"
#endif

#include "pycore_tls.h" /* PyRuntimeState */
#include "pycore_runtime.h" /* PyRuntimeState */


Expand All @@ -20,15 +21,8 @@ _Py_IsMainThread(void)
return (thread == _PyRuntime.main_thread);
}


static inline int
_Py_IsMainInterpreter(PyInterpreterState *interp)
{
/* Use directly _PyRuntime rather than tstate->interp->runtime, since
this function is used in performance critical code path (ceval) */
return (interp == _PyRuntime.interpreters.main);
}

extern int
_Py_IsMainInterpreter(PyInterpreterState *interp);

static inline const PyConfig *
_Py_GetMainConfig(void)
Expand All @@ -40,6 +34,9 @@ _Py_GetMainConfig(void)
return _PyInterpreterState_GetConfig(interp);
}

#ifdef _Py_THREAD_LOCAL
extern _Py_THREAD_LOCAL PyInterpreterState *_py_current_interpreter;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why the current interpreter and not the current PyThreadState?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

#endif

/* Only handle signals on the main thread of the main interpreter. */
static inline int
Expand Down Expand Up @@ -107,7 +104,6 @@ _Py_EnsureFuncTstateNotNULL(const char *func, PyThreadState *tstate)
#define _Py_EnsureTstateNotNULL(tstate) \
_Py_EnsureFuncTstateNotNULL(__func__, tstate)


/* Get the current interpreter state.

The macro is unsafe: it does not check for error and it can return NULL.
Expand All @@ -116,14 +112,19 @@ _Py_EnsureFuncTstateNotNULL(const char *func, PyThreadState *tstate)

See also _PyInterpreterState_Get()
and _PyGILState_GetInterpreterStateUnsafe(). */
#ifdef _Py_THREAD_LOCAL
#define _PyInterpreterState_GET() _py_current_interpreter
#else
static inline PyInterpreterState* _PyInterpreterState_GET(void) {
PyThreadState *tstate = _PyThreadState_GET();
#ifdef Py_DEBUG
_Py_EnsureTstateNotNULL(tstate);
#endif
return tstate->interp;
PyInterpreterState *interp = tstate->interp;
if (interp == NULL) {
Py_FatalError("no current interpreter");
}
return interp;
}

#endif

// PyThreadState functions

Expand Down
18 changes: 18 additions & 0 deletions Include/internal/pycore_tls.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@

#ifndef Py_CORE_TLS_H
#define Py_CORE_TLS_H

#ifndef Py_BUILD_CORE
# error "this header requires Py_BUILD_CORE to be defined"
#endif

#ifndef _Py_THREAD_LOCAL
#if defined(__GNUC__)
#define _Py_THREAD_LOCAL __thread
#endif
#if defined(_MSC_VER)
#define _Py_THREAD_LOCAL __declspec(thread)
#endif
#endif

#endif /* Py_CORE_TLS_H */
1 change: 1 addition & 0 deletions Makefile.pre.in
Original file line number Diff line number Diff line change
Expand Up @@ -1274,6 +1274,7 @@ PYTHON_HEADERS= \
$(srcdir)/Include/internal/pycore_structseq.h \
$(srcdir)/Include/internal/pycore_symtable.h \
$(srcdir)/Include/internal/pycore_sysmodule.h \
$(srcdir)/Include/internal/pycore_tls.h \
$(srcdir)/Include/internal/pycore_traceback.h \
$(srcdir)/Include/internal/pycore_tuple.h \
$(srcdir)/Include/internal/pycore_ucnhash.h \
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
A pointer to the interpreter state is stored in a thread local variable
on platforms that support it.
2 changes: 1 addition & 1 deletion Modules/_functoolsmodule.c
Original file line number Diff line number Diff line change
Expand Up @@ -703,7 +703,7 @@ functools_reduce(PyObject *self, PyObject *args)
// bpo-42536: The GC may have untracked this args tuple. Since we're
// recycling it, make sure it's tracked again:
if (!_PyObject_GC_IS_TRACKED(args)) {
_PyObject_GC_TRACK(args);
PyObject_GC_Track(args);
}
}
}
Expand Down
8 changes: 4 additions & 4 deletions Modules/_io/_iomodule.c
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
#define PY_SSIZE_T_CLEAN
#include "Python.h"
#include "_iomodule.h"
#include "pycore_pystate.h" // _PyInterpreterState_GET()
#include "pycore_pystate.h" // PyInterpreterState_Get()

#ifdef HAVE_SYS_TYPES_H
#include <sys/types.h>
Expand Down Expand Up @@ -91,7 +91,7 @@ PyDoc_STRVAR(module_doc,
" I/O classes. open() uses the file's blksize (as obtained by os.stat) if\n"
" possible.\n"
);


/*
* The main open() function
Expand Down Expand Up @@ -509,7 +509,7 @@ _io_text_encoding_impl(PyObject *module, PyObject *encoding, int stacklevel)
/*[clinic end generated code: output=91b2cfea6934cc0c input=bf70231213e2a7b4]*/
{
if (encoding == NULL || encoding == Py_None) {
PyInterpreterState *interp = _PyInterpreterState_GET();
PyInterpreterState *interp = PyInterpreterState_Get();
Copy link
Member

@ericsnowcurrently ericsnowcurrently Dec 7, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why the switch to PyInterpreterState_Get()?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

if (_PyInterpreterState_GetConfig(interp)->warn_default_encoding) {
if (PyErr_WarnEx(PyExc_EncodingWarning,
"'encoding' argument not specified", stacklevel)) {
Expand Down Expand Up @@ -542,7 +542,7 @@ _io_open_code_impl(PyObject *module, PyObject *path)
{
return PyFile_OpenCodeObject(path);
}

/*
* Private helpers for the io module.
*/
Expand Down
6 changes: 3 additions & 3 deletions Modules/_io/textio.c
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
#include "pycore_long.h" // _PyLong_GetZero()
#include "pycore_fileutils.h" // _Py_GetLocaleEncoding()
#include "pycore_object.h"
#include "pycore_pystate.h" // _PyInterpreterState_GET()
#include "pycore_pystate.h" // PyInterpreterState_Get()
#include "structmember.h" // PyMemberDef
#include "_iomodule.h"

Expand Down Expand Up @@ -996,7 +996,7 @@ io_check_errors(PyObject *errors)
{
assert(errors != NULL && errors != Py_None);

PyInterpreterState *interp = _PyInterpreterState_GET();
PyInterpreterState *interp = PyInterpreterState_Get();
#ifndef Py_DEBUG
/* In release mode, only check in development mode (-X dev) */
if (!_PyInterpreterState_GetConfig(interp)->dev_mode) {
Expand Down Expand Up @@ -1086,7 +1086,7 @@ _io_TextIOWrapper___init___impl(textio *self, PyObject *buffer,
self->detached = 0;

if (encoding == NULL) {
PyInterpreterState *interp = _PyInterpreterState_GET();
PyInterpreterState *interp = PyInterpreterState_Get();
if (_PyInterpreterState_GetConfig(interp)->warn_default_encoding) {
if (PyErr_WarnEx(PyExc_EncodingWarning,
"'encoding' argument not specified", 1)) {
Expand Down
8 changes: 4 additions & 4 deletions Modules/_threadmodule.c
Original file line number Diff line number Diff line change
Expand Up @@ -850,7 +850,7 @@ local_clear(localobject *self)
Py_CLEAR(self->wr_callback);
/* Remove all strong references to dummies from the thread states */
if (self->key) {
PyInterpreterState *interp = _PyInterpreterState_GET();
PyInterpreterState *interp = PyInterpreterState_Get();
PyThreadState *tstate = PyInterpreterState_ThreadHead(interp);
for(; tstate; tstate = PyThreadState_Next(tstate)) {
if (tstate->dict == NULL) {
Expand Down Expand Up @@ -1139,7 +1139,7 @@ thread_PyThread_start_new_thread(PyObject *self, PyObject *fargs)
return NULL;
}

PyInterpreterState *interp = _PyInterpreterState_GET();
PyInterpreterState *interp = PyInterpreterState_Get();
if (interp->config._isolated_interpreter) {
PyErr_SetString(PyExc_RuntimeError,
"thread is not supported for isolated subinterpreters");
Expand All @@ -1150,7 +1150,7 @@ thread_PyThread_start_new_thread(PyObject *self, PyObject *fargs)
if (boot == NULL) {
return PyErr_NoMemory();
}
boot->interp = _PyInterpreterState_GET();
boot->interp = PyInterpreterState_Get();
boot->tstate = _PyThreadState_Prealloc(boot->interp);
if (boot->tstate == NULL) {
PyMem_Free(boot);
Expand Down Expand Up @@ -1278,7 +1278,7 @@ particular thread within a system.");
static PyObject *
thread__count(PyObject *self, PyObject *Py_UNUSED(ignored))
{
PyInterpreterState *interp = _PyInterpreterState_GET();
PyInterpreterState *interp = PyInterpreterState_Get();
return PyLong_FromLong(interp->num_threads);
}

Expand Down
4 changes: 2 additions & 2 deletions Modules/atexitmodule.c
Original file line number Diff line number Diff line change
Expand Up @@ -9,15 +9,15 @@
#include "Python.h"
#include "pycore_initconfig.h" // _PyStatus_NO_MEMORY
#include "pycore_interp.h" // PyInterpreterState.atexit
#include "pycore_pystate.h" // _PyInterpreterState_GET
#include "pycore_pystate.h" // PyInterpreterState_Get

/* ===================================================================== */
/* Callback machinery. */

static inline struct atexit_state*
get_atexit_state(void)
{
PyInterpreterState *interp = _PyInterpreterState_GET();
PyInterpreterState *interp = PyInterpreterState_Get();
return &interp->atexit;
}

Expand Down
2 changes: 1 addition & 1 deletion Modules/gcmodule.c
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ gc_decref(PyGC_Head *g)
static GCState *
get_gc_state(void)
{
PyInterpreterState *interp = _PyInterpreterState_GET();
PyInterpreterState *interp = PyInterpreterState_Get();
return &interp->gc;
}

Expand Down
4 changes: 2 additions & 2 deletions Modules/main.c
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
#include "pycore_interp.h" // _PyInterpreterState.sysdict
#include "pycore_pathconfig.h" // _PyPathConfig_ComputeSysPath0()
#include "pycore_pylifecycle.h" // _Py_PreInitializeFromPyArgv()
#include "pycore_pystate.h" // _PyInterpreterState_GET()
#include "pycore_pystate.h" // PyInterpreterState_Get()

/* Includes for exit_sigint() */
#include <stdio.h> // perror()
Expand Down Expand Up @@ -534,7 +534,7 @@ pymain_repl(PyConfig *config, int *exitcode)
static void
pymain_run_python(int *exitcode)
{
PyInterpreterState *interp = _PyInterpreterState_GET();
PyInterpreterState *interp = PyInterpreterState_Get();
/* pymain_run_stdin() modify the config */
PyConfig *config = (PyConfig*)_PyInterpreterState_GetConfig(interp);

Expand Down
16 changes: 8 additions & 8 deletions Modules/posixmodule.c
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
#include "pycore_ceval.h" // _PyEval_ReInitThreads()
#include "pycore_import.h" // _PyImport_ReInitLock()
#include "pycore_initconfig.h" // _PyStatus_EXCEPTION()
#include "pycore_pystate.h" // _PyInterpreterState_GET()
#include "pycore_pystate.h" // PyInterpreterState_Get()

#include "structmember.h" // PyMemberDef
#ifndef MS_WINDOWS
Expand Down Expand Up @@ -575,7 +575,7 @@ run_at_forkers(PyObject *lst, int reverse)
void
PyOS_BeforeFork(void)
{
run_at_forkers(_PyInterpreterState_GET()->before_forkers, 1);
run_at_forkers(PyInterpreterState_Get()->before_forkers, 1);

_PyImport_AcquireLock();
}
Expand All @@ -586,7 +586,7 @@ PyOS_AfterFork_Parent(void)
if (_PyImport_ReleaseLock() <= 0)
Py_FatalError("failed releasing import lock after fork");

run_at_forkers(_PyInterpreterState_GET()->after_forkers_parent, 0);
run_at_forkers(PyInterpreterState_Get()->after_forkers_parent, 0);
}

void
Expand Down Expand Up @@ -6650,7 +6650,7 @@ os_register_at_fork_impl(PyObject *module, PyObject *before,
check_null_or_callable(after_in_parent, "after_in_parent")) {
return NULL;
}
interp = _PyInterpreterState_GET();
interp = PyInterpreterState_Get();

if (register_at_forker(&interp->before_forkers, before)) {
return NULL;
Expand Down Expand Up @@ -6680,8 +6680,8 @@ os_fork1_impl(PyObject *module)
/*[clinic end generated code: output=0de8e67ce2a310bc input=12db02167893926e]*/
{
pid_t pid;

if (_PyInterpreterState_GET() != PyInterpreterState_Main()) {
PyInterpreterState *interp = PyInterpreterState_Get();
if (_Py_IsMainInterpreter(interp)) {
PyErr_SetString(PyExc_RuntimeError, "fork not supported for subinterpreters");
return NULL;
}
Expand Down Expand Up @@ -6715,7 +6715,7 @@ os_fork_impl(PyObject *module)
/*[clinic end generated code: output=3626c81f98985d49 input=13c956413110eeaa]*/
{
pid_t pid;
PyInterpreterState *interp = _PyInterpreterState_GET();
PyInterpreterState *interp = PyInterpreterState_Get();
if (interp->config._isolated_interpreter) {
PyErr_SetString(PyExc_RuntimeError,
"fork not supported for isolated subinterpreters");
Expand Down Expand Up @@ -7333,7 +7333,7 @@ os_forkpty_impl(PyObject *module)
int master_fd = -1;
pid_t pid;

if (_PyInterpreterState_GET() != PyInterpreterState_Main()) {
if (PyInterpreterState_Get() != PyInterpreterState_Main()) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

PyErr_SetString(PyExc_RuntimeError, "fork not supported for subinterpreters");
return NULL;
}
Expand Down
7 changes: 3 additions & 4 deletions Modules/signalmodule.c
Original file line number Diff line number Diff line change
Expand Up @@ -1655,8 +1655,8 @@ signal_module_exec(PyObject *m)
}
#endif

PyThreadState *tstate = _PyThreadState_GET();
if (_Py_IsMainInterpreter(tstate->interp)) {
PyInterpreterState *interp = PyInterpreterState_Get();
if (_Py_IsMainInterpreter(interp)) {
if (signal_get_set_handlers(state, d) < 0) {
return -1;
}
Expand Down Expand Up @@ -2032,8 +2032,7 @@ _PySignal_AfterFork(void)
int
_PyOS_IsMainThread(void)
{
PyInterpreterState *interp = _PyInterpreterState_GET();
return _Py_ThreadCanHandleSignals(interp);
return _Py_ThreadCanHandleSignals(PyInterpreterState_Get());
}

#ifdef MS_WINDOWS
Expand Down
2 changes: 1 addition & 1 deletion Objects/complexobject.c
Original file line number Diff line number Diff line change
Expand Up @@ -907,7 +907,7 @@ complex_new_impl(PyTypeObject *type, PyObject *r, PyObject *i)
int ci_is_complex = 0;

if (r == NULL) {
r = _PyLong_GetZero();
r = PY_ZERO();
}

/* Special-case for a single argument when type(arg) is complex. */
Expand Down
7 changes: 3 additions & 4 deletions Objects/floatobject.c
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,10 @@ class float "PyObject *" "&PyFloat_Type"


#if PyFloat_MAXFREELIST > 0
static struct _Py_float_state *
static inline struct _Py_float_state *
get_float_state(void)
{
PyInterpreterState *interp = _PyInterpreterState_GET();
return &interp->float_state;
return &_PyInterpreterState_GET()->float_state;
}
#endif

Expand Down Expand Up @@ -1630,7 +1629,7 @@ float_new_impl(PyTypeObject *type, PyObject *x)
{
if (type != &PyFloat_Type) {
if (x == NULL) {
x = _PyLong_GetZero();
x = PY_ZERO();
}
return float_subtype_new(type, x); /* Wimp out */
}
Expand Down
2 changes: 1 addition & 1 deletion Objects/frameobject.c
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ static PyMemberDef frame_memberlist[] = {
};

#if PyFrame_MAXFREELIST > 0
static struct _Py_frame_state *
static inline struct _Py_frame_state *
get_frame_state(void)
{
PyInterpreterState *interp = _PyInterpreterState_GET();
Expand Down
Loading