From df6ea14f15ac729c87fa1fa9e17cca35f3a69527 Mon Sep 17 00:00:00 2001 From: Mikhail Efimov Date: Mon, 17 Nov 2025 21:16:36 +0300 Subject: [PATCH 1/4] Another flag on immortal --- Include/internal/pycore_stackref.h | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/Include/internal/pycore_stackref.h b/Include/internal/pycore_stackref.h index e59611c07fa793..03a00a86804a17 100644 --- a/Include/internal/pycore_stackref.h +++ b/Include/internal/pycore_stackref.h @@ -502,7 +502,12 @@ _PyStackRef_FromPyObjectSteal(PyObject *obj) assert(obj != NULL); // Make sure we don't take an already tagged value. assert(((uintptr_t)obj & Py_TAG_BITS) == 0); - return (_PyStackRef){ .bits = (uintptr_t)obj }; + if (_Py_IsImmortal(obj)) { + return (_PyStackRef){ .bits = (uintptr_t)obj | Py_TAG_DEFERRED }; + } + else { + return (_PyStackRef){ .bits = (uintptr_t)obj }; + } } # define PyStackRef_FromPyObjectSteal(obj) _PyStackRef_FromPyObjectSteal(_PyObject_CAST(obj)) @@ -542,7 +547,7 @@ PyStackRef_FromPyObjectNew(PyObject *obj) // Make sure we don't take an already tagged value. assert(((uintptr_t)obj & Py_TAG_BITS) == 0); assert(obj != NULL); - if (_PyObject_HasDeferredRefcount(obj)) { + if (_PyObject_HasDeferredRefcount(obj) || _Py_IsImmortal(obj)) { return (_PyStackRef){ .bits = (uintptr_t)obj | Py_TAG_DEFERRED }; } else { From fbda208492d2e5db9349e94c1b77982579fa2d8d Mon Sep 17 00:00:00 2001 From: Mikhail Efimov Date: Mon, 17 Nov 2025 21:37:44 +0300 Subject: [PATCH 2/4] Use _Py_IMMORTAL_FLAGS == Py_TAG_REFCNT check on all builds --- Include/internal/pycore_stackref.h | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/Include/internal/pycore_stackref.h b/Include/internal/pycore_stackref.h index 03a00a86804a17..e56ab807045c54 100644 --- a/Include/internal/pycore_stackref.h +++ b/Include/internal/pycore_stackref.h @@ -57,6 +57,13 @@ extern "C" { #define Py_TAGGED_SHIFT 2 +/* References to immortal objects always have their tag bit set to Py_TAG_REFCNT + * as they can (must) have their reclamation deferred */ + +#if _Py_IMMORTAL_FLAGS != Py_TAG_REFCNT +# error "_Py_IMMORTAL_FLAGS != Py_TAG_REFCNT" +#endif + #if !defined(Py_GIL_DISABLED) && defined(Py_STACKREF_DEBUG) PyAPI_FUNC(PyObject *) _Py_stackref_get_object(_PyStackRef ref); @@ -636,13 +643,6 @@ PyStackRef_AsStrongReference(_PyStackRef stackref) // With GIL -/* References to immortal objects always have their tag bit set to Py_TAG_REFCNT - * as they can (must) have their reclamation deferred */ - -#if _Py_IMMORTAL_FLAGS != Py_TAG_REFCNT -# error "_Py_IMMORTAL_FLAGS != Py_TAG_REFCNT" -#endif - #define BITS_TO_PTR(REF) ((PyObject *)((REF).bits)) #define BITS_TO_PTR_MASKED(REF) ((PyObject *)(((REF).bits) & (~Py_TAG_REFCNT))) From 7b87facf0b1b7b814be21d8fae6e203a830b3535 Mon Sep 17 00:00:00 2001 From: Mikhail Efimov Date: Wed, 19 Nov 2025 14:42:17 +0300 Subject: [PATCH 3/4] Use _PyGC_BITS_DEFERRED for all immortal objects --- Include/internal/pycore_stackref.h | 4 ++-- Objects/object.c | 4 +++- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/Include/internal/pycore_stackref.h b/Include/internal/pycore_stackref.h index e56ab807045c54..c9902ea0fd22a4 100644 --- a/Include/internal/pycore_stackref.h +++ b/Include/internal/pycore_stackref.h @@ -523,7 +523,7 @@ PyStackRef_IsHeapSafe(_PyStackRef stackref) { if (PyStackRef_IsDeferred(stackref)) { PyObject *obj = PyStackRef_AsPyObjectBorrow(stackref); - return obj == NULL || _Py_IsImmortal(obj) || _PyObject_HasDeferredRefcount(obj); + return obj == NULL || _PyObject_HasDeferredRefcount(obj); } return true; } @@ -554,7 +554,7 @@ PyStackRef_FromPyObjectNew(PyObject *obj) // Make sure we don't take an already tagged value. assert(((uintptr_t)obj & Py_TAG_BITS) == 0); assert(obj != NULL); - if (_PyObject_HasDeferredRefcount(obj) || _Py_IsImmortal(obj)) { + if (_PyObject_HasDeferredRefcount(obj)) { return (_PyStackRef){ .bits = (uintptr_t)obj | Py_TAG_DEFERRED }; } else { diff --git a/Objects/object.c b/Objects/object.c index 0540112d7d2acf..d910b49e545d8f 100644 --- a/Objects/object.c +++ b/Objects/object.c @@ -2675,6 +2675,9 @@ _Py_SetImmortalUntracked(PyObject *op) assert(PyUnicode_CHECK_INTERNED(op) == SSTATE_INTERNED_IMMORTAL || PyUnicode_CHECK_INTERNED(op) == SSTATE_INTERNED_IMMORTAL_STATIC); } +#endif +#ifdef Py_GIL_DISABLED + _Py_atomic_or_uint8(&op->ob_gc_bits, _PyGC_BITS_DEFERRED); #endif // Check if already immortal to avoid degrading from static immortal to plain immortal if (_Py_IsImmortal(op)) { @@ -2684,7 +2687,6 @@ _Py_SetImmortalUntracked(PyObject *op) op->ob_tid = _Py_UNOWNED_TID; op->ob_ref_local = _Py_IMMORTAL_REFCNT_LOCAL; op->ob_ref_shared = 0; - _Py_atomic_or_uint8(&op->ob_gc_bits, _PyGC_BITS_DEFERRED); #elif SIZEOF_VOID_P > 4 op->ob_flags = _Py_IMMORTAL_FLAGS; op->ob_refcnt = _Py_IMMORTAL_INITIAL_REFCNT; From 50e0fe3c75d5920255e429373c880185b04c6a9f Mon Sep 17 00:00:00 2001 From: Mikhail Efimov Date: Wed, 19 Nov 2025 16:16:41 +0300 Subject: [PATCH 4/4] Address review: add comment --- Objects/object.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Objects/object.c b/Objects/object.c index d910b49e545d8f..21a3e2fcb768cf 100644 --- a/Objects/object.c +++ b/Objects/object.c @@ -2677,6 +2677,9 @@ _Py_SetImmortalUntracked(PyObject *op) } #endif #ifdef Py_GIL_DISABLED + // We set these bits for all immortals, even static one, to simplify + // further checks on hot path. In fact, if _Py_IsImmortal(op) == 1, + // then _PyObject_HasDeferredRefcount(op) == 1. _Py_atomic_or_uint8(&op->ob_gc_bits, _PyGC_BITS_DEFERRED); #endif // Check if already immortal to avoid degrading from static immortal to plain immortal