From c4db85a814498e9ddfb288f0550acc8f26bd8529 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Tue, 10 Jan 2023 14:54:05 -0700 Subject: [PATCH 01/12] Add a thread_local macro. --- Include/pyport.h | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/Include/pyport.h b/Include/pyport.h index eef0fe1bfd71d8..5857cb4f391949 100644 --- a/Include/pyport.h +++ b/Include/pyport.h @@ -663,6 +663,20 @@ extern char * _getpty(int *, int, mode_t, int); # define WITH_THREAD #endif +#ifdef WITH_THREAD +# ifndef thread_local +# if __STDC_VERSION__ >= 201112L && !defined(__STDC_NO_THREADS__) +# define thread_local _Thread_local +# elif defined(_MSC_VER) /* AKA NT_THREADS */ +# define thread_local __declspec(thread) +# elif defined(__GNUC__) /* includes clang */ +# define thread_local __thread +# else +# error "no supported thread-local variable storage classifier" +# endif +# endif +#endif + /* Check that ALT_SOABI is consistent with Py_TRACE_REFS: ./configure --with-trace-refs should must be used to define Py_TRACE_REFS */ #if defined(ALT_SOABI) && defined(Py_TRACE_REFS) From 47a70947c82cd0380fc23cff50950f5d18465a3d Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Thu, 6 Apr 2023 16:02:20 -0600 Subject: [PATCH 02/12] tstate_current -> thread_local. --- Include/internal/pycore_pystate.h | 8 ++++---- Include/internal/pycore_runtime.h | 3 --- Python/pystate.c | 15 +++++++++------ 3 files changed, 13 insertions(+), 13 deletions(-) diff --git a/Include/internal/pycore_pystate.h b/Include/internal/pycore_pystate.h index b5408622d9d4b2..e068e7489c4c17 100644 --- a/Include/internal/pycore_pystate.h +++ b/Include/internal/pycore_pystate.h @@ -64,17 +64,17 @@ _Py_ThreadCanHandlePendingCalls(void) /* Variable and macro for in-line access to current thread and interpreter state */ +PyAPI_DATA(thread_local PyThreadState *) _Py_tss_tstate; + static inline PyThreadState* _PyRuntimeState_GetThreadState(_PyRuntimeState *runtime) { - return (PyThreadState*)_Py_atomic_load_relaxed(&runtime->tstate_current); + return _Py_tss_tstate; } /* Get the current Python thread state. - Efficient macro reading directly the 'tstate_current' atomic - variable. The macro is unsafe: it does not check for error and it can - return NULL. + This function is unsafe: it does not check for error and it can return NULL. The caller must hold the GIL. diff --git a/Include/internal/pycore_runtime.h b/Include/internal/pycore_runtime.h index 3ebe49926edda6..2a3fd8ab2813ea 100644 --- a/Include/internal/pycore_runtime.h +++ b/Include/internal/pycore_runtime.h @@ -119,9 +119,6 @@ typedef struct pyruntimestate { unsigned long main_thread; - /* Assuming the current thread holds the GIL, this is the - PyThreadState for the current thread. */ - _Py_atomic_address tstate_current; /* Used for the thread state bound to the current thread. */ Py_tss_t autoTSSkey; diff --git a/Python/pystate.c b/Python/pystate.c index 1e59a8c5f89717..668778249411da 100644 --- a/Python/pystate.c +++ b/Python/pystate.c @@ -60,23 +60,26 @@ extern "C" { For each of these functions, the GIL must be held by the current thread. */ + +thread_local PyThreadState *_Py_tss_tstate; + static inline PyThreadState * -current_fast_get(_PyRuntimeState *runtime) +current_fast_get(_PyRuntimeState *Py_UNUSED(runtime)) { - return (PyThreadState*)_Py_atomic_load_relaxed(&runtime->tstate_current); + return _Py_tss_tstate; } static inline void -current_fast_set(_PyRuntimeState *runtime, PyThreadState *tstate) +current_fast_set(_PyRuntimeState *Py_UNUSED(runtime), PyThreadState *tstate) { assert(tstate != NULL); - _Py_atomic_store_relaxed(&runtime->tstate_current, (uintptr_t)tstate); + _Py_tss_tstate = tstate; } static inline void -current_fast_clear(_PyRuntimeState *runtime) +current_fast_clear(_PyRuntimeState *Py_UNUSED(runtime)) { - _Py_atomic_store_relaxed(&runtime->tstate_current, (uintptr_t)NULL); + _Py_tss_tstate = NULL; } #define tstate_verify_not_active(tstate) \ From cf22de1b1229033aac3eb780eaad889fafe9c6c6 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Fri, 7 Apr 2023 07:58:15 -0600 Subject: [PATCH 03/12] Add _PyThraedState_GetCurrent(). --- Include/internal/pycore_pystate.h | 6 +++--- Python/pystate.c | 8 +++++++- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/Include/internal/pycore_pystate.h b/Include/internal/pycore_pystate.h index e068e7489c4c17..c07827828c2edb 100644 --- a/Include/internal/pycore_pystate.h +++ b/Include/internal/pycore_pystate.h @@ -64,12 +64,12 @@ _Py_ThreadCanHandlePendingCalls(void) /* Variable and macro for in-line access to current thread and interpreter state */ -PyAPI_DATA(thread_local PyThreadState *) _Py_tss_tstate; +PyAPI_DATA(PyThreadState *) _PyThreadState_GetCurrent(void); static inline PyThreadState* _PyRuntimeState_GetThreadState(_PyRuntimeState *runtime) { - return _Py_tss_tstate; + return _PyThreadState_GetCurrent(); } /* Get the current Python thread state. @@ -82,7 +82,7 @@ _PyRuntimeState_GetThreadState(_PyRuntimeState *runtime) static inline PyThreadState* _PyThreadState_GET(void) { - return _PyRuntimeState_GetThreadState(&_PyRuntime); + return _PyThreadState_GetCurrent(); } static inline void diff --git a/Python/pystate.c b/Python/pystate.c index 668778249411da..1b97f13caea07c 100644 --- a/Python/pystate.c +++ b/Python/pystate.c @@ -61,7 +61,13 @@ extern "C" { */ -thread_local PyThreadState *_Py_tss_tstate; +thread_local PyThreadState *_Py_tss_tstate = NULL; + +PyThreadState * +_PyThreadState_GetCurrent(void) +{ + return _Py_tss_tstate; +} static inline PyThreadState * current_fast_get(_PyRuntimeState *Py_UNUSED(runtime)) From d4136d28419d17316492b6c351ee021b600e8d3b Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Fri, 7 Apr 2023 10:12:19 -0600 Subject: [PATCH 04/12] Add HAVE_THREAD_LOCAL. --- Include/pyport.h | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/Include/pyport.h b/Include/pyport.h index 5857cb4f391949..e65936e6d3d36c 100644 --- a/Include/pyport.h +++ b/Include/pyport.h @@ -667,11 +667,15 @@ extern char * _getpty(int *, int, mode_t, int); # ifndef thread_local # if __STDC_VERSION__ >= 201112L && !defined(__STDC_NO_THREADS__) # define thread_local _Thread_local +# define HAVE_THREAD_LOCAL 1 # elif defined(_MSC_VER) /* AKA NT_THREADS */ # define thread_local __declspec(thread) +# define HAVE_THREAD_LOCAL 1 # elif defined(__GNUC__) /* includes clang */ # define thread_local __thread +# define HAVE_THREAD_LOCAL 1 # else + // XXX Fall back to the PyThread_tss_*() API. # error "no supported thread-local variable storage classifier" # endif # endif From f8c659898bdb786af780631e9d45667d67374085 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Fri, 7 Apr 2023 10:13:09 -0600 Subject: [PATCH 05/12] Support the faster approach, if available. --- Include/internal/pycore_pystate.h | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/Include/internal/pycore_pystate.h b/Include/internal/pycore_pystate.h index c07827828c2edb..2a5731b41fd69f 100644 --- a/Include/internal/pycore_pystate.h +++ b/Include/internal/pycore_pystate.h @@ -64,14 +64,22 @@ _Py_ThreadCanHandlePendingCalls(void) /* Variable and macro for in-line access to current thread and interpreter state */ +#if defined(HAVE_THREAD_LOCAL) && !defined(Py_BUILD_CORE_MODULE) +extern thread_local PyThreadState *_Py_tss_tstate; +#endif PyAPI_DATA(PyThreadState *) _PyThreadState_GetCurrent(void); static inline PyThreadState* _PyRuntimeState_GetThreadState(_PyRuntimeState *runtime) { +#if defined(HAVE_THREAD_LOCAL) && !defined(Py_BUILD_CORE_MODULE) + return _Py_tss_tstate; +#else return _PyThreadState_GetCurrent(); +#endif } + /* Get the current Python thread state. This function is unsafe: it does not check for error and it can return NULL. @@ -82,7 +90,7 @@ _PyRuntimeState_GetThreadState(_PyRuntimeState *runtime) static inline PyThreadState* _PyThreadState_GET(void) { - return _PyThreadState_GetCurrent(); + return _PyRuntimeState_GetThreadState(&_PyRuntime); } static inline void From 9496df0e88bb552967784db6859a39a987cae26f Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Fri, 7 Apr 2023 10:17:20 -0600 Subject: [PATCH 06/12] Do not fail if thread_local not supported. --- Include/pyport.h | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/Include/pyport.h b/Include/pyport.h index e65936e6d3d36c..202dc92943049f 100644 --- a/Include/pyport.h +++ b/Include/pyport.h @@ -674,9 +674,7 @@ extern char * _getpty(int *, int, mode_t, int); # elif defined(__GNUC__) /* includes clang */ # define thread_local __thread # define HAVE_THREAD_LOCAL 1 -# else - // XXX Fall back to the PyThread_tss_*() API. -# error "no supported thread-local variable storage classifier" + // else: fall back to the PyThread_tss_*() API, or ignore. # endif # endif #endif From 2c335a399c02545a5d390077fa523063f7a89c6c Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Fri, 7 Apr 2023 10:33:48 -0600 Subject: [PATCH 07/12] thread_local -> _Py_thread_local --- Include/pyport.h | 27 +++++++++++++++------------ 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/Include/pyport.h b/Include/pyport.h index 202dc92943049f..0c098977a984f0 100644 --- a/Include/pyport.h +++ b/Include/pyport.h @@ -664,18 +664,21 @@ extern char * _getpty(int *, int, mode_t, int); #endif #ifdef WITH_THREAD -# ifndef thread_local -# if __STDC_VERSION__ >= 201112L && !defined(__STDC_NO_THREADS__) -# define thread_local _Thread_local -# define HAVE_THREAD_LOCAL 1 -# elif defined(_MSC_VER) /* AKA NT_THREADS */ -# define thread_local __declspec(thread) -# define HAVE_THREAD_LOCAL 1 -# elif defined(__GNUC__) /* includes clang */ -# define thread_local __thread -# define HAVE_THREAD_LOCAL 1 - // else: fall back to the PyThread_tss_*() API, or ignore. -# endif +# ifdef HAVE_THREAD_LOCAL +# error "unexpectedly, HAVE_THREAD_LOCAL is already defined" +# endif +# define HAVE_THREAD_LOCAL 1 +# ifdef thread_local +# define _Py_thread_local thread_local +# elif __STDC_VERSION__ >= 201112L && !defined(__STDC_NO_THREADS__) +# define _Py_thread_local _Thread_local +# elif defined(_MSC_VER) /* AKA NT_THREADS */ +# define _Py_thread_local __declspec(thread) +# elif defined(__GNUC__) /* includes clang */ +# define _Py_thread_local __thread +# else + // fall back to the PyThread_tss_*() API, or ignore. +# undef HAVE_THREAD_LOCAL # endif #endif From 4af0ce7e8d06831a72d39566ca8b02093fe6ec05 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Fri, 7 Apr 2023 10:40:54 -0600 Subject: [PATCH 08/12] Only define _Py_thread_local for the core runtime. --- Include/pyport.h | 32 +++++++++++++++++--------------- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/Include/pyport.h b/Include/pyport.h index 0c098977a984f0..4d9c56c1312a9f 100644 --- a/Include/pyport.h +++ b/Include/pyport.h @@ -664,21 +664,23 @@ extern char * _getpty(int *, int, mode_t, int); #endif #ifdef WITH_THREAD -# ifdef HAVE_THREAD_LOCAL -# error "unexpectedly, HAVE_THREAD_LOCAL is already defined" -# endif -# define HAVE_THREAD_LOCAL 1 -# ifdef thread_local -# define _Py_thread_local thread_local -# elif __STDC_VERSION__ >= 201112L && !defined(__STDC_NO_THREADS__) -# define _Py_thread_local _Thread_local -# elif defined(_MSC_VER) /* AKA NT_THREADS */ -# define _Py_thread_local __declspec(thread) -# elif defined(__GNUC__) /* includes clang */ -# define _Py_thread_local __thread -# else - // fall back to the PyThread_tss_*() API, or ignore. -# undef HAVE_THREAD_LOCAL +# ifdef Py_BUILD_CORE +# ifdef HAVE_THREAD_LOCAL +# error "unexpectedly, HAVE_THREAD_LOCAL is already defined" +# endif +# define HAVE_THREAD_LOCAL 1 +# ifdef thread_local +# define _Py_thread_local thread_local +# elif __STDC_VERSION__ >= 201112L && !defined(__STDC_NO_THREADS__) +# define _Py_thread_local _Thread_local +# elif defined(_MSC_VER) /* AKA NT_THREADS */ +# define _Py_thread_local __declspec(thread) +# elif defined(__GNUC__) /* includes clang */ +# define _Py_thread_local __thread +# else + // fall back to the PyThread_tss_*() API, or ignore. +# undef HAVE_THREAD_LOCAL +# endif # endif #endif From 3db400735ac2ce1a7814a2878fa3ec308e46dcfe Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Fri, 7 Apr 2023 10:42:45 -0600 Subject: [PATCH 09/12] Fix pystate.c. --- Include/internal/pycore_pystate.h | 2 +- Python/pystate.c | 31 ++++++++++++++++++++++++------- 2 files changed, 25 insertions(+), 8 deletions(-) diff --git a/Include/internal/pycore_pystate.h b/Include/internal/pycore_pystate.h index 2a5731b41fd69f..f3d38acf44adc5 100644 --- a/Include/internal/pycore_pystate.h +++ b/Include/internal/pycore_pystate.h @@ -65,7 +65,7 @@ _Py_ThreadCanHandlePendingCalls(void) and interpreter state */ #if defined(HAVE_THREAD_LOCAL) && !defined(Py_BUILD_CORE_MODULE) -extern thread_local PyThreadState *_Py_tss_tstate; +extern _Py_thread_local PyThreadState *_Py_tss_tstate; #endif PyAPI_DATA(PyThreadState *) _PyThreadState_GetCurrent(void); diff --git a/Python/pystate.c b/Python/pystate.c index 1b97f13caea07c..7cd397812de657 100644 --- a/Python/pystate.c +++ b/Python/pystate.c @@ -61,31 +61,42 @@ extern "C" { */ -thread_local PyThreadState *_Py_tss_tstate = NULL; - -PyThreadState * -_PyThreadState_GetCurrent(void) -{ - return _Py_tss_tstate; -} +#ifdef HAVE_THREAD_LOCAL +_Py_thread_local PyThreadState *_Py_tss_tstate = NULL; +#endif static inline PyThreadState * current_fast_get(_PyRuntimeState *Py_UNUSED(runtime)) { +#ifdef HAVE_THREAD_LOCAL return _Py_tss_tstate; +#else + // XXX Fall back to the PyThread_tss_*() API. +# error "no supported thread-local variable storage classifier" +#endif } static inline void current_fast_set(_PyRuntimeState *Py_UNUSED(runtime), PyThreadState *tstate) { assert(tstate != NULL); +#ifdef HAVE_THREAD_LOCAL _Py_tss_tstate = tstate; +#else + // XXX Fall back to the PyThread_tss_*() API. +# error "no supported thread-local variable storage classifier" +#endif } static inline void current_fast_clear(_PyRuntimeState *Py_UNUSED(runtime)) { +#ifdef HAVE_THREAD_LOCAL _Py_tss_tstate = NULL; +#else + // XXX Fall back to the PyThread_tss_*() API. +# error "no supported thread-local variable storage classifier" +#endif } #define tstate_verify_not_active(tstate) \ @@ -93,6 +104,12 @@ current_fast_clear(_PyRuntimeState *Py_UNUSED(runtime)) _Py_FatalErrorFormat(__func__, "tstate %p is still current", tstate); \ } +PyThreadState * +_PyThreadState_GetCurrent(void) +{ + return current_fast_get(&_PyRuntime); +} + //------------------------------------------------ // the thread state bound to the current OS thread From d57305336a00b68563b59ac6dfdd424f18e2a998 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Fri, 7 Apr 2023 10:44:51 -0600 Subject: [PATCH 10/12] Call _PyThreadState_GET() from _PyRuntimeState_GetThreadState(). --- Include/internal/pycore_pystate.h | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/Include/internal/pycore_pystate.h b/Include/internal/pycore_pystate.h index f3d38acf44adc5..71944bd29ae642 100644 --- a/Include/internal/pycore_pystate.h +++ b/Include/internal/pycore_pystate.h @@ -69,8 +69,15 @@ extern _Py_thread_local PyThreadState *_Py_tss_tstate; #endif PyAPI_DATA(PyThreadState *) _PyThreadState_GetCurrent(void); +/* Get the current Python thread state. + + This function is unsafe: it does not check for error and it can return NULL. + + The caller must hold the GIL. + + See also PyThreadState_Get() and _PyThreadState_UncheckedGet(). */ static inline PyThreadState* -_PyRuntimeState_GetThreadState(_PyRuntimeState *runtime) +_PyThreadState_GET(void) { #if defined(HAVE_THREAD_LOCAL) && !defined(Py_BUILD_CORE_MODULE) return _Py_tss_tstate; @@ -79,20 +86,13 @@ _PyRuntimeState_GetThreadState(_PyRuntimeState *runtime) #endif } - -/* Get the current Python thread state. - - This function is unsafe: it does not check for error and it can return NULL. - - The caller must hold the GIL. - - See also PyThreadState_Get() and _PyThreadState_UncheckedGet(). */ static inline PyThreadState* -_PyThreadState_GET(void) +_PyRuntimeState_GetThreadState(_PyRuntimeState *Py_UNUSED(runtime)) { - return _PyRuntimeState_GetThreadState(&_PyRuntime); + return _PyThreadState_GET(); } + static inline void _Py_EnsureFuncTstateNotNULL(const char *func, PyThreadState *tstate) { From feb8ef525120044eb974dbcdaf631e65db1287be Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Fri, 7 Apr 2023 10:46:06 -0600 Subject: [PATCH 11/12] Fix the error message. --- Include/pyport.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Include/pyport.h b/Include/pyport.h index 4d9c56c1312a9f..89149976734ccb 100644 --- a/Include/pyport.h +++ b/Include/pyport.h @@ -666,7 +666,7 @@ extern char * _getpty(int *, int, mode_t, int); #ifdef WITH_THREAD # ifdef Py_BUILD_CORE # ifdef HAVE_THREAD_LOCAL -# error "unexpectedly, HAVE_THREAD_LOCAL is already defined" +# error "HAVE_THREAD_LOCAL is already defined" # endif # define HAVE_THREAD_LOCAL 1 # ifdef thread_local From 2332a2e7e04e13a6dbc8f78e5ed1b6c730b01cc6 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Fri, 7 Apr 2023 12:18:52 -0600 Subject: [PATCH 12/12] Add a NEWS entry. --- .../2023-04-07-12-18-41.gh-issue-103323.9802br.rst | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2023-04-07-12-18-41.gh-issue-103323.9802br.rst diff --git a/Misc/NEWS.d/next/Core and Builtins/2023-04-07-12-18-41.gh-issue-103323.9802br.rst b/Misc/NEWS.d/next/Core and Builtins/2023-04-07-12-18-41.gh-issue-103323.9802br.rst new file mode 100644 index 00000000000000..347c91d973e5ce --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2023-04-07-12-18-41.gh-issue-103323.9802br.rst @@ -0,0 +1,3 @@ +We've replaced our use of ``_PyRuntime.tstate_current`` with a thread-local +variable. This is a fairly low-level implementation detail, and there +should be no change in behavior.