From 8b597fc05fe9a1684ada6b5cbe2898a0c6436e51 Mon Sep 17 00:00:00 2001 From: Araq Date: Mon, 27 Jul 2026 18:24:25 +0200 Subject: [PATCH] YRC: honour .acyclic instead of tracing acyclic data 'for correctness' nimAsgnYrc/nimSinkYrc were emitted for EVERY ref type, ignoring .acyclic. The barrier defers the dec into a stripe queue, drainStripe hands the cell to registerLocal, and it enters the collector as a capture ROOT -- so types annotated precisely to stay out of the cycle collector were traced by it anyway, through capture/deadness/commit. Gate that barrier on canFormAcycle (the same predicate ccgtypes.nim:1903 uses to set the descriptor's acyclic flag, so codegen and runtime cannot disagree) and let acyclic refs fall through to the prompt arc-style path. nimDecRefIsLastDyn stops forwarding to nimDecRefIsLastCyclicDyn and does a prompt atomic dec. No grace period is needed for those cells and that is not an accident: the collector cannot reach one by traversal (liftdestructors only emits nimTraceRef when isCyclic) and cannot hold one as a root (roots come only from registerLocal on a drained dec). The queued dec was the only way in. This also removes the reason for the earlier nimDecRefIsLastCyclicStatic workaround for final acyclic types, so that is reverted. compiler/astdef.nim now emits 97 nimDecRefIsLastDyn + 9 nimDecRefIsLast + 38 nimIncRef and zero nimAsgnYrc/nimDecRefIsLastCyclic*, matching ORC. Compiler self-compile (--compileOnly compiler/nim.nim): 37.65s -> 6.99s, against orc 6.96s and arc 6.77s. RSS 743MB vs orc 746MB. yrcbench is unchanged (0.78s/5.13s CPU) and still passes its leak assertion, so cyclic collection is unaffected. koch boot -d:release --mm:yrc reaches 'executables are equal: SUCCESS!'. Co-Authored-By: Claude Opus 5 (1M context) --- compiler/liftdestructors.nim | 32 +++++++++++++++++--------------- lib/system/yrc.nim | 22 +++++++++++++++++++++- 2 files changed, 38 insertions(+), 16 deletions(-) diff --git a/compiler/liftdestructors.nim b/compiler/liftdestructors.nim index d8a4076993..032a4623f2 100644 --- a/compiler/liftdestructors.nim +++ b/compiler/liftdestructors.nim @@ -806,8 +806,22 @@ proc atomicRefOp(c: var TLiftCtx; t: PType; body, x, y: PNode) = createTypeBoundOps(c.g, c.c, elemType, c.info, c.idgen) - # YRC uses dedicated runtime procs for the entire write barrier: - if c.g.config.selectedGC == gcYrc: + # YRC uses dedicated runtime procs for the entire write barrier -- but ONLY + # for refs that can actually form cycles. Routing an acyclic ref through + # `nimAsgnYrc` defeats the entire purpose of `.acyclic`: the barrier defers + # the dec into a stripe queue, `drainStripe` then hands the cell to + # `registerLocal`, and it enters the collector as a capture ROOT -- so a + # type annotated precisely to stay out of the cycle collector gets traced + # by it anyway. (The collector never reaches such a cell by TRAVERSAL: the + # attachedTrace hook below only emits `nimTraceRef` when `isCyclic`. The + # queued dec was the only way in.) + # + # Falling through instead gives acyclic refs the same prompt arc-style + # reclamation they get under --mm:arc/orc, which is also what lets a thread + # that avoids cycles at compile time avoid the collector entirely at run + # time. `canFormAcycle` is the same predicate ccgtypes.nim:1903 uses to set + # the descriptor's acyclic flag, so codegen and runtime cannot disagree. + if c.g.config.selectedGC == gcYrc and types.canFormAcycle(c.g, elemType): let desc = if isFinal(elemType): let ti = genBuiltin(c, mGetTypeInfoV2, "getTypeInfoV2", newNodeIT(nkType, x.info, elemType)) @@ -857,19 +871,7 @@ proc atomicRefOp(c: var TLiftCtx; t: PType; body, x, y: PNode) = else: cond = callCodegenProc(c.g, "nimDecRefIsLastCyclicDyn", c.info, tmp) elif isInheritableAcyclicRef: - if c.g.config.selectedGC == gcYrc and useStatic: - # YRC defers every dec, so the runtime must record a type descriptor - # at dec time instead of destroying immediately. The `Dyn` hook - # derives that descriptor from the object's m_type field -- which a - # FINAL object does not have, so it would read the first data field - # as a type pointer. Pass the static descriptor instead, the same - # convention the nimAsgnYrc/nimSinkYrc path above uses. (ORC is - # unaffected: its `Dyn` hook never looks at the descriptor.) - let typInfo = genBuiltin(c, mGetTypeInfoV2, "getTypeInfoV2", newNodeIT(nkType, x.info, elemType)) - typInfo.typ = getSysType(c.g, c.info, tyPointer) - cond = callCodegenProc(c.g, "nimDecRefIsLastCyclicStatic", c.info, x, typInfo) - else: - cond = callCodegenProc(c.g, "nimDecRefIsLastDyn", c.info, x) + cond = callCodegenProc(c.g, "nimDecRefIsLastDyn", c.info, x) else: cond = callCodegenProc(c.g, "nimDecRefIsLast", c.info, x) cond.typ = getSysType(c.g, x.info, tyBool) diff --git a/lib/system/yrc.nim b/lib/system/yrc.nim index 1d769c0563..767ed48da8 100644 --- a/lib/system/yrc.nim +++ b/lib/system/yrc.nim @@ -1586,7 +1586,27 @@ proc nimDecRefIsLastCyclicDyn(p: pointer): bool {.compilerRtl, inl.} = enqueueDec(head(p), cast[ptr PNimTypeV2](p)[]) proc nimDecRefIsLastDyn(p: pointer): bool {.compilerRtl, inl.} = - nimDecRefIsLastCyclicDyn(p) + ## ACYCLIC ref: prompt reclamation, exactly as under --mm:arc. This used to + ## forward to `nimDecRefIsLastCyclicDyn`, which enqueued the dec and so + ## dragged every `.acyclic` type through capture/deadness/commit -- the + ## precise opposite of what the annotation asks for. + ## + ## No grace period is needed here, and that is not an accident: the + ## collector has no way to be holding this cell. It cannot reach it by + ## traversal, because liftdestructors only emits `nimTraceRef` for fields + ## whose type is cyclic; and it cannot hold it as a capture root, because + ## roots come only from `registerLocal` on a drained dec, and an acyclic + ## dec is never queued now that `nimAsgnYrc` is gated on `canFormAcycle`. + ## Both halves must stay true together -- prompt reclamation here is only + ## sound while nothing else puts an acyclic cell into the collector. + result = false + if p != nil: + when hasThreadSupport: + result = atomicDec(head(p).rc, rcIncrement) == -rcIncrement + else: + let cell = head(p) + if (cell.rc and not rcMask) == 0: result = true + else: cell.rc = cell.rc -% rcIncrement proc nimDecRefIsLastCyclicStatic(p: pointer; desc: PNimTypeV2): bool {.compilerRtl, inl.} = result = false