Skip to content

gh-150277: Make _STORE_ATTR_SLOT lock-free on the free-threaded build - #150324

Closed
eendebakpt wants to merge 5 commits into
python:mainfrom
eendebakpt:ft_store_attr_slot_lockfree
Closed

gh-150277: Make _STORE_ATTR_SLOT lock-free on the free-threaded build#150324
eendebakpt wants to merge 5 commits into
python:mainfrom
eendebakpt:ft_store_attr_slot_lockfree

Conversation

@eendebakpt

@eendebakpteendebakpt commented May 23, 2026

Copy link
Copy Markdown
Contributor

Both the bytecode handler (_STORE_ATTR_SLOT) and the C-API path (PyMember_SetOne 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.

We 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.

Adds a concurrent _Py_T_OBJECT test (test_T_OBJECT in test_slots.py).

Benchmarks:

micro_store_attr_slot:
[baseline] 79.0 ns +- 1.3 ns
-> [patched] 54.5 ns +- 0.3 ns
1.45x faster
micro_attr_idiv:
[baseline] 180 ns +- 2 ns
-> [patched] 149 ns +- 9 ns
1.21x faster
bm_float:
[baseline] 87.4 ms +- 0.7 ms
-> [patched] 83.7 ms +- 0.7 ms
1.04x faster
Geometric mean: 1.22x faster
Interpretation of results by Claude - **`micro_store_attr_slot` (1.45x)**: tight loop of four `STORE_ATTR_SLOT`s. Goes from `LOCK_OBJECT + atomic-release-store + UNLOCK_OBJECT` (3 atomic memory ops) to a single `lock xchg`. - **`micro_attr_idiv` (1.21x)**: realistic mixed workload — `self.x /= norm` is `LOAD_ATTR_SLOT + BINARY_OP(/) + STORE_ATTR_SLOT`; the saving tracks the STORE share of the op cost. - **`bm_float` (1.04x)**: the pyperformance `bm_float` benchmark unmodified. ~6 `STORE_ATTR_SLOT`s per Point (3 in `__init__`, 3 in `normalize()`) × 100 000 Points = ~600 k store ops, each ~25 ns faster, gives the ~4% wall-time improvement we see.
bench_store_attr_slot.py — pyperf benchmark used above
"""pyperf benchmark for the ft_store_attr_slot_lockfree change.Three workloads: micro_store_attr_slot Tight loop of 4 attribute writes to a __slots__ instance per iter. Each write is a single STORE_ATTR_SLOT. micro_attr_idiv The `self.x /= norm` pattern from Point.normalize: 3 in-place float divides per iter (LOAD_ATTR_SLOT + BINARY_OP + STORE_ATTR_SLOT) plus 3 plain reset stores. bm_float The pyperformance `bm_float` benchmark: 100_000 Point() + .normalize() cycle, ending with a maximize() reduction.Usage: python bench_store_attr_slot.py # one-shot python bench_store_attr_slot.py -o results.json # save python -m pyperf compare_to a.json b.json # compare runs"""frommathimportsin, cos, sqrtimportpyperf# --- microbench: STORE_ATTR_SLOT on a __slots__ class ---classS:
__slots__= ('a', 'b', 'c', 'd')
def__init__(self):
self.a=1.0self.b=1.0self.c=1.0self.d=1.0defmicro_store_attr_slot(loops):
s=S()
range_it=range(loops)
t0=pyperf.perf_counter()
for_inrange_it:
s.a=1.5s.b=2.5s.c=3.5s.d=4.5returnpyperf.perf_counter() -t0defmicro_attr_idiv(loops):
s=S()
norm=2.0range_it=range(loops)
t0=pyperf.perf_counter()
for_inrange_it:
s.a/=norms.b/=norms.c/=norms.a=1.0s.b=1.0s.c=1.0returnpyperf.perf_counter() -t0# --- bm_float (vendored from pyperformance/benchmarks/bm_float) ---POINTS=100_000classPoint(object):
__slots__= ('x', 'y', 'z')
def__init__(self, i):
self.x=x=sin(i)
self.y=cos(i) *3self.z= (x*x) /2defnormalize(self):
x=self.xy=self.yz=self.znorm=sqrt(x*x+y*y+z*z)
self.x/=normself.y/=normself.z/=normdefmaximize(self, other):
self.x=self.xifself.x>other.xelseother.xself.y=self.yifself.y>other.yelseother.yself.z=self.zifself.z>other.zelseother.zreturnselfdefmaximize(points):
next=points[0]
forpinpoints[1:]:
next=next.maximize(p)
returnnextdefbm_float_body(n):
points= [None] *nforiinrange(n):
points[i] =Point(i)
forpinpoints:
p.normalize()
returnmaximize(points)
if__name__=="__main__":
runner=pyperf.Runner()
runner.metadata['description'] = (
"Microbench + bm_float for STORE_ATTR_SLOT on the free-threaded ""build (gh-NNNNN: lock-free atomic exchange)"
)
runner.bench_time_func('micro_store_attr_slot', micro_store_attr_slot,
inner_loops=1)
runner.bench_time_func('micro_attr_idiv', micro_attr_idiv,
inner_loops=1)
runner.bench_func('bm_float', bm_float_body, POINTS)

A side effect is that _STORE_ATTR_SLOT does not deopt any longer, this improves the jit traces:

For

def slot_loop(n, p):
for _ in range(n):
p.a = 1.0
p.b = 2.0
p.c = 3.0

The trace on main:

[ 0] bc= 32 _START_EXECUTOR [ 1] bc= - _MAKE_WARM [ 2] bc= 38 _SET_IP [ 3] bc=2162688 _CHECK_PERIODIC // emits one side-exit uop
[ 4] bc= 34 _CHECK_VALIDITY // emits one side-exit uop
[ 5] bc= 35 _ITER_CHECK_RANGE (24) // emits one side-exit uop
[ 6] bc= 36 _GUARD_NOT_EXHAUSTED_RANGE (24) // emits one side-exit uop
[ 7] bc=2424832 _ITER_NEXT_RANGE (24) [ 8] bc= 16 _SET_IP [ 9] bc= 16 _SWAP_FAST_2 (2) [ 10] bc= - _SPILL_OR_RELOAD [ 11] bc= 16 _POP_TOP [ 12] bc= 38 _CHECK_VALIDITY // emits one side-exit uop
[ 13] bc= 17 _LOAD_CONST_INLINE_BORROW [ 14] bc= 18 _LOAD_FAST_BORROW_1 (1) [ 15] bc= 19 _SET_IP [ 16] bc= 39 _GUARD_TYPE_VERSION (1) // emits one side-exit uop
[ 17] bc= 40 _STORE_ATTR_SLOT (1) <<< HAS_DEOPT_FLAG REMOVED by ft_store_attr_slot_lockfree
[ 18] bc= 19 _POP_TOP_NOP [ 19] bc= 41 _CHECK_VALIDITY // emits one side-exit uop
[ 20] bc= 24 _LOAD_CONST_INLINE_BORROW [ 21] bc= 25 _LOAD_FAST_BORROW_1 (1) [ 22] bc= 26 _SET_IP [ 23] bc= 42 _STORE_ATTR_SLOT (2) <<< HAS_DEOPT_FLAG REMOVED by ft_store_attr_slot_lockfree
[ 24] bc= 26 _POP_TOP_NOP [ 25] bc= 43 _CHECK_VALIDITY // emits one side-exit uop
[ 26] bc= 31 _LOAD_CONST_INLINE_BORROW [ 27] bc= 32 _LOAD_FAST_BORROW_1 (1) [ 28] bc= 33 _SET_IP [ 29] bc= 44 _STORE_ATTR_SLOT (3) <<< HAS_DEOPT_FLAG REMOVED by ft_store_attr_slot_lockfree
[ 30] bc= 33 _POP_TOP_NOP [ 31] bc= 1 _JUMP_TO_TOP // emits one side-exit uop
[ 32] bc= 38 _DEOPT [ 33] bc= - _ERROR_POP_N [ 34] bc= 14 _DEOPT [ 35] bc= 14 _EXIT_TRACE [ 36] bc= 41 _EXIT_TRACE [ 37] bc= - _ERROR_POP_N [ 38] bc= 17 _DEOPT [ 39] bc= 19 _EXIT_TRACE [ 40] bc= 19 _DEOPT [ 41] bc= 24 _DEOPT [ 42] bc= 26 _DEOPT [ 43] bc= 31 _DEOPT [ 44] bc= 33 _DEOPT 

Trace with PR:

 [ 0] bc= 32 _START_EXECUTOR [ 1] bc= - _MAKE_WARM [ 2] bc= 38 _SET_IP [ 3] bc=2162688 _CHECK_PERIODIC // emits one side-exit uop
[ 4] bc= 34 _CHECK_VALIDITY // emits one side-exit uop
[ 5] bc= 35 _ITER_CHECK_RANGE (24) // emits one side-exit uop
[ 6] bc= 36 _GUARD_NOT_EXHAUSTED_RANGE (24) // emits one side-exit uop
[ 7] bc=2424832 _ITER_NEXT_RANGE (24) [ 8] bc= 16 _SET_IP [ 9] bc= 16 _SWAP_FAST_2 (2) [ 10] bc= - _SPILL_OR_RELOAD [ 11] bc= 16 _POP_TOP [ 12] bc= 38 _CHECK_VALIDITY // emits one side-exit uop
[ 13] bc= 17 _LOAD_CONST_INLINE_BORROW [ 14] bc= 18 _LOAD_FAST_BORROW_1 (1) [ 15] bc= 19 _SET_IP [ 16] bc= 39 _GUARD_TYPE_VERSION (1) // emits one side-exit uop
[ 17] bc= 19 _STORE_ATTR_SLOT (1) <<< HAS_DEOPT_FLAG REMOVED by ft_store_attr_slot_lockfree
[ 18] bc= 19 _POP_TOP_NOP [ 19] bc= 40 _CHECK_VALIDITY // emits one side-exit uop
[ 20] bc= 24 _LOAD_CONST_INLINE_BORROW [ 21] bc= 25 _LOAD_FAST_BORROW_1 (1) [ 22] bc= 26 _SET_IP [ 23] bc= 26 _STORE_ATTR_SLOT (2) <<< HAS_DEOPT_FLAG REMOVED by ft_store_attr_slot_lockfree
[ 24] bc= 26 _POP_TOP_NOP [ 25] bc= 41 _CHECK_VALIDITY // emits one side-exit uop
[ 26] bc= 31 _LOAD_CONST_INLINE_BORROW [ 27] bc= 32 _LOAD_FAST_BORROW_1 (1) [ 28] bc= 33 _SET_IP [ 29] bc= 33 _STORE_ATTR_SLOT (3) <<< HAS_DEOPT_FLAG REMOVED by ft_store_attr_slot_lockfree
[ 30] bc= 33 _POP_TOP_NOP [ 31] bc= 1 _JUMP_TO_TOP // emits one side-exit uop
[ 32] bc= 38 _DEOPT [ 33] bc= - _ERROR_POP_N [ 34] bc= 14 _DEOPT [ 35] bc= 14 _EXIT_TRACE [ 36] bc= 41 _EXIT_TRACE [ 37] bc= - _ERROR_POP_N [ 38] bc= 17 _DEOPT [ 39] bc= 19 _EXIT_TRACE [ 40] bc= 24 _DEOPT [ 41] bc= 31 _DEOPT 

Replace the LOCK_OBJECT / atomic-release-store / UNLOCK_OBJECT sequence
in _STORE_ATTR_SLOT (and the matching critical section in
PyMember_SetOne for Py_T_OBJECT_EX / _Py_T_OBJECT) with a single atomic
exchange on the slot pointer. Atomic exchange returns a unique old
value per writer, so Py_XDECREF cannot double-free across concurrent
writers. Concurrent readers (_LOAD_ATTR_SLOT, PyMember_GetOne) already
use atomic load + _Py_TryIncrefCompare; PyMember_GetOne's locked
fallback is replaced by _Py_XGetRef so it also stays correct against
lock-free writers.
Drops HAS_DEOPT_FLAG from _STORE_ATTR_SLOT (no LOCK can fail anymore),
which removes one _DEOPT exit per occurrence from Tier 2 traces. Adds
_Py_atomic_exchange_ptr to the cases-generator's NON_ESCAPING_FUNCTIONS
allowlist so the generator stops wrapping it in
_PyFrame_SetStackPointer / _GetStackPointer.
Adds a concurrent _Py_T_OBJECT test (test_T_OBJECT in test_slots.py).
Benchmarks (PGO+LTO, taskset -c 4, pyperf):
micro_store_attr_slot 79.0 ns -> 54.5 ns 1.45x faster
micro_attr_idiv 180 ns -> 149 ns 1.21x faster
bm_float 87.4 ms -> 83.7 ms 1.04x faster
bm_nbody (control) unchanged
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Comment threadPython/structmember.c
// section fallback assumed writers also took ob_mutex, which they no
// longer do, so a plain Py_XINCREF inside the CS could resurrect a
// refcount-0 (about-to-be-freed) object.
v = _Py_XGetRef((PyObject **)addr);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't address the problem of the old object being concurrently freed. The docs of _Py_XGetRef read:

NOTE: The writer must set maybe-weakref on the stored object!

Which is crucial for avoiding a use-after-free.

A similar lock-free implementation of this was rejected in a previous PR: #119368 (review)

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. I see no simple way to address this, so I will close the PR.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@eendebakpt@dpdani