From ff1d60d549943281ac2bb794c171ca40991d26b9 Mon Sep 17 00:00:00 2001 From: Neil Schemenauer Date: Fri, 4 Apr 2025 12:40:15 -0700 Subject: [PATCH 01/16] wip: lock-free set contains --- Lib/test/test_free_threading/test_set.py | 31 ++ Objects/setobject.c | 413 +++++++++++++++++------ 2 files changed, 332 insertions(+), 112 deletions(-) diff --git a/Lib/test/test_free_threading/test_set.py b/Lib/test/test_free_threading/test_set.py index a66e03bcc4c9d1..31b43fb217f8d2 100644 --- a/Lib/test/test_free_threading/test_set.py +++ b/Lib/test/test_free_threading/test_set.py @@ -36,6 +36,37 @@ def repr_set(): for set_repr in set_reprs: self.assertIn(set_repr, ("set()", "{1, 2, 3, 4, 5, 6, 7, 8}")) + def test_contains_mutate(self): + """Test set contains operation combined with mutation.""" + barrier = Barrier(2) + s = set() + done = False + + def read_set(): + barrier.wait() + while not done: + for i in range(64): + result = (i % 16) in s + + def mutate_set(): + nonlocal done + barrier.wait() + for i in range(10): + s.clear() + for j in range(16): + s.add(j) + for j in range(16): + s.discard(j) + # executes the set_swap_bodies() function + s.__iand__(set(k for k in range(10, 20))) + done = True + + threads = [Thread(target=read_set), Thread(target=mutate_set)] + for t in threads: + t.start() + for t in threads: + t.join() + if __name__ == "__main__": unittest.main() diff --git a/Objects/setobject.c b/Objects/setobject.c index acbb53aafc0a26..a9e70b8e5a6a66 100644 --- a/Objects/setobject.c +++ b/Objects/setobject.c @@ -62,6 +62,152 @@ static PyObject _dummy_struct; #define dummy (&_dummy_struct) +#define SET_LOOKKEY_FOUND 1 +#define SET_LOOKKEY_NO_MATCH 0 +#define SET_LOOKKEY_ERROR (-1) +#define SET_LOOKKEY_CHANGED (-2) +#define SET_LOOKKEY_EMPTY (-3) + +#ifdef Py_GIL_DISABLED + +#define LOAD_HASH(entry) _Py_atomic_load_ssize_acquire(&(entry)->hash) +#define STORE_HASH(entry, v) _Py_atomic_store_ssize_release(&(entry)->hash, v) + +#define LOAD_KEY(entry) _Py_atomic_load_ptr_acquire(&(entry)->key) +#define STORE_KEY(entry, v) _Py_atomic_store_ptr_release(&(entry)->key, v) + +#define LOAD_TABLE(so) _Py_atomic_load_ptr_acquire(&(so)->table) +#define STORE_TABLE(so, v) _Py_atomic_store_ptr_release(&(so)->table, v) + +#define LOAD_MASK(so) _Py_atomic_load_ssize_acquire(&(so)->mask) +#define STORE_MASK(so, v) _Py_atomic_store_ssize_release(&(so)->mask, v) + +#define LOAD_USED(so) _Py_atomic_load_ssize_acquire(&(so)->used) +#define STORE_USED(so, v) _Py_atomic_store_ssize_release(&(so)->used, v) + +#define LOAD_SIZE(so) _Py_atomic_load_ssize_acquire(&(so)->size) +#define STORE_SIZE(so, v) _Py_atomic_store_ssize_release(&(so)->size, v) + +#define IS_SHARED(so) _PyObject_GC_IS_SHARED(so) +#define SET_SHARED(so) _PyObject_GC_SET_SHARED(so) + +static void +ensure_shared_on_read(PySetObject *so) +{ + if (!_Py_IsOwnedByCurrentThread((PyObject *)so) && !IS_SHARED(so)) { + // The first time we access a set from a non-owning thread we mark it + // as shared. This ensures that a concurrent resize operation will + // delay freeing the old entries using QSBR, which is necessary + // to safely allow concurrent reads without locking... + Py_BEGIN_CRITICAL_SECTION(so); + if (!IS_SHARED(so)) { + SET_SHARED(so); + } + Py_END_CRITICAL_SECTION(); + } +} + +static inline Py_ALWAYS_INLINE int +set_compare_threadsafe(PySetObject *so, setentry *table, setentry *ep, + PyObject *key, Py_hash_t hash) +{ + PyObject *startkey = LOAD_KEY(ep); + if (startkey == NULL) { + return SET_LOOKKEY_EMPTY; + } + if (startkey == key) { + return SET_LOOKKEY_FOUND; + } + Py_ssize_t ep_hash = LOAD_HASH(ep); + if (ep_hash == hash) { + if (startkey == NULL || !_Py_TryIncrefCompare(&ep->key, startkey)) { + return SET_LOOKKEY_CHANGED; + } + int cmp = PyObject_RichCompareBool(startkey, key, Py_EQ); + Py_DECREF(startkey); + if (cmp < 0) { + return SET_LOOKKEY_ERROR; + } + if (table == LOAD_TABLE(so) && + startkey == LOAD_KEY(ep)) { + assert(cmp == SET_LOOKKEY_FOUND || cmp == SET_LOOKKEY_NO_MATCH); + return cmp; + } + else { + /* The set was mutated, restart */ + return SET_LOOKKEY_CHANGED; + } + } + return SET_LOOKKEY_NO_MATCH; +} + +#else + +#define LOAD_HASH(entry) ((entry)->hash) +#define STORE_HASH(entry, v) ((entry)->hash = v) + +#define LOAD_KEY(entry) ((entry)->key) +#define STORE_KEY(entry, v) ((entry)->key = v) + +#define LOAD_TABLE(so) ((so)->table) +#define STORE_TABLE(so, v) ((so)->table = v) + +#define LOAD_MASK(so) ((so)->mask) +#define STORE_MASK(so, v) ((so)->mask = v) + +#define LOAD_USED(so) ((so)->used) +#define STORE_USED(so, v) ((so)->used = v) + +#define LOAD_SIZE(so) ((so)->size) +#define STORE_SIZE(so, v) ((so)->size = v) + +#define IS_SHARED(so) 0 +#define SET_SHARED(so) + +#endif + +static inline Py_ALWAYS_INLINE int +set_compare_entry(PySetObject *so, setentry *table, setentry *entry, + PyObject *key, Py_hash_t hash) +{ + if (entry->hash == 0 && entry->key == NULL) + return SET_LOOKKEY_EMPTY; + if (entry->hash == hash) { + PyObject *startkey = entry->key; + assert(startkey != dummy); + if (startkey == key) + return SET_LOOKKEY_FOUND; + if (PyUnicode_CheckExact(startkey) + && PyUnicode_CheckExact(key) + && unicode_eq(startkey, key)) + return SET_LOOKKEY_FOUND; + table = so->table; + Py_INCREF(startkey); + int cmp = PyObject_RichCompareBool(startkey, key, Py_EQ); + Py_DECREF(startkey); + if (cmp < 0) + return SET_LOOKKEY_ERROR; + if (table != so->table || entry->key != startkey) + return SET_LOOKKEY_CHANGED; + if (cmp > 0) + return SET_LOOKKEY_FOUND; + } + return SET_LOOKKEY_NO_MATCH; +} + +static void +set_zero_table(setentry *table, size_t size) +{ +#ifdef Py_GIL_DISABLED + for (size_t i = 0; i < size; i++) { + setentry *entry = &table[i]; + STORE_HASH(entry, 0); + STORE_KEY(entry, NULL); + } +#else + memset(table, 0, sizeof(setentry)*size); +#endif +} /* ======================================================================== */ /* ======= Begin logic for probing the hash table ========================= */ @@ -74,49 +220,36 @@ static PyObject _dummy_struct; /* This must be >= 1 */ #define PERTURB_SHIFT 5 -static setentry * -set_lookkey(PySetObject *so, PyObject *key, Py_hash_t hash) +static int +set_do_lookup(PySetObject *so, setentry *table, size_t mask, PyObject *key, + Py_hash_t hash, setentry **epp, + int (*compare_entry)(PySetObject *so, setentry *table, setentry *ep, + PyObject *key, Py_hash_t hash)) { - setentry *table; setentry *entry; size_t perturb = hash; - size_t mask = so->mask; size_t i = (size_t)hash & mask; /* Unsigned for defined overflow behavior */ int probes; - int cmp; + int status; while (1) { - entry = &so->table[i]; + entry = &table[i]; probes = (i + LINEAR_PROBES <= mask) ? LINEAR_PROBES: 0; do { - if (entry->hash == 0 && entry->key == NULL) - return entry; - if (entry->hash == hash) { - PyObject *startkey = entry->key; - assert(startkey != dummy); - if (startkey == key) - return entry; - if (PyUnicode_CheckExact(startkey) - && PyUnicode_CheckExact(key) - && unicode_eq(startkey, key)) - return entry; - table = so->table; - Py_INCREF(startkey); - cmp = PyObject_RichCompareBool(startkey, key, Py_EQ); - Py_DECREF(startkey); - if (cmp < 0) - return NULL; - if (table != so->table || entry->key != startkey) - return set_lookkey(so, key, hash); - if (cmp > 0) - return entry; - mask = so->mask; + status = compare_entry(so, table, entry, key, hash); + if (status != SET_LOOKKEY_NO_MATCH) { + if (status == SET_LOOKKEY_EMPTY) { + return SET_LOOKKEY_NO_MATCH; + } + *epp = entry; + return status; } entry++; } while (probes--); perturb >>= PERTURB_SHIFT; i = (i * 5 + 1 + perturb) & mask; } + Py_UNREACHABLE(); } static int set_table_resize(PySetObject *, Py_ssize_t); @@ -180,16 +313,16 @@ set_add_entry_takeref(PySetObject *so, PyObject *key, Py_hash_t hash) found_unused_or_dummy: if (freeslot == NULL) goto found_unused; - FT_ATOMIC_STORE_SSIZE_RELAXED(so->used, so->used + 1); - freeslot->key = key; - freeslot->hash = hash; + STORE_USED(so, so->used + 1); + STORE_KEY(freeslot, key); + STORE_HASH(freeslot, hash); return 0; found_unused: so->fill++; - FT_ATOMIC_STORE_SSIZE_RELAXED(so->used, so->used + 1); - entry->key = key; - entry->hash = hash; + STORE_USED(so, so->used + 1); + STORE_KEY(entry, key); + STORE_HASH(entry, hash); if ((size_t)so->fill*5 < mask*3) return 0; return set_table_resize(so, so->used>50000 ? so->used*2 : so->used*4); @@ -255,13 +388,62 @@ set_insert_clean(setentry *table, size_t mask, PyObject *key, Py_hash_t hash) i = (i * 5 + 1 + perturb) & mask; } found_null: - entry->key = key; - entry->hash = hash; + STORE_KEY(entry, key); + STORE_HASH(entry, hash); } /* ======== End logic for probing the hash table ========================== */ /* ======================================================================== */ +static int +set_lookkey(PySetObject *so, PyObject *key, Py_hash_t hash, setentry **epp) +{ + int status; + Py_BEGIN_CRITICAL_SECTION(so); + do { + status = set_do_lookup(so, so->table, so->mask, key, hash, epp, set_compare_entry); + } while (status == SET_LOOKKEY_CHANGED); + Py_END_CRITICAL_SECTION(); + assert(status == SET_LOOKKEY_FOUND || + status == SET_LOOKKEY_NO_MATCH || + status == SET_LOOKKEY_ERROR); + return status; +} + +#ifdef Py_GIL_DISABLED +static int +set_lookkey_threadsafe(PySetObject *so, PyObject *key, Py_hash_t hash) +{ + int status; + setentry *entry; + ensure_shared_on_read(so); + setentry *table = LOAD_TABLE(so); + size_t mask = LOAD_MASK(so); + if (table == NULL || table != LOAD_TABLE(so)) { + return set_lookkey(so, key, hash, &entry); + } + status = set_do_lookup(so, table, mask, key, hash, &entry, set_compare_threadsafe); + if (status == SET_LOOKKEY_CHANGED) { + return set_lookkey(so, key, hash, &entry); + } + assert(status == SET_LOOKKEY_FOUND || + status == SET_LOOKKEY_NO_MATCH || + status == SET_LOOKKEY_ERROR); + return status; +} +#endif + +static void free_entries(setentry *entries, bool use_qsbr) +{ +#ifdef Py_GIL_DISABLED + if (use_qsbr) { + _PyMem_FreeDelayed(entries); + return; + } +#endif + PyMem_Free(entries); +} + /* Restructure the table by allocating a new table and reinserting all keys again. When entries have been deleted, the new table may @@ -319,9 +501,9 @@ set_table_resize(PySetObject *so, Py_ssize_t minused) /* Make the set empty, using the new table. */ assert(newtable != oldtable); - memset(newtable, 0, sizeof(setentry) * newsize); - so->mask = newsize - 1; - so->table = newtable; + set_zero_table(newtable, newsize); + STORE_TABLE(so, NULL); + STORE_MASK(so, newsize - 1); /* Copy the data over; this is refcount-neutral for active entries; dummy entries aren't copied over, of course */ @@ -341,20 +523,22 @@ set_table_resize(PySetObject *so, Py_ssize_t minused) } } + STORE_TABLE(so, newtable); + if (is_oldtable_malloced) - PyMem_Free(oldtable); + free_entries(oldtable, IS_SHARED(so)); return 0; } static int set_contains_entry(PySetObject *so, PyObject *key, Py_hash_t hash) { - setentry *entry; - - entry = set_lookkey(so, key, hash); - if (entry != NULL) - return entry->key != NULL; - return -1; +#ifdef Py_GIL_DISABLED + return set_lookkey_threadsafe(so, key, hash); +#else + setentry *entry; // unused + return set_lookkey(so, key, hash, &entry); +#endif } #define DISCARD_NOTFOUND 0 @@ -365,16 +549,18 @@ set_discard_entry(PySetObject *so, PyObject *key, Py_hash_t hash) { setentry *entry; PyObject *old_key; - - entry = set_lookkey(so, key, hash); - if (entry == NULL) + int status = set_lookkey(so, key, hash, &entry); + if (status < 0) { return -1; - if (entry->key == NULL) + } + if (status == SET_LOOKKEY_NO_MATCH) { return DISCARD_NOTFOUND; + } + assert(status == SET_LOOKKEY_FOUND); old_key = entry->key; - entry->key = dummy; - entry->hash = -1; - FT_ATOMIC_STORE_SSIZE_RELAXED(so->used, so->used - 1); + STORE_KEY(entry, dummy); + STORE_HASH(entry, -1); + STORE_USED(so, so->used - 1); Py_DECREF(old_key); return DISCARD_FOUND; } @@ -412,12 +598,13 @@ set_discard_key(PySetObject *so, PyObject *key) static void set_empty_to_minsize(PySetObject *so) { - memset(so->smalltable, 0, sizeof(so->smalltable)); + STORE_TABLE(so, NULL); + set_zero_table(so->smalltable, PySet_MINSIZE); so->fill = 0; - FT_ATOMIC_STORE_SSIZE_RELAXED(so->used, 0); - so->mask = PySet_MINSIZE - 1; - so->table = so->smalltable; - FT_ATOMIC_STORE_SSIZE_RELAXED(so->hash, -1); + STORE_USED(so, 0); + STORE_MASK(so, PySet_MINSIZE - 1); + STORE_HASH(so, -1); + STORE_TABLE(so, so->smalltable); } static int @@ -466,7 +653,7 @@ set_clear_internal(PyObject *self) } if (table_is_malloced) - PyMem_Free(table); + free_entries(table, IS_SHARED(so)); return 0; } @@ -527,7 +714,7 @@ set_dealloc(PyObject *self) } } if (so->table != so->smalltable) - PyMem_Free(so->table); + free_entries(so->table, IS_SHARED(so)); Py_TYPE(so)->tp_free(so); Py_TRASHCAN_END } @@ -601,7 +788,7 @@ static Py_ssize_t set_len(PyObject *self) { PySetObject *so = _PySet_CAST(self); - return FT_ATOMIC_LOAD_SSIZE_RELAXED(so->used); + return LOAD_USED(so); } static int @@ -639,12 +826,12 @@ set_merge_lock_held(PySetObject *so, PyObject *otherset) key = other_entry->key; if (key != NULL) { assert(so_entry->key == NULL); - so_entry->key = Py_NewRef(key); - so_entry->hash = other_entry->hash; + STORE_KEY(so_entry, Py_NewRef(key)); + STORE_HASH(so_entry, other_entry->hash); } } so->fill = other->fill; - FT_ATOMIC_STORE_SSIZE_RELAXED(so->used, other->used); + STORE_USED(so, other->used); return 0; } @@ -653,7 +840,7 @@ set_merge_lock_held(PySetObject *so, PyObject *otherset) setentry *newtable = so->table; size_t newmask = (size_t)so->mask; so->fill = other->used; - FT_ATOMIC_STORE_SSIZE_RELAXED(so->used, other->used); + STORE_USED(so, other->used); for (i = other->mask + 1; i > 0 ; i--, other_entry++) { key = other_entry->key; if (key != NULL && key != dummy) { @@ -705,9 +892,9 @@ set_pop_impl(PySetObject *so) entry = so->table; } key = entry->key; - entry->key = dummy; - entry->hash = -1; - FT_ATOMIC_STORE_SSIZE_RELAXED(so->used, so->used - 1); + STORE_KEY(entry, dummy); + STORE_HASH(entry, -1); + STORE_USED(so, so->used - 1); so->finger = entry - so->table + 1; /* next place to start */ return key; } @@ -793,12 +980,12 @@ frozenset_hash(PyObject *self) PySetObject *so = _PySet_CAST(self); Py_uhash_t hash; - if (FT_ATOMIC_LOAD_SSIZE_RELAXED(so->hash) != -1) { - return FT_ATOMIC_LOAD_SSIZE_RELAXED(so->hash); + if (LOAD_HASH(so) != -1) { + return LOAD_HASH(so); } hash = frozenset_hash_impl(self); - FT_ATOMIC_STORE_SSIZE_RELAXED(so->hash, hash); + STORE_HASH(so, hash); return hash; } @@ -880,7 +1067,7 @@ static PyObject *setiter_iternext(PyObject *self) return NULL; assert (PyAnySet_Check(so)); - Py_ssize_t so_used = FT_ATOMIC_LOAD_SSIZE(so->used); + Py_ssize_t so_used = LOAD_USED(so); Py_ssize_t si_used = FT_ATOMIC_LOAD_SSIZE(si->si_used); if (si_used != so_used) { PyErr_SetString(PyExc_RuntimeError, @@ -1221,21 +1408,28 @@ set_swap_bodies(PySetObject *a, PySetObject *b) setentry tab[PySet_MINSIZE]; Py_hash_t h; + setentry *a_table = a->table; + setentry *b_table = b->table; + STORE_TABLE(a, NULL); + STORE_TABLE(b, NULL); + t = a->fill; a->fill = b->fill; b->fill = t; t = a->used; - FT_ATOMIC_STORE_SSIZE_RELAXED(a->used, b->used); - FT_ATOMIC_STORE_SSIZE_RELAXED(b->used, t); - t = a->mask; a->mask = b->mask; b->mask = t; - - u = a->table; - if (a->table == a->smalltable) + STORE_USED(a, b->used); + STORE_USED(b, t); + t = a->mask; + STORE_MASK(a, b->mask); + STORE_MASK(b, t); + + u = a_table; + if (a_table == a->smalltable) u = b->smalltable; - a->table = b->table; - if (b->table == b->smalltable) - a->table = a->smalltable; - b->table = u; + a_table = b_table; + if (b_table == b->smalltable) + a_table = a->smalltable; + b_table = u; - if (a->table == a->smalltable || b->table == b->smalltable) { + if (a_table == a->smalltable || b_table == b->smalltable) { memcpy(tab, a->smalltable, sizeof(tab)); memcpy(a->smalltable, b->smalltable, sizeof(tab)); memcpy(b->smalltable, tab, sizeof(tab)); @@ -1243,13 +1437,21 @@ set_swap_bodies(PySetObject *a, PySetObject *b) if (PyType_IsSubtype(Py_TYPE(a), &PyFrozenSet_Type) && PyType_IsSubtype(Py_TYPE(b), &PyFrozenSet_Type)) { - h = FT_ATOMIC_LOAD_SSIZE_RELAXED(a->hash); - FT_ATOMIC_STORE_SSIZE_RELAXED(a->hash, FT_ATOMIC_LOAD_SSIZE_RELAXED(b->hash)); - FT_ATOMIC_STORE_SSIZE_RELAXED(b->hash, h); + h = LOAD_HASH(a); + STORE_HASH(a, LOAD_HASH(b)); + STORE_HASH(b, h); } else { - FT_ATOMIC_STORE_SSIZE_RELAXED(a->hash, -1); - FT_ATOMIC_STORE_SSIZE_RELAXED(b->hash, -1); + STORE_HASH(a, -1); + STORE_HASH(b, -1); + } + if (IS_SHARED(a)) { + SET_SHARED(b); } + if (IS_SHARED(b)) { + SET_SHARED(a); + } + STORE_TABLE(a, a_table); + STORE_TABLE(b, b_table); } /*[clinic input] @@ -2143,8 +2345,8 @@ set_richcompare(PyObject *self, PyObject *w, int op) case Py_EQ: if (PySet_GET_SIZE(v) != PySet_GET_SIZE(w)) Py_RETURN_FALSE; - Py_hash_t v_hash = FT_ATOMIC_LOAD_SSIZE_RELAXED(v->hash); - Py_hash_t w_hash = FT_ATOMIC_LOAD_SSIZE_RELAXED(((PySetObject *)w)->hash); + Py_hash_t v_hash = LOAD_HASH(v); + Py_hash_t w_hash = LOAD_HASH((PySetObject *)w); if (v_hash != -1 && w_hash != -1 && v_hash != w_hash) Py_RETURN_FALSE; return set_issubset((PyObject*)v, w); @@ -2194,33 +2396,21 @@ set_add_impl(PySetObject *so, PyObject *key) Py_RETURN_NONE; } -static int -set_contains_lock_held(PySetObject *so, PyObject *key) +int +_PySet_Contains(PySetObject *so, PyObject *key) { - int rv; - - rv = set_contains_key(so, key); - if (rv < 0) { - if (!PySet_Check(key) || !PyErr_ExceptionMatches(PyExc_TypeError)) + Py_hash_t hash = _PyObject_HashFast(key); + if (hash == -1) { + if (!PySet_Check(key) || !PyErr_ExceptionMatches(PyExc_TypeError)) { return -1; + } PyErr_Clear(); - Py_hash_t hash; Py_BEGIN_CRITICAL_SECTION(key); hash = frozenset_hash_impl(key); Py_END_CRITICAL_SECTION(); - rv = set_contains_entry(so, key, hash); + return set_contains_entry(so, key, hash); } - return rv; -} - -int -_PySet_Contains(PySetObject *so, PyObject *key) -{ - int rv; - Py_BEGIN_CRITICAL_SECTION(so); - rv = set_contains_lock_held(so, key); - Py_END_CRITICAL_SECTION(); - return rv; + return set_contains_entry(so, key, hash); } static int @@ -2231,7 +2421,6 @@ set_contains(PyObject *self, PyObject *key) } /*[clinic input] -@critical_section @coexist set.__contains__ so: setobject @@ -2247,7 +2436,7 @@ set___contains___impl(PySetObject *so, PyObject *key) { long result; - result = set_contains_lock_held(so, key); + result = _PySet_Contains(so, key); if (result < 0) return NULL; return PyBool_FromLong(result); From 55ab02a16ec1b7b31b085a8f63c37de1e8bde29d Mon Sep 17 00:00:00 2001 From: Neil Schemenauer Date: Fri, 11 Jul 2025 16:41:18 -0700 Subject: [PATCH 02/16] Use FT_ATOMIC_* macros. This makes for longer code vs using the custom LOAD_*/STORE_* macros. However, I think this makes the code more clear. --- .../internal/pycore_pyatomic_ft_wrappers.h | 3 + Objects/setobject.c | 176 +++++++----------- 2 files changed, 73 insertions(+), 106 deletions(-) diff --git a/Include/internal/pycore_pyatomic_ft_wrappers.h b/Include/internal/pycore_pyatomic_ft_wrappers.h index d755d03a5fa190..fb54b22d74b239 100644 --- a/Include/internal/pycore_pyatomic_ft_wrappers.h +++ b/Include/internal/pycore_pyatomic_ft_wrappers.h @@ -55,6 +55,8 @@ extern "C" { _Py_atomic_store_uintptr_release(&value, new_value) #define FT_ATOMIC_STORE_SSIZE_RELAXED(value, new_value) \ _Py_atomic_store_ssize_relaxed(&value, new_value) +#define FT_ATOMIC_STORE_SSIZE_RELEASE(value, new_value) \ + _Py_atomic_store_ssize_release(&value, new_value) #define FT_ATOMIC_STORE_UINT8_RELAXED(value, new_value) \ _Py_atomic_store_uint8_relaxed(&value, new_value) #define FT_ATOMIC_STORE_UINT16_RELAXED(value, new_value) \ @@ -129,6 +131,7 @@ extern "C" { #define FT_ATOMIC_STORE_PTR_RELEASE(value, new_value) value = new_value #define FT_ATOMIC_STORE_UINTPTR_RELEASE(value, new_value) value = new_value #define FT_ATOMIC_STORE_SSIZE_RELAXED(value, new_value) value = new_value +#define FT_ATOMIC_STORE_SSIZE_RELEASE(value, new_value) value = new_value #define FT_ATOMIC_STORE_UINT8_RELAXED(value, new_value) value = new_value #define FT_ATOMIC_STORE_UINT16_RELAXED(value, new_value) value = new_value #define FT_ATOMIC_STORE_UINT32_RELAXED(value, new_value) value = new_value diff --git a/Objects/setobject.c b/Objects/setobject.c index a9e70b8e5a6a66..42198956d3cf3a 100644 --- a/Objects/setobject.c +++ b/Objects/setobject.c @@ -70,38 +70,20 @@ static PyObject _dummy_struct; #ifdef Py_GIL_DISABLED -#define LOAD_HASH(entry) _Py_atomic_load_ssize_acquire(&(entry)->hash) -#define STORE_HASH(entry, v) _Py_atomic_store_ssize_release(&(entry)->hash, v) - -#define LOAD_KEY(entry) _Py_atomic_load_ptr_acquire(&(entry)->key) -#define STORE_KEY(entry, v) _Py_atomic_store_ptr_release(&(entry)->key, v) - -#define LOAD_TABLE(so) _Py_atomic_load_ptr_acquire(&(so)->table) -#define STORE_TABLE(so, v) _Py_atomic_store_ptr_release(&(so)->table, v) - -#define LOAD_MASK(so) _Py_atomic_load_ssize_acquire(&(so)->mask) -#define STORE_MASK(so, v) _Py_atomic_store_ssize_release(&(so)->mask, v) - -#define LOAD_USED(so) _Py_atomic_load_ssize_acquire(&(so)->used) -#define STORE_USED(so, v) _Py_atomic_store_ssize_release(&(so)->used, v) - -#define LOAD_SIZE(so) _Py_atomic_load_ssize_acquire(&(so)->size) -#define STORE_SIZE(so, v) _Py_atomic_store_ssize_release(&(so)->size, v) - -#define IS_SHARED(so) _PyObject_GC_IS_SHARED(so) -#define SET_SHARED(so) _PyObject_GC_SET_SHARED(so) +#define SET_IS_SHARED(so) _PyObject_GC_IS_SHARED(so) +#define SET_MARK_SHARED(so) _PyObject_GC_SET_SHARED(so) static void ensure_shared_on_read(PySetObject *so) { - if (!_Py_IsOwnedByCurrentThread((PyObject *)so) && !IS_SHARED(so)) { + if (!_Py_IsOwnedByCurrentThread((PyObject *)so) && !SET_IS_SHARED(so)) { // The first time we access a set from a non-owning thread we mark it // as shared. This ensures that a concurrent resize operation will // delay freeing the old entries using QSBR, which is necessary // to safely allow concurrent reads without locking... Py_BEGIN_CRITICAL_SECTION(so); - if (!IS_SHARED(so)) { - SET_SHARED(so); + if (!SET_IS_SHARED(so)) { + SET_MARK_SHARED(so); } Py_END_CRITICAL_SECTION(); } @@ -111,14 +93,14 @@ static inline Py_ALWAYS_INLINE int set_compare_threadsafe(PySetObject *so, setentry *table, setentry *ep, PyObject *key, Py_hash_t hash) { - PyObject *startkey = LOAD_KEY(ep); + PyObject *startkey = FT_ATOMIC_LOAD_PTR_ACQUIRE(ep->key); if (startkey == NULL) { return SET_LOOKKEY_EMPTY; } if (startkey == key) { return SET_LOOKKEY_FOUND; } - Py_ssize_t ep_hash = LOAD_HASH(ep); + Py_ssize_t ep_hash = FT_ATOMIC_LOAD_SSIZE_ACQUIRE(ep->hash); if (ep_hash == hash) { if (startkey == NULL || !_Py_TryIncrefCompare(&ep->key, startkey)) { return SET_LOOKKEY_CHANGED; @@ -128,8 +110,8 @@ set_compare_threadsafe(PySetObject *so, setentry *table, setentry *ep, if (cmp < 0) { return SET_LOOKKEY_ERROR; } - if (table == LOAD_TABLE(so) && - startkey == LOAD_KEY(ep)) { + if (table == FT_ATOMIC_LOAD_PTR_ACQUIRE(so->table) && + startkey == FT_ATOMIC_LOAD_PTR_ACQUIRE(ep->key)) { assert(cmp == SET_LOOKKEY_FOUND || cmp == SET_LOOKKEY_NO_MATCH); return cmp; } @@ -143,26 +125,8 @@ set_compare_threadsafe(PySetObject *so, setentry *table, setentry *ep, #else -#define LOAD_HASH(entry) ((entry)->hash) -#define STORE_HASH(entry, v) ((entry)->hash = v) - -#define LOAD_KEY(entry) ((entry)->key) -#define STORE_KEY(entry, v) ((entry)->key = v) - -#define LOAD_TABLE(so) ((so)->table) -#define STORE_TABLE(so, v) ((so)->table = v) - -#define LOAD_MASK(so) ((so)->mask) -#define STORE_MASK(so, v) ((so)->mask = v) - -#define LOAD_USED(so) ((so)->used) -#define STORE_USED(so, v) ((so)->used = v) - -#define LOAD_SIZE(so) ((so)->size) -#define STORE_SIZE(so, v) ((so)->size = v) - -#define IS_SHARED(so) 0 -#define SET_SHARED(so) +#define SET_IS_SHARED(so) 0 +#define SET_MARK_SHARED(so) #endif @@ -201,8 +165,8 @@ set_zero_table(setentry *table, size_t size) #ifdef Py_GIL_DISABLED for (size_t i = 0; i < size; i++) { setentry *entry = &table[i]; - STORE_HASH(entry, 0); - STORE_KEY(entry, NULL); + FT_ATOMIC_STORE_SSIZE_RELAXED(entry->hash, 0); + FT_ATOMIC_STORE_PTR_RELEASE(entry->key, NULL); } #else memset(table, 0, sizeof(setentry)*size); @@ -313,16 +277,16 @@ set_add_entry_takeref(PySetObject *so, PyObject *key, Py_hash_t hash) found_unused_or_dummy: if (freeslot == NULL) goto found_unused; - STORE_USED(so, so->used + 1); - STORE_KEY(freeslot, key); - STORE_HASH(freeslot, hash); + FT_ATOMIC_STORE_SSIZE_RELAXED(so->used, so->used + 1); + FT_ATOMIC_STORE_PTR_RELEASE(freeslot->key, key); + FT_ATOMIC_STORE_SSIZE_RELAXED(freeslot->hash, hash); return 0; found_unused: so->fill++; - STORE_USED(so, so->used + 1); - STORE_KEY(entry, key); - STORE_HASH(entry, hash); + FT_ATOMIC_STORE_SSIZE_RELAXED(so->used, so->used + 1); + FT_ATOMIC_STORE_PTR_RELEASE(entry->key, key); + FT_ATOMIC_STORE_SSIZE_RELAXED(entry->hash, hash); if ((size_t)so->fill*5 < mask*3) return 0; return set_table_resize(so, so->used>50000 ? so->used*2 : so->used*4); @@ -388,8 +352,8 @@ set_insert_clean(setentry *table, size_t mask, PyObject *key, Py_hash_t hash) i = (i * 5 + 1 + perturb) & mask; } found_null: - STORE_KEY(entry, key); - STORE_HASH(entry, hash); + FT_ATOMIC_STORE_PTR_RELEASE(entry->key, key); + FT_ATOMIC_STORE_SSIZE_RELAXED(entry->hash, hash); } /* ======== End logic for probing the hash table ========================== */ @@ -417,9 +381,9 @@ set_lookkey_threadsafe(PySetObject *so, PyObject *key, Py_hash_t hash) int status; setentry *entry; ensure_shared_on_read(so); - setentry *table = LOAD_TABLE(so); - size_t mask = LOAD_MASK(so); - if (table == NULL || table != LOAD_TABLE(so)) { + setentry *table = FT_ATOMIC_LOAD_PTR_ACQUIRE(so->table); + size_t mask = FT_ATOMIC_LOAD_SSIZE_RELAXED(so->mask); + if (table == NULL || table != FT_ATOMIC_LOAD_PTR_ACQUIRE(so->table)) { return set_lookkey(so, key, hash, &entry); } status = set_do_lookup(so, table, mask, key, hash, &entry, set_compare_threadsafe); @@ -502,8 +466,8 @@ set_table_resize(PySetObject *so, Py_ssize_t minused) /* Make the set empty, using the new table. */ assert(newtable != oldtable); set_zero_table(newtable, newsize); - STORE_TABLE(so, NULL); - STORE_MASK(so, newsize - 1); + FT_ATOMIC_STORE_PTR_RELEASE(so->table, NULL); + FT_ATOMIC_STORE_SSIZE_RELAXED(so->mask, newsize - 1); /* Copy the data over; this is refcount-neutral for active entries; dummy entries aren't copied over, of course */ @@ -523,10 +487,10 @@ set_table_resize(PySetObject *so, Py_ssize_t minused) } } - STORE_TABLE(so, newtable); + FT_ATOMIC_STORE_PTR_RELEASE(so->table, newtable); if (is_oldtable_malloced) - free_entries(oldtable, IS_SHARED(so)); + free_entries(oldtable, SET_IS_SHARED(so)); return 0; } @@ -558,9 +522,9 @@ set_discard_entry(PySetObject *so, PyObject *key, Py_hash_t hash) } assert(status == SET_LOOKKEY_FOUND); old_key = entry->key; - STORE_KEY(entry, dummy); - STORE_HASH(entry, -1); - STORE_USED(so, so->used - 1); + FT_ATOMIC_STORE_PTR_RELEASE(entry->key, dummy); + FT_ATOMIC_STORE_SSIZE_RELAXED(entry->hash, -1); + FT_ATOMIC_STORE_SSIZE_RELAXED(so->used, so->used - 1); Py_DECREF(old_key); return DISCARD_FOUND; } @@ -598,13 +562,13 @@ set_discard_key(PySetObject *so, PyObject *key) static void set_empty_to_minsize(PySetObject *so) { - STORE_TABLE(so, NULL); + FT_ATOMIC_STORE_PTR_RELEASE(so->table, NULL); set_zero_table(so->smalltable, PySet_MINSIZE); so->fill = 0; - STORE_USED(so, 0); - STORE_MASK(so, PySet_MINSIZE - 1); - STORE_HASH(so, -1); - STORE_TABLE(so, so->smalltable); + FT_ATOMIC_STORE_SSIZE_RELAXED(so->used, 0); + FT_ATOMIC_STORE_SSIZE_RELAXED(so->mask, PySet_MINSIZE - 1); + FT_ATOMIC_STORE_SSIZE_RELAXED(so->hash, -1); + FT_ATOMIC_STORE_PTR_RELEASE(so->table, so->smalltable); } static int @@ -653,7 +617,7 @@ set_clear_internal(PyObject *self) } if (table_is_malloced) - free_entries(table, IS_SHARED(so)); + free_entries(table, SET_IS_SHARED(so)); return 0; } @@ -714,7 +678,7 @@ set_dealloc(PyObject *self) } } if (so->table != so->smalltable) - free_entries(so->table, IS_SHARED(so)); + free_entries(so->table, SET_IS_SHARED(so)); Py_TYPE(so)->tp_free(so); Py_TRASHCAN_END } @@ -788,7 +752,7 @@ static Py_ssize_t set_len(PyObject *self) { PySetObject *so = _PySet_CAST(self); - return LOAD_USED(so); + return FT_ATOMIC_LOAD_SSIZE_RELAXED(so->used); } static int @@ -826,12 +790,12 @@ set_merge_lock_held(PySetObject *so, PyObject *otherset) key = other_entry->key; if (key != NULL) { assert(so_entry->key == NULL); - STORE_KEY(so_entry, Py_NewRef(key)); - STORE_HASH(so_entry, other_entry->hash); + FT_ATOMIC_STORE_PTR_RELEASE(so_entry->key, Py_NewRef(key)); + FT_ATOMIC_STORE_SSIZE_RELAXED(so_entry->hash, other_entry->hash); } } so->fill = other->fill; - STORE_USED(so, other->used); + FT_ATOMIC_STORE_SSIZE_RELAXED(so->used, other->used); return 0; } @@ -840,7 +804,7 @@ set_merge_lock_held(PySetObject *so, PyObject *otherset) setentry *newtable = so->table; size_t newmask = (size_t)so->mask; so->fill = other->used; - STORE_USED(so, other->used); + FT_ATOMIC_STORE_SSIZE_RELAXED(so->used, other->used); for (i = other->mask + 1; i > 0 ; i--, other_entry++) { key = other_entry->key; if (key != NULL && key != dummy) { @@ -892,9 +856,9 @@ set_pop_impl(PySetObject *so) entry = so->table; } key = entry->key; - STORE_KEY(entry, dummy); - STORE_HASH(entry, -1); - STORE_USED(so, so->used - 1); + FT_ATOMIC_STORE_PTR_RELEASE(entry->key, dummy); + FT_ATOMIC_STORE_SSIZE_RELAXED(entry->hash, -1); + FT_ATOMIC_STORE_SSIZE_RELAXED(so->used, so->used - 1); so->finger = entry - so->table + 1; /* next place to start */ return key; } @@ -980,12 +944,12 @@ frozenset_hash(PyObject *self) PySetObject *so = _PySet_CAST(self); Py_uhash_t hash; - if (LOAD_HASH(so) != -1) { - return LOAD_HASH(so); + if (FT_ATOMIC_LOAD_SSIZE_RELAXED(so->hash) != -1) { + return FT_ATOMIC_LOAD_SSIZE_ACQUIRE(so->hash); } hash = frozenset_hash_impl(self); - STORE_HASH(so, hash); + FT_ATOMIC_STORE_SSIZE_RELEASE(so->hash, hash); return hash; } @@ -1067,8 +1031,8 @@ static PyObject *setiter_iternext(PyObject *self) return NULL; assert (PyAnySet_Check(so)); - Py_ssize_t so_used = LOAD_USED(so); - Py_ssize_t si_used = FT_ATOMIC_LOAD_SSIZE(si->si_used); + Py_ssize_t so_used = FT_ATOMIC_LOAD_SSIZE_RELAXED(so->used); + Py_ssize_t si_used = FT_ATOMIC_LOAD_SSIZE_RELAXED(si->si_used); if (si_used != so_used) { PyErr_SetString(PyExc_RuntimeError, "Set changed size during iteration"); @@ -1410,16 +1374,16 @@ set_swap_bodies(PySetObject *a, PySetObject *b) setentry *a_table = a->table; setentry *b_table = b->table; - STORE_TABLE(a, NULL); - STORE_TABLE(b, NULL); + FT_ATOMIC_STORE_PTR_RELEASE(a->table, NULL); + FT_ATOMIC_STORE_PTR_RELEASE(b->table, NULL); t = a->fill; a->fill = b->fill; b->fill = t; t = a->used; - STORE_USED(a, b->used); - STORE_USED(b, t); + FT_ATOMIC_STORE_SSIZE_RELAXED(a->used, b->used); + FT_ATOMIC_STORE_SSIZE_RELAXED(b->used, t); t = a->mask; - STORE_MASK(a, b->mask); - STORE_MASK(b, t); + FT_ATOMIC_STORE_SSIZE_RELAXED(a->mask, b->mask); + FT_ATOMIC_STORE_SSIZE_RELAXED(b->mask, t); u = a_table; if (a_table == a->smalltable) @@ -1437,21 +1401,21 @@ set_swap_bodies(PySetObject *a, PySetObject *b) if (PyType_IsSubtype(Py_TYPE(a), &PyFrozenSet_Type) && PyType_IsSubtype(Py_TYPE(b), &PyFrozenSet_Type)) { - h = LOAD_HASH(a); - STORE_HASH(a, LOAD_HASH(b)); - STORE_HASH(b, h); + h = FT_ATOMIC_LOAD_SSIZE_RELAXED(a->hash); + FT_ATOMIC_STORE_SSIZE_RELAXED(a->hash, FT_ATOMIC_LOAD_SSIZE_RELAXED(b->hash)); + FT_ATOMIC_STORE_SSIZE_RELAXED(b->hash, h); } else { - STORE_HASH(a, -1); - STORE_HASH(b, -1); + FT_ATOMIC_STORE_SSIZE_RELAXED(a->hash, -1); + FT_ATOMIC_STORE_SSIZE_RELAXED(b->hash, -1); } - if (IS_SHARED(a)) { - SET_SHARED(b); + if (!SET_IS_SHARED(b) && SET_IS_SHARED(a)) { + SET_MARK_SHARED(b); } - if (IS_SHARED(b)) { - SET_SHARED(a); + if (!SET_IS_SHARED(a) && SET_IS_SHARED(b)) { + SET_MARK_SHARED(a); } - STORE_TABLE(a, a_table); - STORE_TABLE(b, b_table); + FT_ATOMIC_STORE_PTR_RELEASE(a->table, a_table); + FT_ATOMIC_STORE_PTR_RELEASE(b->table, b_table); } /*[clinic input] @@ -2345,8 +2309,8 @@ set_richcompare(PyObject *self, PyObject *w, int op) case Py_EQ: if (PySet_GET_SIZE(v) != PySet_GET_SIZE(w)) Py_RETURN_FALSE; - Py_hash_t v_hash = LOAD_HASH(v); - Py_hash_t w_hash = LOAD_HASH((PySetObject *)w); + Py_hash_t v_hash = FT_ATOMIC_LOAD_SSIZE_RELAXED(v->hash); + Py_hash_t w_hash = FT_ATOMIC_LOAD_SSIZE_RELAXED(((PySetObject *)w)->hash); if (v_hash != -1 && w_hash != -1 && v_hash != w_hash) Py_RETURN_FALSE; return set_issubset((PyObject*)v, w); From 7df8f02fed10e361e3a4fa7d52b6290bdcc6302e Mon Sep 17 00:00:00 2001 From: Neil Schemenauer Date: Fri, 11 Jul 2025 16:44:36 -0700 Subject: [PATCH 03/16] Increase items and loops for set test. --- Lib/test/test_free_threading/test_set.py | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/Lib/test/test_free_threading/test_set.py b/Lib/test/test_free_threading/test_set.py index 31b43fb217f8d2..b71e7dcb742ecd 100644 --- a/Lib/test/test_free_threading/test_set.py +++ b/Lib/test/test_free_threading/test_set.py @@ -42,20 +42,24 @@ def test_contains_mutate(self): s = set() done = False + NUM_ITEMS = 2_000 + NUM_LOOPS = 20 + def read_set(): barrier.wait() while not done: - for i in range(64): - result = (i % 16) in s + for i in range(NUM_ITEMS): + item = i >> 1 + result = item in s def mutate_set(): nonlocal done barrier.wait() - for i in range(10): + for i in range(NUM_LOOPS): s.clear() - for j in range(16): + for j in range(NUM_ITEMS): s.add(j) - for j in range(16): + for j in range(NUM_ITEMS): s.discard(j) # executes the set_swap_bodies() function s.__iand__(set(k for k in range(10, 20))) From 157cd60fb4c23cd1c46428ad6e5e61730b127f7b Mon Sep 17 00:00:00 2001 From: Neil Schemenauer Date: Fri, 11 Jul 2025 17:12:47 -0700 Subject: [PATCH 04/16] Re-order some atomic store operations. --- Objects/setobject.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/Objects/setobject.c b/Objects/setobject.c index 42198956d3cf3a..b9b2c02bd54cce 100644 --- a/Objects/setobject.c +++ b/Objects/setobject.c @@ -278,15 +278,15 @@ set_add_entry_takeref(PySetObject *so, PyObject *key, Py_hash_t hash) if (freeslot == NULL) goto found_unused; FT_ATOMIC_STORE_SSIZE_RELAXED(so->used, so->used + 1); - FT_ATOMIC_STORE_PTR_RELEASE(freeslot->key, key); FT_ATOMIC_STORE_SSIZE_RELAXED(freeslot->hash, hash); + FT_ATOMIC_STORE_PTR_RELEASE(freeslot->key, key); return 0; found_unused: so->fill++; FT_ATOMIC_STORE_SSIZE_RELAXED(so->used, so->used + 1); - FT_ATOMIC_STORE_PTR_RELEASE(entry->key, key); FT_ATOMIC_STORE_SSIZE_RELAXED(entry->hash, hash); + FT_ATOMIC_STORE_PTR_RELEASE(entry->key, key); if ((size_t)so->fill*5 < mask*3) return 0; return set_table_resize(so, so->used>50000 ? so->used*2 : so->used*4); @@ -352,8 +352,8 @@ set_insert_clean(setentry *table, size_t mask, PyObject *key, Py_hash_t hash) i = (i * 5 + 1 + perturb) & mask; } found_null: - FT_ATOMIC_STORE_PTR_RELEASE(entry->key, key); FT_ATOMIC_STORE_SSIZE_RELAXED(entry->hash, hash); + FT_ATOMIC_STORE_PTR_RELEASE(entry->key, key); } /* ======== End logic for probing the hash table ========================== */ @@ -522,9 +522,9 @@ set_discard_entry(PySetObject *so, PyObject *key, Py_hash_t hash) } assert(status == SET_LOOKKEY_FOUND); old_key = entry->key; - FT_ATOMIC_STORE_PTR_RELEASE(entry->key, dummy); FT_ATOMIC_STORE_SSIZE_RELAXED(entry->hash, -1); FT_ATOMIC_STORE_SSIZE_RELAXED(so->used, so->used - 1); + FT_ATOMIC_STORE_PTR_RELEASE(entry->key, dummy); Py_DECREF(old_key); return DISCARD_FOUND; } @@ -790,8 +790,8 @@ set_merge_lock_held(PySetObject *so, PyObject *otherset) key = other_entry->key; if (key != NULL) { assert(so_entry->key == NULL); - FT_ATOMIC_STORE_PTR_RELEASE(so_entry->key, Py_NewRef(key)); FT_ATOMIC_STORE_SSIZE_RELAXED(so_entry->hash, other_entry->hash); + FT_ATOMIC_STORE_PTR_RELEASE(so_entry->key, Py_NewRef(key)); } } so->fill = other->fill; @@ -855,10 +855,10 @@ set_pop_impl(PySetObject *so) if (entry > limit) entry = so->table; } - key = entry->key; - FT_ATOMIC_STORE_PTR_RELEASE(entry->key, dummy); FT_ATOMIC_STORE_SSIZE_RELAXED(entry->hash, -1); FT_ATOMIC_STORE_SSIZE_RELAXED(so->used, so->used - 1); + key = entry->key; + FT_ATOMIC_STORE_PTR_RELEASE(entry->key, dummy); so->finger = entry - so->table + 1; /* next place to start */ return key; } From 4c3596c713d48826ba757db66a535ef084b33752 Mon Sep 17 00:00:00 2001 From: Neil Schemenauer Date: Fri, 11 Jul 2025 18:13:11 -0700 Subject: [PATCH 05/16] Add and use set_compare_frozenset(). --- Objects/setobject.c | 65 ++++++++++++++++++++++++++++++++++++++------- 1 file changed, 56 insertions(+), 9 deletions(-) diff --git a/Objects/setobject.c b/Objects/setobject.c index b9b2c02bd54cce..23c1a9af7d3657 100644 --- a/Objects/setobject.c +++ b/Objects/setobject.c @@ -68,6 +68,9 @@ static PyObject _dummy_struct; #define SET_LOOKKEY_CHANGED (-2) #define SET_LOOKKEY_EMPTY (-3) +typedef int (*compare_func)(PySetObject *so, setentry *table, setentry *ep, + PyObject *key, Py_hash_t hash); + #ifdef Py_GIL_DISABLED #define SET_IS_SHARED(so) _PyObject_GC_IS_SHARED(so) @@ -159,6 +162,36 @@ set_compare_entry(PySetObject *so, setentry *table, setentry *entry, return SET_LOOKKEY_NO_MATCH; } +// This is similar to set_compare_entry() but we don't need to incref startkey +// before comparing and we don't need to check if the set has changed. +static inline Py_ALWAYS_INLINE int +set_compare_frozenset(PySetObject *so, setentry *table, setentry *ep, + PyObject *key, Py_hash_t hash) +{ + assert(PyFrozenSet_CheckExact(so)); + PyObject *startkey = ep->key; + if (startkey == NULL) { + return SET_LOOKKEY_EMPTY; + } + if (startkey == key) { + return SET_LOOKKEY_FOUND; + } + Py_ssize_t ep_hash = ep->hash; + if (ep_hash == hash) { + if (PyUnicode_CheckExact(startkey) + && PyUnicode_CheckExact(key) + && unicode_eq(startkey, key)) + return SET_LOOKKEY_FOUND; + int cmp = PyObject_RichCompareBool(startkey, key, Py_EQ); + if (cmp < 0) { + return SET_LOOKKEY_ERROR; + } + assert(cmp == SET_LOOKKEY_FOUND || cmp == SET_LOOKKEY_NO_MATCH); + return cmp; + } + return SET_LOOKKEY_NO_MATCH; +} + static void set_zero_table(setentry *table, size_t size) { @@ -186,9 +219,7 @@ set_zero_table(setentry *table, size_t size) static int set_do_lookup(PySetObject *so, setentry *table, size_t mask, PyObject *key, - Py_hash_t hash, setentry **epp, - int (*compare_entry)(PySetObject *so, setentry *table, setentry *ep, - PyObject *key, Py_hash_t hash)) + Py_hash_t hash, setentry **epp, compare_func compare_entry) { setentry *entry; size_t perturb = hash; @@ -363,11 +394,18 @@ static int set_lookkey(PySetObject *so, PyObject *key, Py_hash_t hash, setentry **epp) { int status; - Py_BEGIN_CRITICAL_SECTION(so); - do { - status = set_do_lookup(so, so->table, so->mask, key, hash, epp, set_compare_entry); - } while (status == SET_LOOKKEY_CHANGED); - Py_END_CRITICAL_SECTION(); + if (PyFrozenSet_CheckExact(so)) { + status = set_do_lookup(so, so->table, so->mask, key, hash, epp, + set_compare_frozenset); + } + else { + Py_BEGIN_CRITICAL_SECTION(so); + do { + status = set_do_lookup(so, so->table, so->mask, key, hash, epp, + set_compare_entry); + } while (status == SET_LOOKKEY_CHANGED); + Py_END_CRITICAL_SECTION(); + } assert(status == SET_LOOKKEY_FOUND || status == SET_LOOKKEY_NO_MATCH || status == SET_LOOKKEY_ERROR); @@ -380,13 +418,22 @@ set_lookkey_threadsafe(PySetObject *so, PyObject *key, Py_hash_t hash) { int status; setentry *entry; + if (PyFrozenSet_CheckExact(so)) { + status = set_do_lookup(so, so->table, so->mask, key, hash, &entry, + set_compare_frozenset); + assert(status == SET_LOOKKEY_FOUND || + status == SET_LOOKKEY_NO_MATCH || + status == SET_LOOKKEY_ERROR); + return status; + } ensure_shared_on_read(so); setentry *table = FT_ATOMIC_LOAD_PTR_ACQUIRE(so->table); size_t mask = FT_ATOMIC_LOAD_SSIZE_RELAXED(so->mask); if (table == NULL || table != FT_ATOMIC_LOAD_PTR_ACQUIRE(so->table)) { return set_lookkey(so, key, hash, &entry); } - status = set_do_lookup(so, table, mask, key, hash, &entry, set_compare_threadsafe); + status = set_do_lookup(so, table, mask, key, hash, &entry, + set_compare_threadsafe); if (status == SET_LOOKKEY_CHANGED) { return set_lookkey(so, key, hash, &entry); } From 8ff7dbd91131848539e934e5423a1c039ca1c696 Mon Sep 17 00:00:00 2001 From: Neil Schemenauer Date: Fri, 11 Jul 2025 19:29:51 -0700 Subject: [PATCH 06/16] Fix _PyMem_FreeDelayed() calls, need size. --- Objects/setobject.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/Objects/setobject.c b/Objects/setobject.c index e5823480594a1f..e11b564a8f2d34 100644 --- a/Objects/setobject.c +++ b/Objects/setobject.c @@ -462,11 +462,11 @@ set_lookkey_threadsafe(PySetObject *so, PyObject *key, Py_hash_t hash) } #endif -static void free_entries(setentry *entries, bool use_qsbr) +static void free_entries(setentry *entries, size_t size, bool use_qsbr) { #ifdef Py_GIL_DISABLED if (use_qsbr) { - _PyMem_FreeDelayed(entries); + _PyMem_FreeDelayed(entries, size * sizeof(setentry)); return; } #endif @@ -483,6 +483,7 @@ set_table_resize(PySetObject *so, Py_ssize_t minused) { setentry *oldtable, *newtable, *entry; Py_ssize_t oldmask = so->mask; + Py_ssize_t oldsize = (size_t)oldmask + 1; size_t newmask; int is_oldtable_malloced; setentry small_copy[PySet_MINSIZE]; @@ -555,7 +556,7 @@ set_table_resize(PySetObject *so, Py_ssize_t minused) FT_ATOMIC_STORE_PTR_RELEASE(so->table, newtable); if (is_oldtable_malloced) - free_entries(oldtable, SET_IS_SHARED(so)); + free_entries(oldtable, oldsize, SET_IS_SHARED(so)); return 0; } @@ -647,6 +648,7 @@ set_clear_internal(PyObject *self) setentry *table = so->table; Py_ssize_t fill = so->fill; Py_ssize_t used = so->used; + Py_ssize_t oldsize = (size_t)so->mask + 1; int table_is_malloced = table != so->smalltable; setentry small_copy[PySet_MINSIZE]; @@ -685,7 +687,7 @@ set_clear_internal(PyObject *self) } if (table_is_malloced) - free_entries(table, SET_IS_SHARED(so)); + free_entries(table, oldsize, SET_IS_SHARED(so)); return 0; } @@ -732,6 +734,7 @@ set_dealloc(PyObject *self) PySetObject *so = _PySet_CAST(self); setentry *entry; Py_ssize_t used = so->used; + Py_ssize_t oldsize = (size_t)so->mask + 1; /* bpo-31095: UnTrack is needed before calling any callbacks */ PyObject_GC_UnTrack(so); @@ -744,7 +747,7 @@ set_dealloc(PyObject *self) } } if (so->table != so->smalltable) - free_entries(so->table, SET_IS_SHARED(so)); + free_entries(so->table, oldsize, SET_IS_SHARED(so)); Py_TYPE(so)->tp_free(so); } From 87278ef13e730d5b29ea7a6b7631862d82c0fc4f Mon Sep 17 00:00:00 2001 From: Neil Schemenauer Date: Fri, 11 Jul 2025 19:30:46 -0700 Subject: [PATCH 07/16] Fix frozenset contains method. --- Objects/setobject.c | 24 ++++++++++++++++++------ 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/Objects/setobject.c b/Objects/setobject.c index e11b564a8f2d34..8b27598943e4ca 100644 --- a/Objects/setobject.c +++ b/Objects/setobject.c @@ -169,7 +169,7 @@ static inline Py_ALWAYS_INLINE int set_compare_frozenset(PySetObject *so, setentry *table, setentry *ep, PyObject *key, Py_hash_t hash) { - assert(PyFrozenSet_CheckExact(so)); + assert(PyFrozenSet_Check(so)); PyObject *startkey = ep->key; if (startkey == NULL) { return SET_LOOKKEY_EMPTY; @@ -2434,6 +2434,7 @@ _PySet_Contains(PySetObject *so, PyObject *key) Py_hash_t hash = _PyObject_HashFast(key); if (hash == -1) { if (!PySet_Check(key) || !PyErr_ExceptionMatches(PyExc_TypeError)) { + set_unhashable_type(key); return -1; } PyErr_Clear(); @@ -2488,12 +2489,23 @@ static PyObject * frozenset___contains___impl(PySetObject *so, PyObject *key) /*[clinic end generated code: output=2301ed91bc3a6dd5 input=2f04922a98d8bab7]*/ { - long result; - - result = set_contains_lock_held(so, key); - if (result < 0) + Py_hash_t hash = _PyObject_HashFast(key); + if (hash == -1) { + if (!PySet_Check(key) || !PyErr_ExceptionMatches(PyExc_TypeError)) { + set_unhashable_type(key); + return NULL; + } + PyErr_Clear(); + Py_BEGIN_CRITICAL_SECTION(key); + hash = frozenset_hash_impl(key); + Py_END_CRITICAL_SECTION(); + } + setentry *entry; // unused + int status = set_do_lookup(so, so->table, so->mask, key, hash, &entry, + set_compare_frozenset); + if (status < 0) return NULL; - return PyBool_FromLong(result); + return PyBool_FromLong(status); } /*[clinic input] From b2affbf50e331c0e7d589353f46b4b1ab21dcba5 Mon Sep 17 00:00:00 2001 From: Neil Schemenauer Date: Fri, 11 Jul 2025 20:03:54 -0700 Subject: [PATCH 08/16] Add NEWS. --- .../2025-07-11-19-57-27.gh-issue-132657.vwDuO2.rst | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 Misc/NEWS.d/next/Core_and_Builtins/2025-07-11-19-57-27.gh-issue-132657.vwDuO2.rst diff --git a/Misc/NEWS.d/next/Core_and_Builtins/2025-07-11-19-57-27.gh-issue-132657.vwDuO2.rst b/Misc/NEWS.d/next/Core_and_Builtins/2025-07-11-19-57-27.gh-issue-132657.vwDuO2.rst new file mode 100644 index 00000000000000..87eafbce8838f2 --- /dev/null +++ b/Misc/NEWS.d/next/Core_and_Builtins/2025-07-11-19-57-27.gh-issue-132657.vwDuO2.rst @@ -0,0 +1,2 @@ +For the free-threaded build, avoid locking the set object for the +``__contains__`` method. From 70a1c1f2388dd415c5f427eb3a8134b5f4e178a5 Mon Sep 17 00:00:00 2001 From: Neil Schemenauer Date: Fri, 11 Jul 2025 20:11:01 -0700 Subject: [PATCH 09/16] Re-generate clinic output. --- Objects/clinic/setobject.c.h | 4 +--- Objects/setobject.c | 2 +- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/Objects/clinic/setobject.c.h b/Objects/clinic/setobject.c.h index 488be4435f81d7..098c4bcb53d3fb 100644 --- a/Objects/clinic/setobject.c.h +++ b/Objects/clinic/setobject.c.h @@ -425,9 +425,7 @@ set___contains__(PyObject *so, PyObject *key) { PyObject *return_value = NULL; - Py_BEGIN_CRITICAL_SECTION(so); return_value = set___contains___impl((PySetObject *)so, key); - Py_END_CRITICAL_SECTION(); return return_value; } @@ -554,4 +552,4 @@ set___sizeof__(PyObject *so, PyObject *Py_UNUSED(ignored)) return return_value; } -/*[clinic end generated code: output=7f7fe845ca165078 input=a9049054013a1b77]*/ +/*[clinic end generated code: output=5800c0bf136a5a0a input=a9049054013a1b77]*/ diff --git a/Objects/setobject.c b/Objects/setobject.c index 8b27598943e4ca..85b510e869697b 100644 --- a/Objects/setobject.c +++ b/Objects/setobject.c @@ -2465,7 +2465,7 @@ x.__contains__(y) <==> y in x. static PyObject * set___contains___impl(PySetObject *so, PyObject *key) -/*[clinic end generated code: output=b44863d034b3c70e input=4a7d568459617f24]*/ +/*[clinic end generated code: output=b44863d034b3c70e input=cf4c72db704e4cf0]*/ { long result; From 64b17af7c5f0d7895d3369b4907426b3218613e2 Mon Sep 17 00:00:00 2001 From: Neil Schemenauer Date: Mon, 14 Jul 2025 10:48:20 -0700 Subject: [PATCH 10/16] Better markup in NEWS. --- .../2025-07-11-19-57-27.gh-issue-132657.vwDuO2.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Misc/NEWS.d/next/Core_and_Builtins/2025-07-11-19-57-27.gh-issue-132657.vwDuO2.rst b/Misc/NEWS.d/next/Core_and_Builtins/2025-07-11-19-57-27.gh-issue-132657.vwDuO2.rst index 87eafbce8838f2..27099329327158 100644 --- a/Misc/NEWS.d/next/Core_and_Builtins/2025-07-11-19-57-27.gh-issue-132657.vwDuO2.rst +++ b/Misc/NEWS.d/next/Core_and_Builtins/2025-07-11-19-57-27.gh-issue-132657.vwDuO2.rst @@ -1,2 +1,2 @@ -For the free-threaded build, avoid locking the set object for the +For the free-threaded build, avoid locking the :class:`set` object for the ``__contains__`` method. From 6c339f489d8a15e085dc4714747437de95106d1e Mon Sep 17 00:00:00 2001 From: Neil Schemenauer Date: Mon, 14 Jul 2025 10:58:45 -0700 Subject: [PATCH 11/16] Remove unicode case for set_compare_frozenset. --- Objects/setobject.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/Objects/setobject.c b/Objects/setobject.c index 85b510e869697b..7328005acecf24 100644 --- a/Objects/setobject.c +++ b/Objects/setobject.c @@ -164,7 +164,9 @@ set_compare_entry(PySetObject *so, setentry *table, setentry *entry, } // This is similar to set_compare_entry() but we don't need to incref startkey -// before comparing and we don't need to check if the set has changed. +// before comparing and we don't need to check if the set has changed. This +// also omits the PyUnicode_CheckExact() special case since it doesn't help +// much for frozensets. static inline Py_ALWAYS_INLINE int set_compare_frozenset(PySetObject *so, setentry *table, setentry *ep, PyObject *key, Py_hash_t hash) @@ -179,10 +181,6 @@ set_compare_frozenset(PySetObject *so, setentry *table, setentry *ep, } Py_ssize_t ep_hash = ep->hash; if (ep_hash == hash) { - if (PyUnicode_CheckExact(startkey) - && PyUnicode_CheckExact(key) - && unicode_eq(startkey, key)) - return SET_LOOKKEY_FOUND; int cmp = PyObject_RichCompareBool(startkey, key, Py_EQ); if (cmp < 0) { return SET_LOOKKEY_ERROR; From 89f4696083339e70fa1f7cfdc8eb33b46e1d503a Mon Sep 17 00:00:00 2001 From: Neil Schemenauer Date: Mon, 14 Jul 2025 11:37:12 -0700 Subject: [PATCH 12/16] Add multi-threaded frozenset contains test. --- Lib/test/test_free_threading/test_set.py | 29 ++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/Lib/test/test_free_threading/test_set.py b/Lib/test/test_free_threading/test_set.py index b71e7dcb742ecd..6924f9f525dfad 100644 --- a/Lib/test/test_free_threading/test_set.py +++ b/Lib/test/test_free_threading/test_set.py @@ -71,6 +71,35 @@ def mutate_set(): for t in threads: t.join() + def test_contains_frozenset(self): + barrier = Barrier(3) + done = False + + NUM_ITEMS = 2_000 + NUM_LOOPS = 20 + + s = frozenset() + def make_set(): + nonlocal s + barrier.wait() + while not done: + s = frozenset(range(NUM_ITEMS)) + + def read_set(): + nonlocal done + barrier.wait() + for _ in range(NUM_LOOPS): + for i in range(NUM_ITEMS): + item = i >> 1 + result = item in s + done = True + + threads = [Thread(target=read_set), Thread(target=read_set), Thread(target=make_set)] + for t in threads: + t.start() + for t in threads: + t.join() + if __name__ == "__main__": unittest.main() From 4ffd3cdb8db2616a3de2dbf5bf9c1d4bb30ac838 Mon Sep 17 00:00:00 2001 From: Neil Schemenauer Date: Tue, 30 Sep 2025 15:40:29 -0700 Subject: [PATCH 13/16] Remove unused line of code, from merge. --- Objects/setobject.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/Objects/setobject.c b/Objects/setobject.c index 22793f6e68478b..7f10a0239e2c3c 100644 --- a/Objects/setobject.c +++ b/Objects/setobject.c @@ -226,8 +226,6 @@ set_do_lookup(PySetObject *so, setentry *table, size_t mask, PyObject *key, int probes; int status; - int frozenset = PyFrozenSet_CheckExact(so); - while (1) { entry = &table[i]; probes = (i + LINEAR_PROBES <= mask) ? LINEAR_PROBES: 0; From 2d493038b51d80606eda2f59999b3246925f7306 Mon Sep 17 00:00:00 2001 From: Neil Schemenauer Date: Mon, 3 Nov 2025 14:17:51 -0800 Subject: [PATCH 14/16] Minor code cleanup based on review. * Add comment to _PySet_Contains() * Rename set_compare_entry() to set_compare_entry_lock_held(). * Remove unneeded `startkey == NULL` test. --- Objects/setobject.c | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/Objects/setobject.c b/Objects/setobject.c index 7f10a0239e2c3c..4c26a41fb9f0f4 100644 --- a/Objects/setobject.c +++ b/Objects/setobject.c @@ -106,7 +106,7 @@ set_compare_threadsafe(PySetObject *so, setentry *table, setentry *ep, } Py_ssize_t ep_hash = FT_ATOMIC_LOAD_SSIZE_ACQUIRE(ep->hash); if (ep_hash == hash) { - if (startkey == NULL || !_Py_TryIncrefCompare(&ep->key, startkey)) { + if (!_Py_TryIncrefCompare(&ep->key, startkey)) { return SET_LOOKKEY_CHANGED; } int cmp = PyObject_RichCompareBool(startkey, key, Py_EQ); @@ -135,9 +135,10 @@ set_compare_threadsafe(PySetObject *so, setentry *table, setentry *ep, #endif static inline Py_ALWAYS_INLINE int -set_compare_entry(PySetObject *so, setentry *table, setentry *entry, - PyObject *key, Py_hash_t hash) +set_compare_entry_lock_held(PySetObject *so, setentry *table, setentry *entry, + PyObject *key, Py_hash_t hash) { + _Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(so); if (entry->hash == 0 && entry->key == NULL) return SET_LOOKKEY_EMPTY; if (entry->hash == hash) { @@ -163,10 +164,10 @@ set_compare_entry(PySetObject *so, setentry *table, setentry *entry, return SET_LOOKKEY_NO_MATCH; } -// This is similar to set_compare_entry() but we don't need to incref startkey -// before comparing and we don't need to check if the set has changed. This -// also omits the PyUnicode_CheckExact() special case since it doesn't help -// much for frozensets. +// This is similar to set_compare_entry_lock_held() but we don't need to +// incref startkey before comparing and we don't need to check if the set has +// changed. This also omits the PyUnicode_CheckExact() special case since it +// doesn't help much for frozensets. static inline Py_ALWAYS_INLINE int set_compare_frozenset(PySetObject *so, setentry *table, setentry *ep, PyObject *key, Py_hash_t hash) @@ -418,7 +419,7 @@ set_lookkey(PySetObject *so, PyObject *key, Py_hash_t hash, setentry **epp) Py_BEGIN_CRITICAL_SECTION(so); do { status = set_do_lookup(so, so->table, so->mask, key, hash, epp, - set_compare_entry); + set_compare_entry_lock_held); } while (status == SET_LOOKKEY_CHANGED); Py_END_CRITICAL_SECTION(); } @@ -2439,10 +2440,12 @@ _PySet_Contains(PySetObject *so, PyObject *key) return -1; } PyErr_Clear(); + // Note that 'key' could be a set() or frozenset() object. Unlike most + // container types, set allows membership testing with a set key, even + // though it is not hashable. Py_BEGIN_CRITICAL_SECTION(key); hash = frozenset_hash_impl(key); Py_END_CRITICAL_SECTION(); - return set_contains_entry(so, key, hash); } return set_contains_entry(so, key, hash); } From a0956612fa732a7594e6dd4e42088c7ad6350788 Mon Sep 17 00:00:00 2001 From: Neil Schemenauer Date: Mon, 3 Nov 2025 17:42:27 -0800 Subject: [PATCH 15/16] Improve unit tests. The test_contains_frozenset() was not actually doing anything useful, rewrite it. Add test_contains_hash_mutate() which triggers unusual code paths due to `__hash__` on the key mutating the container. --- Lib/test/test_free_threading/test_set.py | 83 +++++++++++++++++++----- 1 file changed, 68 insertions(+), 15 deletions(-) diff --git a/Lib/test/test_free_threading/test_set.py b/Lib/test/test_free_threading/test_set.py index 6924f9f525dfad..6e224c45bc3e90 100644 --- a/Lib/test/test_free_threading/test_set.py +++ b/Lib/test/test_free_threading/test_set.py @@ -12,7 +12,7 @@ def test_repr_clear(self): """Test repr() of a set while another thread is calling clear()""" NUM_ITERS = 10 NUM_REPR_THREADS = 10 - barrier = Barrier(NUM_REPR_THREADS + 1) + barrier = Barrier(NUM_REPR_THREADS + 1, timeout=2) s = {1, 2, 3, 4, 5, 6, 7, 8} def clear_set(): @@ -38,12 +38,12 @@ def repr_set(): def test_contains_mutate(self): """Test set contains operation combined with mutation.""" - barrier = Barrier(2) + barrier = Barrier(2, timeout=2) s = set() done = False - NUM_ITEMS = 2_000 - NUM_LOOPS = 20 + NUM_ITEMS = 200 + NUM_LOOPS = 200 def read_set(): barrier.wait() @@ -72,29 +72,82 @@ def mutate_set(): t.join() def test_contains_frozenset(self): - barrier = Barrier(3) + barrier = Barrier(3, timeout=2) done = False - NUM_ITEMS = 2_000 - NUM_LOOPS = 20 + NUM_LOOPS = 1_000 - s = frozenset() - def make_set(): - nonlocal s + # This mutates the key used for contains test, not the container + # itself. This works because frozenset allows the key to be a set(). + s = set() + + def mutate_set(): barrier.wait() while not done: - s = frozenset(range(NUM_ITEMS)) + s.add(0) + s.add(1) + s.clear() def read_set(): nonlocal done barrier.wait() + container = frozenset([frozenset([0])]) + self.assertTrue(set([0]) in container) for _ in range(NUM_LOOPS): - for i in range(NUM_ITEMS): - item = i >> 1 - result = item in s + # Will return True when {0} is the key and False otherwise + result = s in container done = True - threads = [Thread(target=read_set), Thread(target=read_set), Thread(target=make_set)] + threads = [ + Thread(target=read_set), + Thread(target=read_set), + Thread(target=mutate_set), + ] + for t in threads: + t.start() + for t in threads: + t.join() + + def test_contains_hash_mutate(self): + """Test set contains operation with mutating hash method.""" + barrier = Barrier(2, timeout=2) + + NUM_ITEMS = 20 # should be larger than PySet_MINSIZE + NUM_LOOPS = 1_000 + + s = set(range(NUM_ITEMS)) + + class Key: + def __init__(self): + self.count = 0 + self.value = 0 + + def __hash__(self): + self.count += 1 + # This intends to trigger the SET_LOOKKEY_CHANGED case + # of set_lookkey_threadsafe() since calling clear() + # will cause the 'table' pointer to change. + if self.count % 2 == 0: + s.clear() + else: + s.update(range(NUM_ITEMS)) + return hash(self.value) + + def __eq__(self, other): + return self.value == other + + key = Key() + self.assertTrue(key in s) + self.assertFalse(key in s) + self.assertTrue(key in s) + self.assertFalse(key in s) + + def read_set(): + barrier.wait() + for i in range(NUM_LOOPS): + result = key in s + + threads = [Thread(target=read_set), Thread(target=read_set)] for t in threads: t.start() for t in threads: From 33838a848c7a4286359487be8262a1b6de2a51a2 Mon Sep 17 00:00:00 2001 From: Neil Schemenauer Date: Thu, 6 Nov 2025 14:46:38 -0800 Subject: [PATCH 16/16] wip: improve test --- Lib/test/test_free_threading/test_set.py | 33 ++++++++++++++---------- 1 file changed, 20 insertions(+), 13 deletions(-) diff --git a/Lib/test/test_free_threading/test_set.py b/Lib/test/test_free_threading/test_set.py index 6e224c45bc3e90..251020319b20f3 100644 --- a/Lib/test/test_free_threading/test_set.py +++ b/Lib/test/test_free_threading/test_set.py @@ -1,13 +1,9 @@ import unittest - from threading import Thread, Barrier -from unittest import TestCase - from test.support import threading_helper -@threading_helper.requires_working_threading() -class TestSet(TestCase): +class TestSetRepr(unittest.TestCase): def test_repr_clear(self): """Test repr() of a set while another thread is calling clear()""" NUM_ITERS = 10 @@ -36,19 +32,20 @@ def repr_set(): for set_repr in set_reprs: self.assertIn(set_repr, ("set()", "{1, 2, 3, 4, 5, 6, 7, 8}")) + +class RaceTestBase: def test_contains_mutate(self): """Test set contains operation combined with mutation.""" barrier = Barrier(2, timeout=2) s = set() done = False - NUM_ITEMS = 200 - NUM_LOOPS = 200 + NUM_LOOPS = 1000 def read_set(): barrier.wait() while not done: - for i in range(NUM_ITEMS): + for i in range(self.SET_SIZE): item = i >> 1 result = item in s @@ -57,9 +54,9 @@ def mutate_set(): barrier.wait() for i in range(NUM_LOOPS): s.clear() - for j in range(NUM_ITEMS): + for j in range(self.SET_SIZE): s.add(j) - for j in range(NUM_ITEMS): + for j in range(self.SET_SIZE): s.discard(j) # executes the set_swap_bodies() function s.__iand__(set(k for k in range(10, 20))) @@ -112,10 +109,10 @@ def test_contains_hash_mutate(self): """Test set contains operation with mutating hash method.""" barrier = Barrier(2, timeout=2) - NUM_ITEMS = 20 # should be larger than PySet_MINSIZE NUM_LOOPS = 1_000 + SET_SIZE = self.SET_SIZE - s = set(range(NUM_ITEMS)) + s = set(range(SET_SIZE)) class Key: def __init__(self): @@ -130,7 +127,7 @@ def __hash__(self): if self.count % 2 == 0: s.clear() else: - s.update(range(NUM_ITEMS)) + s.update(range(SET_SIZE)) return hash(self.value) def __eq__(self, other): @@ -154,5 +151,15 @@ def read_set(): t.join() +@threading_helper.requires_working_threading() +class SmallSetTest(RaceTestBase, unittest.TestCase): + SET_SIZE = 6 # smaller than PySet_MINSIZE + + +@threading_helper.requires_working_threading() +class LargeSetTest(RaceTestBase, unittest.TestCase): + SET_SIZE = 20 # larger than PySet_MINSIZE + + if __name__ == "__main__": unittest.main()