From 82404dfa969f04aa99dfddfe966054932317a7d8 Mon Sep 17 00:00:00 2001 From: Araq Date: Mon, 20 Jul 2026 13:38:09 +0200 Subject: [PATCH] YRC: fixes epoch logic --- lib/system/yrc.nim | 40 ++++++++++----- lib/system/yrc_proof.lean | 31 ++++++++++-- tests/yrc/tyrc_fuzz_graph.nim | 84 ++++++++++++++++++++++++++++++++ tests/yrc/tyrc_parallel_loop.nim | 74 ++++++++++++++++++++++++++++ 4 files changed, 213 insertions(+), 16 deletions(-) create mode 100644 tests/yrc/tyrc_fuzz_graph.nim create mode 100644 tests/yrc/tyrc_parallel_loop.nim diff --git a/lib/system/yrc.nim b/lib/system/yrc.nim index 85420333a3..681fb1db94 100644 --- a/lib/system/yrc.nim +++ b/lib/system/yrc.nim @@ -372,13 +372,22 @@ const SpinBeforePark = 4000 YrcEpochLen {.intdefine.} = 64 # collections per epoch; bounds how long a # stale "proven live" stamp defers rescans. - # Short epochs resonate badly with the - # adaptive threshold: pruned collections - # are cheap, so collections speed up — and - # with them the epoch clock, forcing full - # re-traces MORE often than no stamps at - # all. (A work-based clock would fix this - # properly.) + # Short epochs (≲ 4) resonate with the + # adaptive threshold: pruned collections are + # cheap, so collections speed up — and with + # them the epoch clock, forcing full re-traces + # MORE often than no stamps at all. 64 sits + # clear of that. A WORK-based clock (advance + # per N cells traced, decoupled from the + # collection count) was tried to kill the + # resonance at its root; it lost on webbench + # at every threshold — both slower (~65-90 vs + # ~50 ms) and ~30% more float — because + # pruning shrinks trace work, so the clock + # stalls exactly when a long-lived web should + # be re-examined. The resonance only bites + # below ~4, which 64 already avoids, so there + # was no live problem to trade float for. YrcPromoteAge {.intdefine.} = 3 # captures a cell must survive before its # stamp prunes. Die-young data must never # be deferred: torcbench-style lists are @@ -953,10 +962,19 @@ proc demoteTouchedDead(j: var GcEnv; cap: ptr CaptureBufs) = cap.sccs.d[t].flags = cap.sccs.d[t].flags or flagDirty let m = cap.sccMembers.d[cap.sccs.d[s].memStart] registerLocal(cap.recs.d[m].cell, cap.recs.d[m].desc) - elif (cap.sccs.d[s].flags and flagPruned) != 0: - # survives, but its liveness may rest on a stale epoch stamp: keep - # one member registered so the verdict is retried (fully re-examined - # once the epoch advances) + elif cap.prunedSrc.len > 0: + # A prune happened somewhere in THIS collection, so every "live" verdict + # it produced is suspect: a pruned cell is not traced, yet its out-edges + # still count toward its targets' rc. If that pruned cell is itself dead + # (promoted while live, died later this epoch), its phantom references + # inflate unrelated SCCs' external counts and misclassify genuinely dead + # SCCs as plain survivors. Such a survivor is not flagPruned, so without + # this it would be re-stamped, dropped from the retry set, and orphaned + # forever once its last flagPruned neighbor resolves. Keeping ONE member + # of every survivor registered guarantees it is re-examined until the + # epoch advances, captures the dead promoted cells, and decrements the + # phantom edges away. Cheap in practice: pruning keeps the captured set + # small, so "every survivor" is only the few cells actually traced. let m = cap.sccMembers.d[cap.sccs.d[s].memStart] registerLocal(cap.recs.d[m].cell, cap.recs.d[m].desc) diff --git a/lib/system/yrc_proof.lean b/lib/system/yrc_proof.lean index e54fd66bec..64a501ed03 100644 --- a/lib/system/yrc_proof.lean +++ b/lib/system/yrc_proof.lean @@ -724,17 +724,38 @@ theorem no_deadlock_from_total_order {n : Nat} registered: E1 roots never prune: a registered candidate is always fully root-scanned, stamps notwithstanding; - E2 an SCC that pruned an out-edge and survives keeps one member - registered (flagPruned) — its "live" verdict may lean on a stamp - that went stale within the epoch; + E2 when a collection prunes ANY out-edge, it keeps one member of + EVERY surviving SCC registered — not just the SCCs that pruned an + edge themselves. This is broader than it first looks and the + breadth is load-bearing: a pruned cell is not traced, yet its + out-edges still count toward its targets' rc, so a pruned cell + that is ITSELF dead (promoted while live, died later this epoch) + contributes phantom refs that inflate an UNRELATED SCC's external + count and misclassify that genuinely-dead SCC as a plain (non- + prune-source) survivor. Registering only prune-source SCCs + (the original hook) let such a survivor be re-stamped, dropped + from the retry set, and orphaned permanently once its last + prune-source neighbour resolved — a real leak the dumpster `fuzz` + port surfaced (tests/yrc/tyrc_fuzz_graph.nim: ~1% of allocations + lost, ORC-clean, un-recoverable even by repeated GC_fullCollect). + Registering every survivor of a pruning collection closes it; the + cost is bounded because pruning keeps the captured set small, so + "every survivor" is only the handful actually traced; E3 a commit-time dec into a stamped cell re-registers the target — the dec may be the death blow to a cell no collection analyzed; E4 explicit full collects advance the epoch first, so all stamps are stale and nothing is pruned. - Not formalized. Also noted: the epoch clock counts collections, and + Not formalized (and E2's original narrow form was empirically wrong — + see above; the broadened form is validated by the fuzz port across seeds + and sizes, not machine-checked). The epoch clock counts collections + (YrcEpochLen=64); short epochs (≲ 4) resonate with the adaptive threshold — pruned collections are cheap, so collections and hence epoch turns speed up, - re-tracing MORE than with no stamps; a work-based clock would fix it. + re-tracing MORE than with no stamps — but 64 sits clear of that. A + work-based clock (advance per N cells traced) was measured and lost on + long-lived structures: pruning shrinks trace work, so the clock stalls + exactly when a stale web should be re-examined, trading float for a + resonance the default length already avoids. Reference: D.F. Bacon and V.T. Rajan, "Concurrent Cycle Collection in Reference Counted Systems", ECOOP 2001 — the deadness arithmetic is diff --git a/tests/yrc/tyrc_fuzz_graph.nim b/tests/yrc/tyrc_fuzz_graph.nim new file mode 100644 index 0000000000..c0729926cf --- /dev/null +++ b/tests/yrc/tyrc_fuzz_graph.nim @@ -0,0 +1,84 @@ +discard """ + cmd: "nim c --mm:yrc -d:useMalloc --threads:on $file" + output: "ok" + disabled: "windows" + disabled: "freebsd" + disabled: "openbsd" +""" + +# Deterministic port of dumpster's `fuzz` test +# (https://claytonwramsey.com/blog/dumpster/): drive a mutable object graph +# through a long random sequence of node/edge inserts and removals, then drop +# every root and assert that *every allocation ever made is destroyed exactly +# once* -- no leak (count 0) and no double free (count > 1). The graph grows +# thick with overlapping and self cycles, so only the cycle collector can wind +# it down. A fixed LCG seed makes the shape reproducible across runs. + +type + DropCount = object + id: int + live: bool # false in any moved-from temporary -> never miscounts + Node = ref object + refs: seq[Node] + dc: DropCount + +var counts: seq[int] # counts[id] == times allocation `id` was destroyed + +proc `=destroy`(x: DropCount) = + if x.live: inc counts[x.id] + +var nextId = 0 +proc newNode(): Node = + counts.add 0 + result = Node(refs: @[], dc: DropCount(id: nextId, live: true)) + inc nextId + +# `child` is a by-value borrow (dumpster's `Gc::clone`): storing it copies the +# reference, leaving the caller's root slot still owning. Using `.refs.add` +# directly would move the root at its last read and change the graph shape. +proc link(parent, child: Node) = parent.refs.add child + +# Small fixed-seed LCG (Numerical Recipes constants) for reproducible shape. +var rngState: uint32 = 12345 +proc rnd(n: int): int = + rngState = rngState * 1664525'u32 + 1013904223'u32 + int((rngState shr 16) mod uint32(n)) + +proc run = + const N = 20_000 + var roots: seq[Node] + for i in 0 ..< 50: roots.add newNode() + + for _ in 0 ..< N: + if roots.len == 0: roots.add newNode() + case rnd(4) + of 0: # allocate a fresh root + roots.add newNode() + of 1: # add edge from -> to (may self-loop) + let a = rnd(roots.len) + let b = rnd(roots.len) + link(roots[a], roots[b]) + of 2: # drop a root handle (swap-remove) + let i = rnd(roots.len) + roots[i] = roots[roots.high] + roots.setLen roots.len - 1 + else: # drop one outgoing edge of a root + let a = rnd(roots.len) + if roots[a].refs.len > 0: + let j = rnd(roots[a].refs.len) + roots[a].refs[j] = roots[a].refs[roots[a].refs.high] + roots[a].refs.setLen roots[a].refs.len - 1 + + roots.setLen 0 # release every remaining root + GC_fullCollect() + GC_fullCollect() + +run() + +var missing = 0 +for id in 0 ..< nextId: + if counts[id] != 1: + inc missing +doAssert missing == 0, "graph not fully reclaimed: " & $missing & " of " & + $nextId & " allocations leaked or double-freed" +echo "ok" diff --git a/tests/yrc/tyrc_parallel_loop.nim b/tests/yrc/tyrc_parallel_loop.nim new file mode 100644 index 0000000000..875264918d --- /dev/null +++ b/tests/yrc/tyrc_parallel_loop.nim @@ -0,0 +1,74 @@ +discard """ + cmd: "nim c --mm:yrc -d:useMalloc --threads:on $file" + output: "ok" + valgrind: "leaks" + disabled: "windows" + disabled: "freebsd" + disabled: "openbsd" +""" + +# The "parallel_loop" complex graph from Clayton Ramsey's `dumpster` collector +# (https://claytonwramsey.com/blog/dumpster/). Four allocations form a single +# SCC built from two *overlapping* cycles that share nodes 1 and 4: +# +# 1 -> 4 4 -> 2, 4 -> 3 2 -> 1, 3 -> 1 +# +# so 1->4->2->1 and 1->4->3->1 traverse the same 1 and 4. Every node keeps a +# nonzero refcount from *inside* the SCC, so plain reference counting can never +# free any of them; only cycle collection can, and only once the last external +# handle is gone. We drop the four root handles one at a time and assert that +# nothing is reclaimed until the final drop, then all four die together -- the +# exact assertion sequence dumpster's test makes. + +type + # A field whose destructor bumps a per-node counter when the cell is freed; + # `slot` is nil in any moved-from temporary, so those don't miscount. + DropCount = object + slot: ptr int + Node = ref object + refs: seq[Node] + dc: DropCount + +proc `=destroy`(x: DropCount) = + if x.slot != nil: inc x.slot[] + +# Add an edge parent -> child. `child` is a by-value borrow, so the caller's +# handle keeps owning its reference -- this is Nim's equivalent of dumpster's +# `Gc::clone`. Building edges with `g1.refs.add g2` instead would *move* g2 at +# its last read and silently collapse the graph's root set. +proc link(parent, child: Node) = parent.refs.add child + +# drops[0] is unused; nodes are 1..4 to mirror the blog's gc1..gc4. The four +# handles live in an array so each stays an independent, still-owning root. +var drops: array[5, int] + +proc scenario = + var g: array[1..4, Node] + for i in 1..4: g[i] = Node(dc: DropCount(slot: addr drops[i])) + link(g[2], g[1]) # 2 -> 1 + link(g[3], g[1]) # 3 -> 1 + link(g[4], g[2]) # 4 -> 2 + link(g[4], g[3]) # 4 -> 3 + link(g[1], g[4]) # 1 -> 4 (closes both cycles) + + GC_fullCollect() + doAssert drops == [0, 0, 0, 0, 0], "nothing dead yet" + + g[1] = nil # node1 still held by node2 and node3 + GC_fullCollect() + doAssert drops == [0, 0, 0, 0, 0], "dropping root 1 frees nothing" + + g[2] = nil # node2 still held by node4 + GC_fullCollect() + doAssert drops == [0, 0, 0, 0, 0], "dropping root 2 frees nothing" + + g[3] = nil # node3 still held by node4 + GC_fullCollect() + doAssert drops == [0, 0, 0, 0, 0], "dropping root 3 frees nothing" + + g[4] = nil # last external handle gone: the whole SCC is garbage + GC_fullCollect() + doAssert drops == [0, 1, 1, 1, 1], "the full cycle is reclaimed at once" + +scenario() +echo "ok"