From 39c251e5c65d716d1a0cc6931613f37070cff1c6 Mon Sep 17 00:00:00 2001 From: Dino Viehland Date: Wed, 24 Jan 2024 10:23:28 -0800 Subject: [PATCH 1/3] Bring in a subset of biased reference counting: https://github.com/colesbury/nogil/commit/b6b12a9a94e Co-Authored-By: Sam Gross --- Include/internal/pycore_object.h | 151 +++++++++++++++++++++++++++++++ 1 file changed, 151 insertions(+) diff --git a/Include/internal/pycore_object.h b/Include/internal/pycore_object.h index f413b8451e5ab4..fceec303c0f72a 100644 --- a/Include/internal/pycore_object.h +++ b/Include/internal/pycore_object.h @@ -376,6 +376,157 @@ static inline void _PyObject_GC_UNTRACK( _PyObject_GC_UNTRACK(__FILE__, __LINE__, _PyObject_CAST(op)) #endif +#ifdef Py_GIL_DISABLED + +/* Tries to increment an object's reference count + * + * This is a specialized version of _Py_TryIncref that only succeeds if the + * object is immortal or local to this thread. It does not handle the case + * where the reference count modification requires an atomic operation. This + * allows call sites to specialize for the immortal/local case. + */ +static inline Py_ALWAYS_INLINE int +_Py_TryIncrefFast(PyObject *op) { + uint32_t local = _Py_atomic_load_uint32_relaxed(&op->ob_ref_local); + local += 1; + if (local == 0) { + // immortal + return 1; + } + if (_Py_IsOwnedByCurrentThread(op)) { + _Py_atomic_store_uint32_relaxed(&op->ob_ref_local, local); +#ifdef Py_REF_DEBUG + _Py_IncRefTotal(_PyInterpreterState_GET()); +#endif + return 1; + } + return 0; +} + +static inline Py_ALWAYS_INLINE int +_Py_TryIncRefShared(PyObject *op) +{ + for (;;) { + Py_ssize_t shared = _Py_atomic_load_ssize_relaxed(&op->ob_ref_shared); + + // If the shared refcount is zero and the object is either merged + // or may not have weak references, then we cannot incref it. + if (shared == 0 || shared == _Py_REF_MERGED) { + return 0; + } + + if (_Py_atomic_compare_exchange_ssize( + &op->ob_ref_shared, + &shared, + shared + (1 << _Py_REF_SHARED_SHIFT))) { +#ifdef Py_REF_DEBUG + _Py_IncRefTotal(_PyInterpreterState_GET()); +#endif + return 1; + } + } +} + +/* Tries to incref the object op and ensures that *src still points to it. */ +static inline int +_Py_TryAcquireObject(PyObject **src, PyObject *op) +{ + if (_Py_TryIncrefFast(op)) { + return 1; + } + if (!_Py_TryIncRefShared(op)) { + return 0; + } + if (op != _Py_atomic_load_ptr(src)) { + Py_DECREF(op); + return 0; + } + return 1; +} + +/* Loads and increfs an object from ptr, which may contain a NULL value. + Safe with concurrent (atomic) updates to ptr. + NOTE: The writer must set maybe-weakref on the stored object! */ +static inline Py_ALWAYS_INLINE PyObject * +_Py_XFetchRef(PyObject **ptr) +{ +#ifdef Py_NOGIL + for (;;) { + PyObject *value = _Py_atomic_load_ptr(ptr); + if (value == NULL) { + return value; + } + if (_Py_TryAcquireObject(ptr, value)) { + return value; + } + } +#else + return Py_XNewRef(*ptr); +#endif +} + +/* Attempts to loads and increfs an object from ptr. Returns NULL + on failure, which may be due to a NULL value or a concurrent update. */ +static inline Py_ALWAYS_INLINE PyObject * +_Py_TryXFetchRef(PyObject **ptr) +{ + PyObject *value = _Py_atomic_load_ptr(ptr); + if (value == NULL) { + return value; + } + if (_Py_TryAcquireObject(ptr, value)) { + return value; + } + return NULL; +} + +/* Like Py_NewRef but also optimistically sets _Py_REF_MAYBE_WEAKREF + on objects owned by a different thread. */ +static inline PyObject * +_Py_NewRefWithLock(PyObject *op) +{ + _Py_INCREF_STAT_INC(); + uint32_t local = _Py_atomic_load_uint32_relaxed(&op->ob_ref_local); + local += 1; + if (local == 0) { + return op; + } + +#ifdef Py_REF_DEBUG + _Py_IncRefTotal(_PyInterpreterState_GET()); +#endif + if (_Py_IsOwnedByCurrentThread(op)) { + _Py_atomic_store_uint32_relaxed(&op->ob_ref_local, local); + } + else { + for (;;) { + Py_ssize_t shared = _Py_atomic_load_ssize_relaxed(&op->ob_ref_shared); + Py_ssize_t new_shared = shared + (1 << _Py_REF_SHARED_SHIFT); + if ((shared & _Py_REF_SHARED_FLAG_MASK) == 0) { + new_shared |= _Py_REF_MAYBE_WEAKREF; + } + if (_Py_atomic_compare_exchange_ssize( + &op->ob_ref_shared, + &shared, + new_shared)) { + return op; + } + } + } + return op; +} + +static inline PyObject * +_Py_XNewRefWithLock(PyObject *obj) +{ + if (obj == NULL) { + return NULL; + } + return _Py_NewRefWithLock(obj); +} + +#endif + #ifdef Py_REF_DEBUG extern void _PyInterpreterState_FinalizeRefTotal(PyInterpreterState *); extern void _Py_FinalizeRefTotal(_PyRuntimeState *); From a923bc912ac75348267a3493a5eef63bf2ba1083 Mon Sep 17 00:00:00 2001 From: Dino Viehland Date: Wed, 24 Jan 2024 12:39:31 -0800 Subject: [PATCH 2/3] Review feedback --- Include/internal/pycore_object.h | 56 ++++++++++++-------------------- 1 file changed, 21 insertions(+), 35 deletions(-) diff --git a/Include/internal/pycore_object.h b/Include/internal/pycore_object.h index fceec303c0f72a..ff71472f7ce8d5 100644 --- a/Include/internal/pycore_object.h +++ b/Include/internal/pycore_object.h @@ -385,7 +385,7 @@ static inline void _PyObject_GC_UNTRACK( * where the reference count modification requires an atomic operation. This * allows call sites to specialize for the immortal/local case. */ -static inline Py_ALWAYS_INLINE int +static inline int _Py_TryIncrefFast(PyObject *op) { uint32_t local = _Py_atomic_load_uint32_relaxed(&op->ob_ref_local); local += 1; @@ -394,6 +394,7 @@ _Py_TryIncrefFast(PyObject *op) { return 1; } if (_Py_IsOwnedByCurrentThread(op)) { + _Py_INCREF_STAT_INC(); _Py_atomic_store_uint32_relaxed(&op->ob_ref_local, local); #ifdef Py_REF_DEBUG _Py_IncRefTotal(_PyInterpreterState_GET()); @@ -403,12 +404,11 @@ _Py_TryIncrefFast(PyObject *op) { return 0; } -static inline Py_ALWAYS_INLINE int +static inline int _Py_TryIncRefShared(PyObject *op) { + Py_ssize_t shared = _Py_atomic_load_ssize_relaxed(&op->ob_ref_shared); for (;;) { - Py_ssize_t shared = _Py_atomic_load_ssize_relaxed(&op->ob_ref_shared); - // If the shared refcount is zero and the object is either merged // or may not have weak references, then we cannot incref it. if (shared == 0 || shared == _Py_REF_MERGED) { @@ -422,6 +422,7 @@ _Py_TryIncRefShared(PyObject *op) #ifdef Py_REF_DEBUG _Py_IncRefTotal(_PyInterpreterState_GET()); #endif + _Py_INCREF_STAT_INC(); return 1; } } @@ -447,10 +448,9 @@ _Py_TryAcquireObject(PyObject **src, PyObject *op) /* Loads and increfs an object from ptr, which may contain a NULL value. Safe with concurrent (atomic) updates to ptr. NOTE: The writer must set maybe-weakref on the stored object! */ -static inline Py_ALWAYS_INLINE PyObject * -_Py_XFetchRef(PyObject **ptr) +static inline PyObject * +_Py_XGetRef(PyObject **ptr) { -#ifdef Py_NOGIL for (;;) { PyObject *value = _Py_atomic_load_ptr(ptr); if (value == NULL) { @@ -460,14 +460,11 @@ _Py_XFetchRef(PyObject **ptr) return value; } } -#else - return Py_XNewRef(*ptr); -#endif } /* Attempts to loads and increfs an object from ptr. Returns NULL on failure, which may be due to a NULL value or a concurrent update. */ -static inline Py_ALWAYS_INLINE PyObject * +static inline PyObject * _Py_TryXFetchRef(PyObject **ptr) { PyObject *value = _Py_atomic_load_ptr(ptr); @@ -485,32 +482,21 @@ _Py_TryXFetchRef(PyObject **ptr) static inline PyObject * _Py_NewRefWithLock(PyObject *op) { - _Py_INCREF_STAT_INC(); - uint32_t local = _Py_atomic_load_uint32_relaxed(&op->ob_ref_local); - local += 1; - if (local == 0) { + if (_Py_TryIncrefFast(op)) { return op; } - -#ifdef Py_REF_DEBUG - _Py_IncRefTotal(_PyInterpreterState_GET()); -#endif - if (_Py_IsOwnedByCurrentThread(op)) { - _Py_atomic_store_uint32_relaxed(&op->ob_ref_local, local); - } - else { - for (;;) { - Py_ssize_t shared = _Py_atomic_load_ssize_relaxed(&op->ob_ref_shared); - Py_ssize_t new_shared = shared + (1 << _Py_REF_SHARED_SHIFT); - if ((shared & _Py_REF_SHARED_FLAG_MASK) == 0) { - new_shared |= _Py_REF_MAYBE_WEAKREF; - } - if (_Py_atomic_compare_exchange_ssize( - &op->ob_ref_shared, - &shared, - new_shared)) { - return op; - } + _Py_INCREF_STAT_INC(); + for (;;) { + Py_ssize_t shared = _Py_atomic_load_ssize_relaxed(&op->ob_ref_shared); + Py_ssize_t new_shared = shared + (1 << _Py_REF_SHARED_SHIFT); + if ((shared & _Py_REF_SHARED_FLAG_MASK) == 0) { + new_shared |= _Py_REF_MAYBE_WEAKREF; + } + if (_Py_atomic_compare_exchange_ssize( + &op->ob_ref_shared, + &shared, + new_shared)) { + return op; } } return op; From 87e94657436bc27a9d6eea6255465e3898bf72d3 Mon Sep 17 00:00:00 2001 From: Dino Viehland Date: Wed, 24 Jan 2024 16:41:17 -0800 Subject: [PATCH 3/3] More renames --- Include/internal/pycore_object.h | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/Include/internal/pycore_object.h b/Include/internal/pycore_object.h index ff71472f7ce8d5..ec14c07a2991ff 100644 --- a/Include/internal/pycore_object.h +++ b/Include/internal/pycore_object.h @@ -430,7 +430,7 @@ _Py_TryIncRefShared(PyObject *op) /* Tries to incref the object op and ensures that *src still points to it. */ static inline int -_Py_TryAcquireObject(PyObject **src, PyObject *op) +_Py_TryIncref(PyObject **src, PyObject *op) { if (_Py_TryIncrefFast(op)) { return 1; @@ -456,7 +456,7 @@ _Py_XGetRef(PyObject **ptr) if (value == NULL) { return value; } - if (_Py_TryAcquireObject(ptr, value)) { + if (_Py_TryIncref(ptr, value)) { return value; } } @@ -465,13 +465,13 @@ _Py_XGetRef(PyObject **ptr) /* Attempts to loads and increfs an object from ptr. Returns NULL on failure, which may be due to a NULL value or a concurrent update. */ static inline PyObject * -_Py_TryXFetchRef(PyObject **ptr) +_Py_TryXGetRef(PyObject **ptr) { PyObject *value = _Py_atomic_load_ptr(ptr); if (value == NULL) { return value; } - if (_Py_TryAcquireObject(ptr, value)) { + if (_Py_TryIncref(ptr, value)) { return value; } return NULL; @@ -499,7 +499,6 @@ _Py_NewRefWithLock(PyObject *op) return op; } } - return op; } static inline PyObject *