From 927ff7e28bf84b229d8a24b6734a5e0d665d6cb5 Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Thu, 23 Jul 2026 14:07:54 +0300 Subject: [PATCH 1/4] Fix GC freeing live objects referenced by untraced fresh BiBOP objects The fresh-page-stack grace scheme introduced in #5436 queued a page onto an alternating per-epoch stack on its FIRST allocation in a GC epoch and consumed both stacks once, mid-mark. The queue-once-per-epoch dedup left a wide uncovered window: every allocation into an already-consumed page for the REST of that epoch -- the remainder of the mark plus the entire unbarriered inter-cycle gap -- was skipped, and if the page received no next-epoch allocation before the next grace pass, its fresh (gcMark==-1) slots were never grace-traced. The sweep then freed any object reachable only through such an untraced fresh object while the fresh object itself survived via grace: a dangling reference inside a surviving object. With compact strings inlining the byte payload into the String's BiBOP slot, recycling those slots rewrites word bytes in place -- the corrupted dictionary entries and impossible NPE reported in issue 5425. Replace the queue with a full-registry walk pruned by the existing gcAllocedSinceSweep flag. The pruning invariant is exact and race-free: a mark==-1 slot can only exist on a page allocated into since that page's last sweep (the sweep converts every -1 it sees), every allocation path already sets the flag, pre-mark stores are published by the mark-start thread sync, and only the sweep -- which never touches an owned page -- clears it. Flag-FALSE pages (the retained-survivor bulk on exactly the workloads #5436 targets) are skipped without touching their slots, so the pause win of #5436 is preserved while the whole fresh-page queueing machinery (two page-header fields, the epoch mirror check and queue call in three allocation paths) is deleted from the hot path. Add a QA-only grace-completeness gate (-DCN1_GRACE_AUDIT): snapshot each page's bump cursor at mark start and, right before the sweep, full-walk the registry tracing any pre-snapshot slot still fresh. It reports missedFresh (fresh slots the grace pass never visited) and doomedChildren (objects that became marked ONLY through them -- each one would otherwise be swept while still referenced; any nonzero value is a collector bug). The new GraceAudit driver allocates dropped fresh nodes holding sole references to older objects WHILE the concurrent mark runs (System.gc is asynchronous), then goes quiet a cycle: against the fresh-page stacks it reports 100-370 missed / 100-250 doomed per cycle (12,178 doomed in one run); with this fix doomedChildren is zero across the suite. StormAB and LoadLoop are the matching perf A/B drivers: wall time and RSS are unchanged vs the pre-fix tree (storm 5-8x faster than pre-#5436, repeated dictionary loads flat), run-bibop-adaptive.sh stays green (adaptive 0.94x time / 0.62x RSS vs legacy), and the full gauntlet passes byte-identical in both thread-stop modes. Fixes the corruption regression reported in #5425. Co-Authored-By: Claude Fable 5 --- vm/ByteCodeTranslator/src/cn1_globals.h | 29 ++-- vm/ByteCodeTranslator/src/cn1_globals.m | 153 +++++++++++++------- vm/benchmarks/README.md | 37 ++++- vm/benchmarks/src/com/bench/GraceAudit.java | 104 +++++++++++++ vm/benchmarks/src/com/bench/LoadLoop.java | 57 ++++++++ vm/benchmarks/src/com/bench/StormAB.java | 57 ++++++++ 6 files changed, 362 insertions(+), 75 deletions(-) create mode 100644 vm/benchmarks/src/com/bench/GraceAudit.java create mode 100644 vm/benchmarks/src/com/bench/LoadLoop.java create mode 100644 vm/benchmarks/src/com/bench/StormAB.java diff --git a/vm/ByteCodeTranslator/src/cn1_globals.h b/vm/ByteCodeTranslator/src/cn1_globals.h index 7974bae6cb6..d0adc7794be 100644 --- a/vm/ByteCodeTranslator/src/cn1_globals.h +++ b/vm/ByteCodeTranslator/src/cn1_globals.h @@ -1290,7 +1290,6 @@ const int currentCodenameOneCallStackOffset = threadStateData->callStackOffset; typedef struct CN1BibopPage { struct CN1BibopPage* _Atomic nextAll; // append-only global registry chain - struct CN1BibopPage* nextFresh[2]; // alternating per-GC fresh-page stack links struct CN1BibopPage* nextPool; // FREE/PARTIAL pool / SWEEP stack link int classIndex; int slotSize; @@ -1335,7 +1334,9 @@ typedef struct CN1BibopPage { // idempotent across parallel markers) int gcGraceEpoch; // upper bound on survivor epochs as of the last // full walk (GC-thread only) - _Atomic int gcFreshEpoch[2]; // queued once on each alternating epoch stack +#ifdef CN1_GRACE_AUDIT + int gcAuditSnapshot; // QA builds only: bumpIndex at mark start +#endif } CN1BibopPage; // Per-thread current page per size class; defined in cn1_globals.m. Touched only @@ -1343,11 +1344,10 @@ typedef struct CN1BibopPage { extern __thread CN1BibopPage* bibopCurrent[CN1_BIBOP_NUM_CLASSES]; extern _Atomic long bibopBytesSinceGc; extern _Atomic long bibopGcTriggerBytes; -// Atomic mirror of currentGcMarkValue for mutator-side adaptive/fresh-page +// Atomic mirror of currentGcMarkValue for mutator-side adaptive-policy // decisions. currentGcMarkValue itself remains owned by the GC/mark threads. extern _Atomic int bibopGcEpoch; extern _Atomic int bibopBypassGeneration[CN1_BIBOP_NUM_CLASSES]; -extern CN1BibopPage* _Atomic bibopFreshPages[2]; #if defined(CN1_GC_INSTRUMENT) && !defined(CN1_DISABLE_BIBOP) // QA-only diagnostics. Production builds contain neither the counters nor // their atomic updates. @@ -1359,7 +1359,6 @@ extern _Atomic long cn1BibopBeltRuns; extern _Atomic long cn1BibopAdoptedRescanSkips; #endif extern int currentGcMarkValue; -extern void cn1BibopNoteFreshAllocation(CN1BibopPage* page, int epoch); #ifndef CN1_BIBOP_NO_FASTSWEEP // Called from monitorEnter (any thread) when a monitor (CN1ThreadData) is freshly // attached to a heap object. If the object is a BiBOP slot it bumps a global live-monitor @@ -1471,16 +1470,13 @@ static inline JAVA_OBJECT cn1BibopFastAlloc(CODENAME_ONE_THREAD_STATE, int size, #endif __atomic_store_n(&o->__codenameOneGcMark, -1, __ATOMIC_RELEASE); atomic_store_explicit(&p->bumpIndex, bi + 1, memory_order_release); - int __cn1FreshEpoch = atomic_load_explicit(&bibopGcEpoch, memory_order_relaxed); - if(__builtin_expect(atomic_load_explicit(&p->gcFreshEpoch[__cn1FreshEpoch & 1], memory_order_relaxed) - != __cn1FreshEpoch, 0)) { - cn1BibopNoteFreshAllocation(p, __cn1FreshEpoch); - } #ifndef CN1_BIBOP_NO_FASTSWEEP - // Mark the page dirty so the O(1) sweep never treats a page that still has - // fresh mark==-1 (grace-candidate) slots as homogeneous. Single plain store - // to the already-hot page header; published to the GC by the eventual - // retire release-push. + // Mark the page dirty: the O(1) sweep never treats a page that still has + // fresh mark==-1 (grace-candidate) slots as homogeneous, and the grace + // pass slot-scans exactly the flagged pages ("-1 slot present" implies + // "allocated into since last sweep" -- the sweep converts every -1 it + // sees). Single plain store to the already-hot page header; pre-mark + // stores are published to the GC by the mark-start thread sync. p->gcAllocedSinceSweep = JAVA_TRUE; #endif CN1_BIBOP_ACCOUNT_BYTES(threadStateData, p->slotSize); @@ -1559,11 +1555,6 @@ static inline JAVA_OBJECT cn1BibopFastAllocNoZero(CODENAME_ONE_THREAD_STATE, int #endif __atomic_store_n(&o->__codenameOneGcMark, -1, __ATOMIC_RELEASE); atomic_store_explicit(&p->bumpIndex, bi + 1, memory_order_release); - int __cn1FreshEpoch = atomic_load_explicit(&bibopGcEpoch, memory_order_relaxed); - if(__builtin_expect(atomic_load_explicit(&p->gcFreshEpoch[__cn1FreshEpoch & 1], memory_order_relaxed) - != __cn1FreshEpoch, 0)) { - cn1BibopNoteFreshAllocation(p, __cn1FreshEpoch); - } #ifndef CN1_BIBOP_NO_FASTSWEEP p->gcAllocedSinceSweep = JAVA_TRUE; #endif diff --git a/vm/ByteCodeTranslator/src/cn1_globals.m b/vm/ByteCodeTranslator/src/cn1_globals.m index 6acec7c55ef..33de9a4367d 100644 --- a/vm/ByteCodeTranslator/src/cn1_globals.m +++ b/vm/ByteCodeTranslator/src/cn1_globals.m @@ -946,6 +946,9 @@ static void cn1DrainDeadThreadPending() { // walks the page registry and its slots before their definitions. static inline JAVA_OBJECT cn1BibopSlot(CN1BibopPage* p, int i); static CN1BibopPage* _Atomic bibopAllPages; +#ifdef CN1_GRACE_AUDIT +static void cn1GraceAuditPreSweep(CODENAME_ONE_THREAD_STATE); +#endif #endif #ifdef CN1_BIBOP_VALIDATE // Belt diagnostic: while set, gcMarkObject logs the class of each newly-marked object @@ -1283,29 +1286,43 @@ void codenameOneGCMark() { // dangling child -> the intermittent Property->Double / container->content crash. Drain // every fresh NON-LEAF object here so a surviving grace object's subtree survives // WITH it. Primitive arrays and other leaf classes have no subtree and are left to - // the sweep's normal one-cycle grace. The alternating dirty-page stacks prevent a - // page allocated into during this mark from corrupting the stack being consumed. + // the sweep's normal one-cycle grace. + // + // Walk the FULL page registry, pruned by gcAllocedSinceSweep. The invariant is + // exact: a mark==-1 slot can only exist on a page allocated into since that + // page's last sweep (the sweep converts every -1 it sees to V), and EVERY + // allocation path sets the flag before the mark-start thread sync publishes it + // -- so a flag-FALSE page provably holds no fresh slot and is skipped without + // touching its slots. Only the sweep clears the flag, and it only processes + // retired (owner==0) pages, so no mutator/GC race on the flag exists. This + // replaced a queue-of-fresh-pages scheme (issue 5425): queue-once-per-epoch + // dedup left every allocation AFTER the queue was consumed (rest of the mark + // plus the whole unbarriered inter-cycle window) untraced when the page was + // not re-queued the next epoch, and the sweep then freed objects reachable + // only through those untraced fresh objects -> user-visible heap corruption. { - for(int lane = 0 ; lane < 2 ; lane++) { - CN1BibopPage* gp = atomic_exchange_explicit(&bibopFreshPages[lane], - (CN1BibopPage*)0, - memory_order_acquire); - while(gp != 0) { -#ifdef CN1_GC_INSTRUMENT - atomic_fetch_add_explicit(&cn1BibopFreshPagesScanned, 1, - memory_order_relaxed); + CN1BibopPage* gp = atomic_load_explicit(&bibopAllPages, memory_order_acquire); + while(gp != 0) { +#ifndef CN1_BIBOP_NO_FASTSWEEP + if(gp->gcAllocedSinceSweep == JAVA_FALSE) { + gp = atomic_load_explicit(&gp->nextAll, memory_order_acquire); + continue; + } #endif - int gn = atomic_load_explicit(&gp->bumpIndex, memory_order_acquire); - for(int gi = 0 ; gi < gn ; gi++) { - JAVA_OBJECT go = cn1BibopSlot(gp, gi); - if(__atomic_load_n(&go->__codenameOneGcMark, __ATOMIC_ACQUIRE) == -1 - && go->__codenameOneParentClsReference != 0 - && go->__codenameOneParentClsReference->markFunction != 0) { - gcMarkObject(d, go, JAVA_FALSE); - } +#ifdef CN1_GC_INSTRUMENT + atomic_fetch_add_explicit(&cn1BibopFreshPagesScanned, 1, + memory_order_relaxed); +#endif + int gn = atomic_load_explicit(&gp->bumpIndex, memory_order_acquire); + for(int gi = 0 ; gi < gn ; gi++) { + JAVA_OBJECT go = cn1BibopSlot(gp, gi); + if(__atomic_load_n(&go->__codenameOneGcMark, __ATOMIC_ACQUIRE) == -1 + && go->__codenameOneParentClsReference != 0 + && go->__codenameOneParentClsReference->markFunction != 0) { + gcMarkObject(d, go, JAVA_FALSE); } - gp = gp->nextFresh[lane]; } + gp = atomic_load_explicit(&gp->nextAll, memory_order_acquire); } gcMarkDrain(d); } @@ -1365,6 +1382,11 @@ void codenameOneGCMark() { } if(n > 0) gcMarkDrain(d); } +#if defined(CN1_GRACE_AUDIT) && !defined(CN1_DISABLE_BIBOP) + // QA builds only: right before the sweep, verify the grace pass reached every + // pre-mark fresh object; trace and report anything it missed (issue 5425). + cn1GraceAuditPreSweep(d); +#endif #if CN1_ADOPT_POLICY != 0 && !defined(CN1_DISABLE_BIBOP) // Marking (incl. grace, belt and SATB) is fully done. Register the objects matured // this cycle into allObjectsInHeap now -- single-threaded, locked, before the sweep. @@ -1886,7 +1908,6 @@ JAVA_INT java_lang_System_identityHashCode___java_lang_Object_R_int(CODENAME_ONE // struct CN1BibopPage is defined in cn1_globals.h (shared with the inlined bump). static CN1BibopPage* _Atomic bibopAllPages = 0; // registry head (atomic) -CN1BibopPage* _Atomic bibopFreshPages[2]; // alternating epoch stacks static _Atomic long long bibopAllPagesCount = 0; // grow-only registration count static CN1BibopPage* bibopFreePool = 0; // bibopMutex static CN1BibopPage* bibopPartialPool[CN1_BIBOP_NUM_CLASSES]; // bibopMutex @@ -1961,8 +1982,6 @@ static void cn1BibopFormatPage(CN1BibopPage* p, int ci) { p->freeList = 0; p->freeCount = 0; p->owned = JAVA_FALSE; - p->nextFresh[0] = 0; - p->nextFresh[1] = 0; #ifndef CN1_BIBOP_NO_FASTSWEEP p->gcAllocedSinceSweep = JAVA_FALSE; p->gcNeedsReclaim = JAVA_FALSE; @@ -1971,29 +1990,9 @@ static void cn1BibopFormatPage(CN1BibopPage* p, int ci) { atomic_store_explicit(&p->gcLastMarkedEpoch, 0, memory_order_relaxed); p->gcGraceEpoch = 0; #endif - atomic_store_explicit(&p->gcFreshEpoch[0], 0, memory_order_relaxed); - atomic_store_explicit(&p->gcFreshEpoch[1], 0, memory_order_relaxed); -} - -// Queue a page only on its first allocation in a GC epoch. The grace pass can -// now walk pages that actually received fresh objects instead of rescanning the -// grow-only registry on every collection. -void cn1BibopNoteFreshAllocation(CN1BibopPage* page, int epoch) { - int lane = epoch & 1; - int seen = atomic_load_explicit(&page->gcFreshEpoch[lane], memory_order_relaxed); - while(seen != epoch) { - if(atomic_compare_exchange_weak_explicit(&page->gcFreshEpoch[lane], &seen, epoch, - memory_order_acq_rel, - memory_order_relaxed)) { - CN1BibopPage* head = atomic_load_explicit(&bibopFreshPages[lane], memory_order_relaxed); - do { - page->nextFresh[lane] = head; - } while(!atomic_compare_exchange_weak_explicit(&bibopFreshPages[lane], &head, page, - memory_order_release, - memory_order_relaxed)); - return; - } - } +#ifdef CN1_GRACE_AUDIT + p->gcAuditSnapshot = 0; +#endif } void cn1BibopBeginGcCycle(void) { @@ -2005,6 +2004,18 @@ void cn1BibopBeginGcCycle(void) { // sustained allocator. bibopCycleAllocatedBytes = atomic_exchange_explicit(&bibopBytesSinceGc, 0, memory_order_acq_rel); +#ifdef CN1_GRACE_AUDIT + // QA builds only: snapshot every page's cursor at mark start. Slots below the + // snapshot existed before the grace pass ran, so a complete grace pass must + // have traced every one of them that is still fresh at pre-sweep time. + { + CN1BibopPage* ap = atomic_load_explicit(&bibopAllPages, memory_order_acquire); + while(ap != 0) { + ap->gcAuditSnapshot = atomic_load_explicit(&ap->bumpIndex, memory_order_acquire); + ap = atomic_load_explicit(&ap->nextAll, memory_order_acquire); + } + } +#endif } // Raw 64KB page memory comes from large arenas -- one posix_memalign per @@ -2493,11 +2504,6 @@ static JAVA_OBJECT cn1BibopAlloc(CODENAME_ONE_THREAD_STATE, int size, struct cla // publish the new cursor with release AFTER the slot (incl. its // mark) is fully initialized. atomic_store_explicit(&p->bumpIndex, bi + 1, memory_order_release); - int epoch = atomic_load_explicit(&bibopGcEpoch, memory_order_relaxed); - if(atomic_load_explicit(&p->gcFreshEpoch[epoch & 1], memory_order_relaxed) - != epoch) { - cn1BibopNoteFreshAllocation(p, epoch); - } #ifndef CN1_BIBOP_NO_FASTSWEEP p->gcAllocedSinceSweep = JAVA_TRUE; #endif @@ -2515,11 +2521,6 @@ static JAVA_OBJECT cn1BibopAlloc(CODENAME_ONE_THREAD_STATE, int size, struct cla } // free-list slot path cn1BibopInitSlot(threadStateData, o, size, parent); - int epoch = atomic_load_explicit(&bibopGcEpoch, memory_order_relaxed); - if(atomic_load_explicit(&p->gcFreshEpoch[epoch & 1], memory_order_relaxed) - != epoch) { - cn1BibopNoteFreshAllocation(p, epoch); - } #ifndef CN1_BIBOP_NO_FASTSWEEP p->gcAllocedSinceSweep = JAVA_TRUE; #endif @@ -2935,6 +2936,48 @@ static void cn1BibopSweep(CODENAME_ONE_THREAD_STATE) { classSlots, classLive); } +#ifdef CN1_GRACE_AUDIT +// QA builds only (grace-completeness gate, born from issue 5425): walk the FULL +// page registry right before the sweep, ignoring every pruning heuristic, and +// trace any slot that (a) existed before the grace pass ran (below the +// mark-start snapshot) and (b) is still fresh (gcMark == -1) with a published +// non-leaf class. missedFresh counts fresh objects the grace pass did not visit +// (small counts can be benign: a free-list slot re-allocated mid-mark below the +// snapshot after the grace pass ran is SATB-covered this cycle and re-traced +// next cycle). doomedChildren counts objects that became newly marked ONLY by +// tracing them -- ANY nonzero value is a collector bug: without this pass the +// sweep frees those children while a surviving fresh object still references +// them (dangling reference -> heap corruption). +static void cn1GraceAuditPreSweep(CODENAME_ONE_THREAD_STATE) { + long missedFresh = 0; + long beforeFresh = gcMarkNewObjectCount; + CN1BibopPage* gp = atomic_load_explicit(&bibopAllPages, memory_order_acquire); + while(gp != 0) { + int gn = gp->gcAuditSnapshot; + int bi = atomic_load_explicit(&gp->bumpIndex, memory_order_acquire); + if(gn > bi) gn = bi; // page was reformatted mid-cycle; stale snapshot + for(int gi = 0 ; gi < gn ; gi++) { + JAVA_OBJECT go = cn1BibopSlot(gp, gi); + if(__atomic_load_n(&go->__codenameOneGcMark, __ATOMIC_ACQUIRE) == -1 + && go->__codenameOneParentClsReference != 0 + && go->__codenameOneParentClsReference->markFunction != 0) { + missedFresh++; + gcMarkObject(threadStateData, go, JAVA_FALSE); + } + } + gp = atomic_load_explicit(&gp->nextAll, memory_order_acquire); + } + long freshMarked = gcMarkNewObjectCount - beforeFresh; + gcMarkDrain(threadStateData); + long recovered = gcMarkNewObjectCount - beforeFresh - freshMarked; + if(missedFresh > 0 || recovered > 0) { + fprintf(stderr, "[GRACE-AUDIT] epoch=%d missedFresh=%ld doomedChildren=%ld\n", + currentGcMarkValue, missedFresh, recovered); + fflush(stderr); + } +} +#endif + // (The overflow-rescan helpers cn1BibopRescanStart / cn1BibopRescanStep live // further down, next to gcMarkDrain, because they use the mark worklist.) diff --git a/vm/benchmarks/README.md b/vm/benchmarks/README.md index 973066a7f9f..6d47e06fce8 100644 --- a/vm/benchmarks/README.md +++ b/vm/benchmarks/README.md @@ -77,13 +77,48 @@ by the host JVM. The harness additionally requires runtime evidence that: - the BiBOP-only allocator thread graduated to the high-throughput pacing tier; - the 24 MiB baseline trigger grew under sustained survival; - a survivor-heavy size class activated the bounded legacy bypass/reprobe path; -- grace marking used the fresh-page set rather than the grow-only page registry. +- grace marking slot-scanned only pages flagged `gcAllocedSinceSweep` rather + than every slot of the grow-only page registry. It then measures best wall time and per-process peak RSS for the production adaptive collector against the legacy collector and a no-pacing diagnostic build. Those compile-time variants are QA controls only; applications ship one collector with the adaptive behavior enabled, not user-selectable GC flags. +## Grace-completeness audit (`-DCN1_GRACE_AUDIT`) + +The concurrent collector gives fresh (`gcMark == -1`) BiBOP objects one cycle +of sweep grace, so an object reachable ONLY through a surviving fresh object +must be traced by the mark's grace pass or the sweep frees it while it is +still referenced (the issue-5425 dictionary corruption). `-DCN1_GRACE_AUDIT` +compiles in a QA-only pre-sweep pass that snapshots every page's bump cursor +at mark start and, right before the sweep, full-walks the registry tracing +any pre-snapshot slot that is still fresh. It reports per cycle: + +- `missedFresh` — fresh slots the grace pass did not visit. Small counts can + be benign (a free-list slot re-allocated mid-mark, below the snapshot, after + the grace pass ran — SATB covers its links this cycle and the sticky + `gcAllocedSinceSweep` flag re-traces it next cycle). +- `doomedChildren` — objects that became marked ONLY by tracing those missed + slots. **Any nonzero value is a collector bug**: without the audit pass the + sweep would free each of them while a surviving object still references it. + +`GraceAudit` is the driver shaped to break queue/dedup-based grace schemes: +`System.gc()` is asynchronous, so a single thread allocates dropped fresh +nodes (each holding the only reference to an older object) WHILE the mark +runs, then goes quiet across the next cycle. Gate: + +```bash +./translate-and-build.sh GraceAudit target/grace-audit -DCN1_GRACE_AUDIT +./target/grace-audit # stderr must show doomedChildren=0 on every line +``` + +The fresh-page-stack grace scheme this audit was written against reported +100-370 missed slots and 100-250 doomed children per cycle; the +`gcAllocedSinceSweep`-pruned registry walk reports zero doomed across the +suite. `StormAB` (sustained single-thread storm) and `LoadLoop` (repeated +dictionary build/drop) are the matching wall-time/RSS A/B drivers. + `ClinitThrow` is a standalone liveness reproducer (not byte-identical to the host JVM by design — ParparVM's initialization-failure semantics differ): a throwing `` must release the class-init monitor so other threads diff --git a/vm/benchmarks/src/com/bench/GraceAudit.java b/vm/benchmarks/src/com/bench/GraceAudit.java new file mode 100644 index 00000000000..2c097e22337 --- /dev/null +++ b/vm/benchmarks/src/com/bench/GraceAudit.java @@ -0,0 +1,104 @@ +/* + * Copyright (c) 2012, Codename One and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. Codename One designates this + * particular file as subject to the "Classpath" exception as provided + * by Oracle in the LICENSE file that accompanied this code. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Codename One through http://www.codenameone.com/ if you + * need additional information or have any questions. + */ +package com.bench; + +/** + * Repro driver for issue 5425: a bursty small-object size class that goes + * quiet across GC cycle boundaries. Fresh objects allocated into a page AFTER + * that page's fresh-stack entry was consumed by the grace pass (same epoch) + * are never grace-traced if the size class receives no allocation in the next + * epoch before its grace pass. Run with -DCN1_GRACE_AUDIT to count them. + */ +public class GraceAudit { + static class Node { + Object a, b, c; + } + + static class Filler { + long a, b, c, d, e, f, g, h, i2, j, k, l; + } + + static Object sink; + static Object[] keep = new Object[256]; + static Object[] tmp = new Object[16]; + static long checksum; + + // deterministic LCG so runs are comparable without java.util.Random + static long seed = 42; + + static int next(int bound) { + seed = seed * 6364136223846793005L + 1442695040888963407L; + int v = (int) (seed >>> 33) % bound; + return v < 0 ? v + bound : v; + } + + public static void main(String[] args) throws Exception { + for (int round = 0; round < 120; round++) { + // Refill payload children (each will end up referenced ONLY by an + // unpublished fresh node). + for (int j = 0; j < 256; j++) { + if (keep[j] == null) { + Node k = new Node(); + k.b = k; + keep[j] = k; + } + } + // Kick a concurrent mark, then keep allocating fresh dropped nodes + // WHILE it runs: allocations landing after the grace pass consumed + // this page's fresh-stack entry stay unqueued for this epoch. + System.gc(); + for (int slice = 0; slice < 40; slice++) { + for (int i = 0; i < 8; i++) { + Node n = new Node(); + int j = (slice * 8 + i) & 255; + n.a = keep[j]; + keep[j] = null; + tmp[0] = n; + tmp[0] = null; + } + Thread.sleep(3); + } + // Quiet phase: no Node allocation at all across the next cycle, so + // the Node page is never re-queued; filler drives the byte trigger. + for (int i = 0; i < 120000; i++) { + Filler f = new Filler(); + f.b = i; + tmp[i & 15] = f; + } + System.gc(); + Thread.sleep(150); + } + for (int i = 0; i < 16; i++) { + if (tmp[i] != null) { + checksum++; + } + } + for (int i = 0; i < 256; i++) { + if (keep[i] != null) { + checksum += 3; + } + } + System.out.println("GRACE_AUDIT_DRIVER_DONE checksum=" + checksum + " sink=" + (sink == null)); + } +} diff --git a/vm/benchmarks/src/com/bench/LoadLoop.java b/vm/benchmarks/src/com/bench/LoadLoop.java new file mode 100644 index 00000000000..803dc7fcbf4 --- /dev/null +++ b/vm/benchmarks/src/com/bench/LoadLoop.java @@ -0,0 +1,57 @@ +/* + * Copyright (c) 2012, Codename One and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. Codename One designates this + * particular file as subject to the "Classpath" exception as provided + * by Oracle in the LICENSE file that accompanied this code. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Codename One through http://www.codenameone.com/ if you + * need additional information or have any questions. + */ +package com.bench; + +import java.util.Hashtable; + +/** + * Models the issue-5425 looping Dtest: repeatedly build a large dictionary + * (Hashtable of String -> small byte[]), drop the previous one, and report + * per-round wall time. Steady-state per-round time should be flat; growth + * means the collector degrades as loads repeat. + */ +public class LoadLoop { + static Hashtable dict; + + public static void main(String[] args) { + for (int round = 0; round < 12; round++) { + long start = System.currentTimeMillis(); + Hashtable h = new Hashtable(); + StringBuilder sb = new StringBuilder(); + for (int i = 0; i < 120000; i++) { + sb.setLength(0); + sb.append("word"); + sb.append(i); + String key = sb.toString(); + byte[] def = new byte[16 + (i & 31)]; + def[0] = (byte) i; + h.put(key, def); + } + dict = h; + long ms = System.currentTimeMillis() - start; + System.out.println("round " + round + " ms=" + ms + " size=" + h.size()); + } + System.out.println("LOAD_LOOP_DONE"); + } +} diff --git a/vm/benchmarks/src/com/bench/StormAB.java b/vm/benchmarks/src/com/bench/StormAB.java new file mode 100644 index 00000000000..c3e243e9d06 --- /dev/null +++ b/vm/benchmarks/src/com/bench/StormAB.java @@ -0,0 +1,57 @@ +/* + * Copyright (c) 2012, Codename One and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. Codename One designates this + * particular file as subject to the "Classpath" exception as provided + * by Oracle in the LICENSE file that accompanied this code. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Codename One through http://www.codenameone.com/ if you + * need additional information or have any questions. + */ +package com.bench; + +/** + * A/B driver for the issue-5425 pacing question: a single thread that + * sustains a small-object allocation storm (the Dtest dictionary-load shape) + * with a modest retained set. Compare wall time and peak RSS across VM + * revisions. + */ +public class StormAB { + static class Filler { + long a, b, c, d, e, f, g, h, i2, j, k, l; + } + + static Object[] tmp = new Object[16]; + static long checksum; + + public static void main(String[] args) { + long start = System.currentTimeMillis(); + for (int round = 0; round < 40; round++) { + for (int i = 0; i < 1000000; i++) { + Filler f = new Filler(); + f.a = i; + tmp[i & 15] = f; + } + } + for (int i = 0; i < 16; i++) { + if (tmp[i] != null) { + checksum++; + } + } + System.out.println("STORM_AB_DONE checksum=" + checksum + + " ms=" + (System.currentTimeMillis() - start)); + } +} From bc7b403510fc5ba81dc8bf1c9b611d2d07bdd3e0 Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Thu, 23 Jul 2026 14:57:28 +0300 Subject: [PATCH 2/4] Address review: relaxed atomics on the grace flag, audit docs, dead code - gcAllocedSinceSweep: the concurrent pair (mutator set on allocation / grace-pass read) now uses relaxed __atomic ops -- identical machine code to the previous plain access, but removes the formal C11 data race. Sweep/format keep plain access: they only touch retired or pooled pages no mutator holds. Comments spell out why relaxed suffices: pre-mark stores are ordered ahead of the grace pass by the mark-start thread pause, a store the pass can still miss is by definition a during-mark allocation (SATB-covered this cycle), and only the sweep clears the flag so a missed store is re-observed next cycle. - Document that the CN1_GRACE_AUDIT mark-start snapshot deliberately under-approximates: boundary slots racing mark start are during-mark allocations, the class the grace guarantee does not cover this cycle, and excluding them keeps the audit free of false positives. - GraceAudit: drop the unused LCG helper and vestigial sink field, reword stale fresh-stack phrasing to be scheme-agnostic. Revalidated: GraceAudit doomedChildren=0, run-gauntlet.sh GREEN (both stop modes), run-bibop-adaptive.sh GREEN (0.93x time / 0.60x RSS vs legacy), StormAB wall time unchanged. Co-Authored-By: Claude Fable 5 --- vm/ByteCodeTranslator/src/cn1_globals.h | 19 +++++++++---- vm/ByteCodeTranslator/src/cn1_globals.m | 26 ++++++++++++++---- vm/benchmarks/src/com/bench/GraceAudit.java | 30 +++++++++------------ 3 files changed, 47 insertions(+), 28 deletions(-) diff --git a/vm/ByteCodeTranslator/src/cn1_globals.h b/vm/ByteCodeTranslator/src/cn1_globals.h index d0adc7794be..024702a2f98 100644 --- a/vm/ByteCodeTranslator/src/cn1_globals.h +++ b/vm/ByteCodeTranslator/src/cn1_globals.h @@ -1304,7 +1304,11 @@ typedef struct CN1BibopPage { // page in O(1) -- without the per-slot walk -- whenever it can PROVE the page is // homogeneous. The fields are always present (so the struct layout is identical in // A/B builds); only the writes/reads are gated. See cn1BibopSweep for the proof. - JAVA_BOOLEAN gcAllocedSinceSweep; // any alloc into the page since last sweep/reset + JAVA_BOOLEAN gcAllocedSinceSweep; // any alloc into the page since last sweep/reset. + // Alloc paths set it / the grace pass reads it + // via relaxed __atomic ops (concurrent pair); + // sweep/format access it plain -- they only + // touch retired/pooled pages no mutator holds // (owner-thread single-writer; published to the // GC via the sweep-stack release-push) JAVA_BOOLEAN gcNeedsReclaim; // a survivor carries a finalizer or monitor -> @@ -1475,9 +1479,13 @@ static inline JAVA_OBJECT cn1BibopFastAlloc(CODENAME_ONE_THREAD_STATE, int size, // fresh mark==-1 (grace-candidate) slots as homogeneous, and the grace // pass slot-scans exactly the flagged pages ("-1 slot present" implies // "allocated into since last sweep" -- the sweep converts every -1 it - // sees). Single plain store to the already-hot page header; pre-mark - // stores are published to the GC by the mark-start thread sync. - p->gcAllocedSinceSweep = JAVA_TRUE; + // sees). Relaxed atomic (compiles to the same plain store on the hot + // path) because the GRACE PASS reads this concurrently: pre-mark stores + // are ordered ahead of it by the mark-start thread pause, and a store + // it can still miss is by definition a during-mark allocation -- + // SATB-covered this cycle and rescanned next cycle since only the + // sweep (never a concurrent phase) clears the flag. + __atomic_store_n(&p->gcAllocedSinceSweep, JAVA_TRUE, __ATOMIC_RELAXED); #endif CN1_BIBOP_ACCOUNT_BYTES(threadStateData, p->slotSize); // allocationsSinceLastGC / totalAllocations (the isHighFrequencyGC heuristic) @@ -1556,7 +1564,8 @@ static inline JAVA_OBJECT cn1BibopFastAllocNoZero(CODENAME_ONE_THREAD_STATE, int __atomic_store_n(&o->__codenameOneGcMark, -1, __ATOMIC_RELEASE); atomic_store_explicit(&p->bumpIndex, bi + 1, memory_order_release); #ifndef CN1_BIBOP_NO_FASTSWEEP - p->gcAllocedSinceSweep = JAVA_TRUE; + // relaxed: concurrently read by the grace pass (see cn1BibopFastAlloc) + __atomic_store_n(&p->gcAllocedSinceSweep, JAVA_TRUE, __ATOMIC_RELAXED); #endif CN1_BIBOP_ACCOUNT_BYTES(threadStateData, p->slotSize); return o; diff --git a/vm/ByteCodeTranslator/src/cn1_globals.m b/vm/ByteCodeTranslator/src/cn1_globals.m index 33de9a4367d..416c733a363 100644 --- a/vm/ByteCodeTranslator/src/cn1_globals.m +++ b/vm/ByteCodeTranslator/src/cn1_globals.m @@ -1293,8 +1293,13 @@ void codenameOneGCMark() { // page's last sweep (the sweep converts every -1 it sees to V), and EVERY // allocation path sets the flag before the mark-start thread sync publishes it // -- so a flag-FALSE page provably holds no fresh slot and is skipped without - // touching its slots. Only the sweep clears the flag, and it only processes - // retired (owner==0) pages, so no mutator/GC race on the flag exists. This + // touching its slots. The flag is read with a relaxed atomic (its writers + // mirror this; same machine code as the old plain access): pre-mark stores + // are ordered ahead of this pass by the mark-start thread pause, a store + // this read can still miss is by definition a during-mark allocation (SATB + // covers its links this cycle), and only the sweep -- never a phase running + // concurrently with mutators or with this pass -- clears the flag, so a + // missed store is re-observed next cycle. This // replaced a queue-of-fresh-pages scheme (issue 5425): queue-once-per-epoch // dedup left every allocation AFTER the queue was consumed (rest of the mark // plus the whole unbarriered inter-cycle window) untraced when the page was @@ -1304,7 +1309,7 @@ void codenameOneGCMark() { CN1BibopPage* gp = atomic_load_explicit(&bibopAllPages, memory_order_acquire); while(gp != 0) { #ifndef CN1_BIBOP_NO_FASTSWEEP - if(gp->gcAllocedSinceSweep == JAVA_FALSE) { + if(__atomic_load_n(&gp->gcAllocedSinceSweep, __ATOMIC_RELAXED) == JAVA_FALSE) { gp = atomic_load_explicit(&gp->nextAll, memory_order_acquire); continue; } @@ -2008,6 +2013,15 @@ void cn1BibopBeginGcCycle(void) { // QA builds only: snapshot every page's cursor at mark start. Slots below the // snapshot existed before the grace pass ran, so a complete grace pass must // have traced every one of them that is still fresh at pre-sweep time. + // Mutators are still running here, so a snapshot may trail a page's true + // cursor by the allocations racing mark start. That is INTENTIONAL + // under-approximation: boundary slots are during-mark allocations -- the + // class the grace guarantee does not cover this cycle (SATB + the sticky + // dirty flag cover them) -- and excluding them keeps the audit free of + // false positives. The audited set still spans every clearly-pre-mark slot, + // which is exactly the population the issue-5425 bug dropped; snapshotting + // later (after the pause) would widen coverage by only those boundary slots + // while making benign mid-mark allocations report as misses. { CN1BibopPage* ap = atomic_load_explicit(&bibopAllPages, memory_order_acquire); while(ap != 0) { @@ -2505,7 +2519,8 @@ static JAVA_OBJECT cn1BibopAlloc(CODENAME_ONE_THREAD_STATE, int size, struct cla // mark) is fully initialized. atomic_store_explicit(&p->bumpIndex, bi + 1, memory_order_release); #ifndef CN1_BIBOP_NO_FASTSWEEP - p->gcAllocedSinceSweep = JAVA_TRUE; + // relaxed: concurrently read by the grace pass (see cn1BibopFastAlloc) + __atomic_store_n(&p->gcAllocedSinceSweep, JAVA_TRUE, __ATOMIC_RELAXED); #endif CN1_BIBOP_ACCOUNT_BYTES(threadStateData, p->slotSize); return o; @@ -2522,7 +2537,8 @@ static JAVA_OBJECT cn1BibopAlloc(CODENAME_ONE_THREAD_STATE, int size, struct cla // free-list slot path cn1BibopInitSlot(threadStateData, o, size, parent); #ifndef CN1_BIBOP_NO_FASTSWEEP - p->gcAllocedSinceSweep = JAVA_TRUE; + // relaxed: concurrently read by the grace pass (see cn1BibopFastAlloc) + __atomic_store_n(&p->gcAllocedSinceSweep, JAVA_TRUE, __ATOMIC_RELAXED); #endif CN1_BIBOP_ACCOUNT_BYTES(threadStateData, p->slotSize); return o; diff --git a/vm/benchmarks/src/com/bench/GraceAudit.java b/vm/benchmarks/src/com/bench/GraceAudit.java index 2c097e22337..4304c3071e4 100644 --- a/vm/benchmarks/src/com/bench/GraceAudit.java +++ b/vm/benchmarks/src/com/bench/GraceAudit.java @@ -24,11 +24,14 @@ package com.bench; /** - * Repro driver for issue 5425: a bursty small-object size class that goes - * quiet across GC cycle boundaries. Fresh objects allocated into a page AFTER - * that page's fresh-stack entry was consumed by the grace pass (same epoch) - * are never grace-traced if the size class receives no allocation in the next - * epoch before its grace pass. Run with -DCN1_GRACE_AUDIT to count them. + * Grace-completeness gate born from issue 5425: a bursty small-object size + * class allocates fresh objects WHILE the concurrent mark runs (System.gc is + * asynchronous), each holding the only reference to an older object, then + * goes quiet across the next GC cycle. Any grace scheme that tracks "pages + * with fresh slots" incrementally must still trace those objects; the + * fresh-page-stack scheme this driver was written against dropped them and + * the sweep freed their children while still referenced. Run with + * -DCN1_GRACE_AUDIT: every reported doomedChildren value must be zero. */ public class GraceAudit { static class Node { @@ -39,20 +42,10 @@ static class Filler { long a, b, c, d, e, f, g, h, i2, j, k, l; } - static Object sink; static Object[] keep = new Object[256]; static Object[] tmp = new Object[16]; static long checksum; - // deterministic LCG so runs are comparable without java.util.Random - static long seed = 42; - - static int next(int bound) { - seed = seed * 6364136223846793005L + 1442695040888963407L; - int v = (int) (seed >>> 33) % bound; - return v < 0 ? v + bound : v; - } - public static void main(String[] args) throws Exception { for (int round = 0; round < 120; round++) { // Refill payload children (each will end up referenced ONLY by an @@ -65,8 +58,9 @@ public static void main(String[] args) throws Exception { } } // Kick a concurrent mark, then keep allocating fresh dropped nodes - // WHILE it runs: allocations landing after the grace pass consumed - // this page's fresh-stack entry stay unqueued for this epoch. + // WHILE it runs: some land after the grace pass already visited (or + // dismissed) this page, the window where queue/dedup-based grace + // schemes lose track of fresh objects. System.gc(); for (int slice = 0; slice < 40; slice++) { for (int i = 0; i < 8; i++) { @@ -99,6 +93,6 @@ public static void main(String[] args) throws Exception { checksum += 3; } } - System.out.println("GRACE_AUDIT_DRIVER_DONE checksum=" + checksum + " sink=" + (sink == null)); + System.out.println("GRACE_AUDIT_DRIVER_DONE checksum=" + checksum); } } From 1c7f68bdd04333d2bb94f00d0140e441f144965a Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Thu, 23 Jul 2026 15:37:05 +0300 Subject: [PATCH 3/4] Address review: atomics on the registry-visible reformat path cn1BibopAcquirePage reformats a FREE-pool page that is already in the page registry, on a mutator thread, possibly during a concurrent mark -- so its resets of gcAllocedSinceSweep (and gcAuditSnapshot in audit builds) can overlap the grace pass / audit reads. Convert that pair to relaxed __atomic ops: the flag store is value-identical (the sweep already reset it before pooling), so this only removes the formal race. The new-page format path is untouched by observers (it runs before registry insertion) but shares the same code, and format is cold either way. The audit snapshot field now uses relaxed atomics at all three sites (format reset, mark-start snapshot, pre-sweep read). The sweep's three plain accesses remain intentionally plain and the header comment now states the precise reason: the sweep runs on the GC thread after mark completes (program-ordered against the grace pass) on retired pages no mutator holds, and the pool-handoff mutex orders it against the next owner's stores. Revalidated: GraceAudit doomedChildren=0, gauntlet GREEN in both stop modes, run-bibop-adaptive GREEN (0.95x time / 0.61x RSS vs legacy), StormAB unchanged. Co-Authored-By: Claude Fable 5 --- vm/ByteCodeTranslator/src/cn1_globals.h | 19 ++++++++++++++----- vm/ByteCodeTranslator/src/cn1_globals.m | 19 +++++++++++++++---- 2 files changed, 29 insertions(+), 9 deletions(-) diff --git a/vm/ByteCodeTranslator/src/cn1_globals.h b/vm/ByteCodeTranslator/src/cn1_globals.h index 024702a2f98..77ff2afdec9 100644 --- a/vm/ByteCodeTranslator/src/cn1_globals.h +++ b/vm/ByteCodeTranslator/src/cn1_globals.h @@ -1305,10 +1305,15 @@ typedef struct CN1BibopPage { // homogeneous. The fields are always present (so the struct layout is identical in // A/B builds); only the writes/reads are gated. See cn1BibopSweep for the proof. JAVA_BOOLEAN gcAllocedSinceSweep; // any alloc into the page since last sweep/reset. - // Alloc paths set it / the grace pass reads it - // via relaxed __atomic ops (concurrent pair); - // sweep/format access it plain -- they only - // touch retired/pooled pages no mutator holds + // Alloc paths set it, the FREE-pool reformat + // resets it, and the grace pass reads it, all + // via relaxed __atomic ops (those can overlap + // a concurrent mark). Only the sweep accesses + // it plain: it runs on the GC thread after + // mark (program-ordered vs the grace pass) on + // retired pages no mutator holds, and the + // pool handoff mutex orders it vs the next + // owner's stores // (owner-thread single-writer; published to the // GC via the sweep-stack release-push) JAVA_BOOLEAN gcNeedsReclaim; // a survivor carries a finalizer or monitor -> @@ -1339,7 +1344,11 @@ typedef struct CN1BibopPage { int gcGraceEpoch; // upper bound on survivor epochs as of the last // full walk (GC-thread only) #ifdef CN1_GRACE_AUDIT - int gcAuditSnapshot; // QA builds only: bumpIndex at mark start + int gcAuditSnapshot; // QA builds only: bumpIndex at mark start. + // Relaxed __atomic access everywhere -- the + // GC writes/reads it during marking while a + // mutator can reformat the page from the + // FREE pool #endif } CN1BibopPage; diff --git a/vm/ByteCodeTranslator/src/cn1_globals.m b/vm/ByteCodeTranslator/src/cn1_globals.m index 416c733a363..6103caf1b8d 100644 --- a/vm/ByteCodeTranslator/src/cn1_globals.m +++ b/vm/ByteCodeTranslator/src/cn1_globals.m @@ -1988,7 +1988,14 @@ static void cn1BibopFormatPage(CN1BibopPage* p, int ci) { p->freeCount = 0; p->owned = JAVA_FALSE; #ifndef CN1_BIBOP_NO_FASTSWEEP - p->gcAllocedSinceSweep = JAVA_FALSE; + // Relaxed atomic, not plain: the acquire-path format (cn1BibopAcquirePage) + // reformats a FREE-pool page that is already in the registry, on a mutator + // thread, possibly while the grace pass concurrently reads this flag. The + // store is value-identical (the sweep already reset the flag before pooling + // the page) so any interleaving reads FALSE; the atomic just keeps the + // concurrent read/write pair well-defined. The new-page path formats before + // registry insertion, where nothing can observe the page. + __atomic_store_n(&p->gcAllocedSinceSweep, JAVA_FALSE, __ATOMIC_RELAXED); p->gcNeedsReclaim = JAVA_FALSE; p->gcHasMonitors = JAVA_FALSE; p->gcHasAdopted = JAVA_FALSE; @@ -1996,7 +2003,9 @@ static void cn1BibopFormatPage(CN1BibopPage* p, int ci) { p->gcGraceEpoch = 0; #endif #ifdef CN1_GRACE_AUDIT - p->gcAuditSnapshot = 0; + // Same registry-visible reformat race as the flag above: the GC thread reads + // and rewrites this field during marking in audit builds. + __atomic_store_n(&p->gcAuditSnapshot, 0, __ATOMIC_RELAXED); #endif } @@ -2025,7 +2034,9 @@ void cn1BibopBeginGcCycle(void) { { CN1BibopPage* ap = atomic_load_explicit(&bibopAllPages, memory_order_acquire); while(ap != 0) { - ap->gcAuditSnapshot = atomic_load_explicit(&ap->bumpIndex, memory_order_acquire); + __atomic_store_n(&ap->gcAuditSnapshot, + atomic_load_explicit(&ap->bumpIndex, memory_order_acquire), + __ATOMIC_RELAXED); ap = atomic_load_explicit(&ap->nextAll, memory_order_acquire); } } @@ -2969,7 +2980,7 @@ static void cn1GraceAuditPreSweep(CODENAME_ONE_THREAD_STATE) { long beforeFresh = gcMarkNewObjectCount; CN1BibopPage* gp = atomic_load_explicit(&bibopAllPages, memory_order_acquire); while(gp != 0) { - int gn = gp->gcAuditSnapshot; + int gn = __atomic_load_n(&gp->gcAuditSnapshot, __ATOMIC_RELAXED); int bi = atomic_load_explicit(&gp->bumpIndex, memory_order_acquire); if(gn > bi) gn = bi; // page was reformatted mid-cycle; stale snapshot for(int gi = 0 ; gi < gn ; gi++) { From a5925103820d79b4989d33045a918244ad572238 Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Thu, 23 Jul 2026 15:46:42 +0300 Subject: [PATCH 4/4] Address review: clarify that grace-audit silence is success The audit prints a [GRACE-AUDIT] line only when a cycle misses something; the README implied a line per cycle. State that an empty stderr is a fully clean run and the gate is that no line reports doomedChildren != 0. Co-Authored-By: Claude Fable 5 --- vm/benchmarks/README.md | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/vm/benchmarks/README.md b/vm/benchmarks/README.md index 6d47e06fce8..81f2ff419ed 100644 --- a/vm/benchmarks/README.md +++ b/vm/benchmarks/README.md @@ -93,7 +93,9 @@ must be traced by the mark's grace pass or the sweep frees it while it is still referenced (the issue-5425 dictionary corruption). `-DCN1_GRACE_AUDIT` compiles in a QA-only pre-sweep pass that snapshots every page's bump cursor at mark start and, right before the sweep, full-walks the registry tracing -any pre-snapshot slot that is still fresh. It reports per cycle: +any pre-snapshot slot that is still fresh. A cycle in which the grace pass +missed nothing prints nothing -- **silence is success**. A cycle with a miss +prints one `[GRACE-AUDIT]` line reporting: - `missedFresh` — fresh slots the grace pass did not visit. Small counts can be benign (a free-list slot re-allocated mid-mark, below the snapshot, after @@ -110,7 +112,9 @@ runs, then goes quiet across the next cycle. Gate: ```bash ./translate-and-build.sh GraceAudit target/grace-audit -DCN1_GRACE_AUDIT -./target/grace-audit # stderr must show doomedChildren=0 on every line +./target/grace-audit # PASS: no line reports doomedChildren != 0 + # (an empty stderr is a fully clean run; benign + # missedFresh-only lines may still appear) ``` The fresh-page-stack grace scheme this audit was written against reported