From 4b27f3a774e127e415a0386394cc4a916e482eda Mon Sep 17 00:00:00 2001 From: Sam Gross Date: Mon, 12 Aug 2024 18:48:48 +0000 Subject: [PATCH 1/3] gh-117376: Make `Py_DECREF` a macro in ceval.c in free-threaded build `Py_DECREF` and `PyStackRef_CLOSE` are now implemented as macros in the free-threaded build in ceval.c. There are two motivations; * MSVC has problems inlining functions in ceval.c in the PGO build. * We will want to mark escaping calls in order to spill the stack pointer in ceval.c and we will want to do this around `_Py_Dealloc` (or `_Py_MergeZeroLocalRefcount` or `_Py_DecRefShared`), not around the entire `Py_DECREF` or `PyStackRef_CLOSE` call. --- Include/internal/pycore_stackref.h | 21 +++++----- Python/ceval.c | 61 ++++++++++++++++++++++-------- 2 files changed, 55 insertions(+), 27 deletions(-) diff --git a/Include/internal/pycore_stackref.h b/Include/internal/pycore_stackref.h index 1b35a3e3269257..edf442689917c4 100644 --- a/Include/internal/pycore_stackref.h +++ b/Include/internal/pycore_stackref.h @@ -191,18 +191,15 @@ PyStackRef_FromPyObjectImmortal(PyObject *obj) } while (0) #ifdef Py_GIL_DISABLED -static inline void -PyStackRef_CLOSE(_PyStackRef stackref) -{ - if (PyStackRef_IsDeferred(stackref)) { - // No assert for being immortal or deferred here. - // The GC unsets deferred objects right before clearing. - return; - } - Py_DECREF(PyStackRef_AsPyObjectBorrow(stackref)); -} +# define PyStackRef_CLOSE(stackref) \ + do { \ + _PyStackRef _close_tmp = (stackref); \ + if (!PyStackRef_IsDeferred(_close_tmp)) { \ + Py_DECREF(PyStackRef_AsPyObjectBorrow(_close_tmp)); \ + } \ + } while (0) #else -# define PyStackRef_CLOSE(stackref) Py_DECREF(PyStackRef_AsPyObjectBorrow(stackref)); +# define PyStackRef_CLOSE(stackref) Py_DECREF(PyStackRef_AsPyObjectBorrow(stackref)) #endif #define PyStackRef_XCLOSE(stackref) \ @@ -227,7 +224,7 @@ PyStackRef_DUP(_PyStackRef stackref) return stackref; } #else -# define PyStackRef_DUP(stackref) PyStackRef_FromPyObjectSteal(Py_NewRef(PyStackRef_AsPyObjectBorrow(stackref))); +# define PyStackRef_DUP(stackref) PyStackRef_FromPyObjectSteal(Py_NewRef(PyStackRef_AsPyObjectBorrow(stackref))) #endif static inline void diff --git a/Python/ceval.c b/Python/ceval.c index c685a95b2ef088..384d2dbd423f47 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -52,13 +52,27 @@ # error "ceval.c must be build with Py_BUILD_CORE define for best performance" #endif -#if !defined(Py_DEBUG) && !defined(Py_TRACE_REFS) && !defined(Py_GIL_DISABLED) +#if !defined(Py_DEBUG) && !defined(Py_TRACE_REFS) // GH-89279: The MSVC compiler does not inline these static inline functions // in PGO build in _PyEval_EvalFrameDefault(), because this function is over // the limit of PGO, and that limit cannot be configured. // Define them as macros to make sure that they are always inlined by the // preprocessor. -// TODO: implement Py_DECREF macro for Py_GIL_DISABLED + +#undef Py_IS_TYPE +#define Py_IS_TYPE(ob, type) \ + (_PyObject_CAST(ob)->ob_type == (type)) + +#undef Py_XDECREF +#define Py_XDECREF(arg) \ + do { \ + PyObject *xop = _PyObject_CAST(arg); \ + if (xop != NULL) { \ + Py_DECREF(xop); \ + } \ + } while (0) + +#ifndef Py_GIL_DISABLED #undef Py_DECREF #define Py_DECREF(arg) \ @@ -74,19 +88,6 @@ } \ } while (0) -#undef Py_XDECREF -#define Py_XDECREF(arg) \ - do { \ - PyObject *xop = _PyObject_CAST(arg); \ - if (xop != NULL) { \ - Py_DECREF(xop); \ - } \ - } while (0) - -#undef Py_IS_TYPE -#define Py_IS_TYPE(ob, type) \ - (_PyObject_CAST(ob)->ob_type == (type)) - #undef _Py_DECREF_SPECIALIZED #define _Py_DECREF_SPECIALIZED(arg, dealloc) \ do { \ @@ -100,6 +101,36 @@ d(op); \ } \ } while (0) + +#else // Py_GIL_DISABLED + +#define _DECREF(arg, DEALLOC) \ + do { \ + PyObject *op = _PyObject_CAST(arg); \ + uint32_t local = _Py_atomic_load_uint32_relaxed(&op->ob_ref_local); \ + if (local == _Py_IMMORTAL_REFCNT_LOCAL) { \ + break; \ + } \ + _Py_DECREF_STAT_INC(); \ + if (_Py_IsOwnedByCurrentThread(op)) { \ + local--; \ + _Py_atomic_store_uint32_relaxed(&op->ob_ref_local, local); \ + if (local == 0) { \ + _Py_MergeZeroLocalRefcount(op); \ + } \ + } \ + else { \ + _Py_DecRefShared(op); \ + } \ + } while (0) + +#undef Py_DECREF +#define Py_DECREF(arg) _DECREF(arg, Py_TYPE(op)->tp_dealloc) + +#undef _Py_DECREF_SPECIALIZED +#define _Py_DECREF_SPECIALIZED(arg, dealloc) _DECREF(arg, (destructor)(dealloc)) + +#endif #endif From 65dcdfedd4a3face3044f392c2da40a39220d6d6 Mon Sep 17 00:00:00 2001 From: Sam Gross Date: Wed, 21 Aug 2024 19:32:25 +0000 Subject: [PATCH 2/3] Clean-up macro definition. The `DEALLOC` argument isn't currently used. _Py_DECREF_SPECIALIZED is the same as `Py_DECREF` in the free-threaded build. This might change in the future, but keep things simpler for now. --- Python/ceval.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/Python/ceval.c b/Python/ceval.c index 384d2dbd423f47..2aa252a940b71d 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -104,7 +104,8 @@ #else // Py_GIL_DISABLED -#define _DECREF(arg, DEALLOC) \ +#undef Py_DECREF +#define Py_DECREF(arg) \ do { \ PyObject *op = _PyObject_CAST(arg); \ uint32_t local = _Py_atomic_load_uint32_relaxed(&op->ob_ref_local); \ @@ -124,11 +125,8 @@ } \ } while (0) -#undef Py_DECREF -#define Py_DECREF(arg) _DECREF(arg, Py_TYPE(op)->tp_dealloc) - #undef _Py_DECREF_SPECIALIZED -#define _Py_DECREF_SPECIALIZED(arg, dealloc) _DECREF(arg, (destructor)(dealloc)) +#define _Py_DECREF_SPECIALIZED(arg, dealloc) Py_DECREF(arg) #endif #endif From b92425816c934eb485741143577adb02a5fb52fc Mon Sep 17 00:00:00 2001 From: Sam Gross Date: Wed, 21 Aug 2024 19:58:16 +0000 Subject: [PATCH 3/3] Format PyStackRef_CLOSE --- Include/internal/pycore_stackref.h | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/Include/internal/pycore_stackref.h b/Include/internal/pycore_stackref.h index ef82f6f8f2d072..7c106577671b5f 100644 --- a/Include/internal/pycore_stackref.h +++ b/Include/internal/pycore_stackref.h @@ -190,12 +190,12 @@ PyStackRef_FromPyObjectImmortal(PyObject *obj) } while (0) #ifdef Py_GIL_DISABLED -# define PyStackRef_CLOSE(stackref) \ - do { \ - _PyStackRef _close_tmp = (stackref); \ - if (!PyStackRef_IsDeferred(_close_tmp)) { \ - Py_DECREF(PyStackRef_AsPyObjectBorrow(_close_tmp)); \ - } \ +# define PyStackRef_CLOSE(REF) \ + do { \ + _PyStackRef _close_tmp = (REF); \ + if (!PyStackRef_IsDeferred(_close_tmp)) { \ + Py_DECREF(PyStackRef_AsPyObjectBorrow(_close_tmp)); \ + } \ } while (0) #else # define PyStackRef_CLOSE(stackref) Py_DECREF(PyStackRef_AsPyObjectBorrow(stackref))