From 219a0e45980a608162cc500536a03f59bd3c1fe8 Mon Sep 17 00:00:00 2001 From: Kevin Newton Date: Thu, 20 Aug 2026 12:30:14 -0400 Subject: [PATCH] whatever gh-156132: Relax memory ordering of shared refcount atomics In the free-threaded build, all operations on ob_ref_shared use sequentially consistent atomics. Full ordering is stronger than the biased reference counting protocol requires, and on ARM64 the ordered instructions are measurably slower under contention. Increfs need atomicity but no ordering: a thread incrementing ob_ref_shared already holds a valid reference, so the increment only extends the object's lifetime and neither publishes nor consumes any of the object's data. Visibility of the object's contents is provided by whatever operation gave the reference to this thread. Switch the incref paths to relaxed ordering: * Py_INCREF and _Py_RefcntAdd: relaxed fetch_add on ob_ref_shared, via a new _Py_atomic_add_ssize_relaxed. * _Py_TryIncRefShared and _Py_NewRefWithLock: relaxed CAS, via a new _Py_atomic_compare_exchange_ssize_relaxed. Decrefs do need ordering, but acquire/release rather than seq_cst. The successful CAS in _Py_DecRefShared releases this thread's accesses through the dying reference, and acquires other threads' released accesses in case this decref is the one that makes the object dead and proceeds to deallocate (or queues it to the owning thread). Switch it to a new _Py_atomic_compare_exchange_ssize_acq_rel with a relaxed failure ordering, since the loop reloads and retries. On x86-64 the generated code is unchanged (locked RMW instructions are always fully ordered). On ARM64 with clang the decref change is also codegen-neutral (both seq_cst and acq_rel CAS lower to casal); the incref sites lower to ldadd/cas instead of ldaddal/casal. The MSVC ARM64 backend uses the _nf Interlocked variants for the relaxed operations and the _rel variant plus a __dmb(ISHLD) fence for the acq_rel CAS. --- Include/cpython/pyatomic.h | 11 +++++++ Include/cpython/pyatomic_gcc.h | 16 ++++++++++ Include/cpython/pyatomic_msc.h | 50 ++++++++++++++++++++++++++++++++ Include/cpython/pyatomic_std.h | 28 ++++++++++++++++++ Include/internal/pycore_object.h | 7 +++-- Include/refcount.h | 5 +++- Objects/object.c | 6 ++-- 7 files changed, 117 insertions(+), 6 deletions(-) diff --git a/Include/cpython/pyatomic.h b/Include/cpython/pyatomic.h index e85b360c986668c..d0daef54723c0ef 100644 --- a/Include/cpython/pyatomic.h +++ b/Include/cpython/pyatomic.h @@ -129,6 +129,9 @@ _Py_atomic_add_uintptr(uintptr_t *obj, uintptr_t value); static inline Py_ssize_t _Py_atomic_add_ssize(Py_ssize_t *obj, Py_ssize_t value); +static inline Py_ssize_t +_Py_atomic_add_ssize_relaxed(Py_ssize_t *obj, Py_ssize_t value); + // --- _Py_atomic_compare_exchange ------------------------------------------- // Performs an atomic compare-and-exchange. @@ -179,6 +182,14 @@ _Py_atomic_compare_exchange_uintptr(uintptr_t *obj, uintptr_t *expected, uintptr static inline int _Py_atomic_compare_exchange_ssize(Py_ssize_t *obj, Py_ssize_t *expected, Py_ssize_t desired); +static inline int +_Py_atomic_compare_exchange_ssize_relaxed(Py_ssize_t *obj, Py_ssize_t *expected, + Py_ssize_t desired); + +static inline int +_Py_atomic_compare_exchange_ssize_acq_rel(Py_ssize_t *obj, Py_ssize_t *expected, + Py_ssize_t desired); + // NOTE: `obj` and `expected` are logically `void**` types, but we use `void*` // so that we can pass types like `PyObject**` without a cast. static inline int diff --git a/Include/cpython/pyatomic_gcc.h b/Include/cpython/pyatomic_gcc.h index 253b35082aafcd2..3919463dcf0d8e3 100644 --- a/Include/cpython/pyatomic_gcc.h +++ b/Include/cpython/pyatomic_gcc.h @@ -62,6 +62,10 @@ static inline Py_ssize_t _Py_atomic_add_ssize(Py_ssize_t *obj, Py_ssize_t value) { return __atomic_fetch_add(obj, value, __ATOMIC_SEQ_CST); } +static inline Py_ssize_t +_Py_atomic_add_ssize_relaxed(Py_ssize_t *obj, Py_ssize_t value) +{ return __atomic_fetch_add(obj, value, __ATOMIC_RELAXED); } + // --- _Py_atomic_compare_exchange ------------------------------------------- @@ -130,6 +134,18 @@ _Py_atomic_compare_exchange_ssize(Py_ssize_t *obj, Py_ssize_t *expected, Py_ssiz { return __atomic_compare_exchange_n(obj, expected, desired, 0, __ATOMIC_SEQ_CST, __ATOMIC_SEQ_CST); } +static inline int +_Py_atomic_compare_exchange_ssize_relaxed(Py_ssize_t *obj, Py_ssize_t *expected, + Py_ssize_t desired) +{ return __atomic_compare_exchange_n(obj, expected, desired, 0, + __ATOMIC_RELAXED, __ATOMIC_RELAXED); } + +static inline int +_Py_atomic_compare_exchange_ssize_acq_rel(Py_ssize_t *obj, Py_ssize_t *expected, + Py_ssize_t desired) +{ return __atomic_compare_exchange_n(obj, expected, desired, 0, + __ATOMIC_ACQ_REL, __ATOMIC_RELAXED); } + static inline int _Py_atomic_compare_exchange_ptr(void *obj, void *expected, void *desired) { return __atomic_compare_exchange_n((void **)obj, (void **)expected, desired, 0, diff --git a/Include/cpython/pyatomic_msc.h b/Include/cpython/pyatomic_msc.h index 3b3c5f7017e9575..0f55a27d9b0e592 100644 --- a/Include/cpython/pyatomic_msc.h +++ b/Include/cpython/pyatomic_msc.h @@ -124,6 +124,19 @@ _Py_atomic_add_ssize(Py_ssize_t *obj, Py_ssize_t value) return (Py_ssize_t)_Py_atomic_add_intptr((intptr_t *)obj, (intptr_t)value); } +static inline Py_ssize_t +_Py_atomic_add_ssize_relaxed(Py_ssize_t *obj, Py_ssize_t value) +{ +#if defined(_M_ARM64) + _Py_atomic_ASSERT_ARG_TYPE(__int64); + return (Py_ssize_t)_InterlockedExchangeAdd64_nf( + (volatile __int64 *)obj, (__int64)value); +#else + // Interlocked RMW operations are inherently ordered on x86 and x64. + return _Py_atomic_add_ssize(obj, value); +#endif +} + // --- _Py_atomic_compare_exchange ------------------------------------------- @@ -279,6 +292,43 @@ _Py_atomic_compare_exchange_ssize(Py_ssize_t *obj, Py_ssize_t *expected, Py_ssiz (void*)value); } +static inline int +_Py_atomic_compare_exchange_ssize_relaxed(Py_ssize_t *obj, Py_ssize_t *expected, + Py_ssize_t desired) +{ +#if defined(_M_ARM64) + _Py_atomic_ASSERT_ARG_TYPE(__int64); + Py_ssize_t initial = (Py_ssize_t)_InterlockedCompareExchange64_nf( + (volatile __int64 *)obj, (__int64)desired, (__int64)*expected); + if (initial == *expected) { + return 1; + } + *expected = initial; + return 0; +#else + return _Py_atomic_compare_exchange_ssize(obj, expected, desired); +#endif +} + +static inline int +_Py_atomic_compare_exchange_ssize_acq_rel(Py_ssize_t *obj, Py_ssize_t *expected, + Py_ssize_t desired) +{ +#if defined(_M_ARM64) + _Py_atomic_ASSERT_ARG_TYPE(__int64); + Py_ssize_t initial = (Py_ssize_t)_InterlockedCompareExchange64_rel( + (volatile __int64 *)obj, (__int64)desired, (__int64)*expected); + if (initial == *expected) { + // The release CAS and acquire fence provide acq-rel ordering on success. + __dmb(_ARM64_BARRIER_ISHLD); + return 1; + } + *expected = initial; + return 0; +#else + return _Py_atomic_compare_exchange_ssize(obj, expected, desired); +#endif +} // --- _Py_atomic_exchange --------------------------------------------------- diff --git a/Include/cpython/pyatomic_std.h b/Include/cpython/pyatomic_std.h index faef303da70314c..cb24d6703a8606a 100644 --- a/Include/cpython/pyatomic_std.h +++ b/Include/cpython/pyatomic_std.h @@ -111,6 +111,14 @@ _Py_atomic_add_ssize(Py_ssize_t *obj, Py_ssize_t value) return atomic_fetch_add((_Atomic(Py_ssize_t)*)obj, value); } +static inline Py_ssize_t +_Py_atomic_add_ssize_relaxed(Py_ssize_t *obj, Py_ssize_t value) +{ + _Py_USING_STD; + return atomic_fetch_add_explicit((_Atomic(Py_ssize_t)*)obj, value, + memory_order_relaxed); +} + // --- _Py_atomic_compare_exchange ------------------------------------------- @@ -218,6 +226,26 @@ _Py_atomic_compare_exchange_ssize(Py_ssize_t *obj, Py_ssize_t *expected, Py_ssiz expected, desired); } +static inline int +_Py_atomic_compare_exchange_ssize_relaxed(Py_ssize_t *obj, Py_ssize_t *expected, + Py_ssize_t desired) +{ + _Py_USING_STD; + return atomic_compare_exchange_strong_explicit( + (_Atomic(Py_ssize_t)*)obj, expected, desired, + memory_order_relaxed, memory_order_relaxed); +} + +static inline int +_Py_atomic_compare_exchange_ssize_acq_rel(Py_ssize_t *obj, Py_ssize_t *expected, + Py_ssize_t desired) +{ + _Py_USING_STD; + return atomic_compare_exchange_strong_explicit( + (_Atomic(Py_ssize_t)*)obj, expected, desired, + memory_order_acq_rel, memory_order_relaxed); +} + static inline int _Py_atomic_compare_exchange_ptr(void *obj, void *expected, void *desired) { diff --git a/Include/internal/pycore_object.h b/Include/internal/pycore_object.h index 41786cb267c2e96..87dbe3900a8054b 100644 --- a/Include/internal/pycore_object.h +++ b/Include/internal/pycore_object.h @@ -165,7 +165,8 @@ static inline void _Py_RefcntAdd(PyObject* op, Py_ssize_t n) _Py_atomic_store_uint32_relaxed(&op->ob_ref_local, (uint32_t)refcnt); } else { - _Py_atomic_add_ssize(&op->ob_ref_shared, (n << _Py_REF_SHARED_SHIFT)); + _Py_atomic_add_ssize_relaxed(&op->ob_ref_shared, + (n << _Py_REF_SHARED_SHIFT)); } # ifdef Py_REF_DEBUG _Py_AddRefTotal(_PyThreadState_GET(), n); @@ -547,7 +548,7 @@ _Py_TryIncRefShared(PyObject *op) return 0; } - if (_Py_atomic_compare_exchange_ssize( + if (_Py_atomic_compare_exchange_ssize_relaxed( &op->ob_ref_shared, &shared, shared + (1 << _Py_REF_SHARED_SHIFT))) { @@ -627,7 +628,7 @@ _Py_NewRefWithLock(PyObject *op) if ((shared & _Py_REF_SHARED_FLAG_MASK) == 0) { new_shared |= _Py_REF_MAYBE_WEAKREF; } - if (_Py_atomic_compare_exchange_ssize( + if (_Py_atomic_compare_exchange_ssize_relaxed( &op->ob_ref_shared, &shared, new_shared)) { diff --git a/Include/refcount.h b/Include/refcount.h index 80fe7ff70a11e87..7fa9744aab28dec 100644 --- a/Include/refcount.h +++ b/Include/refcount.h @@ -280,7 +280,10 @@ static inline Py_ALWAYS_INLINE void Py_INCREF(PyObject *op) _Py_atomic_store_uint32_relaxed(&op->ob_ref_local, new_local); } else { - _Py_atomic_add_ssize(&op->ob_ref_shared, (1 << _Py_REF_SHARED_SHIFT)); + // The existing reference keeps the object alive; the increment only + // needs to extend its lifetime. + _Py_atomic_add_ssize_relaxed(&op->ob_ref_shared, + (1 << _Py_REF_SHARED_SHIFT)); } #elif SIZEOF_VOID_P > 4 uint32_t cur_refcnt = op->ob_refcnt; diff --git a/Objects/object.c b/Objects/object.c index fadd9273a36607c..6c8533776a49633 100644 --- a/Objects/object.c +++ b/Objects/object.c @@ -403,8 +403,10 @@ _Py_DecRefSharedIsDead(PyObject *o, const char *filename, int lineno) _Py_NegativeRefcount(filename, lineno, o); } #endif - } while (!_Py_atomic_compare_exchange_ssize(&o->ob_ref_shared, - &shared, new_shared)); + // Publish accesses through this reference and acquire prior releases + // if this is the last reference and the object will be deallocated. + } while (!_Py_atomic_compare_exchange_ssize_acq_rel( + &o->ob_ref_shared, &shared, new_shared)); if (should_queue) { #ifdef Py_REF_DEBUG