Skip to content

Lock free _STORE_ATTR_SLOT#62

Open
eendebakpt wants to merge 1 commit into
mainfrom
ft_store_attr_slot_lockfree
Open

Lock free _STORE_ATTR_SLOT#62
eendebakpt wants to merge 1 commit into
mainfrom
ft_store_attr_slot_lockfree

Conversation

@eendebakpt
Copy link
Copy Markdown
Owner

Both the bytecode handler (_STORE_ATTR_SLOT) and the C-API path (PyMember_SetOne for Py_T_OBJECT_EX / _Py_T_OBJECT) currently take per-object locks (ob_mutex via LOCK_OBJECT or Py_BEGIN_CRITICAL_SECTION) to sequence the read-of-old-value, store-of-new-value, and decref-of-old.

Replace the lock with a single atomic exchange on the slot pointer, similar with how the read side already works: _LOAD_ATTR_SLOT and PyMember_GetOne are lock-free for the fast path, using FT_ATOMIC_LOAD_PTR plus _Py_TryIncrefCompare to cope with mid-flight pointer changes.

Three changes are required:

  1. _STORE_ATTR_SLOT in Python/bytecodes.c: drop LOCK_OBJECT / UNLOCK_OBJECT and the DEOPT_IF on lock-acquire failure; use _Py_atomic_exchange_ptr on Py_GIL_DISABLED, plain read/write on the GIL build.

  2. PyMember_SetOne in Python/structmember.c, Py_T_OBJECT_EX and _Py_T_OBJECT cases: drop Py_BEGIN_CRITICAL_SECTION; use _Py_atomic_exchange_ptr to match the bytecode path. The two write paths must change together: if only the bytecode were lock-free, PyMember_SetOne's plain "oldv = *addr" inside the CS would race with concurrent atomic-exchange writers, returning a "stolen" old value that another writer is also about to DECREF (double-free).

  3. PyMember_GetOne in Python/structmember.c, Py_T_OBJECT_EX and _Py_T_OBJECT cases: replace the CS+Py_XINCREF fallback (taken on _Py_TryIncrefCompare failure) with _Py_XGetRef, which retries the atomic-load + try-incref loop. The previous fallback relied on writers also taking ob_mutex to keep the slot stable inside the CS; with lock-free writers, the CS no longer excludes them, and a plain Py_XINCREF on a re-loaded slot value could resurrect a refcount-0 (about-to-be-freed) object.

Benchmarks:

Workload Unit GIL (ref) FT baseline FT patched This patch (patched − baseline)
STORE_ATTR_SLOT (alone) ns/op 5.41 15.40 9.30 −6.10 (−39.6%)
attr_idiv (3× /= + 3× reset per iter) ns/op 22.56 41.22 30.20 −11.02 (−26.7%)*
Point.normalize (per Point) ns/Point 358.4 417.4 398.0 −19.4 (−4.6%)
pyperformance bm_float (best) ms 46.6 53.2 48.5 −4.7 (−8.8%)
pyperformance bm_float (median) ms 47.7 56.4 51.7 −4.7 (−8.3%)
pyperformance bm_nbody (best, control) ms 57.3 78.7 78.0 −0.7 (−0.9%)

Interpretation

  • STORE_ATTR_SLOT itself: closes 61% of the FT-vs-GIL per-op gap (2.85× slower → 1.72× slower).
  • attr_idiv (the self.x /= norm pattern in Point.normalize): closes 59% of the gap.
  • bm_float: closes 71% of the FT-vs-GIL benchmark gap (was +14% slower, now +4%).
  • bm_nbody (control): unchanged. Confirms the improvement is from STORE_ATTR specifically, not a global side-effect. bm_nbody's STORE-heavy work is on STORE_SUBSCR_LIST_INT, which still takes a per-object lock.

Both the bytecode handler (_STORE_ATTR_SLOT) and the C-API path
(PyMember_SetOne for Py_T_OBJECT_EX / _Py_T_OBJECT) currently take
per-object locks (ob_mutex via LOCK_OBJECT or Py_BEGIN_CRITICAL_SECTION)
to sequence the read-of-old-value, store-of-new-value, and decref-of-old.

Replace the lock with a single atomic exchange on the slot pointer.
Atomic exchange returns the previous value and stores the new one in one
atomic op, which is both sufficient for correctness on the write side
(each concurrent writer observes a unique old value, so no double-decref)
and consistent with how the read side already works: _LOAD_ATTR_SLOT and
PyMember_GetOne are lock-free for the fast path, using FT_ATOMIC_LOAD_PTR
plus _Py_TryIncrefCompare to cope with mid-flight pointer changes.

This is the WRITE-side counterpart to the existing lock-free LOAD_ATTR_SLOT
read path, and follows the pattern Dino Viehland established with
pythongh-130373 (Avoid locking in _LOAD_ATTR_WITH_HINT, merged Feb 2025) and
companions pythongh-130312 / pythongh-130313 (don't lock when populating sets in
bytecode / when clearing attribute-less objects). The primitive being
used here, _Py_atomic_exchange_ptr, is being formally proposed in
pythongh-150044 for an internal _Py_SETREF_ATOMIC macro.

