mirror of
https://github.com/nim-lang/Nim.git
synced 2026-08-04 14:38:38 +00:00
atomicArc: skip the atomic RMW when the cell is uniquely referenced (#26073)
`nimDecRefIsLast` always performed an atomic decrement. When the biased count is already zero the destroying thread holds the only reference, so there is nothing to adjudicate and the read-modify-write can be skipped. Soundness: a counted reference can only be derived from the location being destroyed -- which happens-before this destructor unless the program races on that location -- or from another counted reference, whose contribution is already in `rc` and therefore forces the slow path. Observing zero proves no other thread holds a reference and that none can appear. This relies on `--mm:atomicArc` having no collector; ORC and YRC mutate `rc` from a participant that holds no counted reference at all, so the fast path is deliberately not enabled for them. The slow path keeps deciding on the value its own RMW returned. That is what separates this from nim-lang/threading#45, where the "who frees" role was decided from a separate load and the RMW result was discarded, so the role could be dropped by every participant at once. gcbench, -d:danger, median of 21 pinned runs: --mm:arc (non-atomic RC) 0.1310 --mm:atomicArc 0.1742 --mm:atomicArc + this 0.1330 -23.7%, closing 95% of the gap to non-atomic reference counting. gcbench builds its trees with `sink` parameters, so it performs almost no incRefs and the whole atomicArc penalty is decRef traffic. The worst case -- a decrement that always sees rc > 0, so the load never pays off -- measures +1.1%. `-d:nimNoAtomicArcFastPath` restores the previous code path.
This commit is contained in:
@@ -252,7 +252,36 @@ proc nimDecRefIsLast(p: pointer): bool {.compilerRtl, inl.} =
|
||||
writeStackTrace()
|
||||
cfprintf(cstderr, "[DecRef] %p %ld\n", p, cell.count)
|
||||
|
||||
when (defined(gcAtomicArc) or defined(gcYrc)) and hasThreadSupport:
|
||||
when defined(gcAtomicArc) and hasThreadSupport and
|
||||
not defined(nimNoAtomicArcFastPath):
|
||||
# Uniquely-referenced fast path: skip the RMW entirely.
|
||||
#
|
||||
# A counted reference can only be derived from the location being
|
||||
# destroyed (which happens-before this destructor, or the program races
|
||||
# on that location) or from another counted reference (whose
|
||||
# contribution is already in `rc`, forcing the RMW below). So observing
|
||||
# a zero count proves no other thread holds a reference to this cell and
|
||||
# therefore none can be inside this destructor: there is nothing to
|
||||
# adjudicate and no RMW is needed. This is only sound because
|
||||
# `--mm:atomicArc` has no collector -- ORC/YRC mutate `rc` from a
|
||||
# participant that holds no counted reference at all.
|
||||
#
|
||||
# The load must be ACQUIRE: the count may have reached zero because
|
||||
# another thread's release-decrement got there first, and we have to see
|
||||
# its writes before destroying the object.
|
||||
#
|
||||
# The slow path stays self-testing (it frees on the value the RMW
|
||||
# returned, never on a separate load), which is what keeps this out of
|
||||
# the nim-lang/threading#45 bug class.
|
||||
if (atomicLoadN(addr cell.rc, ATOMIC_ACQUIRE) and not rcMask) == 0:
|
||||
result = true
|
||||
when traceCollector:
|
||||
cprintf("[ABOUT TO DESTROY] %p\n", cell)
|
||||
elif atomicDec(cell.rc, rcIncrement) == -rcIncrement:
|
||||
result = true
|
||||
when traceCollector:
|
||||
cprintf("[ABOUT TO DESTROY] %p\n", cell)
|
||||
elif (defined(gcAtomicArc) or defined(gcYrc)) and hasThreadSupport:
|
||||
# `atomicDec` returns the new value
|
||||
if atomicDec(cell.rc, rcIncrement) == -rcIncrement:
|
||||
result = true
|
||||
|
||||
71
tests/arc/tconcurrentdecref.nim
Normal file
71
tests/arc/tconcurrentdecref.nim
Normal file
@@ -0,0 +1,71 @@
|
||||
discard """
|
||||
matrix: "--mm:atomicArc --threads:on"
|
||||
output: "ok"
|
||||
"""
|
||||
|
||||
# Every thread here holds its OWN counted reference to the same cell and drops
|
||||
# it concurrently with the others. Exactly one free per object must happen: a
|
||||
# leak (nobody frees) and a double free (two threads free) are both caught.
|
||||
#
|
||||
# This is the shape that went wrong in nim-lang/threading#45, where the
|
||||
# destructor decided who frees from a separate load and discarded the result
|
||||
# of the read-modify-write, so the role could be dropped by every participant
|
||||
# at once. `nimDecRefIsLast` must always decide on the value its own RMW
|
||||
# returned. The uniquely-referenced fast path added on top of it may only
|
||||
# skip the RMW when the load proves no other thread holds a reference.
|
||||
|
||||
import std/atomics
|
||||
|
||||
type
|
||||
Payload = object
|
||||
id: int
|
||||
Obj = ref Payload
|
||||
|
||||
var freeCount: Atomic[int]
|
||||
|
||||
proc `=destroy`(p: Payload) =
|
||||
discard freeCount.fetchAdd(1, moRelease)
|
||||
|
||||
const
|
||||
NumObjects = 2000
|
||||
NumThreads = 6
|
||||
Rounds = 3
|
||||
|
||||
type
|
||||
Arg = object
|
||||
refs: seq[Obj]
|
||||
|
||||
var
|
||||
go: Atomic[bool]
|
||||
threads: array[NumThreads, Thread[ptr Arg]]
|
||||
args: array[NumThreads, Arg]
|
||||
|
||||
proc worker(a: ptr Arg) {.thread.} =
|
||||
while not go.load(moAcquire): cpuRelax()
|
||||
a.refs.setLen(0) # drop them all, as fast as possible
|
||||
|
||||
proc main =
|
||||
var expected = 0
|
||||
for round in 1..Rounds:
|
||||
var mine = newSeq[Obj](NumObjects)
|
||||
for i in 0 ..< NumObjects:
|
||||
mine[i] = Obj(id: i)
|
||||
for t in 0 ..< NumThreads:
|
||||
args[t].refs = newSeq[Obj](NumObjects)
|
||||
for i in 0 ..< NumObjects:
|
||||
args[t].refs[i] = mine[i] # counted copy
|
||||
go.store(false, moRelease)
|
||||
for t in 0 ..< NumThreads:
|
||||
createThread(threads[t], worker, addr args[t])
|
||||
go.store(true, moRelease) # everybody drops at once...
|
||||
mine.setLen(0) # ...including this thread
|
||||
joinThreads(threads)
|
||||
expected += NumObjects
|
||||
let got = freeCount.load(moAcquire)
|
||||
if got != expected:
|
||||
echo "round ", round, ": got ", got, " frees, expected ", expected,
|
||||
(if got < expected: " (leak)" else: " (double free)")
|
||||
quit 1
|
||||
echo "ok"
|
||||
|
||||
main()
|
||||
Reference in New Issue
Block a user