Skip to content

Commit 46ea558

Browse files
committed
Make weakref dereference lock-free.
In the free-threaded build, _PyWeakref_GET_REF() no longer takes the striped weakref list lock. The referent's memory stays readable if it is freed concurrently because object heaps delay page reuse with QSBR, and wr_object only ever transitions from the referent to Py_None, so _Py_TryIncrefCompare() is sufficient. clear_weakref_lock_held() now uses release stores instead of sequentially consistent stores. The weakref list lock orders the list updates, and the lock-free read only requires that the store of Py_None happens before the referent's memory can be reused.
1 parent d3ada45 commit 46ea558

3 files changed

Lines changed: 16 additions & 12 deletions

File tree

‎Include/internal/pycore_weakref.h‎

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -74,26 +74,27 @@ static inline PyObject* _PyWeakref_GET_REF(PyObject *ref_obj)
7474
assert(PyWeakref_Check(ref_obj));
7575
PyWeakReference *ref = _Py_CAST(PyWeakReference*, ref_obj);
7676

77-
PyObject *obj = FT_ATOMIC_LOAD_PTR(ref->wr_object);
77+
#ifdef Py_GIL_DISABLED
78+
PyObject *obj = _Py_atomic_load_ptr(&ref->wr_object);
7879
if (obj == Py_None) {
7980
// clear_weakref() was called
8081
return NULL;
8182
}
82-
83-
LOCK_WEAKREFS(obj);
84-
#ifdef Py_GIL_DISABLED
85-
if (ref->wr_object == Py_None) {
83+
if (_Py_TryIncrefCompare(&ref->wr_object, obj)) {
84+
return obj;
85+
}
86+
return NULL;
87+
#else
88+
PyObject *obj = ref->wr_object;
89+
if (obj == Py_None) {
8690
// clear_weakref() was called
87-
UNLOCK_WEAKREFS(obj);
8891
return NULL;
8992
}
90-
#endif
9193
if (_Py_TryIncref(obj)) {
92-
UNLOCK_WEAKREFS(obj);
9394
return obj;
9495
}
95-
UNLOCK_WEAKREFS(obj);
9696
return NULL;
97+
#endif
9798
}
9899

99100
static inline int _PyWeakref_IS_DEAD(PyObject *ref_obj)
@@ -108,8 +109,8 @@ static inline int _PyWeakref_IS_DEAD(PyObject *ref_obj)
108109
}
109110
else {
110111
LOCK_WEAKREFS(obj);
111-
// See _PyWeakref_GET_REF() for the rationale of this test
112112
#ifdef Py_GIL_DISABLED
113+
// The weakref may have been cleared before the lock was acquired.
113114
ret = (ref->wr_object == Py_None) || _is_dead(obj);
114115
#else
115116
ret = _is_dead(obj);
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
Improve the performance and thread scaling of dereferencing weak references
2+
in the :term:`free-threaded build`. Dereferencing a weak reference no longer
3+
takes a lock.

‎Objects/weakrefobject.c‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -84,9 +84,9 @@ clear_weakref_lock_held(PyWeakReference *self, PyObject **callback)
8484
/* If 'self' is the end of the list (and thus self->wr_next ==
8585
NULL) then the weakref list itself (and thus the value of *list)
8686
will end up being set to NULL. */
87-
FT_ATOMIC_STORE_PTR(*list, self->wr_next);
87+
FT_ATOMIC_STORE_PTR_RELEASE(*list, self->wr_next);
8888
}
89-
FT_ATOMIC_STORE_PTR(self->wr_object, Py_None);
89+
FT_ATOMIC_STORE_PTR_RELEASE(self->wr_object, Py_None);
9090
if (self->wr_prev != NULL) {
9191
self->wr_prev->wr_next = self->wr_next;
9292
}

0 commit comments

Comments
 (0)