Three coordinated changes are required:

  1. _STORE_ATTR_SLOT in Python/bytecodes.c: drop LOCK_OBJECT /
     UNLOCK_OBJECT and the DEOPT_IF on lock-acquire failure; use
     _Py_atomic_exchange_ptr on Py_GIL_DISABLED, plain read/write on
     the GIL build.

  2. PyMember_SetOne in Python/structmember.c, Py_T_OBJECT_EX and
     _Py_T_OBJECT cases: drop Py_BEGIN_CRITICAL_SECTION; use
     _Py_atomic_exchange_ptr to match the bytecode path. The two write
     paths must change together: if only the bytecode were lock-free,
     PyMember_SetOne's plain "oldv = *addr" inside the CS would race
     with concurrent atomic-exchange writers, returning a "stolen" old
     value that another writer is also about to DECREF (double-free).

  3. PyMember_GetOne in Python/structmember.c, Py_T_OBJECT_EX and
     _Py_T_OBJECT cases: replace the CS+Py_XINCREF fallback (taken on
     _Py_TryIncrefCompare failure) with _Py_XGetRef, which retries the
     atomic-load + try-incref loop. The previous fallback relied on
     writers also taking ob_mutex to keep the slot stable inside the
     CS; with lock-free writers, the CS no longer excludes them, and a
     plain Py_XINCREF on a re-loaded slot value could resurrect a
     refcount-0 (about-to-be-freed) object.

History (git blame on the LOCK_OBJECT line):
  * 22b0de2 (2024-06): original FT structure of _STORE_ATTR_SLOT.
  * 1b15c89 (2024-12, pythongh-127838 / pythongh-115999): added LOCK_OBJECT in
    the broader STORE_ATTR specialization commit. For
    _STORE_ATTR_INSTANCE_VALUE the lock is genuinely required (it also
    updates the PyDictValues insertion-order list, which has to be
    coordinated with the NULL->non-NULL transition). For
    _STORE_ATTR_SLOT it was added for symmetry but isn't required
    when atomic exchange is used.
  * bef63d2 (2025-12, pythonGH-134584 / pythongh-142729): removed a redundant
    refcount op from _STORE_ATTR_SLOT, confirming the slot fast path
    is still being actively simplified.

Why this is correct:
  * The slot store is a single pointer write at a fixed offset
    (determined by the type and guarded by _GUARD_TYPE_VERSION before
    _STORE_ATTR_SLOT).
  * Atomic exchange replaces the LOCK_OBJECT + plain-load-old +
    atomic-release-store-new + UNLOCK_OBJECT sequence with one
    lock-prefix xchg.
  * Each writer's exchange returns exactly one old pointer, so
    Py_XDECREF(old_value) cannot double-free across concurrent writers.
  * Readers (LOAD_ATTR_SLOT, PyMember_GetOne) atomically load + try-
    incref, then re-validate that the slot still points at the same
    object before claiming the reference. The TryIncrefCompare path
    correctly fails if ob_ref_shared has reached 0 (object being
    freed), so we never resurrect.
  * Type-change races (`__class__ = NewType`) are caught by the
    _GUARD_TYPE_VERSION uop that runs before _STORE_ATTR_SLOT, same as
    for the already-lock-free _LOAD_ATTR_SLOT.
  * The DEOPT_IF on lock-acquire failure is dropped: atomic exchange
    always succeeds, so the bytecode no longer thrashes between
    specialized and generic forms under contention.

Microbench (origin/main @ 28eac9a, --disable-gil
--enable-optimizations --with-lto=yes, taskset -c 4, best-of-9 ns/op):

  workload                       baseline   patched   ratio
  STORE_ATTR_SLOT (alone)         15.40      9.30     0.604   (-39.6%)
  attr_idiv (LOAD + / + STORE)    41.22     30.20     0.733   (-26.7%)
  Point.normalize (per Point)    417.4     398.0     0.953   (-4.6%)

pyperformance bm_float (direct timing, best-of-5, ms):

  baseline FT    53.2
  patched  FT    48.5    (-4.7 ms, -8.8%)
  GIL (ref)      46.6    (patched FT is within 4% of GIL on this bench)

bm_nbody is unchanged as a control (its STORE-heavy work is on
STORE_SUBSCR_LIST_INT, not STORE_ATTR).

Comprehensive regression sweep on PGO+LTO build: test_descr, test_class,
test_dataclasses, test_dict, test_typing, test_threading,
test_concurrent_futures, test_thread, test_capi, test_inspect,
test_subclassinit, test_long, test_float, test_complex, test_gc,
test_pickle, test_listcomps, test_exceptions, test_compile - all pass.
The full test_free_threading suite (192 tests) also passes.

Concurrent stress test (4 writer threads + 2 reader threads racing on
the same __slots__ object's slots, 200k writes each, 100k reads each):
no crash, no inconsistency, refcount stays in expected range.

Standalone benchmark script: bench_store_attr_slot.py (microbench +
Point.normalize pipeline; see PR comment).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@eendebakpt eendebakpt changed the title Lock free store attr Lock free _STORE_ATTR_SLOT May 22, 2026
@eendebakpt eendebakpt force-pushed the ft_store_attr_slot_lockfree branch from 73d3dc3 to 2cf7c01 Compare May 22, 2026 21:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant