From 6c1bedc7397f8b256671bfe586288985443a60d4 Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Tue, 25 Aug 2026 06:25:09 +0300 Subject: [PATCH 01/21] Stop the SATB barrier logging fresh references (issue #5537) Four merged fixes (#5540, #5563, #5573, #5585) each named a mechanism and the reporter's build still climbed 500MB to 5GB in five minutes on the iOS Simulator with a live set of a few hundred objects, GC pauses lengthening until they were continuous. The reason none of them settled it is structural: every GC workload in vm/tests measures a PEAK under load, and a heap that grows forever at a modest rate passes "peak < 2GB over 50 rounds" without difficulty. Nothing measured whether the VM ever gives the memory back. The instrument comes first, and it is what found this. -DCN1_GC_CONFORM adds a probe that PARTITIONS the footprint -- resident pages, legacy blocks, the legacy table, the allocator's side tables -- and prints the residual the four do not account for, plus a per-phase breakdown of the mark. It deliberately is not CN1_GC_VERIFY: that flag forces cn1BibopReleaseOffset() to 0, which compiles out the page-release path, the major sweep and every madvise call, so the paths a footprint investigation is about cannot be measured in a verifier build. It changes no allocator behaviour, and the emitters are gated at RUNTIME on CN1_GC_PROBE so probe-on and probe-off are the same binary. On the reported shape -- a deep game-tree search on four workers, tiny short-lived reference-carrying objects, a constant live set -- it named the cost immediately: of a 327ms mark, 282ms was SATB termination, draining 2,718,448 logged references in one cycle. Of those, 2,718,413 were references to FRESH objects. A mark == -1 object was allocated after the cycle's snapshot was taken, so it is not in the snapshot the barrier exists to preserve, and both sweeps keep it anyway -- the grace rule promotes a fresh slot to the current epoch instead of freeing it. Its own outgoing references to non-fresh objects are still logged by the same barrier as they are stored, so nothing reachable only through a fresh object is lost, which is the hazard the insertion half was added for. Without that filter the log is a feedback loop rather than a cost: its size is mutation rate times cycle duration, draining it is part of the cycle, so a longer cycle logs more and logging more lengthens the cycle. Both reported symptoms fall out of the one loop -- the footprint climbs because the collector never catches up, and the pauses climb because the log it has to drain keeps growing. Measured, three repetitions each, interleaved in one session: footprint drift before 306,684 / 241,493 / 224,237 KB/min after -31,430 / 36,866 / 18,993 KB/min (noise around zero) page count before 3,947 -> 5,995 over 40s and still climbing after flat at 11,687 for 40s mark time before 38ms -> 180ms; after 9-68ms, no trend under a simulated 1.4GB per-process ceiling: 3.5x the search throughput (237.8M nodes vs 67.6M), peak 1271MB, no kill Throughput, interleaved A/B, checksums bit-identical: geomean 0.944 -- 5.6% faster overall, objectAllocation 1.73x (56.3ms -> 32.5ms). The barrier was that expensive. -DCN1_SATB_LOG_FRESH restores the old behaviour for A/B. GcSteadyStateIntegrationTest is the gate. It asserts the SATB log stays sized by the live set rather than by the allocation rate, and that the page heap stops growing in the second half of the run; then it rebuilds with -DCN1_SATB_LOG_FRESH and requires both to fail, so it cannot go inert. Two pre-existing defects found on the way and fixed here: * -DCN1_DISABLE_CONSERVATIVE_GC_ROOTS, the revert path cn1_globals.h documents, did not compile at all: the grace passes use CN1_GC_TRUSTED_BEGIN/END/SUSPEND/ RESUME unconditionally and those are only defined with conservative roots on. No-op definitions restore it, which is what makes it usable as an A/B arm. * [GC-INSTR] allocs= is not an allocation count -- CN1_FAST_NEW's inlined bump path never reaches that counter, so on a small-object workload it understates allocation by orders of magnitude. Renamed to outOfLineAllocs= with a note. Verified: 520 vm/tests non-benchmark tests green; all six GC benchmark tests green; run-gc-verify.sh green including both fault self-tests; run-gauntlet.sh green with every checksum matching; grace audit reports doomedChildren=0 with and without the filter; and the probe compiles across nine ablation flag combinations. Not addressed, and pre-existing: under a per-process ceiling the process still rides to ceiling-minus-64MB, which #5585 flagged as open. That is now a bounded plateau rather than unbounded growth, but the margin is thin on a device where the renderer shares the same budget. Co-Authored-By: Claude Opus 5 (1M context) --- CLAUDE.md | 56 ++ vm/ByteCodeTranslator/src/cn1_globals.h | 12 + vm/ByteCodeTranslator/src/cn1_globals.m | 515 ++++++++++++++++++ vm/ByteCodeTranslator/src/nativeMethods.m | 20 +- .../src/com/bench/GcSteadyState.java | 228 ++++++++ .../GcSteadyStateIntegrationTest.java | 388 +++++++++++++ .../tools/translator/GcSteadyStateApp.java | 195 +++++++ 7 files changed, 1413 insertions(+), 1 deletion(-) create mode 100644 vm/benchmarks/src/com/bench/GcSteadyState.java create mode 100644 vm/tests/src/test/java/com/codename1/tools/translator/GcSteadyStateIntegrationTest.java create mode 100644 vm/tests/src/test/resources/com/codename1/tools/translator/GcSteadyStateApp.java diff --git a/CLAUDE.md b/CLAUDE.md index 3ec43bd914a..6a0dfc3a5ea 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -248,6 +248,62 @@ scripts/check-cast-semantics.sh scripts/check-cast-semantics.sh --write-baseline # after fixing a method ``` +### GC memory: measure the steady state, not the peak + +Every GC workload in `vm/tests` measures a **peak under load**, and a peak cannot express +the failure mode issue #5537 reported: a heap that grows forever at a modest rate passes +`GcOverflowSpiralIntegrationTest`'s "peak < 2GB over 50 rounds" without difficulty. When +investigating memory, the question to ask is whether the growth **stops**. + +`-DCN1_GC_CONFORM` adds the instrument for that. Unlike `CN1_GC_VERIFY` it changes **no** +allocator behaviour -- which matters, because `CN1_GC_VERIFY` forces +`cn1BibopReleaseOffset()` to return 0 and therefore compiles out the page-release path, +the major sweep and every `madvise` call. Those are exactly the paths a footprint +investigation is about, so they cannot be measured in a verifier build. + +Build with `-DCN1_GC_CONFORM` and set `CN1_GC_PROBE=` at runtime (every nth cycle; +unset = off, so probe-on and probe-off are the same binary). Two emitters: + +- `[GCPROBE]` per cycle, on the GC thread after the sweep. It **partitions the + footprint** -- `residentPgKb`, `legBlockKb`, `legTableKb`, `sideKb` -- and prints the + residual `residKb` that the four do not account for. Read the residual first: if it + carries the drift, the growth is not in the Java heap and every heap hypothesis is dead + in one run. It also breaks the mark down by phase (`waitMs stackMs tdrainMs migrateMs + satbMs poolMs graceMs drainMs`), which is what localises a lengthening pause to a + subsystem rather than to a guess. +- `[GCPROBE-T]` once a second, atomics only. This is the series that survives a collector + that has stopped finishing cycles -- the state in which the per-cycle emitter goes + silent, and the state being investigated. + +`vm/benchmarks/src/com/bench/GcSteadyState.java` is the churn workload, parameterised +through the environment (`CN1_WL_SECONDS`, `CN1_WL_THREADS`, `CN1_WL_DEPTH`, +`CN1_WL_BRANCH`, `CN1_WL_SLEEP_MS`, ...) because the clean target's generated `main()` +passes `JAVA_NULL` for args. Sweeping `CN1_WL_SLEEP_MS` over `{0,1,10,100,1000}` is the +cheapest discriminator between a rate problem and a retention problem, and needs no +rebuild. + +Every GC ablation is a **compile-time** macro, so each A/B arm is a rebuild; use +`vm/benchmarks/translate-and-build.sh` with `CN1_BENCH_CFLAGS` (see `ab-adopt.sh`), which +is ~15s per arm. Useful arms: `-DCN1_ADOPT_POLICY=0`, `-DCN1_DISABLE_BIBOP`, +`-DCN1_BIBOP_NO_FASTSWEEP`, `-DCN1_BIBOP_NO_PAGE_RELEASE`, `-DCN1_DISABLE_SATB`, +`-DCN1_SATB_LOG_FRESH`, and `-DCN1_DISABLE_CONSERVATIVE_GC_ROOTS` (which also needs the +translator run with `-Dcn1.frameless.objects=false -Dcn1.frameless.instance=false`, so it +is confounded with a codegen change -- make it the last arm, not the first). + +Two traps worth knowing before believing a number: + +- **`[GC-INSTR] outOfLineAllocs=` is not an allocation count.** `CN1_FAST_NEW`'s inlined + bump path never reaches that counter, so on a small-object workload it understates + allocation by orders of magnitude. `CN1_ALLOC_CENSUS` counts at every entry point. +- **Physical footprint moves with the host's memory pressure.** A/B by interleaving both + builds inside one session on a non-swapping host; two soaks an hour apart measure the + machine (see the note at `vm/JavaAPI/src/java/lang/System.java`). + +`GcSteadyStateIntegrationTest` is the gate. It asserts that the SATB log stays sized by +the live set rather than by the allocation rate, and that the page heap stops growing in +the second half of the run; then it rebuilds with `-DCN1_SATB_LOG_FRESH` and **requires +both assertions to fail**, so the gate cannot go inert. + ### Working with Native Code Platform-specific native code locations: diff --git a/vm/ByteCodeTranslator/src/cn1_globals.h b/vm/ByteCodeTranslator/src/cn1_globals.h index aed96f7ba37..231157cfbf1 100644 --- a/vm/ByteCodeTranslator/src/cn1_globals.h +++ b/vm/ByteCodeTranslator/src/cn1_globals.h @@ -67,6 +67,18 @@ #define CN1_CONSERVATIVE_GC_ROOTS #endif +// CN1_GC_CONFORM: the footprint probe and (later) the structural conformance verifier +// for issue 5537. UNLIKE CN1_GC_VERIFY it changes no allocator behaviour -- in particular +// it does NOT force cn1BibopReleaseOffset() to 0, so the page-release and major-sweep +// paths that CN1_GC_VERIFY compiles out entirely are live and measurable under it. +// It subsumes CN1_GC_INSTRUMENT because the probe reports that flag's counters, and +// those counters do not exist without it. +#ifdef CN1_GC_CONFORM +#ifndef CN1_GC_INSTRUMENT +#define CN1_GC_INSTRUMENT +#endif +#endif + #ifdef CN1_CONSERVATIVE_GC_ROOTS // PHASE 3b: conservative native-stack scanning as a REAL GC root source. Needs // signal-based universal thread stopping (sig_atomic_t / sigaction / ucontext). diff --git a/vm/ByteCodeTranslator/src/cn1_globals.m b/vm/ByteCodeTranslator/src/cn1_globals.m index 7256c221003..5b4cef8e655 100644 --- a/vm/ByteCodeTranslator/src/cn1_globals.m +++ b/vm/ByteCodeTranslator/src/cn1_globals.m @@ -630,6 +630,57 @@ static void cn1ReportGcOverflow(void) { triggerKb); } +// ---- CN1_GC_CONFORM counters (issue 5537) ------------------------------------------- +// The question these exist to answer is "which partition of the footprint is growing", +// which no existing counter can express: [GC-INSTR] allocs= is bumped only in +// codenameOneGcMalloc, so the inlined BiBOP bump path -- almost every object in a +// small-object workload -- never reaches it, and cn1HeapAccounting samples at four +// fixed cycles, which cannot see a drift that takes minutes. +// +// All of them are compiled out without -DCN1_GC_CONFORM so a shipping build is +// byte-identical. The one on a genuinely hot path (the per-word conservative scan) +// accumulates in locals and does ONE atomic add per range, not one per word. +#ifdef CN1_GC_CONFORM +// Per-cycle phase accounting for the mark. "The pauses get longer" is the reported +// symptom; without a breakdown it is impossible to tell a collector that is walking a +// bigger LIVE graph from one whose fixed per-cycle overhead grows with the HEAP -- and +// only the second is a defect. +long long cn1GcSnapNs = 0; // rebuilding the conservative-root snapshots (incl. the qsort) +long long cn1GcGraceNs = 0; // both grace passes: O(all pages) + O(legacy table), every cycle +long long cn1GcDrainNs = 0; // the root drain +long long cn1GcWaitNs = 0; // waiting for mutators to reach a safepoint +long long cn1GcStackNs = 0; // scanning one thread's stacks (precise + conservative) +long long cn1GcTDrainNs = 0; // the PER-THREAD drain inside the root loop +long long cn1GcMigrateNs = 0; // migrating pendingHeapAllocations into allObjectsInHeap +long cn1GcMigrated = 0; // ...and how many objects that was +long long cn1GcSatbNs = 0; // SATB termination: take-and-drain to a fixpoint +long cn1GcSatbEntries = 0; // ...and how many logged references that processed +long long cn1GcPoolNs = 0; // the constant-pool root scan +// How much of the SATB log was already dead weight when it was written, and how much of +// it was dead weight by the time it was read. The barrier can only filter the first; the +// gap between them is what a bigger batch window or a dedup would buy. +_Atomic long cn1GcSatbAlready = 0; // enqueued while ALREADY at the current epoch +_Atomic long cn1GcSatbFresh = 0; // enqueued while mark == -1 (fresh; grace covers it) +long cn1GcSatbDrainAlready = 0; // already at the current epoch by the time it drained +static long long cn1GcNowNs(void) { + struct timespec t; + clock_gettime(CLOCK_MONOTONIC, &t); + return (long long)t.tv_sec * 1000000000LL + (long long)t.tv_nsec; +} +_Atomic long cn1GcMaturedTotal = 0; // objects graduated into the legacy heap +_Atomic long cn1GcMaturedPages = 0; // pages that gcHasAdopted has ever stuck to +_Atomic long cn1GcMaturedDied = 0; // matured objects the legacy sweep reverted to -3 +_Atomic long cn1GcStaleSkips = 0; // whole sweeps skipped on a stale page index +_Atomic long cn1MonitorEntries = 0; // live entries in the monitor side table +_Atomic long long cn1ConsWords = 0; // aligned words read by the conservative scan +_Atomic long long cn1ConsResolved = 0; // ...that resolved to a heap object +// ...that resolved to an object of a STRICTLY OLDER epoch, i.e. a word that revived +// something the precise roots had not reached yet. It over-counts (a precise root later +// in the same cycle would have marked it anyway), so it is an UPPER BOUND on conservative +// resurrection -- which is what makes it able to refute cheaply and not to confirm. +_Atomic long long cn1ConsFirstMarks = 0; +#endif + static void cn1ReportLowMemoryParks(void) { if(!cn1LowMemoryTraceOn()) { return; @@ -1234,6 +1285,19 @@ static void cn1MatureObject(JAVA_OBJECT obj) { // Sticky-flag the host page so its slots always take the full per-slot sweep walk // (which skips live -4 slots) instead of the O(1) page reset, which would recycle this // still-live object's memory out from under the legacy collector. +#ifdef CN1_GC_CONFORM + { + CN1BibopPage* __mp = (CN1BibopPage*)(((uintptr_t)obj) & ~((uintptr_t)CN1_BIBOP_PAGE_SIZE - 1)); + atomic_fetch_add_explicit(&cn1GcMaturedTotal, 1, memory_order_relaxed); + // Count the page only on the FALSE->TRUE transition: gcHasAdopted is sticky, so + // counting every maturation would report maturations, not pinned pages, and the + // ratio pinnedPages/pagesRegistered is the whole point (a pinned page can never + // take the O(1) reclaim shortcut again). + if(__mp->gcHasAdopted == JAVA_FALSE) { + atomic_fetch_add_explicit(&cn1GcMaturedPages, 1, memory_order_relaxed); + } + } +#endif ((CN1BibopPage*)(((uintptr_t)obj) & ~((uintptr_t)CN1_BIBOP_PAGE_SIZE - 1)))->gcHasAdopted = JAVA_TRUE; // Buffer for post-mark registration (NOT placeObjectInHeapCollection here -- see above). pthread_mutex_lock(&gcAdoptMutex); @@ -1440,6 +1504,44 @@ static void cn1DrainDeadThreadPending() { #endif void cn1SatbEnqueue(JAVA_OBJECT old) { +#ifndef CN1_SATB_LOG_FRESH + // FRESH-REFERENCE FILTER (issue 5537). + // + // A mark == -1 object was allocated after this cycle's snapshot was taken, so it is + // not IN the snapshot this barrier exists to preserve, and the sweep keeps it anyway: + // both halves promote a fresh slot to the current epoch instead of freeing it (the + // grace rule -- cn1BibopSweep's `m == -1` branch and codenameOneGCSweep's `else`). + // Its own outgoing references to NON-fresh objects are still logged by this same + // barrier as they are stored, so nothing reachable only through a fresh object is + // lost -- which is the hazard the insertion half was added for. + // + // Without the filter the log is a positive feedback loop rather than a cost. Measured + // on the game-tree shape at 4 threads: essentially the ENTIRE log was fresh + // references (2,718,413 of 2,718,448 entries in one cycle), and draining them was + // 282ms of a 327ms mark. A longer cycle logs more, and logging more lengthens the + // cycle, so mark time and footprint climb together until the collector is continuous + // -- exactly the reported symptom pair. + // + // Racy read of a GC-thread-owned epoch, deliberately: a stale value can only make the + // test fail and log something that did not need logging, which is the conservative + // direction. A free slot's sentinel mark is neither value, so it still reaches the + // log and is rejected by gcMarkObject exactly as before. + if(old->__codenameOneGcMark == -1) { + return; + } +#endif +#ifdef CN1_GC_CONFORM + { + // Racy read of a GC-thread-owned value on purpose: this is a census, and a stale + // read can only misclassify, never corrupt. + int __m = old->__codenameOneGcMark; + if(__m == currentGcMarkValue) { + atomic_fetch_add_explicit(&cn1GcSatbAlready, 1, memory_order_relaxed); + } else if(__m == -1) { + atomic_fetch_add_explicit(&cn1GcSatbFresh, 1, memory_order_relaxed); + } + } +#endif pthread_mutex_lock(&gcSatbMutex); if(gcSatbTop >= gcSatbCap) { long ncap = gcSatbCap ? gcSatbCap * 2 : 8192; @@ -1508,6 +1610,15 @@ static long cn1SatbTake(JAVA_OBJECT** out) { // follows child words out of mark functions -- so the two really are independent here. #define CN1_GC_TRUSTED_SUSPEND() cn1GcTrustedRoots = 0 #define CN1_GC_TRUSTED_RESUME() cn1GcTrustedRoots = 1 +#else +// Without conservative roots there is no resolve guard to bypass, so trust is +// meaningless -- but the grace passes use these unconditionally, so the documented +// -DCN1_DISABLE_CONSERVATIVE_GC_ROOTS revert path (cn1_globals.h) did not compile at +// all. No-ops here restore it, which is what makes it usable as an A/B arm. +#define CN1_GC_TRUSTED_BEGIN() do { } while(0) +#define CN1_GC_TRUSTED_END() do { } while(0) +#define CN1_GC_TRUSTED_SUSPEND() do { } while(0) +#define CN1_GC_TRUSTED_RESUME() do { } while(0) #endif #ifdef CN1_GC_VERIFY @@ -1650,6 +1761,10 @@ void codenameOneGCMark() { // and would otherwise satisfy useCoop with that stale SP forever, // silently skipping the live region below it (missed roots -> UAF). t->gcParkCaptured = JAVA_FALSE; +#endif +#ifdef CN1_GC_CONFORM + long long __wt0 = cn1GcNowNs(); + int __wtActive = 1; #endif // wait for the thread to pause so we can traverse its stack but not for native threads where // we don't have much control and who barely call into Java anyway @@ -1684,6 +1799,9 @@ void codenameOneGCMark() { // SIGSEGV or a libmalloc abort that wedges the VM). If the slot no // longer holds this thread it died and markDeadThread already migrated // everything under this same lock; skip. +#ifdef CN1_GC_CONFORM + long long __mg0 = cn1GcNowNs(); +#endif lockCriticalSection(); if(allThreads[iter] == t) { if (!t->lightweightThread) { @@ -1697,6 +1815,9 @@ void codenameOneGCMark() { if(obj) { t->pendingHeapAllocations[heapTrav] = 0; placeObjectInHeapCollection(obj); +#ifdef CN1_GC_CONFORM + cn1GcMigrated++; +#endif } } if (!t->lightweightThread) { @@ -1704,6 +1825,9 @@ void codenameOneGCMark() { } } unlockCriticalSection(); +#ifdef CN1_GC_CONFORM + cn1GcMigrateNs += cn1GcNowNs() - __mg0; +#endif // this is a thread that allocates a lot and might demolish RAM. We will hold it until the sweep is finished... @@ -1789,14 +1913,25 @@ void codenameOneGCMark() { // covered: the conservative scan walks the WHOLE native stack regardless. #ifdef CN1_GC_VERIFY { extern const char* cn1GcMarkPhase; cn1GcMarkPhase = "conservative-native-stack"; } +#endif +#ifdef CN1_GC_CONFORM + { long long __s0 = cn1GcNowNs(); cn1GcStackNs -= __s0; } #endif cn1GcScanThreadNativeStack(d, t); +#ifdef CN1_GC_CONFORM + cn1GcStackNs += cn1GcNowNs(); +#endif #ifdef CN1_CONSERVATIVE_GC_SELFCHECK cn1GcSelfCheckThreadStack(t, stackSize); #endif #endif #ifdef CN1_GC_VERIFY { extern const char* cn1GcMarkPhase; cn1GcMarkPhase = "statics"; } +#endif +#ifdef CN1_GC_CONFORM + // The spin above is over by the time control reaches here, whatever path + // it took, so this is the honest close for the safepoint wait. + if(__wtActive) { cn1GcWaitNs += cn1GcNowNs() - __wt0; __wtActive = 0; } #endif markStatics(d); // Drain the worklist before unblocking the thread so that every object @@ -1816,7 +1951,11 @@ void codenameOneGCMark() { // and gcMarkDrainParallel does not return until the entire reachable set // is marked -- it just marks it faster. With a single configured marker // it degrades to the serial gcMarkDrain and is byte-for-byte identical. +#ifdef CN1_GC_CONFORM + { long long __t0 = cn1GcNowNs(); gcMarkDrainParallel(d); cn1GcTDrainNs += cn1GcNowNs() - __t0; } +#else gcMarkDrainParallel(d); +#endif if(!agressiveAllocator) { t->threadBlockedByGC = JAVA_FALSE; } else { @@ -1831,6 +1970,9 @@ void codenameOneGCMark() { // since they are immutable this probably doesn't need as much sync as the statics... #ifdef CN1_GC_VERIFY { extern const char* cn1GcMarkPhase; cn1GcMarkPhase = "constant-pool"; } +#endif +#ifdef CN1_GC_CONFORM + { long long __p0 = cn1GcNowNs(); cn1GcPoolNs -= __p0; } #endif for(int iter = 0 ; iter < CN1_CONSTANT_POOL_SIZE ; iter++) { // Most entries are JAVA_NULL now (the pool fills on first use); the @@ -1842,6 +1984,9 @@ void codenameOneGCMark() { gcMarkObject(d, poolEntry, JAVA_TRUE); } } +#ifdef CN1_GC_CONFORM + cn1GcPoolNs += cn1GcNowNs(); +#endif #ifdef CN1_CONSERVATIVE_GC_ROOTS // PHASE 3b: scan the GC thread's OWN native stack last -- a root could be live only @@ -1857,7 +2002,11 @@ void codenameOneGCMark() { #ifdef CN1_GC_VERIFY { extern const char* cn1GcMarkPhase; cn1GcMarkPhase = "root-drain"; } #endif +#ifdef CN1_GC_CONFORM + { long long __d0 = cn1GcNowNs(); gcMarkDrain(d); cn1GcDrainNs += cn1GcNowNs() - __d0; } +#else gcMarkDrain(d); +#endif #if CN1_ADOPT_POLICY != 0 && !defined(CN1_DISABLE_BIBOP) // Make already-matured slots visible in the legacy table before any safety @@ -1924,6 +2073,9 @@ void codenameOneGCMark() { CN1BibopPage* gp = atomic_load_explicit(&bibopAllPages, memory_order_acquire); #endif cn1GcInGracePass = 1; // see cn1GcGraceFullDrains +#ifdef CN1_GC_CONFORM + { long long __g0 = cn1GcNowNs(); cn1GcGraceNs -= __g0; } +#endif while(gp != 0) { #ifndef CN1_BIBOP_NO_FASTSWEEP if(__atomic_load_n(&gp->gcAllocedSinceSweep, __ATOMIC_RELAXED) == JAVA_FALSE) { @@ -1982,6 +2134,9 @@ void codenameOneGCMark() { } } gcMarkDrain(d); +#ifdef CN1_GC_CONFORM + cn1GcGraceNs += cn1GcNowNs(); +#endif cn1GcInGracePass = 0; } // A single page's slot walk runs between two of those checks, so the worklist must @@ -2018,6 +2173,9 @@ void codenameOneGCMark() { { extern const char* cn1GcMarkPhase; cn1GcMarkPhase = "legacy-grace-pass"; } #endif cn1GcInGracePass = 1; // see cn1GcGraceFullDrains +#ifdef CN1_GC_CONFORM + { long long __g0 = cn1GcNowNs(); cn1GcGraceNs -= __g0; } +#endif int gt = currentSizeOfAllObjectsInHeap; for(int gi = 0 ; gi < gt ; gi++) { JAVA_OBJECT go = allObjectsInHeap[gi]; @@ -2052,6 +2210,9 @@ void codenameOneGCMark() { // whose fields can dangle. That is precisely what the guard exists to stop. CN1_GC_TRUSTED_END(); gcMarkDrain(d); +#ifdef CN1_GC_CONFORM + cn1GcGraceNs += cn1GcNowNs(); +#endif cn1GcInGracePass = 0; } #endif /* CN1_DISABLE_LEGACY_GRACE -- A/B escape hatch, mirrors CN1_DISABLE_SATB */ @@ -2091,15 +2252,26 @@ void codenameOneGCMark() { // the start-of-cycle snapshot is closed. Draining it here (not before grace+belt) is // what keeps the barrier armed through those phases and closes the residual grace // window. Bounded by the live set (only genuinely-new marks reset the fixpoint). +#ifdef CN1_GC_CONFORM + long long __satb0 = cn1GcNowNs(); +#endif for(;;) { #ifdef CN1_GC_VERIFY { extern const char* cn1GcMarkPhase; cn1GcMarkPhase = "satb-drain"; } #endif JAVA_OBJECT* batch; long n = cn1SatbTake(&batch); +#ifdef CN1_GC_CONFORM + cn1GcSatbEntries += n; +#endif if(n == 0) break; // log empty at this instant long before = gcMarkNewObjectCount; for(long i = 0 ; i < n ; i++) { +#ifdef CN1_GC_CONFORM + if(batch[i] != JAVA_NULL && batch[i]->__codenameOneGcMark == currentGcMarkValue) { + cn1GcSatbDrainAlready++; + } +#endif gcMarkObject(d, batch[i], JAVA_FALSE); } gcMarkDrain(d); @@ -2116,6 +2288,9 @@ void codenameOneGCMark() { } if(n > 0) gcMarkDrain(d); } +#ifdef CN1_GC_CONFORM + cn1GcSatbNs += cn1GcNowNs() - __satb0; +#endif #ifdef CN1_GC_VERIFY // Check the objects revived this cycle before the sweep acts on anything. { extern void cn1GcResurrectAudit(CODENAME_ONE_THREAD_STATE); cn1GcResurrectAudit(d); } @@ -2398,6 +2573,9 @@ static void cn1GcReportStaleIndexSkip(void) { static long skips = 0; static long next = 1; skips++; +#ifdef CN1_GC_CONFORM + atomic_store_explicit(&cn1GcStaleSkips, skips, memory_order_relaxed); +#endif if(skips >= next) { next *= 2; fprintf(stderr, "CN1 GC: page resolver index could not be rebuilt; skipped the " @@ -2466,6 +2644,9 @@ void codenameOneGCSweep() { // double-frees a native-resource finalizer's buffer -- the deterministic // mid-suite "corrupted unsorted chunks" heap abort. if(o->__heapPosition == CN1_BIBOP_ADOPTED) { +#ifdef CN1_GC_CONFORM + atomic_fetch_add_explicit(&cn1GcMaturedDied, 1, memory_order_relaxed); +#endif o->__heapPosition = CN1_BIBOP_HEAP_POS; continue; } @@ -4559,6 +4740,9 @@ void cn1MonitorDataSet(JAVA_OBJECT o, void* data) { e = (struct CN1MonitorEntry*)malloc(sizeof(struct CN1MonitorEntry)); e->key = o; e->data = data; e->next = cn1MonitorBuckets[h]; cn1MonitorBuckets[h] = e; +#ifdef CN1_GC_CONFORM + atomic_fetch_add_explicit(&cn1MonitorEntries, 1, memory_order_relaxed); +#endif pthread_mutex_unlock(&cn1MonitorTableMutex); } @@ -4572,6 +4756,9 @@ void cn1MonitorDataSet(JAVA_OBJECT o, void* data) { if((*pp)->key == o) { struct CN1MonitorEntry* d = *pp; r = d->data; *pp = d->next; free(d); +#ifdef CN1_GC_CONFORM + atomic_fetch_add_explicit(&cn1MonitorEntries, -1, memory_order_relaxed); +#endif break; } pp = &(*pp)->next; @@ -5578,6 +5765,9 @@ void cn1GcBuildRootSnapshots(void) { if(cn1ConsSnapEpoch == currentGcMarkValue) { return; // already built this cycle } +#ifdef CN1_GC_CONFORM + long long __snapT0 = cn1GcNowNs(); +#endif cn1ConsSnapEpoch = currentGcMarkValue; cn1ConsExtN = 0; int n = currentSizeOfAllObjectsInHeap; @@ -5711,6 +5901,9 @@ void cn1GcBuildRootSnapshots(void) { #endif currentSizeOfAllObjectsInHeap); } +#ifdef CN1_GC_CONFORM + cn1GcSnapNs += cn1GcNowNs() - __snapT0; +#endif } #ifdef CN1_RESOLVE_DIAG @@ -6454,12 +6647,39 @@ void cn1GcVerifyHeap(CODENAME_ONE_THREAD_STATE) { void cn1ConservativeMarkRange(CODENAME_ONE_THREAD_STATE, char* lo, char* hi) { if(lo == 0 || hi == 0 || hi <= lo) return; char* p = (char*)(((uintptr_t)lo + (sizeof(void*) - 1)) & ~((uintptr_t)(sizeof(void*) - 1))); +#ifdef CN1_GC_CONFORM + long long __words = 0, __resolved = 0, __first = 0; +#endif for(; p + sizeof(void*) <= hi ; p += sizeof(void*)) { JAVA_OBJECT o = cn1ConservativeResolve(*(void**)p); +#ifdef CN1_GC_CONFORM + __words++; +#endif if(o != JAVA_NULL) { +#ifdef CN1_GC_CONFORM + // Read the mark BEFORE marking. Neither the current epoch (already reached + // this cycle) nor -1 (fresh, which grace keeps regardless) says anything; an + // older epoch means this word is the only reason the object is still alive. + __resolved++; + { + int __m = o->__codenameOneGcMark; + if(__m != currentGcMarkValue && __m != -1) { + __first++; + } + } +#endif gcMarkObject(threadStateData, o, JAVA_FALSE); } } +#ifdef CN1_GC_CONFORM + // One atomic add per RANGE, never per word: this loop runs over every aligned word of + // every stopped thread's stack and a per-word RMW would be the measurement. + if(__words != 0) { + atomic_fetch_add_explicit(&cn1ConsWords, __words, memory_order_relaxed); + atomic_fetch_add_explicit(&cn1ConsResolved, __resolved, memory_order_relaxed); + atomic_fetch_add_explicit(&cn1ConsFirstMarks, __first, memory_order_relaxed); + } +#endif } // Portable [high) stack base + size for a given pthread. Stacks grow DOWN, so the base @@ -8832,6 +9052,298 @@ void cn1StartupPhase(const char* name) { void cn1StartupPhase(const char* name) { } #endif +// ======================= CN1_GC_CONFORM: the footprint probe ========================= +// Issue 5537. Four merged fixes each named a mechanism; none of them ever showed that the +// named mechanism ACCOUNTED for the growth, because nothing in the VM could partition the +// footprint. This does, and its primary output is the RESIDUAL: if +// residKb = fpKb - residentPgKb - legBlockKb - legTableKb - sideKb +// carries the drift, the growth is not in the Java heap at all and every heap hypothesis +// dies in one run. +// +// Two emitters, because the reported failure includes "GC pauses get longer until they +// are effectively continuous" -- a per-cycle probe goes blind exactly where the failure +// peaks, so a 1Hz wall-clock series runs alongside it. +// +// Compiled out without -DCN1_GC_CONFORM, and gated at RUNTIME on CN1_GC_PROBE so that +// probe-on and probe-off are the SAME BINARY and the probe can be checked against itself. +#ifdef CN1_GC_CONFORM +// The object header does not record instance size, so the true weight of a legacy block +// comes from the allocator. malloc_size is Apple-only; glibc spells it malloc_usable_size, +// and without it cn1HeapAccounting's Linux legacy figure is a silent zero. +#if defined(__APPLE__) +#include +#define CN1_CONFORM_BLOCK_SIZE(p) ((long long)malloc_size((void*)(p))) +#elif defined(__linux__) +#include +#define CN1_CONFORM_BLOCK_SIZE(p) ((long long)malloc_usable_size((void*)(p))) +#else +#define CN1_CONFORM_BLOCK_SIZE(p) ((long long)0) +#endif + +static _Atomic int cn1GcProbeMode = -1; // -1 = env not probed, 0 = off, else cadence +static long long cn1GcProbeT0 = 0; + +static int cn1GcProbeEvery(void) { + int m = atomic_load_explicit(&cn1GcProbeMode, memory_order_relaxed); + if(m < 0) { + const char* e = getenv("CN1_GC_PROBE"); + m = 0; + if(e != 0) { + m = atoi(e); + if(m <= 0) { + m = 1; // CN1_GC_PROBE=1 / =yes -> every cycle + } + } + atomic_store_explicit(&cn1GcProbeMode, m, memory_order_relaxed); + } + return m; +} + +static long long cn1GcProbeElapsedMs(void) { + return (long long)cn1MonotonicMillis() - cn1GcProbeT0; +} + +// Capacities of the allocator's own side tables. None of these hold Java objects, so +// nothing else in the VM reports them, and a table that ratchets looks exactly like a +// heap leak from the outside. +static long long cn1GcProbeSideBytes(void) { + long long side = 0; + side += (long long)cn1ImmortalRootsCap * (long long)sizeof(JAVA_OBJECT); + side += (long long)gcSatbCap * (long long)sizeof(JAVA_OBJECT); +#ifdef CN1_CONSERVATIVE_GC_ROOTS + side += (long long)CN1_CLAZZ_SET_SIZE * (long long)sizeof(uintptr_t); +#endif +#ifndef CN1_DISABLE_BIBOP + side += (long long)gcAdoptCap * (long long)sizeof(JAVA_OBJECT); +#endif +#ifdef CN1_CONSERVATIVE_GC_ROOTS + side += (long long)cn1ConsExtCap * (long long)sizeof(void*) * 2; + if(cn1ConsExtHashMask >= 0) { + side += (long long)(cn1ConsExtHashMask + 1) * (long long)sizeof(char*); + } +#ifndef CN1_DISABLE_BIBOP + // The page index exists only where there are pages to index. + if(cn1ConsPgMask >= 0) { + side += (long long)(cn1ConsPgMask + 1) * (long long)sizeof(CN1ConsPage); + } +#endif +#endif + side += (long long)CN1_MON_BUCKETS * (long long)sizeof(void*); + side += atomic_load_explicit(&cn1MonitorEntries, memory_order_relaxed) + * (long long)sizeof(struct CN1MonitorEntry); + side += (long long)CN1_FV_BUCKETS * (long long)sizeof(void*); + side += cn1FVLive * (long long)sizeof(struct CN1FVEntry); + return side; +} + +// Runs on the GC thread immediately after the sweep, from java_lang_System_gcMarkSweep__. +// That is the only point in the program where no mark is in flight and no mutator owns a +// retired page, which is what makes walking bibopAllPages here safe -- the 1Hz emitter +// below must never do it (cn1BibopFormatPage rewrites page geometry underneath a reader). +void cn1GcProbeCycle(double markMs, double sweepMs) { + int every = cn1GcProbeEvery(); + if(every == 0 || (currentGcMarkValue % every) != 0) { + return; + } + long long pgTotal = 0, pgEmpty = 0, pgReleased = 0, pgAdopted = 0, pgMon = 0; + long long pgOwned = 0, pgGrace = 0, liveSlots = 0, deadSlots = 0, resvBytes = 0; + long long releasedBytes = 0; +#ifndef CN1_DISABLE_BIBOP + size_t relOff = cn1BibopReleaseOffset(); + CN1BibopPage* p = atomic_load_explicit(&bibopAllPages, memory_order_acquire); + while(p != 0) { + pgTotal++; + resvBytes += CN1_BIBOP_PAGE_SIZE; + int bi = atomic_load_explicit(&p->bumpIndex, memory_order_relaxed); + int live = bi - p->freeCount; + if(live < 0) { + live = 0; + } + if(live == 0) { + pgEmpty++; + } + liveSlots += (long long)live * (long long)p->slotSize; + deadSlots += (long long)p->freeCount * (long long)p->slotSize; + if(p->gcPageReleased) { + pgReleased++; + // Only the slot region is handed back; the header page stays resident. + releasedBytes += (long long)(CN1_BIBOP_PAGE_SIZE - relOff); + } + if(p->gcHasAdopted) { pgAdopted++; } + if(p->gcHasMonitors) { pgMon++; } + if(p->owned) { pgOwned++; } + pgGrace += (long long)atomic_load_explicit(&p->gcGraceMarked, memory_order_relaxed); + p = atomic_load_explicit(&p->nextAll, memory_order_acquire); + } +#endif + // The legacy heap is a separate malloc'd population that no page figure can see. + long long legUsed = 0, legBlockBytes = 0; + int legCap = currentSizeOfAllObjectsInHeap; + for(int i = 0 ; i < legCap ; i++) { + JAVA_OBJECT o = allObjectsInHeap[i]; + if(o == JAVA_NULL) { + continue; + } + legUsed++; + legBlockBytes += CN1_CONFORM_BLOCK_SIZE(o); + } + long long legTableBytes = (long long)sizeOfAllObjectsInHeap * (long long)sizeof(JAVA_OBJECT); + long long sideBytes = cn1GcProbeSideBytes(); + long long residentPgBytes = resvBytes - releasedBytes; + long long fpKb = (long long)cn1ProcFootprintBytes() / 1024; + long long residKb = fpKb - (residentPgBytes + legBlockBytes + legTableBytes + sideBytes) / 1024; + + fprintf(stderr, + "[GCPROBE] v=1 cyc=%d tMs=%lld fpKb=%lld" + " pgTotal=%lld pgEmpty=%lld pgReleased=%lld pgAdopted=%lld pgMon=%lld pgOwned=%lld pgGrace=%lld" + " resvKb=%lld residentPgKb=%lld liveSlotKb=%lld deadSlotKb=%lld" + " legCap=%d legUsed=%lld legTableKb=%lld legBlockKb=%lld" + " matured=%ld maturedDied=%ld maturedPages=%ld" + " triggerKb=%ld bypassActs=%ld bypassAllocs=%ld occKb=%ld liveKb=%ld reclKb=%ld" + " markMs=%.1f sweepMs=%.1f snapMs=%.1f graceMs=%.1f drainMs=%.1f" + " waitMs=%.1f stackMs=%.1f tdrainMs=%.1f migrateMs=%.1f migrated=%ld" + " satbMs=%.1f satbRefs=%ld satbAlready=%ld satbFresh=%ld satbDrainAlready=%ld poolMs=%.1f" + " staleSkips=%ld ovfCycles=%ld graceDrains=%ld" + " consWords=%lld consResolved=%lld consFirstMarks=%lld" + " monitors=%ld immortal=%d fvLive=%ld sideKb=%lld residKb=%lld\n", + currentGcMarkValue, cn1GcProbeElapsedMs(), fpKb, + pgTotal, pgEmpty, pgReleased, pgAdopted, pgMon, pgOwned, pgGrace, + resvBytes / 1024, residentPgBytes / 1024, liveSlots / 1024, deadSlots / 1024, + legCap, legUsed, legTableBytes / 1024, legBlockBytes / 1024, + atomic_load_explicit(&cn1GcMaturedTotal, memory_order_relaxed), + atomic_load_explicit(&cn1GcMaturedDied, memory_order_relaxed), + atomic_load_explicit(&cn1GcMaturedPages, memory_order_relaxed), +#ifdef CN1_DISABLE_BIBOP + 0L, 0L, 0L, 0L, 0L, 0L, +#else + (long)(atomic_load_explicit(&bibopGcTriggerBytes, memory_order_relaxed) / 1024), + atomic_load_explicit(&cn1BibopBypassActivations, memory_order_relaxed), + atomic_load_explicit(&cn1BibopBypassAllocations, memory_order_relaxed), + bibopLastCycleOccupiedBytes / 1024, bibopLastCycleLiveBytes / 1024, + bibopLastCycleReclaimedBytes / 1024, +#endif + markMs, sweepMs, + cn1GcSnapNs / 1e6, cn1GcGraceNs / 1e6, cn1GcDrainNs / 1e6, + cn1GcWaitNs / 1e6, cn1GcStackNs / 1e6, cn1GcTDrainNs / 1e6, + cn1GcMigrateNs / 1e6, cn1GcMigrated, + cn1GcSatbNs / 1e6, cn1GcSatbEntries, + atomic_load_explicit(&cn1GcSatbAlready, memory_order_relaxed), + atomic_load_explicit(&cn1GcSatbFresh, memory_order_relaxed), + cn1GcSatbDrainAlready, cn1GcPoolNs / 1e6, + atomic_load_explicit(&cn1GcStaleSkips, memory_order_relaxed), + atomic_load_explicit(&cn1GcOverflowCycles, memory_order_relaxed), + atomic_load_explicit(&cn1GcGraceDrains, memory_order_relaxed), + atomic_load_explicit(&cn1ConsWords, memory_order_relaxed), + atomic_load_explicit(&cn1ConsResolved, memory_order_relaxed), + atomic_load_explicit(&cn1ConsFirstMarks, memory_order_relaxed), + atomic_load_explicit(&cn1MonitorEntries, memory_order_relaxed), + cn1ImmortalRootsN, cn1FVLive, + sideBytes / 1024, residKb); + fflush(stderr); + // Per-CYCLE, so reset after reporting. A running total cannot show a trend. + cn1GcSnapNs = 0; + cn1GcGraceNs = 0; + cn1GcDrainNs = 0; + cn1GcWaitNs = 0; + cn1GcStackNs = 0; + cn1GcTDrainNs = 0; + cn1GcMigrateNs = 0; + cn1GcMigrated = 0; + cn1GcSatbNs = 0; + cn1GcSatbEntries = 0; + cn1GcSatbDrainAlready = 0; + atomic_store_explicit(&cn1GcSatbAlready, 0, memory_order_relaxed); + atomic_store_explicit(&cn1GcSatbFresh, 0, memory_order_relaxed); + cn1GcPoolNs = 0; +} + +// 1Hz wall-clock series. ATOMICS ONLY -- it must never walk bibopAllPages. This is the +// series that survives a collector which has stopped finishing cycles, which is the state +// the reporter describes and the one in which the per-cycle emitter above goes silent. +static void* cn1GcProbeThread(void* ignored) { + for(;;) { + usleep(1000000); + fprintf(stderr, "[GCPROBE-T] v=1 tMs=%lld fpKb=%lld cyc=%d pgTotal=%lld" + " matured=%ld maturedDied=%ld maturedPages=%ld triggerKb=%ld" + " bytesSinceGc=%lld staleSkips=%ld\n", + cn1GcProbeElapsedMs(), + (long long)cn1ProcFootprintBytes() / 1024, + currentGcMarkValue, +#ifdef CN1_DISABLE_BIBOP + 0LL, +#else + (long long)atomic_load_explicit(&bibopAllPagesCount, memory_order_relaxed), +#endif + atomic_load_explicit(&cn1GcMaturedTotal, memory_order_relaxed), + atomic_load_explicit(&cn1GcMaturedDied, memory_order_relaxed), + atomic_load_explicit(&cn1GcMaturedPages, memory_order_relaxed), +#ifdef CN1_DISABLE_BIBOP + 0L, 0LL, +#else + (long)(atomic_load_explicit(&bibopGcTriggerBytes, memory_order_relaxed) / 1024), + (long long)atomic_load_explicit(&bibopBytesSinceGc, memory_order_relaxed), +#endif + atomic_load_explicit(&cn1GcStaleSkips, memory_order_relaxed)); + fflush(stderr); + } + return ignored; +} + +// ---- workload configuration, read from the environment ------------------------------ +// The clean target's generated main() passes JAVA_NULL for args (see the emitted +// com__
.c), so a translated workload cannot be parameterised through argv the +// way the host-JVM reference run is. These give it env-backed knobs instead, following +// the GcVerifyApp_gcMarkState___R_long precedent of implementing a test class's native +// here under a QA #ifdef. The mangling is load-bearing and unchecked by the compiler: +// `int cfg(int)` is `_cfg` + `__` for the argument list + `_int` for the argument + `_R_int` +// for the return -- see the ParparVM native-name rules in CLAUDE.md. +static int cn1ConformCfg(int which) { + static const char* names[] = { + "CN1_WL_SECONDS", "CN1_WL_THREADS", "CN1_WL_DEPTH", "CN1_WL_BRANCH", + "CN1_WL_SLEEP_MS", "CN1_WL_MOVES", "CN1_WL_LEGACY", "CN1_WL_SCRUB" + }; + static const int defs[] = { 60, 4, 14, 3, 0, 4, 256, 0 }; + int n = (int)(sizeof(defs) / sizeof(defs[0])); + if(which < 0 || which >= n) { + return 0; + } + const char* e = getenv(names[which]); + if(e == 0 || *e == 0) { + return defs[which]; + } + return atoi(e); +} + +JAVA_INT com_bench_GcSteadyState_cfg___int_R_int(CODENAME_ONE_THREAD_STATE, JAVA_INT which) { + return cn1ConformCfg(which); +} + +// Milliseconds since the probe's t0, so a workload's own samples share one timebase with +// [GCPROBE] and [GCPROBE-T] and the three series can be joined on tMs. +JAVA_LONG com_bench_GcSteadyState_probeMs___R_long(CODENAME_ONE_THREAD_STATE) { + return (JAVA_LONG)cn1GcProbeElapsedMs(); +} + +void cn1GcProbeInit(void) { + // t0 is stamped even with the emitters off: a workload's own samples call probeMs() + // and must share the timebase whether or not [GCPROBE] is being printed. + cn1GcProbeT0 = (long long)cn1MonotonicMillis(); + if(cn1GcProbeEvery() == 0) { + return; + } + pthread_t t; + pthread_attr_t a; + pthread_attr_init(&a); + pthread_attr_setdetachstate(&a, PTHREAD_CREATE_DETACHED); + pthread_create(&t, &a, cn1GcProbeThread, 0); + pthread_attr_destroy(&a); + fprintf(stderr, "[GCPROBE] init every=%d pageSize=%d maxObject=%d ptr=%d\n", + cn1GcProbeEvery(), (int)CN1_BIBOP_PAGE_SIZE, (int)CN1_BIBOP_MAX_OBJECT, + (int)sizeof(void*)); + fflush(stderr); +} +#endif /* CN1_GC_CONFORM */ + void initConstantPool() { cn1StartupPhase("main"); __STATIC_INITIALIZER_java_lang_Class(getThreadLocalData()); @@ -8883,6 +9395,9 @@ void initConstantPool() { atexit(cn1ReportPacingParks); atexit(cn1ReportGcOverflow); cn1StartSimulatedMemoryWarnings(); +#ifdef CN1_GC_CONFORM + cn1GcProbeInit(); +#endif // it will wait two seconds unless an explicit GC occurs java_lang_System_startGCThread__(threadStateData); diff --git a/vm/ByteCodeTranslator/src/nativeMethods.m b/vm/ByteCodeTranslator/src/nativeMethods.m index e14acc00d1d..059f5c1ae7c 100644 --- a/vm/ByteCodeTranslator/src/nativeMethods.m +++ b/vm/ByteCodeTranslator/src/nativeMethods.m @@ -1855,6 +1855,10 @@ JAVA_VOID java_lang_System_gcLight__(CODENAME_ONE_THREAD_STATE) { JAVA_BOOLEAN firstTimeGcThread = JAVA_TRUE; JAVA_BOOLEAN gcCurrentlyRunning = JAVA_FALSE; +#ifdef CN1_GC_CONFORM +extern void cn1GcProbeCycle(double markMs, double sweepMs); +double cn1GcProbeMarkMs = 0, cn1GcProbeSweepMs = 0; +#endif JAVA_VOID java_lang_System_gcMarkSweep__(CODENAME_ONE_THREAD_STATE) { gcCurrentlyRunning = JAVA_TRUE; if(firstTimeGcThread) { @@ -1895,8 +1899,19 @@ JAVA_VOID java_lang_System_gcMarkSweep__(CODENAME_ONE_THREAD_STATE) { clock_gettime(CLOCK_MONOTONIC,&_t2); markNs += (_t1.tv_sec-_t0.tv_sec)*1000000000LL+(_t1.tv_nsec-_t0.tv_nsec); sweepNs += (_t2.tv_sec-_t1.tv_sec)*1000000000LL+(_t2.tv_nsec-_t1.tv_nsec); +#ifdef CN1_GC_CONFORM + // PER-CYCLE, not cumulative: "the pauses get longer" is a statement about the trend of + // one cycle's cost, and a running total cannot express it. + cn1GcProbeMarkMs = ((_t1.tv_sec-_t0.tv_sec)*1000000000LL+(_t1.tv_nsec-_t0.tv_nsec)) / 1e6; + cn1GcProbeSweepMs = ((_t2.tv_sec-_t1.tv_sec)*1000000000LL+(_t2.tv_nsec-_t1.tv_nsec)) / 1e6; +#endif gcCount++; - if(gcCount==1 || (gcCount % 20)==0) fprintf(stderr,"[GC-INSTR] cycles=%d allocs=%lld heapTableSize=%d markMs=%.0f sweepMs=%.0f\n", + // outOfLineAllocs, NOT allocations: cn1_instr_allocCount is bumped in + // codenameOneGcMalloc, and CN1_FAST_NEW's inlined bump path never reaches it. On a + // small-object workload -- the shape issue 5537 reported -- that is the overwhelming + // majority of allocation, so reading this as a total understates it by orders of + // magnitude. cn1AllocCensus (CN1_ALLOC_CENSUS) counts at every entry point. + if(gcCount==1 || (gcCount % 20)==0) fprintf(stderr,"[GC-INSTR] cycles=%d outOfLineAllocs=%lld heapTableSize=%d markMs=%.0f sweepMs=%.0f\n", gcCount, cn1_instr_allocCount, currentSizeOfAllObjectsInHeap, markNs/1e6, sweepNs/1e6); #else codenameOneGCMark(); @@ -1908,6 +1923,9 @@ JAVA_VOID java_lang_System_gcMarkSweep__(CODENAME_ONE_THREAD_STATE) { threadStateData->exception = JAVA_NULL; } flushReleaseQueue(); +#ifdef CN1_GC_CONFORM + cn1GcProbeCycle(cn1GcProbeMarkMs, cn1GcProbeSweepMs); +#endif #ifdef CN1_ALLOC_CENSUS { // Several points, not one: allocation during startup and allocation once diff --git a/vm/benchmarks/src/com/bench/GcSteadyState.java b/vm/benchmarks/src/com/bench/GcSteadyState.java new file mode 100644 index 00000000000..2ed29e2bf8d --- /dev/null +++ b/vm/benchmarks/src/com/bench/GcSteadyState.java @@ -0,0 +1,228 @@ +package com.bench; + +/** + * Issue #5537, the steady-state question: does the VM ever GIVE THE MEMORY BACK? + * + *

Every GC test in the repo measures a PEAK under load. GcOverflowSpiralIntegrationTest + * asserts peak < 2GB over 50 bounded rounds; a heap that grows forever at a modest rate + * passes it. The reporter's build climbs 500MB to 5GB over five minutes on the iOS + * Simulator against a live set of a few hundred objects, so the statistic that matters is + * the SLOPE of the footprint at a fixed live set, and nothing here could express it.

+ * + *

The shape is the reporter's: a deep, CPU-bound game-tree search producing millions of + * tiny short-lived objects, on worker threads, with a live set that returns to the same + * baseline every round. It is derived from {@code GcOverflowSpiralApp} and differs from it + * in the five things that make a drift measurable:

+ * + *
    + *
  • Several workers, because the collector is single-threaded + * ({@code gcMarkResolveThreadCount} returns 1) and at lowered priority, so whether it + * keeps up is a function of how many cores the mutator holds.
    • + *
  • Wall-clock duration instead of a fixed round count, because a drift that takes + * minutes cannot be sampled by a run that ends in seconds.
    • + *
  • Depth as a knob, defaulting deep. Depth sets the extent of the native stack, + * which is the input to the conservative root scan: every stale word in a live frame + * marks whatever it points at.
    • + *
  • A wall-clock sampler thread. Sampling every N nodes stops exactly when the + * collector stalls, which is the state being measured.
    • + *
  • A sleep knob. The reporter's own observation -- 1ms and 100ms do not help, + * 1000ms does -- is a crude rate measurement, and sweeping it is the cheapest + * discriminator there is between a rate problem and a retention problem.
    • + *
+ * + *

Knobs come from the environment through a native, because the clean target's generated + * main() passes JAVA_NULL for args. Requires -DCN1_GC_CONFORM.

+ */ +public class GcSteadyState { + + private static native int cfg(int which); + private static native long probeMs(); + + private static final int CFG_SECONDS = 0, CFG_THREADS = 1, CFG_DEPTH = 2, CFG_BRANCH = 3; + private static final int CFG_SLEEP_MS = 4, CFG_MOVES = 5, CFG_LEGACY = 6, CFG_SCRUB = 7; + + private static final int BOARD_CELLS = 64; + private static final int LEGACY_BLOCK_REFS = 128; + + static int seconds, threads, depth, branch, sleepMs, movesPerNode, legacyBlocks, scrubDepth; + static volatile boolean stop = false; + static Object[][] legacyLiveSet; + static final Object SUM_LOCK = new Object(); + static long checksum = 0; + static long nodes = 0; + + /** One node of the search: small, short-lived, and REFERENCE-CARRYING. Only a non-leaf + * object has a mark function, and only such an object is eligible for maturation into + * the legacy heap -- a board of ints is a leaf and never graduates. */ + static final class Move { + int from; + int to; + int score; + int[] board; + Move next; + } + + public static void main(String[] args) { + seconds = cfg(CFG_SECONDS); + threads = cfg(CFG_THREADS); + depth = cfg(CFG_DEPTH); + branch = cfg(CFG_BRANCH); + sleepMs = cfg(CFG_SLEEP_MS); + movesPerNode = cfg(CFG_MOVES); + legacyBlocks = cfg(CFG_LEGACY); + scrubDepth = cfg(CFG_SCRUB); + System.out.println("WLCONFIG seconds=" + seconds + " threads=" + threads + + " depth=" + depth + " branch=" + branch + " sleepMs=" + sleepMs + + " moves=" + movesPerNode + " legacy=" + legacyBlocks + + " scrub=" + scrubDepth); + + // A retained legacy population, held for the whole run. Without it the collector's + // table walk costs nothing and this workload cannot tell a cheap drain from an + // O(heap) one. Reference-carrying, because the rescan skips objects with no mark + // function. + legacyLiveSet = new Object[legacyBlocks][]; + for (int i = 0; i < legacyBlocks; i++) { + Object[] block = new Object[LEGACY_BLOCK_REFS]; + for (int j = 0; j < LEGACY_BLOCK_REFS; j++) { + Move held = new Move(); + held.from = i; + held.to = j; + block[j] = held; + } + legacyLiveSet[i] = block; + } + + Thread sampler = new Thread(new Runnable() { + public void run() { + while (!stop) { + System.out.println("SAMPLE tMs=" + probeMs() + " fpKb=" + footprintKb() + + " nodes=" + nodes); + sleep(250); + } + } + }); + sampler.start(); + + Thread[] workers = new Thread[threads]; + for (int t = 0; t < threads; t++) { + final int seed = t * 7919; + workers[t] = new Thread(new Runnable() { + public void run() { + long sum = 0; + long localNodes = 0; + int[] root = new int[BOARD_CELLS]; + int round = 0; + while (!stop) { + sum += search(root, depth, seed + round); + localNodes = nodes; + round++; + if (sleepMs > 0) { + sleep(sleepMs); + } + } + // Order-independent, so the checksum does not depend on scheduling. + synchronized (SUM_LOCK) { + checksum += sum; + nodes = localNodes; + } + } + }); + } + + long startMs = System.currentTimeMillis(); + for (int t = 0; t < threads; t++) { + workers[t].start(); + } + while (System.currentTimeMillis() - startMs < seconds * 1000L) { + sleep(200); + } + stop = true; + for (int t = 0; t < threads; t++) { + try { + workers[t].join(); + } catch (InterruptedException e) { + } + } + try { + sampler.join(); + } catch (InterruptedException e) { + } + + // Optional: overwrite the deep frames the search left behind. Conservative root + // scanning reads every aligned word in [sp, stackBase), so a returned frame's + // leftover words still pin whatever they point at. Scrubbing is therefore an + // ablation of that retention that costs no rebuild -- which is why it is a knob + // and NOT on during the measurement window. + if (scrubDepth > 0) { + scrub(scrubDepth); + } + + System.out.println("ELAPSED_MS=" + (System.currentTimeMillis() - startMs)); + System.out.println("FINAL_FOOTPRINT_KB=" + footprintKb()); + Move lastHeld = (Move) legacyLiveSet[legacyBlocks - 1][LEGACY_BLOCK_REFS - 1]; + System.out.println("RESULT=" + (checksum + lastHeld.from + lastHeld.to)); + System.out.println("GC_STEADY_STATE_DONE"); + } + + private static int search(int[] board, int d, int seed) { + if (stop) { + return 0; + } + nodes++; + if (d == 0) { + int s = 0; + for (int i = 0; i < BOARD_CELLS; i++) { + s += board[i] * (i + 1); + } + return s & 0xff; + } + int best = -1; + for (int b = 0; b < branch; b++) { + int[] child = new int[BOARD_CELLS]; + for (int i = 0; i < BOARD_CELLS; i++) { + child[i] = board[i] + ((seed + b + i) & 7); + } + Move chain = null; + for (int m = 0; m < movesPerNode; m++) { + Move mv = new Move(); + mv.from = b; + mv.to = m; + mv.score = seed + m; + mv.board = child; + mv.next = chain; + chain = mv; + } + int v = search(child, d - 1, seed + b + chain.to); + if (v > best) { + best = v; + } + } + return best; + } + + /** Writes zeroes over the stack region the search used, so its leftover words stop + * resolving to dead objects. Recursion, not an array: the words to overwrite are the + * frames themselves. */ + private static int scrub(int d) { + int[] pad = new int[16]; + for (int i = 0; i < pad.length; i++) { + pad[i] = 0; + } + if (d <= 0) { + return pad[0]; + } + return pad[0] + scrub(d - 1); + } + + private static long footprintKb() { + Runtime r = Runtime.getRuntime(); + return (r.totalMemory() - r.freeMemory()) / 1024; + } + + private static void sleep(long ms) { + try { + Thread.sleep(ms); + } catch (InterruptedException e) { + } + } +} diff --git a/vm/tests/src/test/java/com/codename1/tools/translator/GcSteadyStateIntegrationTest.java b/vm/tests/src/test/java/com/codename1/tools/translator/GcSteadyStateIntegrationTest.java new file mode 100644 index 00000000000..f44ceef1aaf --- /dev/null +++ b/vm/tests/src/test/java/com/codename1/tools/translator/GcSteadyStateIntegrationTest.java @@ -0,0 +1,388 @@ +/* + * 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.codename1.tools.translator; + +import org.junit.jupiter.api.Tag; +import org.junit.jupiter.api.Test; + +import java.io.BufferedReader; +import java.io.InputStreamReader; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.stream.Collectors; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assertions.fail; + +/** + * Steady-state gate for the collector (issue #5537). + * + *

Every other GC test here measures a PEAK under load, and a peak cannot express the + * failure this issue reported. {@code GcOverflowSpiralIntegrationTest} asserts peak < 2GB + * over 50 bounded rounds; a heap that grows forever at a modest rate passes it. The + * reporter's build climbed 500MB to 5GB over five minutes against a live set of a few + * hundred objects, with GC pauses lengthening until they were continuous -- so the property + * that had to be asserted, and never was, is that the growth STOPS.

+ * + *

The mechanism found underneath it: the SATB write barrier logged a reference on every + * object store during a mark, and on a churn workload essentially every logged reference + * was to a FRESH object -- one allocated after the snapshot was taken, which the sweep's + * grace rule keeps regardless. The log's size is therefore mutation rate x cycle duration, + * and draining it is part of the cycle, so a longer cycle produced a longer log which + * produced a longer cycle. Measured before the fix: 2,718,413 fresh references of + * 2,718,448 logged in one cycle, 282ms of a 327ms mark, page count climbing without bound. + * Both symptoms fall out of that one loop.

+ * + *

This gate builds the workload with {@code -DCN1_GC_CONFORM}, which adds the + * {@code [GCPROBE]} series and changes no allocator behaviour -- deliberately NOT + * {@code CN1_GC_VERIFY}, which forces {@code cn1BibopReleaseOffset()} to 0 and so compiles + * out the page-release and major-sweep paths this measurement depends on.

+ * + *

Two assertions, one on the mechanism and one on the outcome, and then a second run + * that re-injects the defect ({@code -DCN1_SATB_LOG_FRESH}) and requires both to fail. A + * gate that has never been watched failing proves nothing.

+ */ +@Tag("benchmark") +class GcSteadyStateIntegrationTest { + + /** + * Logged references per cycle, as a multiple of the live legacy population. The barrier + * should only see references the snapshot actually needs, which is bounded by the live + * set; before the fix it was bounded by the ALLOCATION RATE and ran to millions. The + * multiple is deliberately loose -- the two regimes are five orders of magnitude apart, + * so this cannot be made tight enough to flake without also being wrong. + */ + private static final double MAX_SATB_REFS_PER_LIVE_OBJECT = 4.0; + + /** + * How much the page heap may still grow in the second half of the run, relative to the + * first. Zero would be wrong: a run reaches its working set at its own pace and a + * partially-filled arena is 64 pages. A COMPOUNDING heap doubles here. + */ + private static final double MAX_SECOND_HALF_PAGE_GROWTH = 0.25; + + /** Cycles needed before the comparison means anything. Anti-vacuousness. */ + private static final int MIN_CYCLES = 24; + + @Test + void aChurningWorkloadReachesAWorkingSetAndStaysThere() throws Exception { + Parser.cleanup(); + List tempDirs = new ArrayList<>(); + try { + runGate(tempDirs); + } finally { + for (Path dir : tempDirs) { + deleteRecursively(dir); + } + } + } + + private void runGate(List tempDirs) throws Exception { + Path sourceDir = Files.createTempDirectory("gc-steady-sources"); + Path classesDir = Files.createTempDirectory("gc-steady-classes"); + Path javaApiDir = Files.createTempDirectory("gc-steady-javaapi"); + tempDirs.add(sourceDir); + tempDirs.add(classesDir); + tempDirs.add(javaApiDir); + + Path source = sourceDir.resolve("GcSteadyStateApp.java"); + Files.write(source, loadAppSource().getBytes(StandardCharsets.UTF_8)); + + CompilerHelper.CompilerConfig config = selectCompiler(); + if (config == null) { + fail("No compatible compiler available for the GC steady-state test"); + } + CompilerHelper.compileJavaAPI(javaApiDir, config); + + List compileArgs = new ArrayList<>(); + compileArgs.add("-source"); + compileArgs.add(config.targetVersion); + compileArgs.add("-target"); + compileArgs.add(config.targetVersion); + if (CompilerHelper.useClasspath(config)) { + compileArgs.add("-classpath"); + compileArgs.add(javaApiDir.toString()); + } else { + compileArgs.add("-bootclasspath"); + compileArgs.add(javaApiDir.toString()); + compileArgs.add("-Xlint:-options"); + } + compileArgs.add("-d"); + compileArgs.add(classesDir.toString()); + compileArgs.add(source.toString()); + assertEquals(0, CompilerHelper.compile(config.jdkHome, compileArgs), + "GcSteadyStateApp should compile. " + CompilerHelper.getLastErrorLog()); + + String javaResult = extractLine(runJavaMain(config, classesDir, javaApiDir), "RESULT="); + assertTrue(javaResult.startsWith("RESULT="), "JavaSE should produce RESULT="); + + CompilerHelper.copyDirectory(javaApiDir, classesDir); + Path outputDir = Files.createTempDirectory("gc-steady-output"); + tempDirs.add(outputDir); + CleanTargetIntegrationTest.runTranslator(classesDir, outputDir, "GcSteadyStateApp"); + Path distDir = outputDir.resolve("dist"); + Path cmakeLists = distDir.resolve("CMakeLists.txt"); + assertTrue(Files.exists(cmakeLists), "Translator should emit a CMake project"); + CleanTargetIntegrationTest.replaceLibraryWithExecutableTarget(cmakeLists, "GcSteadyStateApp-src"); + + // ---- 1. the gate ------------------------------------------------------ + Path fixed = build(distDir, tempDirs, "fixed", "-DCN1_GC_CONFORM"); + Run clean = run(fixed, distDir); + assertEquals(0, clean.exit, "The workload must finish. Output: " + tail(clean.output)); + assertTrue(clean.output.contains("GC_STEADY_STATE_DONE"), + "The workload should run to completion. Output: " + tail(clean.output)); + assertEquals(javaResult, extractLine(clean.output, "RESULT="), + "JavaSE and ParparVM should agree on the workload result"); + Series good = Series.parse(clean.output); + assertTrue(good.cycles >= MIN_CYCLES, + "Only " + good.cycles + " collection cycles ran, so the comparison below " + + "measured nothing. Output: " + tail(clean.output)); + assertTrue(good.satbRefsPerLiveObject() <= MAX_SATB_REFS_PER_LIVE_OBJECT, + describe("The SATB log is sized by the allocation rate, not by the live set", + good)); + assertTrue(good.secondHalfPageGrowth() <= MAX_SECOND_HALF_PAGE_GROWTH, + describe("The page heap is still compounding in the second half of the run", + good)); + + // ---- 2. proof that the gate can fail ---------------------------------- + // CN1_SATB_LOG_FRESH is the escape hatch that restores the pre-fix barrier, so it + // doubles as the fault injection: without this half, a build in which the probe or + // the filter silently compiled out would pass part 1 forever. + Path faulty = build(distDir, tempDirs, "faulted", "-DCN1_GC_CONFORM -DCN1_SATB_LOG_FRESH"); + Run faulted = run(faulty, distDir); + Series bad = Series.parse(faulted.output); + assertTrue(bad.cycles >= MIN_CYCLES, + "The faulted build produced no [GCPROBE] series, so CN1_GC_CONFORM is not " + + "active and the clean run above proved nothing. Output: " + tail(faulted.output)); + assertTrue(bad.satbRefsPerLiveObject() > MAX_SATB_REFS_PER_LIVE_OBJECT, + "Re-injecting the unfiltered SATB barrier did NOT blow the log budget, so " + + "this gate is inert. " + describe("faulted run", bad)); + assertTrue(bad.satbRefsPerLiveObject() > good.satbRefsPerLiveObject() * 10, + "The fresh-reference filter should cut the log by orders of magnitude. " + + describe("fixed", good) + " " + describe("faulted", bad)); + } + + /** One build of the already-translated project, with its own flags and build dir. */ + private Path build(Path distDir, List tempDirs, String name, String cFlags) throws Exception { + Path buildDir = Files.createTempDirectory("gc-steady-build-" + name); + tempDirs.add(buildDir); + List cmake = new ArrayList<>(Arrays.asList( + "cmake", "-S", distDir.toString(), "-B", buildDir.toString(), + "-DCMAKE_BUILD_TYPE=Release")); + cmake.addAll(CompilerHelper.cmakeToolchainArgs()); + // CMAKE_C_FLAGS composes with the target's own options, so the mandatory + // -fwrapv / -fno-strict-aliasing the generated project adds are kept. + cmake.add("-DCMAKE_C_FLAGS=" + cFlags); + CleanTargetIntegrationTest.runCommand(cmake, distDir); + CleanTargetIntegrationTest.runCommand( + Arrays.asList("cmake", "--build", buildDir.toString()), distDir); + Path exe = buildDir.resolve(CompilerHelper.executableName("GcSteadyStateApp")); + assertTrue(Files.exists(exe), "ParparVM build should produce a runnable executable at " + exe); + return exe; + } + + /** The [GCPROBE] series, reduced to the two things this gate decides on. */ + private static final class Series { + int cycles; + long satbRefsTotal; + long liveObjectsMax; + long pagesAtStart; + long pagesAtMid; + long pagesAtEnd; + + static Series parse(String output) { + List> rows = new ArrayList<>(); + for (String line : output.split("\\R")) { + if (!line.startsWith("[GCPROBE] v=1")) { + continue; + } + Map row = new HashMap<>(); + for (String token : line.split("\\s+")) { + int eq = token.indexOf('='); + if (eq <= 0) { + continue; + } + try { + row.put(token.substring(0, eq), + (long) Double.parseDouble(token.substring(eq + 1))); + } catch (NumberFormatException ignored) { + // v=1 and any future non-numeric field + } + } + rows.add(row); + } + Series s = new Series(); + s.cycles = rows.size(); + if (rows.isEmpty()) { + return s; + } + // The first fifth is start-up: the retained population is still being built and + // the page pool has not reached its working set, so it describes neither regime. + int from = rows.size() / 5; + int mid = (from + rows.size()) / 2; + for (int i = from; i < rows.size(); i++) { + s.satbRefsTotal += rows.get(i).getOrDefault("satbRefs", 0L); + s.liveObjectsMax = Math.max(s.liveObjectsMax, rows.get(i).getOrDefault("legUsed", 0L)); + } + s.pagesAtStart = rows.get(from).getOrDefault("pgTotal", 0L); + s.pagesAtMid = rows.get(mid).getOrDefault("pgTotal", 0L); + s.pagesAtEnd = rows.get(rows.size() - 1).getOrDefault("pgTotal", 0L); + return s; + } + + /** Logged references per cycle, per live object. Bounded by the live set once the + * barrier stops logging things the snapshot never contained. */ + double satbRefsPerLiveObject() { + if (cycles == 0 || liveObjectsMax == 0) { + return Double.MAX_VALUE; + } + return ((double) satbRefsTotal / cycles) / liveObjectsMax; + } + + /** Second-half page growth as a fraction of first-half page growth's endpoint. A + * heap that has reached a working set adds almost nothing here; a compounding one + * adds at least as much as it did in the first half. */ + double secondHalfPageGrowth() { + if (pagesAtMid == 0) { + return Double.MAX_VALUE; + } + return (double) (pagesAtEnd - pagesAtMid) / pagesAtMid; + } + } + + private String describe(String what, Series s) { + return what + ": cycles=" + s.cycles + + " satbRefs/cycle/liveObject=" + String.format("%.3f", s.satbRefsPerLiveObject()) + + " (total=" + s.satbRefsTotal + ", live=" + s.liveObjectsMax + ")" + + " pages " + s.pagesAtStart + " -> " + s.pagesAtMid + " -> " + s.pagesAtEnd + + " (second-half growth " + String.format("%.3f", s.secondHalfPageGrowth()) + ")"; + } + + private static final class Run { + final int exit; + final String output; + + Run(int exit, String output) { + this.exit = exit; + this.output = output; + } + } + + private Run run(Path executable, Path workingDir) throws Exception { + ProcessBuilder builder = new ProcessBuilder(executable.toString()); + builder.directory(workingDir.toFile()); + // A developer debugging the collector has CN1_* knobs exported, and several of them + // (CN1_SIMULATE_FREE_MEMORY, CN1_GC_FAULT) would invert this result rather than fail + // loudly. Start the child from a known state and give it only what this test sets. + builder.environment().keySet().removeIf(key -> key.startsWith("CN1_")); + builder.environment().put("CN1_GC_PROBE", "1"); + builder.redirectErrorStream(true); + Process process = builder.start(); + String output; + try (BufferedReader reader = new BufferedReader( + new InputStreamReader(process.getInputStream(), StandardCharsets.UTF_8))) { + output = reader.lines().collect(Collectors.joining("\n")); + } + return new Run(process.waitFor(), output); + } + + private String tail(String output) { + String[] lines = output.split("\\R"); + int from = Math.max(0, lines.length - 25); + return String.join("\n", Arrays.copyOfRange(lines, from, lines.length)); + } + + private String loadAppSource() throws Exception { + java.io.InputStream in = GcSteadyStateIntegrationTest.class + .getResourceAsStream("/com/codename1/tools/translator/GcSteadyStateApp.java"); + assertNotNull(in, "GcSteadyStateApp.java test resource should exist"); + try (BufferedReader reader = new BufferedReader(new InputStreamReader(in, StandardCharsets.UTF_8))) { + return reader.lines().collect(Collectors.joining("\n")) + "\n"; + } + } + + private String runJavaMain(CompilerHelper.CompilerConfig config, Path classesDir, Path javaApiDir) + throws Exception { + String javaExe = config.jdkHome.resolve("bin").resolve("java").toString(); + if (System.getProperty("os.name").toLowerCase().contains("win")) { + javaExe += ".exe"; + } + ProcessBuilder pb = new ProcessBuilder(javaExe, "-cp", + classesDir + System.getProperty("path.separator") + javaApiDir, "GcSteadyStateApp"); + pb.redirectErrorStream(true); + Process process = pb.start(); + String output; + try (BufferedReader reader = new BufferedReader( + new InputStreamReader(process.getInputStream(), StandardCharsets.UTF_8))) { + output = reader.lines().collect(Collectors.joining("\n")); + } + assertEquals(0, process.waitFor(), "JVM run should exit cleanly. Output: " + output); + return output; + } + + private String extractLine(String output, String prefix) { + for (String line : output.split("\\R")) { + if (line.startsWith(prefix)) { + return line.trim(); + } + } + return ""; + } + + private CompilerHelper.CompilerConfig selectCompiler() { + String[] preferredTargets = {"11", "17", "21", "25", "1.8"}; + for (String target : preferredTargets) { + for (CompilerHelper.CompilerConfig config : CompilerHelper.getAvailableCompilers(target)) { + if (CompilerHelper.isJavaApiCompatible(config)) { + return config; + } + } + } + return null; + } + + private static void deleteRecursively(Path root) { + if (root == null || !Files.exists(root)) { + return; + } + try (java.util.stream.Stream walk = Files.walk(root)) { + walk.sorted(java.util.Comparator.reverseOrder()).forEach(p -> { + try { + Files.deleteIfExists(p); + } catch (java.io.IOException ignored) { + // best effort; the OS reclaims the temp tree + } + }); + } catch (java.io.IOException ignored) { + // best effort + } + } +} diff --git a/vm/tests/src/test/resources/com/codename1/tools/translator/GcSteadyStateApp.java b/vm/tests/src/test/resources/com/codename1/tools/translator/GcSteadyStateApp.java new file mode 100644 index 00000000000..ac5ea5f8cf5 --- /dev/null +++ b/vm/tests/src/test/resources/com/codename1/tools/translator/GcSteadyStateApp.java @@ -0,0 +1,195 @@ +/* + * 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. + */ + +/** + * The steady-state half of issue #5537: does the collector reach a working set and STAY + * there, or does it compound? + * + *

Every other GC workload in this suite measures a PEAK under load, and a peak cannot + * express the reported failure. {@code GcOverflowSpiralApp} runs 50 bounded rounds and its + * test asserts peak < 2GB; a heap that grows forever at a modest rate passes that. The + * reporter's build climbed 500MB to 5GB over five minutes against a live set of a few + * hundred objects, so the statistic that matters is whether the growth STOPS.

+ * + *

The shape is the reporter's -- a deep, CPU-bound game-tree search on worker threads, + * allocating millions of tiny short-lived reference-carrying objects -- with two + * properties this gate depends on:

+ * + *
    + *
  • The live set is constant by construction. {@code legacyLiveSet} is built once + * and held for the whole run; the search retains only one path through the tree. If the + * footprint compounds, it is the VM's doing and not the program's.
    • + *
  • Several worker threads. Marking is single-threaded and runs at lowered + * priority, so whether the collector keeps up is a function of how many cores the + * mutator holds. A single-threaded version of this workload does not reproduce.
    • + *
+ * + *

Deterministic by construction -- a fixed round count and fixed seeds rather than a + * wall-clock budget -- so {@code RESULT} can be compared against the same program on a + * stock JVM, and so the gate's own measurement window is reproducible. It declares no + * natives for the same reason: the reference run has to be able to execute it unchanged. + * The VM-side numbers come from the {@code [GCPROBE]} series, which the test reads from + * stderr.

+ */ +public class GcSteadyStateApp { + + /** Board payload: 64 ints + header, a BiBOP size class, and a LEAF (no mark function). */ + private static final int BOARD_CELLS = 64; + + /** Search geometry. Depth is what sets native-stack extent, which is the input to the + * conservative root scan; branch keeps one round to a few hundred thousand nodes. */ + private static final int DEPTH = 12; + private static final int BRANCH = 3; + + /** Reference-carrying allocations per node. Only a non-leaf object reaches the mark + * worklist and only a non-leaf object can be matured into the legacy heap, so this is + * what makes the workload visible to the parts of the collector under test. */ + private static final int MOVES_PER_NODE = 4; + + /** + * Worker threads. FIXED, not derived from the runner: the gate compares two halves of + * one run against each other, so the shape has to be the same on every machine -- and + * Runtime.availableProcessors() is not part of ParparVM's JavaAPI anyway, so a + * translated build cannot ask. Four is enough to keep the single-threaded collector + * behind on any runner with two cores or more; one worker does not reproduce. + */ + private static final int THREADS = 4; + + /** Rounds per worker. Sized for a few hundred collection cycles: the gate compares the + * second half of the run against the first, so it needs enough cycles in each. */ + private static final int ROUNDS = 24; + + /** A retained legacy population, held for the whole run, so the collector's table walks + * cost something. Reference-carrying on purpose -- the rescan skips objects with no + * mark function, so a population of primitive arrays would be free and prove nothing. */ + private static final int LEGACY_BLOCKS = 256; + private static final int LEGACY_BLOCK_REFS = 128; + + static Object[][] legacyLiveSet; + static final Object SUM_LOCK = new Object(); + static long checksum = 0; + + /** One node of the search: small, short-lived, and carrying references. */ + static final class Move { + int from; + int to; + int score; + int[] board; + Move next; + } + + public static void main(String[] args) { + int threads = THREADS; + System.out.println("CONFIG depth=" + DEPTH + " branch=" + BRANCH + + " moves=" + MOVES_PER_NODE + " rounds=" + ROUNDS + " threads=" + threads); + + legacyLiveSet = new Object[LEGACY_BLOCKS][]; + for (int i = 0; i < LEGACY_BLOCKS; i++) { + Object[] block = new Object[LEGACY_BLOCK_REFS]; + for (int j = 0; j < LEGACY_BLOCK_REFS; j++) { + Move held = new Move(); + held.from = i; + held.to = j; + block[j] = held; + } + legacyLiveSet[i] = block; + } + System.out.println("BASELINE_FOOTPRINT_KB=" + footprintKb()); + + long startMs = System.currentTimeMillis(); + Thread[] workers = new Thread[threads]; + for (int t = 0; t < threads; t++) { + final int seed = t * 7919; + workers[t] = new Thread(new Runnable() { + public void run() { + long sum = 0; + int[] root = new int[BOARD_CELLS]; + for (int r = 0; r < ROUNDS; r++) { + sum += search(root, DEPTH, seed + r); + } + // Order-independent, so RESULT does not depend on scheduling. + synchronized (SUM_LOCK) { + checksum += sum; + } + } + }); + workers[t].start(); + } + for (int t = 0; t < threads; t++) { + try { + workers[t].join(); + } catch (InterruptedException e) { + } + } + + System.out.println("ELAPSED_MS=" + (System.currentTimeMillis() - startMs)); + System.out.println("FINAL_FOOTPRINT_KB=" + footprintKb()); + // Keeps the population reachable to the end and folds it into RESULT, so the + // reference comparison covers it too. + Move lastHeld = (Move) legacyLiveSet[LEGACY_BLOCKS - 1][LEGACY_BLOCK_REFS - 1]; + System.out.println("RESULT=" + (checksum + lastHeld.from + lastHeld.to)); + System.out.println("GC_STEADY_STATE_DONE"); + } + + /** + * Recursive search. Every level copies the board and builds a short chain of moves, all + * of it dead the moment the level returns -- the allocation shape of a game-tree search + * with no transposition table. + */ + private static int search(int[] board, int depth, int seed) { + if (depth == 0) { + int s = 0; + for (int i = 0; i < BOARD_CELLS; i++) { + s += board[i] * (i + 1); + } + return s & 0xff; + } + int best = -1; + for (int b = 0; b < BRANCH; b++) { + int[] child = new int[BOARD_CELLS]; + for (int i = 0; i < BOARD_CELLS; i++) { + child[i] = board[i] + ((seed + b + i) & 7); + } + Move chain = null; + for (int m = 0; m < MOVES_PER_NODE; m++) { + Move mv = new Move(); + mv.from = b; + mv.to = m; + mv.score = seed + m; + mv.board = child; + mv.next = chain; + chain = mv; + } + int v = search(child, depth - 1, seed + b + chain.to); + if (v > best) { + best = v; + } + } + return best; + } + + private static long footprintKb() { + Runtime r = Runtime.getRuntime(); + return (r.totalMemory() - r.freeMemory()) / 1024; + } +} From c641e00b81b970e8d61d4a9a8542eabb32892596 Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Tue, 25 Aug 2026 07:46:48 +0300 Subject: [PATCH 02/21] Defend a headroom reserve under a per-process ceiling (issue #5537) The previous commit stopped the heap growing without bound. This one stops the process parking itself on the kill line, which #5585 flagged as open and which is what turns a native spike into a jetsam kill. Budget headroom is not a footprint bound. Admission against os_proc_available_memory answers only "is there budget left", so it keeps saying yes until the budget is gone. Measured on the issue-5537 game-tree shape under a simulated 1.4GB ceiling, seven times: 1,271MB resident and 63MB of headroom left, every time, against a live set of a few hundred objects. That repeatability is the tell -- it is not an accident of the workload, it is the policy converging on ceiling minus CN1_PACING_HEADROOM_MARGIN by construction. The ceiling is not special either: give the same workload an 8GB budget and it rides to 7.5GB. There is no footprint TARGET anywhere in the design. 63MB is the whole margin, and the renderer spends out of the same budget -- #5598 measured one screen texture at 30MB. So the collector now also bounds how far the mutator may run ahead of it, but only once headroom drops inside a reserve of a quarter of the budget (CN1_PACING_RESERVE_SHIFT). Inside the reserve the mutator is clamped to the static cap, the collector gets ahead, and the footprint falls back out. Gating on HEADROOM rather than on footprint is what makes this affordable: it is a control loop that engages only inside the reserve, not a tax on every allocation, and volumeParks in the [PACING] report is 0 for a run that never enters it. Both allocation paths are charged against ONE figure. Bounding them separately is a defect this code has had before -- each running a full cap ahead of a cap derived from the same budget -- and the reserve is derived from the BUDGET, never from the device's free RAM, which is the defect #5563 fixed. cn1BibopPacingCap is deliberately not reused for that reason. Measured, builds interleaved within one session (-DCN1_PACING_NO_RESERVE is the same binary with the bound compiled out), simulated 1.4GB ceiling, four workers: peak footprint smallest headroom seen no reserve 1271MB, x7 63MB, x7 reserve limit>>2 1027-1036MB 298-304MB 4.8x the margin. Throughput across seven interleaved pairs came out at 0.90 to 0.99 of the unbounded build, median 0.94; the spread is session drift, not the bound, and the sign never changed. A single repetition each of the tighter reserves put >> 3 at 1183MB/150MB and >> 4 at 1207MB/127MB, both slower -- a smaller reserve engages later and thrashes closer to the edge -- so a quarter is the knee rather than a compromise. Roughly 6% for that margin is a different trade from the volume brakes #5573 and #5585 measured at 2-4x and rejected. It cannot touch a platform with no per-process budget, because the whole branch is unreachable there: vm/benchmarks measures geomean 0.9398 against master, i.e. still 6% FASTER from the previous commit's SATB fix, with no benchmark regressing and every checksum identical. cn1PacingPastGrowthFloor's rate-limited footprint probe is factored out as cn1PacingFootprintNow so both bounds read through it. Behaviour-preserving: each of its three early returns previously answered FALSE, and the fast path above already established that the cached value is under the floor. GcSteadyStateIntegrationTest gains a third scenario asserting the process defends its reserve under a simulated ceiling, and a fourth that rebuilds with -DCN1_PACING_NO_RESERVE and requires the third to fail -- otherwise a gate that never engages would report green forever. Verified: 520 vm/tests non-benchmark tests green; all seven GC benchmark tests green (ProcessBudgetPacingIntegrationTest included, which exercises the same budgeted path); run-gc-verify.sh green with both fault self-tests; run-gauntlet.sh green with every checksum matching; nine ablation flag combinations compile. Co-Authored-By: Claude Opus 5 (1M context) --- CLAUDE.md | 16 ++ vm/ByteCodeTranslator/src/cn1_globals.m | 181 ++++++++++++++++-- .../GcSteadyStateIntegrationTest.java | 76 ++++++++ 3 files changed, 260 insertions(+), 13 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 6a0dfc3a5ea..4753bc182e6 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -304,6 +304,22 @@ the live set rather than by the allocation rate, and that the page heap stops gr the second half of the run; then it rebuilds with `-DCN1_SATB_LOG_FRESH` and **requires both assertions to fail**, so the gate cannot go inert. +**Under a per-process ceiling, budget headroom is not a footprint bound.** Admission +against `os_proc_available_memory()` answers only "is there budget left", so on its own it +keeps saying yes until the budget is gone and the process converges on ceiling minus +`CN1_PACING_HEADROOM_MARGIN` however small its live set is. The collector therefore also +defends a reserve — `CN1_PACING_RESERVE_SHIFT`, a quarter of the budget — by clamping how +far the mutator may run ahead of it once headroom drops inside that reserve. It is a +control loop, not a tax — `volumeParks` in the `[PACING]` report is 0 for a run that never +enters the reserve — and the whole branch is unreachable on a platform with no per-process +budget, which is why the `vm/benchmarks` numbers are untouched by it. Note the ceiling is +not special: given an 8GB budget the unbounded build rides to 7.5GB, because admission has +no footprint *target*. `-DCN1_PACING_NO_RESERVE` compiles it out for +A/B, and is what the gate's third scenario re-injects to prove it can fail. + +Reach for `CN1_SIMULATE_PROC_MEMORY_LIMIT=` to exercise any of this off-device — +without it the budgeted pacing path never runs, which is how the original bug survived. + ### Working with Native Code Platform-specific native code locations: diff --git a/vm/ByteCodeTranslator/src/cn1_globals.m b/vm/ByteCodeTranslator/src/cn1_globals.m index 5b4cef8e655..32feea0a855 100644 --- a/vm/ByteCodeTranslator/src/cn1_globals.m +++ b/vm/ByteCodeTranslator/src/cn1_globals.m @@ -528,6 +528,10 @@ static void cn1StartSimulatedMemoryWarnings(void) { // -- a park count cannot, since the unbudgeted path parks too. minHeadroom is the least // remaining budget ever observed; -1 means the bounded path never ran. static _Atomic long cn1PacingBoundedChecks = 0; +// Parks caused by the VOLUME bound rather than by an exhausted budget. Separating them +// matters: park counts alone cannot tell "the process is near its ceiling" from "the +// mutator is too far ahead of the collector", and the two call for opposite responses. +static _Atomic long cn1PacingVolumeParks = 0; static _Atomic long cn1PacingMinHeadroom = -1; static _Atomic int cn1PacingTrace = -1; static int cn1PacingTraceOn(void) { @@ -546,12 +550,13 @@ static void cn1ReportPacingParks(void) { long minCap = atomic_load_explicit(&cn1PacingMinCap, memory_order_relaxed); long minHead = atomic_load_explicit(&cn1PacingMinHeadroom, memory_order_relaxed); fprintf(stderr, "[PACING] bibopParks=%ld legacyParks=%ld minCapKb=%ld" - " boundedChecks=%ld minHeadroomKb=%ld\n", + " boundedChecks=%ld minHeadroomKb=%ld volumeParks=%ld\n", atomic_load_explicit(&cn1PacingParksBibop, memory_order_relaxed), atomic_load_explicit(&cn1PacingParksLegacy, memory_order_relaxed), minCap == 0x7fffffffffffffffLL ? -1L : minCap / 1024, atomic_load_explicit(&cn1PacingBoundedChecks, memory_order_relaxed), - minHead < 0 ? -1L : minHead / 1024); + minHead < 0 ? -1L : minHead / 1024, + atomic_load_explicit(&cn1PacingVolumeParks, memory_order_relaxed)); } // Mark-worklist overflow accounting, reported by CN1_LOG_GC_OVERFLOW at exit and @@ -3936,27 +3941,44 @@ static void cn1BibopUpdateThreadPolicy(CODENAME_ONE_THREAD_STATE) { // CN1_PACING_FOOTPRINT_REFRESH_MS across all threads. That bounds how far the mutator can // run past the floor before the bound engages to one refresh interval's worth of // allocation, instead of one COLLECTION's worth. -static JAVA_BOOLEAN cn1PacingPastGrowthFloor(void) { - if(atomic_load_explicit(&cn1CachedProcFootprint, memory_order_relaxed) - > CN1_PACING_GROWTH_FLOOR_BYTES) { - return JAVA_TRUE; - } +/** + * The process footprint, refreshed at most once per CN1_PACING_FOOTPRINT_REFRESH_MS + * across all threads. Returns the cached figure otherwise, and 0 where the platform has + * no probe at all. + * + * cn1CachedProcFootprint is also refreshed once per cycle by cn1RefreshFreeMemCache, but + * a cycle is exactly the interval a runaway happens in -- at a couple of GB/s a 750ms + * cycle is more than a gigabyte -- so any bound that has to decide DURING a cycle reads + * through here instead. + */ +static long long cn1PacingFootprintNow(void) { + long long cached = atomic_load_explicit(&cn1CachedProcFootprint, memory_order_relaxed); JAVA_LONG now = cn1MonotonicMillis(); JAVA_LONG last = atomic_load_explicit(&cn1ProcFootprintStampMs, memory_order_relaxed); if(now - last < CN1_PACING_FOOTPRINT_REFRESH_MS) { - return JAVA_FALSE; // probed recently and it was under; believe that + return cached; // probed recently; believe that } if(!atomic_compare_exchange_strong_explicit(&cn1ProcFootprintStampMs, &last, now, memory_order_relaxed, memory_order_relaxed)) { - return JAVA_FALSE; // another thread is taking this interval's probe + return cached; // another thread is taking this interval's probe } long long fp = (long long)cn1ProcFootprintBytes(); if(fp <= 0) { - return JAVA_FALSE; // no probe on this platform; the bound stays off + return cached; // no probe on this platform } atomic_store_explicit(&cn1CachedProcFootprint, fp, memory_order_relaxed); - return fp > CN1_PACING_GROWTH_FLOOR_BYTES; + return fp; +} + +static JAVA_BOOLEAN cn1PacingPastGrowthFloor(void) { + // Once the cache is over the floor the bound is engaged and a syscall to re-confirm + // it buys nothing, so this stays ahead of the probe. + if(atomic_load_explicit(&cn1CachedProcFootprint, memory_order_relaxed) + > CN1_PACING_GROWTH_FLOOR_BYTES) { + return JAVA_TRUE; + } + return cn1PacingFootprintNow() > CN1_PACING_GROWTH_FLOOR_BYTES; } static long cn1BibopPacingCap(CODENAME_ONE_THREAD_STATE) { @@ -4038,6 +4060,83 @@ static long long cn1PacingVolume(int which) { return (long long)atomic_load_explicit(&bibopBytesSinceGc, memory_order_relaxed); } +#ifndef CN1_DISABLE_BIBOP +/** + * Uncollected bytes across BOTH allocation paths, as ONE figure. + * + * Charging them against separate caps is a defect this code has had before: two paths + * each running a full cap ahead of a cap derived from the same budget, so the process ran + * twice as far ahead as the cap said. + */ +static long long cn1PacingUncollectedBytes(void) { + return (long long)atomic_load_explicit(&bibopBytesSinceGc, memory_order_relaxed) + + (long long)atomic_load_explicit(&cn1LegacyBytesSinceGc, memory_order_relaxed); +} + +// Everything below is derived from the BUDGET, never from the device's free RAM. Sizing +// backpressure against the device is exactly the defect #5563 fixed -- a high-throughput +// thread was licensed half of a big iPad's free memory while the process was allowed a +// fraction of that -- and it is the same defect whether the figure feeds a cap, a claim +// or a reserve. cn1BibopPacingCap is deliberately NOT reused here for that reason. +#ifndef CN1_PACING_RESERVE_SHIFT +// The share of the budget the collector defends as free headroom: limit >> 2, a quarter. +// +// What that headroom has to absorb is a NATIVE spike out of the same budget -- the +// largest single one this codebase knows about is a 30MB screen texture (#5598) -- plus +// whatever the mutator dirties between deciding to park and parking, which is why it is +// a share of the budget rather than a fixed figure. +// +// Measured on the issue-5537 game-tree shape, four workers, under a simulated 1.4GB +// ceiling, builds interleaved within one session (-DCN1_PACING_NO_RESERVE is the same +// binary with this bound compiled out): +// +// peak footprint smallest headroom seen +// no reserve 1271MB, x7 63MB, x7 +// reserve limit>>2 1027-1036MB 298-304MB +// +// Both columns are that repeatable because neither is an accident: without the bound, +// admission converges on ceiling minus CN1_PACING_HEADROOM_MARGIN by construction; with +// it, the control loop holds the reserve. Throughput across seven interleaved pairs came +// out at 0.90 to 0.99 of the unbounded build, median 0.94 -- the spread is session drift, +// not the bound, and the sign never changed. +// +// A single repetition each of the tighter reserves put >> 3 at 1183MB/150MB and >> 4 at +// 1207MB/127MB, both slower than >> 2: a smaller reserve engages later and thrashes +// closer to the edge, so a quarter is the knee rather than a compromise. +// +// Roughly 6% for 4.8x the margin is a different trade from the volume brakes #5573 and +// #5585 measured at 2-4x and rejected, and the reason it is affordable is that it is a +// control loop that engages only inside the reserve, not a tax on every allocation -- +// volumeParks in the [PACING] report is 0 for a run that never enters it. +// +// It cannot touch a platform with no per-process budget at all, because this whole branch +// is unreachable there; that is why vm/benchmarks measures the same with and without it. +// +// The ceiling figure above is not special. Given an 8GB budget instead, the unbounded +// build rides to 7.5GB and this one holds 5.7GB: admission has no footprint TARGET, so +// whatever ceiling a process is given is where it ends up. (Those two numbers are peaks +// only -- at 7.5GB resident the measuring host is itself under pressure, so no throughput +// conclusion can be drawn from that configuration.) +#define CN1_PACING_RESERVE_SHIFT 2 +#endif + +/** + * The headroom, in bytes, that the collector defends for a process whose whole budget is + * limitBytes. + * + * A plain share, with no floor. A floor is tempting and wrong: any absolute one large + * enough to matter on an iPad exceeds the whole budget of a tightly-limited process (two + * admission margins is 128MB, and an app extension can be limited to less than that), + * which would leave the bound permanently engaged there. Below roughly a 256MB budget the + * share falls under CN1_PACING_HEADROOM_MARGIN and this bound goes quiet, which is + * correct rather than a gap: admission already refuses inside the margin, so the reserve + * would have nothing left to defend. + */ +static long long cn1PacingReserveBytes(long long limitBytes) { + return limitBytes >> CN1_PACING_RESERVE_SHIFT; +} +#endif + // Atomically admit this thread if the live budget, minus what other threads have already // been admitted to dirty, still covers this block plus the margin. Test and claim must be // one step: a plain check followed by a separate add lets every waiter observe the same @@ -4130,7 +4229,44 @@ static void cn1PacingPark(CODENAME_ONE_THREAD_STATE, int which, long long pendin memory_order_relaxed)) { } } - JAVA_BOOLEAN admitted = cn1PacingTryAdmit(procHeadroom, need, pendingBytes); + // BUDGET HEADROOM IS NOT A FOOTPRINT BOUND. + // + // Admission against os_proc_available_memory answers "is there budget left", so it + // keeps saying yes until the budget is GONE, and the process converges on + // ceiling-minus-margin however small its live set is. Measured on the issue-5537 + // game-tree shape against a simulated 1.4GB ceiling, seven times: 1,271MB resident + // and 63MB of headroom left, every time, against a live set of a few hundred objects. + // That 63MB IS the margin, and the renderer spends out of the same budget -- #5598 + // measured one screen texture at 30MB -- so anything that spikes lands on the kill + // line. + // + // So also bound how far the mutator may run ahead of the collector once headroom + // drops inside the reserve. That is what keeps the footprint proportional to the + // COLLECTOR'S WORK rather than to the device's budget, and it is the bound the + // unbudgeted branch has always had and this one dropped. + JAVA_BOOLEAN volumeOk = JAVA_TRUE; +#if !defined(CN1_DISABLE_BIBOP) && !defined(CN1_PACING_NO_RESERVE) + { + long long footprint = cn1PacingFootprintNow(); + if(footprint > 0 + && (long long)procHeadroom + < cn1PacingReserveBytes(footprint + (long long)procHeadroom)) { + // Inside the reserve: clamp the mutator to the STATIC cap so the collector + // gets ahead and the footprint falls back out of it. Gating on HEADROOM + // rather than on footprint is what makes this cost nothing until it is + // needed -- a process using three quarters of its budget and holding still + // is not in danger; one with no headroom left is. + long long trigger = (long long)atomic_load_explicit(&bibopGcTriggerBytes, + memory_order_relaxed); + volumeOk = cn1PacingUncollectedBytes() + <= trigger * CN1_BIBOP_GC_HARD_CAP_MULTIPLIER + ? JAVA_TRUE : JAVA_FALSE; + } + } +#endif + // Short-circuit deliberately: cn1PacingTryAdmit CLAIMS on success, so it must not run + // while the volume bound is refusing. + JAVA_BOOLEAN admitted = volumeOk && cn1PacingTryAdmit(procHeadroom, need, pendingBytes); if(admitted) { return; } @@ -4138,6 +4274,9 @@ static void cn1PacingPark(CODENAME_ONE_THREAD_STATE, int which, long long pendin atomic_fetch_add_explicit(which == CN1_PACE_LEGACY ? &cn1PacingParksLegacy : &cn1PacingParksBibop, 1, memory_order_relaxed); + if(!volumeOk) { + atomic_fetch_add_explicit(&cn1PacingVolumeParks, 1, memory_order_relaxed); + } } CN1_GC_PARK_CAPTURE(threadStateData); // fresh capture for the coop conservative scan threadStateData->threadActive = JAVA_FALSE; @@ -4164,7 +4303,23 @@ static void cn1PacingPark(CODENAME_ONE_THREAD_STATE, int which, long long pendin if(headroomNow < 0) { break; // budget disappeared under us; nothing to honour } - if(cn1PacingTryAdmit(headroomNow, need, pendingBytes)) { +#if !defined(CN1_DISABLE_BIBOP) && !defined(CN1_PACING_NO_RESERVE) + if(!volumeOk) { + // Re-test the volume bound too, or a thread refused by it would spin out its + // whole wait against a budget test that was never the thing blocking it. + // bibopBytesSinceGc is exchanged to 0 at cycle START, so this clears as soon + // as the collection this park requested begins. + long long footprintNow = cn1PacingFootprintNow(); + long long trigger = (long long)atomic_load_explicit(&bibopGcTriggerBytes, + memory_order_relaxed); + volumeOk = (footprintNow <= 0 + || headroomNow >= cn1PacingReserveBytes(footprintNow + headroomNow) + || cn1PacingUncollectedBytes() + <= trigger * CN1_BIBOP_GC_HARD_CAP_MULTIPLIER) + ? JAVA_TRUE : JAVA_FALSE; + } +#endif + if(volumeOk && cn1PacingTryAdmit(headroomNow, need, pendingBytes)) { admitted = JAVA_TRUE; break; } diff --git a/vm/tests/src/test/java/com/codename1/tools/translator/GcSteadyStateIntegrationTest.java b/vm/tests/src/test/java/com/codename1/tools/translator/GcSteadyStateIntegrationTest.java index f44ceef1aaf..e8f872abb70 100644 --- a/vm/tests/src/test/java/com/codename1/tools/translator/GcSteadyStateIntegrationTest.java +++ b/vm/tests/src/test/java/com/codename1/tools/translator/GcSteadyStateIntegrationTest.java @@ -92,6 +92,23 @@ class GcSteadyStateIntegrationTest { /** Cycles needed before the comparison means anything. Anti-vacuousness. */ private static final int MIN_CYCLES = 24; + /** + * Synthetic per-process budget for the ceiling scenario. Well above what this workload + * needs, so that whatever the process settles at is the pacing policy's doing and not + * the workload's. + */ + private static final long CEILING_MB = 1400; + + /** + * The share of that budget the collector must keep free (CN1_PACING_RESERVE_SHIFT). + * Asserted at half, because the assertion is about which REGIME the process is in -- + * defending a reserve, or converging on the admission margin -- and those are 300MB + * and 63MB apart. A tolerance tight enough to distinguish 300 from 280 would be + * measuring the runner. + */ + private static final long RESERVE_MB = CEILING_MB / 4; + private static final long MIN_HEADROOM_MB = RESERVE_MB / 2; + @Test void aChurningWorkloadReachesAWorkingSetAndStaysThere() throws Exception { Parser.cleanup(); @@ -188,6 +205,59 @@ private void runGate(List tempDirs) throws Exception { assertTrue(bad.satbRefsPerLiveObject() > good.satbRefsPerLiveObject() * 10, "The fresh-reference filter should cut the log by orders of magnitude. " + describe("fixed", good) + " " + describe("faulted", bad)); + + // ---- 3. under a per-process ceiling, the collector defends a reserve ---- + // Budget headroom is not a footprint bound: admission answers "is there budget + // left", so on its own it keeps saying yes until the budget is gone and the + // process converges on ceiling-minus-margin however small its live set is. That + // is survivable only until something else spends out of the same budget, which + // on iOS the renderer does. + Map ceiling = new HashMap<>(); + ceiling.put("CN1_SIMULATE_PROC_MEMORY_LIMIT", Long.toString(CEILING_MB * 1024 * 1024)); + Run bounded = run(fixed, distDir, ceiling); + assertEquals(0, bounded.exit, + "The workload must finish under a ceiling. Output: " + tail(bounded.output)); + long boundedHeadroomMb = minHeadroomMb(bounded.output); + assertTrue(boundedHeadroomMb >= 0, + "No [PACING] report under a simulated ceiling -- the budgeted path never " + + "ran, so this scenario measured nothing. Output: " + tail(bounded.output)); + assertTrue(boundedHeadroomMb >= MIN_HEADROOM_MB, + "Under a " + CEILING_MB + "MB budget the collector should defend about " + + RESERVE_MB + "MB of headroom, but the smallest seen was " + + boundedHeadroomMb + "MB -- the process is riding the kill line."); + + // ---- 4. proof that scenario 3 can fail --------------------------------- + Path noReserve = build(distDir, tempDirs, "noreserve", + "-DCN1_GC_CONFORM -DCN1_PACING_NO_RESERVE"); + Run unbounded = run(noReserve, distDir, ceiling); + long unboundedHeadroomMb = minHeadroomMb(unbounded.output); + assertTrue(unboundedHeadroomMb >= 0, + "No [PACING] report from the no-reserve build. Output: " + tail(unbounded.output)); + assertTrue(unboundedHeadroomMb < MIN_HEADROOM_MB, + "Compiling the reserve out did NOT put the process back on the admission " + + "margin (smallest headroom " + unboundedHeadroomMb + "MB), so this " + + "scenario is inert."); + } + + /** Smallest headroom the pacing tracer saw, in MB, or -1 if it never reported. */ + private long minHeadroomMb(String output) { + for (String line : output.split("\\R")) { + int at = line.indexOf("minHeadroomKb="); + if (at < 0) { + continue; + } + String rest = line.substring(at + "minHeadroomKb=".length()); + int end = 0; + if (end < rest.length() && rest.charAt(end) == '-') { + end++; + } + while (end < rest.length() && Character.isDigit(rest.charAt(end))) { + end++; + } + long kb = Long.parseLong(rest.substring(0, end)); + return kb < 0 ? -1 : kb / 1024; + } + return -1; } /** One build of the already-translated project, with its own flags and build dir. */ @@ -297,6 +367,10 @@ private static final class Run { } private Run run(Path executable, Path workingDir) throws Exception { + return run(executable, workingDir, new HashMap()); + } + + private Run run(Path executable, Path workingDir, Map env) throws Exception { ProcessBuilder builder = new ProcessBuilder(executable.toString()); builder.directory(workingDir.toFile()); // A developer debugging the collector has CN1_* knobs exported, and several of them @@ -304,6 +378,8 @@ private Run run(Path executable, Path workingDir) throws Exception { // loudly. Start the child from a known state and give it only what this test sets. builder.environment().keySet().removeIf(key -> key.startsWith("CN1_")); builder.environment().put("CN1_GC_PROBE", "1"); + builder.environment().put("CN1_LOG_PACING_PARKS", "1"); + builder.environment().putAll(env); builder.redirectErrorStream(true); Process process = builder.start(); String output; From 012eb18077f49d6a19a0e0a4433dd3ae90680af9 Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Tue, 25 Aug 2026 10:26:57 +0300 Subject: [PATCH 03/21] Give the benchmark driver its GPL header, and scope two helpers to their use check-copyright-headers rejects a new source file without the complete Codename One GPLv2 + Classpath Exception header, and vm/benchmarks/src is in scope. cn1PacingUncollectedBytes and cn1PacingReserveBytes are used only from the reserve bound, so they are guarded on the same condition it is -- otherwise compiling the bound out with -DCN1_PACING_NO_RESERVE leaves them as unused statics. Co-Authored-By: Claude Opus 5 (1M context) --- vm/ByteCodeTranslator/src/cn1_globals.m | 2 +- .../src/com/bench/GcSteadyState.java | 22 +++++++++++++++++++ 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/vm/ByteCodeTranslator/src/cn1_globals.m b/vm/ByteCodeTranslator/src/cn1_globals.m index 32feea0a855..4004caf6d50 100644 --- a/vm/ByteCodeTranslator/src/cn1_globals.m +++ b/vm/ByteCodeTranslator/src/cn1_globals.m @@ -4060,7 +4060,7 @@ static long long cn1PacingVolume(int which) { return (long long)atomic_load_explicit(&bibopBytesSinceGc, memory_order_relaxed); } -#ifndef CN1_DISABLE_BIBOP +#if !defined(CN1_DISABLE_BIBOP) && !defined(CN1_PACING_NO_RESERVE) /** * Uncollected bytes across BOTH allocation paths, as ONE figure. * diff --git a/vm/benchmarks/src/com/bench/GcSteadyState.java b/vm/benchmarks/src/com/bench/GcSteadyState.java index 2ed29e2bf8d..5b5f281fe31 100644 --- a/vm/benchmarks/src/com/bench/GcSteadyState.java +++ b/vm/benchmarks/src/com/bench/GcSteadyState.java @@ -1,3 +1,25 @@ +/* + * 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; /** From c5de00016d8cb5cae53551d8e7c25c5dcd3544ba Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Tue, 25 Aug 2026 10:41:08 +0300 Subject: [PATCH 04/21] Count benchmark nodes per worker, not through a shared racy counter The driver incremented one static long from four workers with an unsynchronised read-modify-write, and the sampler read it concurrently. That is not merely imprecise: the rate at which increments are lost depends on CONTENTION, and contention is exactly what differs between the builds this benchmark compares -- a build whose threads park more loses fewer increments and so reports a throughput advantage it has not got. The per-round `nodes = localNodes` writeback also overwrote the shared total instead of combining the workers' counts. Each worker now counts into its own slot, and NODES= is summed after join(), which gives it a happens-before edge to every worker's last write. The SAMPLE series sums the same slots while they are still being written, so it is renamed nodes~= and documented as a progress indicator rather than a measurement. The CI fixture (GcSteadyStateApp) never had a node counter -- its assertions come from the [GCPROBE] series -- so nothing the gate asserts is affected. Reported by chatgpt-codex-connector on #5599. Co-Authored-By: Claude Opus 5 (1M context) --- .../src/com/bench/GcSteadyState.java | 42 +++++++++++++++---- 1 file changed, 33 insertions(+), 9 deletions(-) diff --git a/vm/benchmarks/src/com/bench/GcSteadyState.java b/vm/benchmarks/src/com/bench/GcSteadyState.java index 5b5f281fe31..2857367dd82 100644 --- a/vm/benchmarks/src/com/bench/GcSteadyState.java +++ b/vm/benchmarks/src/com/bench/GcSteadyState.java @@ -71,7 +71,20 @@ public class GcSteadyState { static Object[][] legacyLiveSet; static final Object SUM_LOCK = new Object(); static long checksum = 0; - static long nodes = 0; + /** + * Per-worker node counts, one slot each, so counting costs no synchronisation and + * loses no increments. A single shared counter cannot be used for this: an + * unsynchronised read-modify-write from four workers drops updates at a rate that + * depends on CONTENTION, and contention is precisely what differs between the builds + * this benchmark compares -- a build whose threads park more would lose fewer + * increments and so report a throughput advantage it does not have. + * + * NODES= at the end is the authoritative figure: it is summed after join(), which + * gives it a happens-before edge to every worker's last write. The SAMPLE series sums + * the same slots while they are still being written, so it is a progress indicator + * and not a measurement. + */ + static long[] nodeCounts; /** One node of the search: small, short-lived, and REFERENCE-CARRYING. Only a non-leaf * object has a mark function, and only such an object is eligible for maturation into @@ -102,6 +115,7 @@ public static void main(String[] args) { // table walk costs nothing and this workload cannot tell a cheap drain from an // O(heap) one. Reference-carrying, because the rescan skips objects with no mark // function. + nodeCounts = new long[threads]; legacyLiveSet = new Object[legacyBlocks][]; for (int i = 0; i < legacyBlocks; i++) { Object[] block = new Object[LEGACY_BLOCK_REFS]; @@ -118,7 +132,7 @@ public static void main(String[] args) { public void run() { while (!stop) { System.out.println("SAMPLE tMs=" + probeMs() + " fpKb=" + footprintKb() - + " nodes=" + nodes); + + " nodes~=" + sumNodes()); sleep(250); } } @@ -128,24 +142,24 @@ public void run() { Thread[] workers = new Thread[threads]; for (int t = 0; t < threads; t++) { final int seed = t * 7919; + final int slot = t; workers[t] = new Thread(new Runnable() { public void run() { long sum = 0; - long localNodes = 0; + long[] counter = new long[1]; int[] root = new int[BOARD_CELLS]; int round = 0; while (!stop) { - sum += search(root, depth, seed + round); - localNodes = nodes; + sum += search(root, depth, seed + round, counter); round++; if (sleepMs > 0) { sleep(sleepMs); } } + nodeCounts[slot] = counter[0]; // Order-independent, so the checksum does not depend on scheduling. synchronized (SUM_LOCK) { checksum += sum; - nodes = localNodes; } } }); @@ -179,6 +193,7 @@ public void run() { scrub(scrubDepth); } + System.out.println("NODES=" + sumNodes()); System.out.println("ELAPSED_MS=" + (System.currentTimeMillis() - startMs)); System.out.println("FINAL_FOOTPRINT_KB=" + footprintKb()); Move lastHeld = (Move) legacyLiveSet[legacyBlocks - 1][LEGACY_BLOCK_REFS - 1]; @@ -186,11 +201,12 @@ public void run() { System.out.println("GC_STEADY_STATE_DONE"); } - private static int search(int[] board, int d, int seed) { + private static int search(int[] board, int d, int seed, long[] c) { if (stop) { return 0; } - nodes++; + // Thread-private: this array belongs to one worker for the whole run. + c[0]++; if (d == 0) { int s = 0; for (int i = 0; i < BOARD_CELLS; i++) { @@ -214,7 +230,7 @@ private static int search(int[] board, int d, int seed) { mv.next = chain; chain = mv; } - int v = search(child, d - 1, seed + b + chain.to); + int v = search(child, d - 1, seed + b + chain.to, c); if (v > best) { best = v; } @@ -236,6 +252,14 @@ private static int scrub(int d) { return pad[0] + scrub(d - 1); } + private static long sumNodes() { + long total = 0; + for (int i = 0; i < nodeCounts.length; i++) { + total += nodeCounts[i]; + } + return total; + } + private static long footprintKb() { Runtime r = Runtime.getRuntime(); return (r.totalMemory() - r.freeMemory()) / 1024; From 3ec70f1bd39d03c0ae59f20a2a1943980f891cf7 Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Tue, 25 Aug 2026 10:54:30 +0300 Subject: [PATCH 05/21] Keep every probe row a single-cycle row, publish node counts live Two review findings on #5599, both real. cn1GcProbeCycle returned early on a skipped cycle without clearing the phase accumulators, so with CN1_GC_PROBE>1 snapMs/graceMs/satbMs and friends carried a whole interval while markMs and sweepMs described only the cycle that just ran -- two time bases in one row, which would attribute an interval's worth of a phase to a single cycle's pause. The resets move into cn1GcProbeResetPhases and run on every cycle, printed or not. The cumulative counters (matured, consWords, staleSkips) are deliberately left alone: those are running totals the reader diffs. The benchmark driver published each worker's node count only after the run stopped, so every SAMPLE line reported zero. It now republishes once per round; a worker that stalls stops publishing and its slot going flat is the signal. Neither affected any measurement reported so far -- every run used CN1_GC_PROBE=1, where the skip path is unreachable, and the throughput figures come from NODES=, which is summed after join(). Also corrects the reserve's throughput figures, which came from the racy counter the previous commit replaced. Re-measured with the exact one, four interleaved pairs: 0.97-1.05 of the unbounded build, median 0.99, two of four faster with the bound on. The previous "median 0.94" overstated the cost. Peak footprint and headroom are unchanged (1271MB/63MB against 1015-1027MB/306-308MB) -- those come from the probe and Runtime, not the counter. The claim that a smaller reserve is "slower" is withdrawn; >>3 and >>4 buy less on peak and headroom, which is the argument that survives. Reported by chatgpt-codex-connector on #5599. Co-Authored-By: Claude Opus 5 (1M context) --- vm/ByteCodeTranslator/src/cn1_globals.m | 80 +++++++++++-------- .../src/com/bench/GcSteadyState.java | 13 ++- 2 files changed, 56 insertions(+), 37 deletions(-) diff --git a/vm/ByteCodeTranslator/src/cn1_globals.m b/vm/ByteCodeTranslator/src/cn1_globals.m index 4004caf6d50..253f285cfac 100644 --- a/vm/ByteCodeTranslator/src/cn1_globals.m +++ b/vm/ByteCodeTranslator/src/cn1_globals.m @@ -4090,32 +4090,28 @@ + (long long)atomic_load_explicit(&cn1LegacyBytesSinceGc, memory_order_relaxed); // ceiling, builds interleaved within one session (-DCN1_PACING_NO_RESERVE is the same // binary with this bound compiled out): // -// peak footprint smallest headroom seen -// no reserve 1271MB, x7 63MB, x7 -// reserve limit>>2 1027-1036MB 298-304MB +// peak footprint smallest headroom throughput +// no reserve 1271MB, x4 63MB, x4 1.00 +// reserve limit>>2 1015-1027MB 306-308MB 0.97-1.05, median 0.99 // -// Both columns are that repeatable because neither is an accident: without the bound, -// admission converges on ceiling minus CN1_PACING_HEADROOM_MARGIN by construction; with -// it, the control loop holds the reserve. Throughput across seven interleaved pairs came -// out at 0.90 to 0.99 of the unbounded build, median 0.94 -- the spread is session drift, -// not the bound, and the sign never changed. +// The first two columns are that repeatable because neither is an accident: without the +// bound, admission converges on ceiling minus CN1_PACING_HEADROOM_MARGIN by construction; +// with it, the control loop holds the reserve. Throughput is a wash -- two of the four +// pairs came out faster with the bound on -- which is what a bound that engages only +// inside the reserve, rather than taxing every allocation, should look like. Note +// volumeParks in the [PACING] report reads 0 for a run that never enters the reserve. // -// A single repetition each of the tighter reserves put >> 3 at 1183MB/150MB and >> 4 at -// 1207MB/127MB, both slower than >> 2: a smaller reserve engages later and thrashes -// closer to the edge, so a quarter is the knee rather than a compromise. -// -// Roughly 6% for 4.8x the margin is a different trade from the volume brakes #5573 and -// #5585 measured at 2-4x and rejected, and the reason it is affordable is that it is a -// control loop that engages only inside the reserve, not a tax on every allocation -- -// volumeParks in the [PACING] report is 0 for a run that never enters it. +// A single repetition each of the tighter reserves put >> 3 at 1183MB of peak and 150MB +// of headroom, and >> 4 at 1207MB/127MB: a smaller reserve engages later and closer to +// the edge, buying less on both axes, so a quarter is a knee rather than a compromise. // // It cannot touch a platform with no per-process budget at all, because this whole branch // is unreachable there; that is why vm/benchmarks measures the same with and without it. // // The ceiling figure above is not special. Given an 8GB budget instead, the unbounded // build rides to 7.5GB and this one holds 5.7GB: admission has no footprint TARGET, so -// whatever ceiling a process is given is where it ends up. (Those two numbers are peaks -// only -- at 7.5GB resident the measuring host is itself under pressure, so no throughput +// whatever ceiling a process is given is where it ends up. (Those two are peaks only -- +// at 7.5GB resident the measuring host is itself under pressure, so no throughput // conclusion can be drawn from that configuration.) #define CN1_PACING_RESERVE_SHIFT 2 #endif @@ -9295,9 +9291,40 @@ static long long cn1GcProbeSideBytes(void) { // That is the only point in the program where no mark is in flight and no mutator owns a // retired page, which is what makes walking bibopAllPages here safe -- the 1Hz emitter // below must never do it (cn1BibopFormatPage rewrites page geometry underneath a reader). +/** + * Clear the phase accumulators. Runs on EVERY cycle, printed or not. + * + * The cumulative counters (matured, consWords, staleSkips, ...) are deliberately left + * alone: they are running totals and the reader diffs them. These are per-cycle, and they + * have to be cleared on a skipped cycle too -- markMs and sweepMs describe only the cycle + * that just ran, so letting the phase figures accumulate over a whole CN1_GC_PROBE>1 + * interval would put two different time bases in one row and attribute an interval's worth + * of a phase to a single cycle's pause. + */ +static void cn1GcProbeResetPhases(void) { + cn1GcSnapNs = 0; + cn1GcGraceNs = 0; + cn1GcDrainNs = 0; + cn1GcWaitNs = 0; + cn1GcStackNs = 0; + cn1GcTDrainNs = 0; + cn1GcMigrateNs = 0; + cn1GcMigrated = 0; + cn1GcSatbNs = 0; + cn1GcSatbEntries = 0; + cn1GcSatbDrainAlready = 0; + atomic_store_explicit(&cn1GcSatbAlready, 0, memory_order_relaxed); + atomic_store_explicit(&cn1GcSatbFresh, 0, memory_order_relaxed); + cn1GcPoolNs = 0; +} + void cn1GcProbeCycle(double markMs, double sweepMs) { int every = cn1GcProbeEvery(); - if(every == 0 || (currentGcMarkValue % every) != 0) { + if(every == 0) { + return; + } + if((currentGcMarkValue % every) != 0) { + cn1GcProbeResetPhases(); return; } long long pgTotal = 0, pgEmpty = 0, pgReleased = 0, pgAdopted = 0, pgMon = 0; @@ -9396,20 +9423,7 @@ void cn1GcProbeCycle(double markMs, double sweepMs) { sideBytes / 1024, residKb); fflush(stderr); // Per-CYCLE, so reset after reporting. A running total cannot show a trend. - cn1GcSnapNs = 0; - cn1GcGraceNs = 0; - cn1GcDrainNs = 0; - cn1GcWaitNs = 0; - cn1GcStackNs = 0; - cn1GcTDrainNs = 0; - cn1GcMigrateNs = 0; - cn1GcMigrated = 0; - cn1GcSatbNs = 0; - cn1GcSatbEntries = 0; - cn1GcSatbDrainAlready = 0; - atomic_store_explicit(&cn1GcSatbAlready, 0, memory_order_relaxed); - atomic_store_explicit(&cn1GcSatbFresh, 0, memory_order_relaxed); - cn1GcPoolNs = 0; + cn1GcProbeResetPhases(); } // 1Hz wall-clock series. ATOMICS ONLY -- it must never walk bibopAllPages. This is the diff --git a/vm/benchmarks/src/com/bench/GcSteadyState.java b/vm/benchmarks/src/com/bench/GcSteadyState.java index 2857367dd82..8f8a8d953bf 100644 --- a/vm/benchmarks/src/com/bench/GcSteadyState.java +++ b/vm/benchmarks/src/com/bench/GcSteadyState.java @@ -79,10 +79,11 @@ public class GcSteadyState { * this benchmark compares -- a build whose threads park more would lose fewer * increments and so report a throughput advantage it does not have. * - * NODES= at the end is the authoritative figure: it is summed after join(), which - * gives it a happens-before edge to every worker's last write. The SAMPLE series sums - * the same slots while they are still being written, so it is a progress indicator - * and not a measurement. + * Each worker republishes its slot once per round, so the SAMPLE series tracks + * progress live. NODES= at the end is the authoritative figure: it is summed after + * join(), which gives it a happens-before edge to every worker's last write. The + * SAMPLE series reads the same slots while they are still being written, so it is a + * progress indicator and not a measurement. */ static long[] nodeCounts; @@ -151,6 +152,10 @@ public void run() { int round = 0; while (!stop) { sum += search(root, depth, seed + round, counter); + // Publish once per round so the SAMPLE series is a live progress + // indicator rather than a row of zeroes. A worker that stalls + // stops publishing, and its slot going flat IS the signal. + nodeCounts[slot] = counter[0]; round++; if (sleepMs > 0) { sleep(sleepMs); From 0dc41ffed09673a182585376ae6b5ef625ca854c Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Tue, 25 Aug 2026 11:06:00 +0300 Subject: [PATCH 06/21] Load the mark word atomically in the barrier, and harden the ceiling scenario Three findings, two from review and one the review's tighter test surfaced. The SATB filter read __codenameOneGcMark with a plain load while the marker reaches the same field through __atomic_*. That is a mixed atomic/non-atomic access to one object -- undefined in C, and the same bug class #5598 fixed in the constant pool. The concrete hazard is not tearing but the compiler caching a -1 across several inlined barriers in one loop, which would keep suppressing entries after the object had aged into a genuine snapshot object. Now __ATOMIC_RELAXED, and the comment says why relaxed and not acquire: nothing is published through this read, both stale answers are safe, and what relaxed buys is that the load happens at all. An acquire fence on every object store buys nothing over that and is not free on arm64. The two CN1_GC_CONFORM census reads of the same field move with it. The fault-injected runs' measurements were accepted without checking exit status or the completion marker, so a build that crashed after emitting enough probe rows would have satisfied the assertions and turned a memory-safety regression into a green gate. Both now go through assertHealthy first. The ceiling scenario used a 1400MB budget, which needs the mutator to actually outrun the collector by 1.3GB -- and how far it outruns depends on how many cores it has to itself, so a two-core runner might never get there and the fourth scenario would go quietly inert. It now uses 768MB, which admission converges on by construction rather than by winning a race. The threshold between the two regimes becomes ABSOLUTE, twice CN1_PACING_HEADROOM_MARGIN, because the margin does not scale with the budget: a proportional threshold silently stops separating them as the budget shrinks, which is exactly what happened at 400MB (reserve 100MB, margin still 63MB, half the reserve below it). Reported by chatgpt-codex-connector on #5599. Co-Authored-By: Claude Opus 5 (1M context) --- vm/ByteCodeTranslator/src/cn1_globals.m | 32 ++++++++--- .../GcSteadyStateIntegrationTest.java | 55 +++++++++++++++---- 2 files changed, 66 insertions(+), 21 deletions(-) diff --git a/vm/ByteCodeTranslator/src/cn1_globals.m b/vm/ByteCodeTranslator/src/cn1_globals.m index 253f285cfac..0637e6ee396 100644 --- a/vm/ByteCodeTranslator/src/cn1_globals.m +++ b/vm/ByteCodeTranslator/src/cn1_globals.m @@ -1527,19 +1527,31 @@ void cn1SatbEnqueue(JAVA_OBJECT old) { // cycle, so mark time and footprint climb together until the collector is continuous // -- exactly the reported symptom pair. // - // Racy read of a GC-thread-owned epoch, deliberately: a stale value can only make the - // test fail and log something that did not need logging, which is the conservative - // direction. A free slot's sentinel mark is neither value, so it still reaches the - // log and is rejected by gcMarkObject exactly as before. - if(old->__codenameOneGcMark == -1) { + // ATOMIC, and RELAXED rather than acquire. The marker reaches this same field through + // __atomic_* accesses, so a plain load here would be a mixed atomic/non-atomic access + // to one object -- undefined in C, and the concrete hazard is not tearing but the + // compiler CACHING a -1 across several inlined barriers in one loop: the filter would + // then keep suppressing after the object had aged into a genuine snapshot object, and + // a later reference move would escape the log. + // + // Relaxed is sufficient and acquire is not wanted. Nothing is published through this + // read -- it decides only whether to log -- and both stale answers are safe: a stale + // -1 for an object that has since been marked suppresses an entry gcMarkObject would + // have dropped anyway, and a stale non-fresh value logs an entry that was not needed. + // What relaxed buys is that the load actually happens at each barrier. An acquire + // fence on every object store, on the mutator's hottest path, buys nothing over that. + // + // A free slot's sentinel mark is neither value, so it still reaches the log and is + // rejected by gcMarkObject exactly as before. + if(__atomic_load_n(&old->__codenameOneGcMark, __ATOMIC_RELAXED) == -1) { return; } #endif #ifdef CN1_GC_CONFORM { - // Racy read of a GC-thread-owned value on purpose: this is a census, and a stale - // read can only misclassify, never corrupt. - int __m = old->__codenameOneGcMark; + // Same reasoning as the filter above; a census may misclassify but must not be a + // mixed atomic/non-atomic access to the field. + int __m = __atomic_load_n(&old->__codenameOneGcMark, __ATOMIC_RELAXED); if(__m == currentGcMarkValue) { atomic_fetch_add_explicit(&cn1GcSatbAlready, 1, memory_order_relaxed); } else if(__m == -1) { @@ -2273,7 +2285,9 @@ void codenameOneGCMark() { long before = gcMarkNewObjectCount; for(long i = 0 ; i < n ; i++) { #ifdef CN1_GC_CONFORM - if(batch[i] != JAVA_NULL && batch[i]->__codenameOneGcMark == currentGcMarkValue) { + if(batch[i] != JAVA_NULL + && __atomic_load_n(&batch[i]->__codenameOneGcMark, __ATOMIC_RELAXED) + == currentGcMarkValue) { cn1GcSatbDrainAlready++; } #endif diff --git a/vm/tests/src/test/java/com/codename1/tools/translator/GcSteadyStateIntegrationTest.java b/vm/tests/src/test/java/com/codename1/tools/translator/GcSteadyStateIntegrationTest.java index e8f872abb70..1cf5df4da7d 100644 --- a/vm/tests/src/test/java/com/codename1/tools/translator/GcSteadyStateIntegrationTest.java +++ b/vm/tests/src/test/java/com/codename1/tools/translator/GcSteadyStateIntegrationTest.java @@ -93,21 +93,37 @@ class GcSteadyStateIntegrationTest { private static final int MIN_CYCLES = 24; /** - * Synthetic per-process budget for the ceiling scenario. Well above what this workload - * needs, so that whatever the process settles at is the pacing policy's doing and not - * the workload's. + * Synthetic per-process budget for the ceiling scenario. + * + * Deliberately TIGHT rather than device-sized. The fourth scenario needs the + * no-reserve build to actually reach its ceiling, and how far a mutator outruns the + * collector depends on how many cores it has to itself -- on a two-core runner the + * single collector thread competes far better than it does on a developer's machine, + * so a 1.4GB budget is reached locally and might not be in CI. A budget this size is + * reached by any runner that can run the workload at all, because admission converges + * on ceiling-minus-margin by construction rather than by winning a race. + * + * Still comfortably above CN1_PACING_HEADROOM_MARGIN x 4, so the reserve is a + * meaningful figure and not swallowed by the admission margin. */ - private static final long CEILING_MB = 1400; + private static final long CEILING_MB = 768; /** - * The share of that budget the collector must keep free (CN1_PACING_RESERVE_SHIFT). - * Asserted at half, because the assertion is about which REGIME the process is in -- - * defending a reserve, or converging on the admission margin -- and those are 300MB - * and 63MB apart. A tolerance tight enough to distinguish 300 from 280 would be - * measuring the runner. + * The reserve the collector should defend at that budget (CN1_PACING_RESERVE_SHIFT). */ private static final long RESERVE_MB = CEILING_MB / 4; - private static final long MIN_HEADROOM_MB = RESERVE_MB / 2; + + /** + * The line between the two regimes: defending a reserve, or converging on the bare + * admission margin. Twice CN1_PACING_HEADROOM_MARGIN, i.e. an ABSOLUTE figure rather + * than a share of the budget -- the margin does not scale with the budget, so a + * proportional threshold silently stops separating the regimes as the budget shrinks + * (at a 400MB budget the reserve is 100MB and the margin still 64MB, and half the + * reserve falls below it). The two regimes here are 192MB and 63MB, so this sits + * clear of both; a tolerance tight enough to distinguish 192 from 180 would be + * measuring the runner. + */ + private static final long HEADROOM_THRESHOLD_MB = 128; @Test void aChurningWorkloadReachesAWorkingSetAndStaysThere() throws Exception { @@ -195,6 +211,7 @@ private void runGate(List tempDirs) throws Exception { // the filter silently compiled out would pass part 1 forever. Path faulty = build(distDir, tempDirs, "faulted", "-DCN1_GC_CONFORM -DCN1_SATB_LOG_FRESH"); Run faulted = run(faulty, distDir); + assertHealthy(faulted, "the -DCN1_SATB_LOG_FRESH build"); Series bad = Series.parse(faulted.output); assertTrue(bad.cycles >= MIN_CYCLES, "The faulted build produced no [GCPROBE] series, so CN1_GC_CONFORM is not " @@ -221,7 +238,7 @@ private void runGate(List tempDirs) throws Exception { assertTrue(boundedHeadroomMb >= 0, "No [PACING] report under a simulated ceiling -- the budgeted path never " + "ran, so this scenario measured nothing. Output: " + tail(bounded.output)); - assertTrue(boundedHeadroomMb >= MIN_HEADROOM_MB, + assertTrue(boundedHeadroomMb >= HEADROOM_THRESHOLD_MB, "Under a " + CEILING_MB + "MB budget the collector should defend about " + RESERVE_MB + "MB of headroom, but the smallest seen was " + boundedHeadroomMb + "MB -- the process is riding the kill line."); @@ -230,15 +247,29 @@ private void runGate(List tempDirs) throws Exception { Path noReserve = build(distDir, tempDirs, "noreserve", "-DCN1_GC_CONFORM -DCN1_PACING_NO_RESERVE"); Run unbounded = run(noReserve, distDir, ceiling); + assertHealthy(unbounded, "the -DCN1_PACING_NO_RESERVE build"); long unboundedHeadroomMb = minHeadroomMb(unbounded.output); assertTrue(unboundedHeadroomMb >= 0, "No [PACING] report from the no-reserve build. Output: " + tail(unbounded.output)); - assertTrue(unboundedHeadroomMb < MIN_HEADROOM_MB, + assertTrue(unboundedHeadroomMb < HEADROOM_THRESHOLD_MB, "Compiling the reserve out did NOT put the process back on the admission " + "margin (smallest headroom " + unboundedHeadroomMb + "MB), so this " + "scenario is inert."); } + /** + * A fault-injected run's measurements are only admissible if the run itself was + * healthy. Without this, a build that crashed or was OOM-killed after emitting enough + * probe rows would satisfy the cycle and inflated-SATB assertions and turn a + * memory-safety regression into a green gate -- the faults injected here are policy + * regressions, not crashes, so a crash means something else is wrong. + */ + private void assertHealthy(Run r, String which) { + assertEquals(0, r.exit, which + " must still exit cleanly. Output: " + tail(r.output)); + assertTrue(r.output.contains("GC_STEADY_STATE_DONE"), + which + " must run to completion. Output: " + tail(r.output)); + } + /** Smallest headroom the pacing tracer saw, in MB, or -1 if it never reported. */ private long minHeadroomMb(String output) { for (String line : output.split("\\R")) { From 24c3ca8e186156d4e15c161942265d59a5e11372 Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Tue, 25 Aug 2026 11:11:18 +0300 Subject: [PATCH 07/21] Do not size an adopted BiBOP slot as if it were a malloc block The probe sized every non-null allObjectsInHeap entry with malloc_size / malloc_usable_size. A MATURED object is in that table but its storage is a slot inside a posix_memalign'd BiBOP arena, so the pointer is interior: glibc's malloc_usable_size reads the chunk header immediately below it and returns a garbage figure, and CI runs this gate on Linux. Its bytes are also already counted in residentPgBytes, so anything it did return double-counted into the residual that is this probe's whole point. Only an object the table INDEXES (__heapPosition >= 0) owns an individual block. The rest are counted as legAdopted instead -- the same population as matured - maturedDied but measured from the table rather than from the counters, so the two disagreeing is itself a finding. Not a small corner: on the game-tree workload legAdopted is 32,907 of a legUsed of 33,164, so 99% of the table was being sized this way. It was harmless on macOS only because malloc_size answers 0 for an interior pointer, which is also why legBlockKb read flat through the original investigation and correctly never carried the drift. Verified after the change: run-gc-verify.sh green with both fault self-tests, and vm/benchmarks geomean 0.9422 against master (0.9398 before the previous commit's atomic load, i.e. that load costs nothing), no benchmark regressing, checksums identical. Reported by chatgpt-codex-connector on #5599. Co-Authored-By: Claude Opus 5 (1M context) --- vm/ByteCodeTranslator/src/cn1_globals.m | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/vm/ByteCodeTranslator/src/cn1_globals.m b/vm/ByteCodeTranslator/src/cn1_globals.m index 0637e6ee396..03ebf385dd0 100644 --- a/vm/ByteCodeTranslator/src/cn1_globals.m +++ b/vm/ByteCodeTranslator/src/cn1_globals.m @@ -9373,7 +9373,17 @@ void cn1GcProbeCycle(double markMs, double sweepMs) { } #endif // The legacy heap is a separate malloc'd population that no page figure can see. - long long legUsed = 0, legBlockBytes = 0; + // Only an object the table INDEXES (__heapPosition >= 0) owns an individual malloc + // block. A matured one carries CN1_BIBOP_ADOPTED and its storage is a slot inside a + // BiBOP page, so it must not be sized here for two separate reasons: the pointer is + // interior to a posix_memalign'd arena, which makes it an invalid argument to + // malloc_size / malloc_usable_size (glibc reads the chunk header immediately below + // the pointer and would hand back a garbage figure), and its bytes are already + // counted in residentPgBytes, so adding them again would corrupt the residual that + // is this probe's whole point. Counted separately instead -- legAdopted is the same + // population as matured minus maturedDied, measured from the table rather than from + // the counters, so the two disagreeing is itself a finding. + long long legUsed = 0, legAdopted = 0, legBlockBytes = 0; int legCap = currentSizeOfAllObjectsInHeap; for(int i = 0 ; i < legCap ; i++) { JAVA_OBJECT o = allObjectsInHeap[i]; @@ -9381,7 +9391,11 @@ void cn1GcProbeCycle(double markMs, double sweepMs) { continue; } legUsed++; - legBlockBytes += CN1_CONFORM_BLOCK_SIZE(o); + if(o->__heapPosition >= 0) { + legBlockBytes += CN1_CONFORM_BLOCK_SIZE(o); + } else { + legAdopted++; + } } long long legTableBytes = (long long)sizeOfAllObjectsInHeap * (long long)sizeof(JAVA_OBJECT); long long sideBytes = cn1GcProbeSideBytes(); @@ -9393,7 +9407,7 @@ void cn1GcProbeCycle(double markMs, double sweepMs) { "[GCPROBE] v=1 cyc=%d tMs=%lld fpKb=%lld" " pgTotal=%lld pgEmpty=%lld pgReleased=%lld pgAdopted=%lld pgMon=%lld pgOwned=%lld pgGrace=%lld" " resvKb=%lld residentPgKb=%lld liveSlotKb=%lld deadSlotKb=%lld" - " legCap=%d legUsed=%lld legTableKb=%lld legBlockKb=%lld" + " legCap=%d legUsed=%lld legAdopted=%lld legTableKb=%lld legBlockKb=%lld" " matured=%ld maturedDied=%ld maturedPages=%ld" " triggerKb=%ld bypassActs=%ld bypassAllocs=%ld occKb=%ld liveKb=%ld reclKb=%ld" " markMs=%.1f sweepMs=%.1f snapMs=%.1f graceMs=%.1f drainMs=%.1f" @@ -9405,7 +9419,7 @@ void cn1GcProbeCycle(double markMs, double sweepMs) { currentGcMarkValue, cn1GcProbeElapsedMs(), fpKb, pgTotal, pgEmpty, pgReleased, pgAdopted, pgMon, pgOwned, pgGrace, resvBytes / 1024, residentPgBytes / 1024, liveSlots / 1024, deadSlots / 1024, - legCap, legUsed, legTableBytes / 1024, legBlockBytes / 1024, + legCap, legUsed, legAdopted, legTableBytes / 1024, legBlockBytes / 1024, atomic_load_explicit(&cn1GcMaturedTotal, memory_order_relaxed), atomic_load_explicit(&cn1GcMaturedDied, memory_order_relaxed), atomic_load_explicit(&cn1GcMaturedPages, memory_order_relaxed), From ed34c06b1d623ff50fbcf166f6a156545a34581f Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Tue, 25 Aug 2026 11:24:01 +0300 Subject: [PATCH 08/21] Close the wait timer at the wait, read the cycle counter atomically Three findings from review; two fixed, one measured and answered in the code. waitMs was opened before the safepoint wait and closed only after the allocation migration and both stack scans, so it double-counted work already attributed to migrateMs and stackMs -- a phase breakdown that overlaps reads a long root scan as mutator wait time, which is the opposite of what it exists to say. It now opens and closes around the wait alone, inside the lightweightThread branch, so a native thread (which is never waited for) contributes 0 instead of everything up to markStatics. The 1Hz emitter read currentGcMarkValue with a plain load while the collector increments that ordinary int -- a data race, in the one emitter documented as "atomics only" and built to keep reporting exactly when the collector is stalled. Now an atomic relaxed load, as is the mutator-side comparison in the SATB census. Not taken: requiring the -DCN1_SATB_LOG_FRESH build to also blow the second-half page-growth bound. Measured across two runs of that build, its second-half growth is 0.446 and then 0.033 -- a runaway's page pool sometimes saturates before the midpoint and the ratio then reads flat while the heap is enormous. That assertion would fail about half the time, and a coin-flip gate is worse than the inertness it guards against. The reasoning, the numbers and what does have teeth (the SATB metric, five orders of magnitude, every time) are recorded on the constant. Both series are now printed on every run so the ratio stays auditable rather than merely asserted. Verified after these changes: phases sum to markMs with no overlap (16.0 of 16.3); 520 vm/tests non-benchmark tests green; all seven GC benchmark tests green. Reported by chatgpt-codex-connector on #5599. Co-Authored-By: Claude Opus 5 (1M context) --- vm/ByteCodeTranslator/src/cn1_globals.m | 30 ++++++++++++------- .../GcSteadyStateIntegrationTest.java | 15 ++++++++++ 2 files changed, 34 insertions(+), 11 deletions(-) diff --git a/vm/ByteCodeTranslator/src/cn1_globals.m b/vm/ByteCodeTranslator/src/cn1_globals.m index 03ebf385dd0..c1e29651424 100644 --- a/vm/ByteCodeTranslator/src/cn1_globals.m +++ b/vm/ByteCodeTranslator/src/cn1_globals.m @@ -1552,7 +1552,9 @@ void cn1SatbEnqueue(JAVA_OBJECT old) { // Same reasoning as the filter above; a census may misclassify but must not be a // mixed atomic/non-atomic access to the field. int __m = __atomic_load_n(&old->__codenameOneGcMark, __ATOMIC_RELAXED); - if(__m == currentGcMarkValue) { + // Read atomically for the same reason: this runs on a mutator, and the collector + // owns and increments currentGcMarkValue. + if(__m == __atomic_load_n(¤tGcMarkValue, __ATOMIC_RELAXED)) { atomic_fetch_add_explicit(&cn1GcSatbAlready, 1, memory_order_relaxed); } else if(__m == -1) { atomic_fetch_add_explicit(&cn1GcSatbFresh, 1, memory_order_relaxed); @@ -1778,10 +1780,6 @@ void codenameOneGCMark() { // and would otherwise satisfy useCoop with that stale SP forever, // silently skipping the live region below it (missed roots -> UAF). t->gcParkCaptured = JAVA_FALSE; -#endif -#ifdef CN1_GC_CONFORM - long long __wt0 = cn1GcNowNs(); - int __wtActive = 1; #endif // wait for the thread to pause so we can traverse its stack but not for native threads where // we don't have much control and who barely call into Java anyway @@ -1789,6 +1787,14 @@ void codenameOneGCMark() { t->threadBlockedByGC = JAVA_TRUE; int totalwait = 0; long now = time(0); +#ifdef CN1_GC_CONFORM + // Opened and closed around the safepoint wait ALONE. Closing it later + // -- after the migration and the stack scans -- would fold their cost + // into waitMs as well as into migrateMs and stackMs, and a phase + // breakdown that double-counts reads a long root scan as mutator wait. + // A non-lightweight thread is never waited for, and now contributes 0. + long long __wt0 = cn1GcNowNs(); +#endif while(t->threadActive) { usleep(500); totalwait += 500; @@ -1803,6 +1809,9 @@ void codenameOneGCMark() { } } } +#ifdef CN1_GC_CONFORM + cn1GcWaitNs += cn1GcNowNs() - __wt0; +#endif } // place allocations from the local thread into the global heap list. @@ -1944,11 +1953,6 @@ void codenameOneGCMark() { #endif #ifdef CN1_GC_VERIFY { extern const char* cn1GcMarkPhase; cn1GcMarkPhase = "statics"; } -#endif -#ifdef CN1_GC_CONFORM - // The spin above is over by the time control reaches here, whatever path - // it took, so this is the honest close for the safepoint wait. - if(__wtActive) { cn1GcWaitNs += cn1GcNowNs() - __wt0; __wtActive = 0; } #endif markStatics(d); // Drain the worklist before unblocking the thread so that every object @@ -9465,7 +9469,11 @@ void cn1GcProbeCycle(double markMs, double sweepMs) { " bytesSinceGc=%lld staleSkips=%ld\n", cn1GcProbeElapsedMs(), (long long)cn1ProcFootprintBytes() / 1024, - currentGcMarkValue, + // Atomic: the collector increments this ordinary int in codenameOneGCMark + // while this detached thread reads it. Plain would be a data race, and this + // emitter exists precisely to keep reporting when the collector is stalled -- + // the moment a stale or invented value would mislead most. + __atomic_load_n(¤tGcMarkValue, __ATOMIC_RELAXED), #ifdef CN1_DISABLE_BIBOP 0LL, #else diff --git a/vm/tests/src/test/java/com/codename1/tools/translator/GcSteadyStateIntegrationTest.java b/vm/tests/src/test/java/com/codename1/tools/translator/GcSteadyStateIntegrationTest.java index 1cf5df4da7d..959e06dde1d 100644 --- a/vm/tests/src/test/java/com/codename1/tools/translator/GcSteadyStateIntegrationTest.java +++ b/vm/tests/src/test/java/com/codename1/tools/translator/GcSteadyStateIntegrationTest.java @@ -86,6 +86,19 @@ class GcSteadyStateIntegrationTest { * How much the page heap may still grow in the second half of the run, relative to the * first. Zero would be wrong: a run reaches its working set at its own pace and a * partially-filled arena is 64 pages. A COMPOUNDING heap doubles here. + * + *

This is the OUTCOME check, and unlike the other three assertions it deliberately + * has no fault twin. The obvious one -- requiring the -DCN1_SATB_LOG_FRESH build to + * exceed this bound -- was measured and rejected: across two runs of that build the + * second-half growth came out 0.446 and then 0.033, because a runaway's page pool + * sometimes saturates before the midpoint and the ratio then reads flat while the heap + * is enormous. Asserting it would fail about half the time, and a coin-flip gate is + * worse than the inertness it would be guarding against.

+ * + *

What has teeth is the MECHANISM check above: the same faulted build separates + * from the fixed one by five orders of magnitude on satbRefs per live object, every + * time. Both series are printed on every run so this ratio stays auditable rather than + * merely asserted.

*/ private static final double MAX_SECOND_HALF_PAGE_GROWTH = 0.25; @@ -222,6 +235,8 @@ private void runGate(List tempDirs) throws Exception { assertTrue(bad.satbRefsPerLiveObject() > good.satbRefsPerLiveObject() * 10, "The fresh-reference filter should cut the log by orders of magnitude. " + describe("fixed", good) + " " + describe("faulted", bad)); + System.err.println("[GcSteadyState] " + describe("fixed", good)); + System.err.println("[GcSteadyState] " + describe("faulted", bad)); // ---- 3. under a per-process ceiling, the collector defends a reserve ---- // Budget headroom is not a footprint bound: admission answers "is there budget From 223d629f4d7ffc1914c4783f8c2e08a008fe58b2 Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Tue, 25 Aug 2026 11:38:26 +0300 Subject: [PATCH 09/21] Read the collector's atomic epoch mirror, and claim a matured page with one edge Two follow-ups from review, both correct. The previous commit made the 1Hz emitter's read of currentGcMarkValue atomic while codenameOneGCMark still increments it with a plain ++. That is half a fix: an atomic read of a plainly-written object is still a mixed access and still undefined. Both sides now go through bibopGcEpoch, the collector's own _Atomic mirror of the same value, published at cycle start -- which is what the reviewer offered as the alternative and what should have been used first. The mutator-side comparison in the SATB census moves with it. Where there is no page heap there is no mirror, so the emitter reports cyc=-1 rather than a figure read through a data race. cn1MaturedPages tested gcHasAdopted and then let the existing plain store set it. The CAS above guarantees one thread matures a given OBJECT, but two markers can mature two different objects on the SAME page, so both could observe FALSE and both count it -- and the plain store is itself a data race the moment gcMarkResolveThreadCount stops returning 1. Now one __atomic_exchange_n: exactly one thread sees the FALSE->TRUE edge, and it does the counting. That the ratio is read chiefly in the CN1_GC_MARK_THREADS>1 arm is the point -- it would have been wrong exactly where it is used. Verified in that arm: maturedPages=2121 of pgTotal=11214, a plausible ratio rather than an inflated one. run-gc-verify.sh green with both fault self-tests; the steady state, heap integrity and process budget gates green; seven ablation combinations compile. Reported by chatgpt-codex-connector on #5599. Co-Authored-By: Claude Opus 5 (1M context) --- vm/ByteCodeTranslator/src/cn1_globals.m | 56 ++++++++++++++++++------- 1 file changed, 40 insertions(+), 16 deletions(-) diff --git a/vm/ByteCodeTranslator/src/cn1_globals.m b/vm/ByteCodeTranslator/src/cn1_globals.m index c1e29651424..7e400cc1fe0 100644 --- a/vm/ByteCodeTranslator/src/cn1_globals.m +++ b/vm/ByteCodeTranslator/src/cn1_globals.m @@ -1290,20 +1290,30 @@ static void cn1MatureObject(JAVA_OBJECT obj) { // Sticky-flag the host page so its slots always take the full per-slot sweep walk // (which skips live -4 slots) instead of the O(1) page reset, which would recycle this // still-live object's memory out from under the legacy collector. -#ifdef CN1_GC_CONFORM { CN1BibopPage* __mp = (CN1BibopPage*)(((uintptr_t)obj) & ~((uintptr_t)CN1_BIBOP_PAGE_SIZE - 1)); + // ONE atomic transition rather than a test and a separate store. The CAS above + // guarantees a single thread matures a given OBJECT, but two markers can mature + // two different objects on the SAME page concurrently -- so a plain store here is + // a data race the moment gcMarkResolveThreadCount stops returning 1, and a + // test-then-increment would count that page twice. Exchange returns the previous + // value, so exactly one thread sees the FALSE->TRUE edge. + JAVA_BOOLEAN __wasAdopted = __atomic_exchange_n(&__mp->gcHasAdopted, JAVA_TRUE, + __ATOMIC_RELAXED); +#ifdef CN1_GC_CONFORM atomic_fetch_add_explicit(&cn1GcMaturedTotal, 1, memory_order_relaxed); - // Count the page only on the FALSE->TRUE transition: gcHasAdopted is sticky, so - // counting every maturation would report maturations, not pinned pages, and the - // ratio pinnedPages/pagesRegistered is the whole point (a pinned page can never - // take the O(1) reclaim shortcut again). - if(__mp->gcHasAdopted == JAVA_FALSE) { + // Count the PAGE only on that edge: gcHasAdopted is sticky, so counting every + // maturation would report maturations rather than pinned pages, and the ratio + // pinnedPages/pagesRegistered is the whole point -- a pinned page can never take + // the O(1) reclaim shortcut again. That ratio is read chiefly in the + // CN1_GC_MARK_THREADS>1 arm, which is exactly where a racy count would be wrong. + if(__wasAdopted == JAVA_FALSE) { atomic_fetch_add_explicit(&cn1GcMaturedPages, 1, memory_order_relaxed); } - } +#else + (void)__wasAdopted; #endif - ((CN1BibopPage*)(((uintptr_t)obj) & ~((uintptr_t)CN1_BIBOP_PAGE_SIZE - 1)))->gcHasAdopted = JAVA_TRUE; + } // Buffer for post-mark registration (NOT placeObjectInHeapCollection here -- see above). pthread_mutex_lock(&gcAdoptMutex); if(gcAdoptTop >= gcAdoptCap) { @@ -1552,9 +1562,14 @@ void cn1SatbEnqueue(JAVA_OBJECT old) { // Same reasoning as the filter above; a census may misclassify but must not be a // mixed atomic/non-atomic access to the field. int __m = __atomic_load_n(&old->__codenameOneGcMark, __ATOMIC_RELAXED); - // Read atomically for the same reason: this runs on a mutator, and the collector - // owns and increments currentGcMarkValue. - if(__m == __atomic_load_n(¤tGcMarkValue, __ATOMIC_RELAXED)) { + // Against the ATOMIC mirror, not currentGcMarkValue: this runs on a mutator while + // the collector increments that plain int, and an atomic read of a plainly-written + // object is still a mixed access. +#ifdef CN1_DISABLE_BIBOP + if(0) { +#else + if(__m == atomic_load_explicit(&bibopGcEpoch, memory_order_relaxed)) { +#endif atomic_fetch_add_explicit(&cn1GcSatbAlready, 1, memory_order_relaxed); } else if(__m == -1) { atomic_fetch_add_explicit(&cn1GcSatbFresh, 1, memory_order_relaxed); @@ -9469,11 +9484,20 @@ void cn1GcProbeCycle(double markMs, double sweepMs) { " bytesSinceGc=%lld staleSkips=%ld\n", cn1GcProbeElapsedMs(), (long long)cn1ProcFootprintBytes() / 1024, - // Atomic: the collector increments this ordinary int in codenameOneGCMark - // while this detached thread reads it. Plain would be a data race, and this - // emitter exists precisely to keep reporting when the collector is stalled -- - // the moment a stale or invented value would mislead most. - __atomic_load_n(¤tGcMarkValue, __ATOMIC_RELAXED), +#ifdef CN1_DISABLE_BIBOP + // No page heap means no atomic mirror of the cycle counter, and making only + // the READER atomic would not make currentGcMarkValue's plain ++ in + // codenameOneGCMark well-defined. Report "unavailable" rather than a figure + // read through a data race. + -1, +#else + // bibopGcEpoch is the collector's own _Atomic mirror of currentGcMarkValue, + // published at cycle start. Reading it keeps both sides of this access atomic + // -- which making just the reader atomic would not have done -- and matters + // here because this emitter exists to keep reporting when the collector is + // stalled, the moment a stale cycle number misleads most. + atomic_load_explicit(&bibopGcEpoch, memory_order_relaxed), +#endif #ifdef CN1_DISABLE_BIBOP 0LL, #else From dee9ec9f57a1bfc4e3c901274f6283c1c35ac411 Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Tue, 25 Aug 2026 11:56:08 +0300 Subject: [PATCH 10/21] Make every collector-side write to the mark word atomic The barrier's read was made atomic two commits ago while gcMarkObject still stamped the same field with a plain store, so the pair was still a mixed access. The field already had an atomic convention here -- gcMarkObject's own read is __ATOMIC_ACQUIRE, the BiBOP publish is __ATOMIC_RELEASE -- and the plain writes were the inconsistency, not the new read. Every write that can run concurrently with a mutator is now a relaxed atomic store: gcMarkObject's stamp, both sweeps' grace promotion, both free-mark stores, the nursery promotion and the CN1_GC_VERIFY poison. Relaxed compiles to the same instruction on every target we build; what it buys is that the write is a write the reader is allowed to observe. Header INITIALISATION deliberately stays plain, in codenameOneGcMalloc and in cn1FusedInstallPrimArray. Those are not concurrent with anything: the barrier only ever reads the mark of an object the mutator holds a reference to, so one already published, and the publishing store orders the initialisation against any reader. That distinction is not free-floating -- making those two atomic as well cost 1.2 points of benchmark geomean (0.9550 against 0.9432, with arraySequential, quicksort and valueEscape all moving and returning), because they sit on the allocation fast path. The reasoning is recorded at the site so the next person does not reintroduce it for symmetry. Verified: vm/benchmarks geomean 0.9432 against master, six rounds interleaved, no benchmark regressing and checksums identical; run-gc-verify.sh green with both fault self-tests; all seven GC gates green; five ablation combinations compile including -DCN1_NURSERY. Reported by chatgpt-codex-connector on #5599. Co-Authored-By: Claude Opus 5 (1M context) --- vm/ByteCodeTranslator/src/cn1_globals.h | 2 +- vm/ByteCodeTranslator/src/cn1_globals.m | 22 +++++++++++++++------- 2 files changed, 16 insertions(+), 8 deletions(-) diff --git a/vm/ByteCodeTranslator/src/cn1_globals.h b/vm/ByteCodeTranslator/src/cn1_globals.h index 231157cfbf1..ff3a80a5d68 100644 --- a/vm/ByteCodeTranslator/src/cn1_globals.h +++ b/vm/ByteCodeTranslator/src/cn1_globals.h @@ -2183,7 +2183,7 @@ extern JAVA_OBJECT cn1AllocFused(CODENAME_ONE_THREAD_STATE, int totalSize, struc static inline JAVA_OBJECT cn1FusedInstallPrimArray(JAVA_OBJECT owner, int off, struct clazz* acls, int esz, int len) { struct JavaArrayPrototype* a = (struct JavaArrayPrototype*)((char*)owner + off); a->__codenameOneParentClsReference = acls; - a->__codenameOneGcMark = -1; + a->__codenameOneGcMark = -1; // not yet published; see codenameOneGcMalloc a->__heapPosition = -1; a->length = len; a->dimensions = 1; diff --git a/vm/ByteCodeTranslator/src/cn1_globals.m b/vm/ByteCodeTranslator/src/cn1_globals.m index 7e400cc1fe0..5cbf12794ad 100644 --- a/vm/ByteCodeTranslator/src/cn1_globals.m +++ b/vm/ByteCodeTranslator/src/cn1_globals.m @@ -2749,7 +2749,7 @@ void codenameOneGCSweep() { //counter++; } } else { - o->__codenameOneGcMark = currentGcMarkValue; + __atomic_store_n(&o->__codenameOneGcMark, currentGcMarkValue, __ATOMIC_RELAXED); } } } @@ -5330,7 +5330,7 @@ static void cn1BibopSweep(CODENAME_ONE_THREAD_STATE) { } } cn1GcVerifyPoisonSlot(__o, page->slotSize); - __o->__codenameOneGcMark = CN1_BIBOP_FREE_MARK; + __atomic_store_n(&__o->__codenameOneGcMark, CN1_BIBOP_FREE_MARK, __ATOMIC_RELAXED); cn1GcVerifyFreedSlots++; } } @@ -5383,7 +5383,7 @@ static void cn1BibopSweep(CODENAME_ONE_THREAD_STATE) { *(void**)o = fl; fl = o; freeCount++; } else if(m == -1) { // fresh, never marked -> one cycle of grace (legacy parity) - o->__codenameOneGcMark = V; + __atomic_store_n(&o->__codenameOneGcMark, V, __ATOMIC_RELAXED); liveCount++; #ifndef CN1_BIBOP_NO_FASTSWEEP // parentCls==0 => a MID-CONSTRUCTION memset-elided object (the @@ -5404,7 +5404,7 @@ static void cn1BibopSweep(CODENAME_ONE_THREAD_STATE) { // are written after, so the page structure is unaffected. cn1GcVerifyPoisonSlot(o, page->slotSize); #endif - o->__codenameOneGcMark = CN1_BIBOP_FREE_MARK; + __atomic_store_n(&o->__codenameOneGcMark, CN1_BIBOP_FREE_MARK, __ATOMIC_RELAXED); *(void**)o = fl; fl = o; freeCount++; } else { liveCount++; @@ -6421,7 +6421,7 @@ JAVA_BOOLEAN cn1GcVerifyQuarantineFree(JAVA_OBJECT obj) { #endif cn1GcVerifyFreedLegacy++; cn1GcPoisonBody(obj, sz); - obj->__codenameOneGcMark = CN1_GC_POISON_MARK; + __atomic_store_n(&obj->__codenameOneGcMark, CN1_GC_POISON_MARK, __ATOMIC_RELAXED); obj->__heapPosition = CN1_GC_POISON_POS; JAVA_OBJECT evicted = cn1GcQRing[cn1GcQRingPos]; cn1GcQRing[cn1GcQRingPos] = obj; @@ -7280,6 +7280,10 @@ JAVA_OBJECT codenameOneGcMalloc(CODENAME_ONE_THREAD_STATE, int size, struct claz memset(o, 0, size); } o->__codenameOneParentClsReference = parent; + // PLAIN, unlike the collector-side writes below: this is header initialisation of an + // object no other thread can reach yet. The SATB barrier only ever reads the mark of + // an object the mutator holds a reference to, i.e. one already published, and the + // publishing store is what orders this write against any reader. o->__codenameOneGcMark = -1; o->__heapPosition = -1; #ifdef DEBUG_GC_ALLOCATIONS @@ -8118,7 +8122,7 @@ void gcMarkObject(CODENAME_ONE_THREAD_STATE, JAVA_OBJECT obj, JAVA_BOOLEAN force } } #endif - obj->__codenameOneGcMark = markVal; + __atomic_store_n(&obj->__codenameOneGcMark, markVal, __ATOMIC_RELAXED); CN1_BIBOP_STAMP_MARKED_GRACE(obj, markVal, markSnapshot); gcMarkFoundUnmarkedChildInPass = JAVA_TRUE; gcMarkNewObjectCount++; // SATB fixpoint detection (mark-thread only) @@ -8405,6 +8409,10 @@ JAVA_OBJECT cn1NurseryAlloc(CODENAME_ONE_THREAD_STATE, int size, struct clazz* p threadStateData->nurseryAllocSinceMinor++; memset(o, 0, size); o->__codenameOneParentClsReference = parent; + // PLAIN, unlike the collector-side writes below: this is header initialisation of an + // object no other thread can reach yet. The SATB barrier only ever reads the mark of + // an object the mutator holds a reference to, i.e. one already published, and the + // publishing store is what orders this write against any reader. o->__codenameOneGcMark = -1; o->__heapPosition = -1; return o; @@ -8439,7 +8447,7 @@ void gcMarkArrayObject(CODENAME_ONE_THREAD_STATE, JAVA_OBJECT obj, JAVA_BOOLEAN // there the array's mark bit is NOT claimed through gcMarkObject, so set it as the // pre-existing code did. if(threadStateData->nurseryPromoting) { - obj->__codenameOneGcMark = currentGcMarkValue; + __atomic_store_n(&obj->__codenameOneGcMark, currentGcMarkValue, __ATOMIC_RELAXED); } #endif // In the concurrent GC drain (serial or parallel) this array's mark bit was already From 3058dd8b4fbc44e92c765e21b74f297a22658116 Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Tue, 25 Aug 2026 12:10:35 +0300 Subject: [PATCH 11/21] Emit the generated mark chain's root store atomically too The previous commit converted every hand-written collector-side write of the mark word and missed the one that matters most, because it is not in the C sources at all: ByteCodeClass emits the root of every generated mark chain, and that store was still plain. It runs on the GC thread for every object marked while the SATB barrier atomically loads the same field from mutators, so the pair stayed a mixed atomic/non-atomic access -- the exact defect the previous commit was for, in the one place a grep of cn1_globals.m could not see. Costs nothing, as the hand-written conversions did not: vm/benchmarks geomean 0.9387 against master over six interleaved rounds (0.9432 before this change, so inside the noise), no benchmark regressing, checksums identical. A codegen change touches every translated class rather than one runtime path, so it is verified against the shapes rather than the sites: run-gc-verify.sh green with both fault self-tests, and run-gauntlet.sh green with every checksum matching. Reported by chatgpt-codex-connector on #5599. Co-Authored-By: Claude Opus 5 (1M context) --- .../codename1/tools/translator/ByteCodeClass.java | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/vm/ByteCodeTranslator/src/com/codename1/tools/translator/ByteCodeClass.java b/vm/ByteCodeTranslator/src/com/codename1/tools/translator/ByteCodeClass.java index 89faf366608..1d029f417b9 100644 --- a/vm/ByteCodeTranslator/src/com/codename1/tools/translator/ByteCodeClass.java +++ b/vm/ByteCodeTranslator/src/com/codename1/tools/translator/ByteCodeClass.java @@ -1065,8 +1065,17 @@ public String generateCCode(List allClasses) { b.append(baseClass.replace('/', '_').replace('$', '_')); b.append("(threadStateData, objToMark, force);\n"); } else { - // we can do this in Object.java only since all code will reach here eventually - b.append(" objToMark->__codenameOneGcMark = currentGcMarkValue;\n"); + // we can do this in Object.java only since all code will reach here eventually. + // + // ATOMIC, and it has to be: this is the root of every generated mark chain, so + // it is THE collector-side write of the mark word, and the SATB barrier + // (cn1SatbEnqueue) atomically loads the same field from mutator threads while + // the mark is running. A plain store here would leave that pair a mixed + // atomic/non-atomic access, which is undefined in C -- the same defect the + // hand-written stores in cn1_globals.m were converted for. Relaxed is the same + // instruction on every target we build; what it buys is that the write is one + // the reader is allowed to observe. + b.append(" __atomic_store_n(&objToMark->__codenameOneGcMark, currentGcMarkValue, __ATOMIC_RELAXED);\n"); } b.append("}\n\n"); From 4b84f246d42849b245b162efb3ed1b51f89b7cb8 Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Tue, 25 Aug 2026 12:25:39 +0300 Subject: [PATCH 12/21] Publish the sampler's counters, and keep the page partition valid under a race Two findings, one taken as offered and one taken but answered differently. The benchmark driver's per-worker slots were published with a plain long[] write against a concurrent reader: no visibility guarantee, and Java 8 permits a 64-bit element to be observed torn, so the live series could sit stale or jump nonsensically exactly when a stalled worker is what it is meant to show. Publication and sumNodes() now share SUM_LOCK. Once per round is about once a second per worker, so it costs nothing, and NODES= after join() remains the authoritative figure regardless. The probe's page walk is a different case. It reads plain page counters while mutators run, which is a race, but it is the same deliberate sample cn1HeapAccounting takes beside it -- "a diagnostic wants the shape, not the last digit" -- and both offered remedies cost more than the unsoundness. Stopping the page owners would perturb collector/mutator timing, which is the quantity this probe reports, and would cost CN1_GC_CONFORM the behaviour-neutrality that is the only reason it is a separate flag from CN1_GC_VERIFY. Making the page fields _Atomic would put atomic accesses on the inlined bump path in cn1_globals.h, the hottest code in the VM, to improve a diagnostic. What is worth fixing is the harm actually named: an internally inconsistent partition. Only an owned page can move under the walk -- at most one per size class per thread out of many thousands -- so freeCount is clamped into [0, bumpIndex] and a stale pair can no longer make live and dead slots sum past the page. Verified: 517295 + 25326 KB against a 776448 KB reservation. The reasoning is recorded at the walk so the next reader does not have to rediscover which of the three options was chosen and why. Verified: run-gc-verify.sh green with both fault self-tests; steady-state, heap integrity and process budget gates green; the sampler now tracks progress live (nodes~=28,697,812 mid-run against a final NODES=34,360,526); and the 520-test non-benchmark suite is green on the regenerated code from the previous commit. Reported by chatgpt-codex-connector on #5599. Co-Authored-By: Claude Opus 5 (1M context) --- vm/ByteCodeTranslator/src/cn1_globals.m | 30 +++++++++++++--- .../src/com/bench/GcSteadyState.java | 34 +++++++++++++------ 2 files changed, 49 insertions(+), 15 deletions(-) diff --git a/vm/ByteCodeTranslator/src/cn1_globals.m b/vm/ByteCodeTranslator/src/cn1_globals.m index 5cbf12794ad..07c45b57a05 100644 --- a/vm/ByteCodeTranslator/src/cn1_globals.m +++ b/vm/ByteCodeTranslator/src/cn1_globals.m @@ -9372,21 +9372,43 @@ void cn1GcProbeCycle(double markMs, double sweepMs) { long long pgOwned = 0, pgGrace = 0, liveSlots = 0, deadSlots = 0, resvBytes = 0; long long releasedBytes = 0; #ifndef CN1_DISABLE_BIBOP + // DELIBERATELY UNSYNCHRONISED, like cn1HeapAccounting beside it, which samples the + // same registry the same way and says so: "a diagnostic wants the shape, not the last + // digit". Mutators are running here and one of them can be bump-allocating out of a + // page it owns while this reads that page's counters. + // + // Both ways of making it sound are worse than the unsoundness. Stopping the page + // owners would perturb collector/mutator timing, which is the quantity this probe + // exists to report, and would cost CN1_GC_CONFORM the behaviour-neutrality that is + // the whole reason it is a separate flag from CN1_GC_VERIFY. Making the page fields + // _Atomic would put atomic accesses on the inlined bump path in cn1_globals.h -- the + // hottest code in the VM -- to improve a diagnostic. + // + // What IS worth fixing is the stated harm: an internally inconsistent partition. Only + // an owned page can move under this walk, at most one per size class per thread out + // of many thousands, and clamping freeCount into [0, bumpIndex] means a stale pair can + // never make live and dead slots sum past the page. The buckets stay a valid partition + // whatever it reads; the conclusions drawn from them are slopes over hundreds of + // cycles, not last digits. size_t relOff = cn1BibopReleaseOffset(); CN1BibopPage* p = atomic_load_explicit(&bibopAllPages, memory_order_acquire); while(p != 0) { pgTotal++; resvBytes += CN1_BIBOP_PAGE_SIZE; int bi = atomic_load_explicit(&p->bumpIndex, memory_order_relaxed); - int live = bi - p->freeCount; - if(live < 0) { - live = 0; + int freeNow = p->freeCount; + if(freeNow < 0) { + freeNow = 0; + } + if(freeNow > bi) { + freeNow = bi; } + int live = bi - freeNow; if(live == 0) { pgEmpty++; } liveSlots += (long long)live * (long long)p->slotSize; - deadSlots += (long long)p->freeCount * (long long)p->slotSize; + deadSlots += (long long)freeNow * (long long)p->slotSize; if(p->gcPageReleased) { pgReleased++; // Only the slot region is handed back; the header page stays resident. diff --git a/vm/benchmarks/src/com/bench/GcSteadyState.java b/vm/benchmarks/src/com/bench/GcSteadyState.java index 8f8a8d953bf..5c3efdfa028 100644 --- a/vm/benchmarks/src/com/bench/GcSteadyState.java +++ b/vm/benchmarks/src/com/bench/GcSteadyState.java @@ -79,11 +79,12 @@ public class GcSteadyState { * this benchmark compares -- a build whose threads park more would lose fewer * increments and so report a throughput advantage it does not have. * - * Each worker republishes its slot once per round, so the SAMPLE series tracks - * progress live. NODES= at the end is the authoritative figure: it is summed after - * join(), which gives it a happens-before edge to every worker's last write. The - * SAMPLE series reads the same slots while they are still being written, so it is a - * progress indicator and not a measurement. + * Each worker republishes its slot once per round under SUM_LOCK, which the sampler + * also holds to read them, so the live series is properly published rather than + * merely hoped for. NODES= at the end is still the authoritative figure -- it is + * summed after join(), which orders it against every worker's last write regardless + * -- and the live series remains a progress indicator rather than a measurement, + * because a worker that is mid-round has not published yet. */ static long[] nodeCounts; @@ -155,15 +156,24 @@ public void run() { // Publish once per round so the SAMPLE series is a live progress // indicator rather than a row of zeroes. A worker that stalls // stops publishing, and its slot going flat IS the signal. - nodeCounts[slot] = counter[0]; + // + // Under the lock sumNodes() reads: a plain long[] element write has + // no visibility guarantee against a concurrent reader and Java 8 + // permits a 64-bit element to be observed torn, so the series could + // sit stale or jump nonsensically -- precisely when a stalled + // worker is what it is meant to show. Once per round is roughly + // once a second per worker, so the lock costs nothing. + synchronized (SUM_LOCK) { + nodeCounts[slot] = counter[0]; + } round++; if (sleepMs > 0) { sleep(sleepMs); } } - nodeCounts[slot] = counter[0]; // Order-independent, so the checksum does not depend on scheduling. synchronized (SUM_LOCK) { + nodeCounts[slot] = counter[0]; checksum += sum; } } @@ -258,11 +268,13 @@ private static int scrub(int d) { } private static long sumNodes() { - long total = 0; - for (int i = 0; i < nodeCounts.length; i++) { - total += nodeCounts[i]; + synchronized (SUM_LOCK) { + long total = 0; + for (int i = 0; i < nodeCounts.length; i++) { + total += nodeCounts[i]; + } + return total; } - return total; } private static long footprintKb() { From c139af1912e82e6b24726320262d946c806133e6 Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Tue, 25 Aug 2026 12:38:27 +0300 Subject: [PATCH 13/21] Survive a stall: publish inside the traversal, bound the run Both findings are the same blind spot from two directions -- a stalled collector is one of the things this gate exists to CATCH, and neither the progress series nor the runner survived one. Publishing between rounds was not enough. One depth-14 traversal is millions of nodes, so if the collector stalls badly enough that no round completes inside the window, nothing is ever published and the series reads zero -- silent in exactly the case it is for. It now also publishes every 1<<20 nodes: a power of two so the test is an AND, coarse enough (about a fifth of a second of work) that the lock traffic is negligible against the sampler's 4Hz. Verified live: 0 -> 4,194,304 at 1s -> 33,554,432 at 9.8s, against a final NODES=35,255,230. The runner read the child's output to EOF on the test thread and only then called waitFor(), so a hung workload would block until the CI job's global timeout -- the guard would stop reporting a regression and start eating the build. It now drains on a background thread and waits with a bound, killing the child on expiry and failing with whatever it printed, which is the only diagnostic a stalled run leaves. That is not a new invention: GcOverflowSpiralIntegrationTest and ProcessBudgetPacingIntegrationTest both already do exactly this, and the naive pattern came from copying GcHeapIntegrityIntegrationTest, which is the one that does not. Reported by chatgpt-codex-connector on #5599. Co-Authored-By: Claude Opus 5 (1M context) --- .../src/com/bench/GcSteadyState.java | 22 ++++++- .../GcSteadyStateIntegrationTest.java | 58 +++++++++++++++++-- 2 files changed, 72 insertions(+), 8 deletions(-) diff --git a/vm/benchmarks/src/com/bench/GcSteadyState.java b/vm/benchmarks/src/com/bench/GcSteadyState.java index 5c3efdfa028..e39bbe97b39 100644 --- a/vm/benchmarks/src/com/bench/GcSteadyState.java +++ b/vm/benchmarks/src/com/bench/GcSteadyState.java @@ -64,6 +64,17 @@ public class GcSteadyState { private static final int CFG_SLEEP_MS = 4, CFG_MOVES = 5, CFG_LEGACY = 6, CFG_SCRUB = 7; private static final int BOARD_CELLS = 64; + + /** + * Publish the running node count every this-many nodes, as well as between rounds. + * Between rounds alone is not enough: one depth-14 traversal is millions of nodes, and + * if the collector stalls badly enough that no round completes inside the window then + * nothing is ever published and the series reads zero -- silent in exactly the case it + * exists to show. A power of two so the test is an AND, and coarse enough (about a + * fifth of a second of work) that the lock traffic stays negligible against the + * sampler's 4Hz. + */ + private static final int PUBLISH_EVERY_NODES = 1 << 20; private static final int LEGACY_BLOCK_REFS = 128; static int seconds, threads, depth, branch, sleepMs, movesPerNode, legacyBlocks, scrubDepth; @@ -152,7 +163,7 @@ public void run() { int[] root = new int[BOARD_CELLS]; int round = 0; while (!stop) { - sum += search(root, depth, seed + round, counter); + sum += search(root, depth, seed + round, counter, slot); // Publish once per round so the SAMPLE series is a live progress // indicator rather than a row of zeroes. A worker that stalls // stops publishing, and its slot going flat IS the signal. @@ -216,12 +227,17 @@ public void run() { System.out.println("GC_STEADY_STATE_DONE"); } - private static int search(int[] board, int d, int seed, long[] c) { + private static int search(int[] board, int d, int seed, long[] c, int slot) { if (stop) { return 0; } // Thread-private: this array belongs to one worker for the whole run. c[0]++; + if ((c[0] & (PUBLISH_EVERY_NODES - 1)) == 0) { + synchronized (SUM_LOCK) { + nodeCounts[slot] = c[0]; + } + } if (d == 0) { int s = 0; for (int i = 0; i < BOARD_CELLS; i++) { @@ -245,7 +261,7 @@ private static int search(int[] board, int d, int seed, long[] c) { mv.next = chain; chain = mv; } - int v = search(child, d - 1, seed + b + chain.to, c); + int v = search(child, d - 1, seed + b + chain.to, c, slot); if (v > best) { best = v; } diff --git a/vm/tests/src/test/java/com/codename1/tools/translator/GcSteadyStateIntegrationTest.java b/vm/tests/src/test/java/com/codename1/tools/translator/GcSteadyStateIntegrationTest.java index 959e06dde1d..0552cabbb97 100644 --- a/vm/tests/src/test/java/com/codename1/tools/translator/GcSteadyStateIntegrationTest.java +++ b/vm/tests/src/test/java/com/codename1/tools/translator/GcSteadyStateIntegrationTest.java @@ -35,6 +35,7 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.concurrent.TimeUnit; import java.util.stream.Collectors; import static org.junit.jupiter.api.Assertions.assertEquals; @@ -102,6 +103,16 @@ class GcSteadyStateIntegrationTest { */ private static final double MAX_SECOND_HALF_PAGE_GROWTH = 0.25; + /** + * Ceiling on one translated run. A stalled collector is one of the failure modes this + * gate exists to catch, and reading the child's output to EOF on this thread would + * block until it closed stdout -- so a stall would hang the surefire fork until the + * CI job's global timeout, and the guard would stop reporting a regression and start + * eating the build. Generous: the four runs here are a fixed 24 rounds each, a few + * minutes on a slow runner. + */ + private static final long VM_RUN_TIMEOUT_SECONDS = 600; + /** Cycles needed before the comparison means anything. Anti-vacuousness. */ private static final int MIN_CYCLES = 24; @@ -427,13 +438,50 @@ private Run run(Path executable, Path workingDir, Map env) throw builder.environment().put("CN1_LOG_PACING_PARKS", "1"); builder.environment().putAll(env); builder.redirectErrorStream(true); - Process process = builder.start(); + final Process process = builder.start(); + + // Drained CONCURRENTLY and waited for with a bound, as GcOverflowSpiralIntegration + // Test and ProcessBudgetPacingIntegrationTest already do. Concurrently, because a + // child that fills the pipe buffer blocks in write() while we block in waitFor(); + // bounded, because a stalled collector is a thing this gate is meant to CATCH, and + // blocking on EOF would turn that into a hung build instead of a failed test. + // Draining as we go also means a killed run still yields whatever it printed, which + // is the only diagnostic a stalled run leaves behind. + final StringBuilder captured = new StringBuilder(); + Thread drain = new Thread(() -> { + try (BufferedReader reader = new BufferedReader( + new InputStreamReader(process.getInputStream(), StandardCharsets.UTF_8))) { + String line; + while ((line = reader.readLine()) != null) { + synchronized (captured) { + captured.append(line).append('\n'); + } + } + } catch (Exception e) { + // The stream ends abruptly when a timed-out child is destroyed. Whatever + // was captured before that is exactly what should be reported. + } + }); + drain.setDaemon(true); + drain.start(); + + boolean exited = process.waitFor(VM_RUN_TIMEOUT_SECONDS, TimeUnit.SECONDS); + if (!exited) { + process.destroyForcibly(); + process.waitFor(10, TimeUnit.SECONDS); + } + drain.join(10_000); String output; - try (BufferedReader reader = new BufferedReader( - new InputStreamReader(process.getInputStream(), StandardCharsets.UTF_8))) { - output = reader.lines().collect(Collectors.joining("\n")); + synchronized (captured) { + output = captured.toString(); } - return new Run(process.waitFor(), output); + assertTrue(exited, + "The workload did not finish within " + VM_RUN_TIMEOUT_SECONDS + "s (env " + + env + "). For this gate that is a result and not an" + + " infrastructure problem -- a collector that stops finishing" + + " cycles is one of the regressions it watches for. Output so far:\n" + + tail(output)); + return new Run(exited ? process.exitValue() : -1, output); } private String tail(String output) { From e7fd0927eb66f9ba3024c25f6d9e51c31104b858 Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Tue, 25 Aug 2026 12:51:29 +0300 Subject: [PATCH 14/21] Do not filter fresh SATB entries where there is no insertion barrier The filter's soundness argument ends "its non-fresh children are still logged by this same barrier as they are stored". That step has a precondition I did not state and did not check: the INSERTION half has to exist. Under CN1_NURSERY it does not. CN1_WRITE_BARRIER is the nursery remembered-set update there and enqueues nothing at all (cn1_globals.h:1020-1039), so a fresh container that takes an older child after the grace pass has that child recorded nowhere -- and dropping the deletion entry for the container then lets the sweep reclaim a child the grace-surviving container still references. That is a use-after-free, in the class of defect #5425 and #5442 were about. The filter is an optimisation and not a correctness requirement, so a build without the insertion half simply does not get it: the condition is now !defined(CN1_SATB_LOG_FRESH) && !defined(CN1_NURSERY). Adding SATB insertion to the nursery barrier was the other option offered and is the riskier one -- it changes barrier behaviour in a configuration nothing exercises, and would have to be justified by measurements no one can take. Latent rather than live: CN1_NURSERY is not defined anywhere in-tree, so no shipping or CI build takes that path. It is a documented, reachable flag, and the comment now records the dependency so the next person to enable it is not relying on an argument that quietly stopped holding. Verified: five ablation combinations compile including -DCN1_NURSERY; run-gc-verify.sh green with both fault self-tests; steady-state and heap-integrity gates green. Reported by chatgpt-codex-connector on #5599. Co-Authored-By: Claude Opus 5 (1M context) --- vm/ByteCodeTranslator/src/cn1_globals.m | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/vm/ByteCodeTranslator/src/cn1_globals.m b/vm/ByteCodeTranslator/src/cn1_globals.m index 07c45b57a05..94a1d77ef2b 100644 --- a/vm/ByteCodeTranslator/src/cn1_globals.m +++ b/vm/ByteCodeTranslator/src/cn1_globals.m @@ -1519,7 +1519,7 @@ static void cn1DrainDeadThreadPending() { #endif void cn1SatbEnqueue(JAVA_OBJECT old) { -#ifndef CN1_SATB_LOG_FRESH +#if !defined(CN1_SATB_LOG_FRESH) && !defined(CN1_NURSERY) // FRESH-REFERENCE FILTER (issue 5537). // // A mark == -1 object was allocated after this cycle's snapshot was taken, so it is @@ -1530,6 +1530,17 @@ void cn1SatbEnqueue(JAVA_OBJECT old) { // barrier as they are stored, so nothing reachable only through a fresh object is // lost -- which is the hazard the insertion half was added for. // + // THAT LAST STEP IS THE WHOLE ARGUMENT, AND IT HAS A PRECONDITION: the INSERTION half + // has to exist. Under CN1_NURSERY it does not -- CN1_WRITE_BARRIER is the nursery + // remembered-set update there and enqueues nothing (cn1_globals.h) -- so a fresh + // container that takes an older child after the grace pass has that child recorded + // nowhere, and dropping the deletion entry for the container would let the sweep + // reclaim a child the graced container still references. The filter is an + // optimisation, not a correctness requirement, so a build without the insertion half + // simply does not get it. CN1_NURSERY is not defined anywhere in-tree today, which is + // why this is a latent hole rather than a live one, but the flag is documented and + // reachable. + // // Without the filter the log is a positive feedback loop rather than a cost. Measured // on the game-tree shape at 4 threads: essentially the ENTIRE log was fresh // references (2,718,413 of 2,718,448 entries in one cycle), and draining them was From 58ad468bcd8bc2382e79038ffbd0b13c91ebf259 Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Tue, 25 Aug 2026 12:59:31 +0300 Subject: [PATCH 15/21] Let CN1_WL_LEGACY=0 actually ablate the legacy population Setting the documented knob to 0 built a zero-length legacyLiveSet and then indexed [-1] on the last line, so the driver threw AFTER the entire timed run had been paid for -- losing RESULT and GC_STEADY_STATE_DONE, which is everything the run was for. Running without the retained legacy population is a legitimate ablation, so it now works rather than crashing: the fold is skipped when there is nothing to fold. Two neighbouring values that would produce a wasted or silently empty run are clamped at the same time. A negative CN1_WL_LEGACY reached new Object[n][]; a CN1_WL_THREADS below one started no workers at all and reported that only by printing zero nodes, which is the exact failure mode -- a measurement that looks like a result -- this whole change has been about. WLCONFIG prints the clamped values, so the log says what actually ran. Verified: CN1_WL_LEGACY=0, CN1_WL_LEGACY=-5 with CN1_WL_THREADS=0, and the defaults all reach RESULT and GC_STEADY_STATE_DONE. Reported by chatgpt-codex-connector on #5599. Co-Authored-By: Claude Opus 5 (1M context) --- .../src/com/bench/GcSteadyState.java | 22 +++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/vm/benchmarks/src/com/bench/GcSteadyState.java b/vm/benchmarks/src/com/bench/GcSteadyState.java index e39bbe97b39..0bb90ca2de4 100644 --- a/vm/benchmarks/src/com/bench/GcSteadyState.java +++ b/vm/benchmarks/src/com/bench/GcSteadyState.java @@ -119,6 +119,16 @@ public static void main(String[] args) { movesPerNode = cfg(CFG_MOVES); legacyBlocks = cfg(CFG_LEGACY); scrubDepth = cfg(CFG_SCRUB); + // CN1_WL_LEGACY=0 is a legitimate ablation -- run without the retained legacy + // population -- so it has to work rather than crash, and a negative value must not + // reach new Object[n][]. Likewise a thread count below one produces a run that + // measures nothing and announces it only by reporting zero. + if (legacyBlocks < 0) { + legacyBlocks = 0; + } + if (threads < 1) { + threads = 1; + } System.out.println("WLCONFIG seconds=" + seconds + " threads=" + threads + " depth=" + depth + " branch=" + branch + " sleepMs=" + sleepMs + " moves=" + movesPerNode + " legacy=" + legacyBlocks @@ -222,8 +232,16 @@ public void run() { System.out.println("NODES=" + sumNodes()); System.out.println("ELAPSED_MS=" + (System.currentTimeMillis() - startMs)); System.out.println("FINAL_FOOTPRINT_KB=" + footprintKb()); - Move lastHeld = (Move) legacyLiveSet[legacyBlocks - 1][LEGACY_BLOCK_REFS - 1]; - System.out.println("RESULT=" + (checksum + lastHeld.from + lastHeld.to)); + // Keeps the population reachable to the end and folds it into RESULT, so the + // host-JVM comparison covers it too. Skipped when it was ablated away: reaching + // for element -1 would throw AFTER the whole timed run had been paid for, losing + // RESULT and the completion marker with it. + long held = 0; + if (legacyBlocks > 0) { + Move lastHeld = (Move) legacyLiveSet[legacyBlocks - 1][LEGACY_BLOCK_REFS - 1]; + held = lastHeld.from + lastHeld.to; + } + System.out.println("RESULT=" + (checksum + held)); System.out.println("GC_STEADY_STATE_DONE"); } From 4b6b82a60df9d94df8b793dd0f8d08411a58c866 Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Tue, 25 Aug 2026 13:09:53 +0300 Subject: [PATCH 16/21] Check the answer, not just the telemetry, in every scenario Three of the four runs checked exit status and the completion marker but never that the workload still computed the right thing. That gap matters most exactly where it was left: the ceiling scenarios exercise the budgeted pacing path -- the code this change touches most -- under an environment the clean run never sees, so a worker could die early or compute a wrong sum while the process still exited cleanly and emitted plenty of [PACING] telemetry for the policy assertions to pass. None of the variants changes what the program computes: the faults injected are a barrier filter and a pacing bound, and the workload is deterministic by construction (fixed rounds, fixed seeds, an order-independent checksum). So RESULT must equal the host JVM's in all of them, and assertHealthy now requires it -- which also picks up the -DCN1_SATB_LOG_FRESH run, which had the same gap. Reported by chatgpt-codex-connector on #5599. Co-Authored-By: Claude Opus 5 (1M context) --- .../GcSteadyStateIntegrationTest.java | 32 +++++++++++++------ 1 file changed, 22 insertions(+), 10 deletions(-) diff --git a/vm/tests/src/test/java/com/codename1/tools/translator/GcSteadyStateIntegrationTest.java b/vm/tests/src/test/java/com/codename1/tools/translator/GcSteadyStateIntegrationTest.java index 0552cabbb97..b8cb8a4a30b 100644 --- a/vm/tests/src/test/java/com/codename1/tools/translator/GcSteadyStateIntegrationTest.java +++ b/vm/tests/src/test/java/com/codename1/tools/translator/GcSteadyStateIntegrationTest.java @@ -235,7 +235,7 @@ private void runGate(List tempDirs) throws Exception { // the filter silently compiled out would pass part 1 forever. Path faulty = build(distDir, tempDirs, "faulted", "-DCN1_GC_CONFORM -DCN1_SATB_LOG_FRESH"); Run faulted = run(faulty, distDir); - assertHealthy(faulted, "the -DCN1_SATB_LOG_FRESH build"); + assertHealthy(faulted, "the -DCN1_SATB_LOG_FRESH build", javaResult); Series bad = Series.parse(faulted.output); assertTrue(bad.cycles >= MIN_CYCLES, "The faulted build produced no [GCPROBE] series, so CN1_GC_CONFORM is not " @@ -258,8 +258,7 @@ private void runGate(List tempDirs) throws Exception { Map ceiling = new HashMap<>(); ceiling.put("CN1_SIMULATE_PROC_MEMORY_LIMIT", Long.toString(CEILING_MB * 1024 * 1024)); Run bounded = run(fixed, distDir, ceiling); - assertEquals(0, bounded.exit, - "The workload must finish under a ceiling. Output: " + tail(bounded.output)); + assertHealthy(bounded, "the run under a simulated ceiling", javaResult); long boundedHeadroomMb = minHeadroomMb(bounded.output); assertTrue(boundedHeadroomMb >= 0, "No [PACING] report under a simulated ceiling -- the budgeted path never " @@ -273,7 +272,7 @@ private void runGate(List tempDirs) throws Exception { Path noReserve = build(distDir, tempDirs, "noreserve", "-DCN1_GC_CONFORM -DCN1_PACING_NO_RESERVE"); Run unbounded = run(noReserve, distDir, ceiling); - assertHealthy(unbounded, "the -DCN1_PACING_NO_RESERVE build"); + assertHealthy(unbounded, "the -DCN1_PACING_NO_RESERVE build", javaResult); long unboundedHeadroomMb = minHeadroomMb(unbounded.output); assertTrue(unboundedHeadroomMb >= 0, "No [PACING] report from the no-reserve build. Output: " + tail(unbounded.output)); @@ -284,16 +283,29 @@ private void runGate(List tempDirs) throws Exception { } /** - * A fault-injected run's measurements are only admissible if the run itself was - * healthy. Without this, a build that crashed or was OOM-killed after emitting enough - * probe rows would satisfy the cycle and inflated-SATB assertions and turn a - * memory-safety regression into a green gate -- the faults injected here are policy - * regressions, not crashes, so a crash means something else is wrong. + * A run's measurements are only admissible if the run itself was healthy AND still + * computed the right answer. + * + *

Exit status and the completion marker rule out a build that crashed or was + * OOM-killed after emitting enough probe rows, which would otherwise satisfy the + * cycle and inflated-SATB assertions and turn a memory-safety regression into a green + * gate. None of the variants here changes what the program computes -- the faults are + * a barrier filter and a pacing bound -- so RESULT must match the host JVM in every + * one of them.

+ * + *

That last check is what covers the ceiling scenarios. They run the budgeted + * pacing path, which is the code this change touches most, under an environment the + * clean run never sees; without a parity check a worker could die early or compute + * the wrong sum while the process still exited cleanly and emitted plenty of + * [PACING] telemetry for the policy assertions to pass.

*/ - private void assertHealthy(Run r, String which) { + private void assertHealthy(Run r, String which, String expectedResult) { assertEquals(0, r.exit, which + " must still exit cleanly. Output: " + tail(r.output)); assertTrue(r.output.contains("GC_STEADY_STATE_DONE"), which + " must run to completion. Output: " + tail(r.output)); + assertEquals(expectedResult, extractLine(r.output, "RESULT="), + which + " must still compute the same answer as the host JVM. Output: " + + tail(r.output)); } /** Smallest headroom the pacing tracer saw, in MB, or -1 if it never reported. */ From 58158bd9d15a3001570ad62a55564862abdc53b0 Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Tue, 25 Aug 2026 13:16:31 +0300 Subject: [PATCH 17/21] Normalise every workload knob, not just the one that was reported CN1_WL_MOVES=0 left the move chain null and the next-seed derivation dereferenced it, so the leaf-only ablation died with an NPE on the first node. That is the second knob found this way, so this fixes the class rather than the instance: all eight are normalised in one place before the timed run, and WLCONFIG prints the normalised values so the log says what actually ran rather than what was asked for. Auditing the rest turned up one more that was worse than the reported one. A negative CN1_WL_DEPTH never matches the d == 0 base case, so it recursed until the stack gave out. CN1_WL_SECONDS and CN1_WL_BRANCH below their floors produced runs that measured nothing and said so only by reporting zero -- the failure mode this entire change is about. Zero stays meaningful where it means something, and both cases are real ablations: no retained legacy population, and no reference-carrying Move per node. The second is worth having, because only a non-leaf object reaches the grace pass's worklist or maturation, so leaf-only allocation is a genuinely different workload for the parts of the collector under test. Verified: CN1_WL_MOVES=0, CN1_WL_MOVES=-3, CN1_WL_DEPTH=-1, CN1_WL_BRANCH=0, CN1_WL_SECONDS=0 and CN1_WL_LEGACY=0 all reach RESULT and GC_STEADY_STATE_DONE, and the default configuration is unchanged. Reported by chatgpt-codex-connector on #5599. Co-Authored-By: Claude Opus 5 (1M context) --- .../src/com/bench/GcSteadyState.java | 42 +++++++++++++++---- 1 file changed, 35 insertions(+), 7 deletions(-) diff --git a/vm/benchmarks/src/com/bench/GcSteadyState.java b/vm/benchmarks/src/com/bench/GcSteadyState.java index 0bb90ca2de4..e562cc2dfca 100644 --- a/vm/benchmarks/src/com/bench/GcSteadyState.java +++ b/vm/benchmarks/src/com/bench/GcSteadyState.java @@ -119,16 +119,43 @@ public static void main(String[] args) { movesPerNode = cfg(CFG_MOVES); legacyBlocks = cfg(CFG_LEGACY); scrubDepth = cfg(CFG_SCRUB); - // CN1_WL_LEGACY=0 is a legitimate ablation -- run without the retained legacy - // population -- so it has to work rather than crash, and a negative value must not - // reach new Object[n][]. Likewise a thread count below one produces a run that - // measures nothing and announces it only by reporting zero. - if (legacyBlocks < 0) { - legacyBlocks = 0; + // Every knob is normalised before the run rather than trusted. These are ablation + // switches: someone WILL set one to zero to remove a term, because that is what + // they are for, and a driver that answers a legitimate ablation with an NPE, an + // unbounded recursion or a silently empty run costs an investigation instead of + // informing one. WLCONFIG below prints the normalised values, so the log always + // says what actually ran rather than what was asked for. + // + // Zero is meaningful for two of these and is preserved: no retained legacy + // population, and no reference-carrying Move per node (leaf-only allocation, which + // is a different workload for the grace pass and for maturation, since only a + // non-leaf object reaches either). The rest have a floor because below it the run + // measures nothing -- or, for a negative depth, never terminates, since the d == 0 + // base case would never match. + if (seconds < 1) { + seconds = 1; } if (threads < 1) { threads = 1; } + if (depth < 0) { + depth = 0; + } + if (branch < 1) { + branch = 1; + } + if (sleepMs < 0) { + sleepMs = 0; + } + if (movesPerNode < 0) { + movesPerNode = 0; + } + if (legacyBlocks < 0) { + legacyBlocks = 0; + } + if (scrubDepth < 0) { + scrubDepth = 0; + } System.out.println("WLCONFIG seconds=" + seconds + " threads=" + threads + " depth=" + depth + " branch=" + branch + " sleepMs=" + sleepMs + " moves=" + movesPerNode + " legacy=" + legacyBlocks @@ -279,7 +306,8 @@ private static int search(int[] board, int d, int seed, long[] c, int slot) { mv.next = chain; chain = mv; } - int v = search(child, d - 1, seed + b + chain.to, c, slot); + // chain is null when movesPerNode is 0, which is the leaf-only ablation. + int v = search(child, d - 1, seed + b + (chain == null ? 0 : chain.to), c, slot); if (v > best) { best = v; } From 0a3412ee61ef270c53742377e3dafe72f960f547 Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Tue, 25 Aug 2026 13:28:53 +0300 Subject: [PATCH 18/21] Flag the probe row when the collection cycle threw gcMarkSweep wraps mark and sweep in a catch-all so a throwing finalizer cannot wedge the collector. On that path control jumps past the timing assignments, so the probe emitted a row carrying the PREVIOUS cycle's markMs and sweepMs beside the partial current cycle's phase counters -- two cycles in one row, and it concealed the exceptional cycle, which is the one a reader most wants to see. This is the same defect as the CN1_GC_PROBE>1 skip path fixed earlier, on a different route out. The timings are now cleared BEFORE the protected region, so a throw cannot inherit them, and the row carries threw=1 rather than being suppressed: hiding it would defeat the reason this probe has a wall-clock emitter at all. The three carriers are file scope, so the setjmp/longjmp indeterminate-local rule does not apply to them. Verified: five ablation combinations compile; run-gc-verify.sh green with both fault self-tests; steady-state and heap-integrity gates green; probe rows carry threw=0 on a healthy run. Reported by chatgpt-codex-connector on #5599. Co-Authored-By: Claude Opus 5 (1M context) --- vm/ByteCodeTranslator/src/cn1_globals.m | 10 +++++++--- vm/ByteCodeTranslator/src/nativeMethods.m | 18 ++++++++++++++++-- 2 files changed, 23 insertions(+), 5 deletions(-) diff --git a/vm/ByteCodeTranslator/src/cn1_globals.m b/vm/ByteCodeTranslator/src/cn1_globals.m index 94a1d77ef2b..5a4a3061eda 100644 --- a/vm/ByteCodeTranslator/src/cn1_globals.m +++ b/vm/ByteCodeTranslator/src/cn1_globals.m @@ -9370,7 +9370,11 @@ static void cn1GcProbeResetPhases(void) { cn1GcPoolNs = 0; } -void cn1GcProbeCycle(double markMs, double sweepMs) { +// threw != 0 means the collection cycle raised into gcMarkSweep's catch-all, so mark and +// sweep timings are zero and every other figure describes a PARTIAL cycle. The row is +// still emitted, and flagged: suppressing it would hide the one cycle a reader most wants +// to see, which is the same reason this probe has a wall-clock emitter at all. +void cn1GcProbeCycle(double markMs, double sweepMs, int threw) { int every = cn1GcProbeEvery(); if(every == 0) { return; @@ -9464,7 +9468,7 @@ void cn1GcProbeCycle(double markMs, double sweepMs) { long long residKb = fpKb - (residentPgBytes + legBlockBytes + legTableBytes + sideBytes) / 1024; fprintf(stderr, - "[GCPROBE] v=1 cyc=%d tMs=%lld fpKb=%lld" + "[GCPROBE] v=1 cyc=%d tMs=%lld fpKb=%lld threw=%d" " pgTotal=%lld pgEmpty=%lld pgReleased=%lld pgAdopted=%lld pgMon=%lld pgOwned=%lld pgGrace=%lld" " resvKb=%lld residentPgKb=%lld liveSlotKb=%lld deadSlotKb=%lld" " legCap=%d legUsed=%lld legAdopted=%lld legTableKb=%lld legBlockKb=%lld" @@ -9476,7 +9480,7 @@ void cn1GcProbeCycle(double markMs, double sweepMs) { " staleSkips=%ld ovfCycles=%ld graceDrains=%ld" " consWords=%lld consResolved=%lld consFirstMarks=%lld" " monitors=%ld immortal=%d fvLive=%ld sideKb=%lld residKb=%lld\n", - currentGcMarkValue, cn1GcProbeElapsedMs(), fpKb, + currentGcMarkValue, cn1GcProbeElapsedMs(), fpKb, threw, pgTotal, pgEmpty, pgReleased, pgAdopted, pgMon, pgOwned, pgGrace, resvBytes / 1024, residentPgBytes / 1024, liveSlots / 1024, deadSlots / 1024, legCap, legUsed, legAdopted, legTableBytes / 1024, legBlockBytes / 1024, diff --git a/vm/ByteCodeTranslator/src/nativeMethods.m b/vm/ByteCodeTranslator/src/nativeMethods.m index 059f5c1ae7c..81137130197 100644 --- a/vm/ByteCodeTranslator/src/nativeMethods.m +++ b/vm/ByteCodeTranslator/src/nativeMethods.m @@ -1856,8 +1856,9 @@ JAVA_VOID java_lang_System_gcLight__(CODENAME_ONE_THREAD_STATE) { JAVA_BOOLEAN firstTimeGcThread = JAVA_TRUE; JAVA_BOOLEAN gcCurrentlyRunning = JAVA_FALSE; #ifdef CN1_GC_CONFORM -extern void cn1GcProbeCycle(double markMs, double sweepMs); +extern void cn1GcProbeCycle(double markMs, double sweepMs, int threw); double cn1GcProbeMarkMs = 0, cn1GcProbeSweepMs = 0; +int cn1GcProbeThrew = 0; #endif JAVA_VOID java_lang_System_gcMarkSweep__(CODENAME_ONE_THREAD_STATE) { gcCurrentlyRunning = JAVA_TRUE; @@ -1881,6 +1882,16 @@ JAVA_VOID java_lang_System_gcMarkSweep__(CODENAME_ONE_THREAD_STATE) { // backstop: on any throw, drop it, and still clear gcCurrentlyRunning so the collector // stays healthy and the next cycle retries. If MARK threw, SWEEP is skipped -- correct, // sweeping a partial mark would free reachable objects. +#ifdef CN1_GC_CONFORM + // Cleared BEFORE the protected region. A cycle that throws jumps past the timing + // assignments below, so without this the row would carry the previous cycle's markMs + // and sweepMs beside the partial current cycle's phase counters -- two cycles in one + // row, concealing the exceptional cycle, which is the one worth seeing. These are + // file-scope, so the setjmp/longjmp indeterminate-local rule does not apply. + cn1GcProbeMarkMs = 0; + cn1GcProbeSweepMs = 0; + cn1GcProbeThrew = 0; +#endif int __gcSavedTryBlock = threadStateData->tryBlockOffset; jmp_buf __gcTryJmp; if(CN1_TRY_SETJMP(__gcTryJmp) == 0) { @@ -1921,10 +1932,13 @@ JAVA_VOID java_lang_System_gcMarkSweep__(CODENAME_ONE_THREAD_STATE) { } else { threadStateData->tryBlockOffset = __gcSavedTryBlock; threadStateData->exception = JAVA_NULL; +#ifdef CN1_GC_CONFORM + cn1GcProbeThrew = 1; +#endif } flushReleaseQueue(); #ifdef CN1_GC_CONFORM - cn1GcProbeCycle(cn1GcProbeMarkMs, cn1GcProbeSweepMs); + cn1GcProbeCycle(cn1GcProbeMarkMs, cn1GcProbeSweepMs, cn1GcProbeThrew); #endif #ifdef CN1_ALLOC_CENSUS { From 675c62b86e8ce42d12ea7345aad2493d852fa331 Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Tue, 25 Aug 2026 14:03:02 +0300 Subject: [PATCH 19/21] Re-evaluate the reserve throughout the wait, and stop the driver perturbing itself Two review findings, and a third defect the first one's verification exposed. The wait loop's copy of the volume bound was guarded on the thread having already been refused, so it could only transition refused->allowed. A thread that parked on BUDGET while outside the reserve then held a stale "allowed" for its whole wait and could be admitted on headroom alone after other mutators had pushed the uncollected total past the cap and the process into the reserve. There is now ONE definition, cn1PacingVolumeOk, called from both sites and recomputed every iteration -- the two copies drifted precisely because they were two. The gate parsed only the per-cycle [GCPROBE] rows, so a collector that completes its early cycles and then never finishes another was invisible to it: the rows stop, the generated main returns as soon as the workers do, and the process exits cleanly with the marker while the heap is still growing. [GCPROBE-T] was added for exactly that state and then not asserted on. The outcome check now covers the wall-clock series too, with its own anti-vacuous row count. And the driver had started perturbing its own experiment. The periodic publication added two commits ago took SUM_LOCK inside the search, and monitorEnter is a GC SAFEPOINT in this VM -- so the workers were being stopped far more often than the workload otherwise permits and the runaway stopped reproducing: peak footprint fell from 1271MB to 126MB with the reserve compiled out, in BOTH builds, which is what gave it away. Publication is now a volatile long per worker: not a safepoint, not a lock, and JLS 17.7 makes volatile long access atomic, so it also answers the visibility and tearing that the plain long[] had. With the runaway restored, the reserve's throughput cost is re-measured across four interleaved pairs at 0.875-1.035, median 0.90 -- about a tenth, not the ~1% the previous figure claimed. Peak and headroom are unchanged (1271/63 against 1022-1064/272-304). This is the third throughput figure this comment has carried and the first two were both apparatus rather than signal, so the comment now says which were which. Verified: four ablation combinations compile; run-gc-verify.sh green with both fault self-tests; the steady-state gate green with all five checks. Reported by chatgpt-codex-connector on #5599. Co-Authored-By: Claude Opus 5 (1M context) --- vm/ByteCodeTranslator/src/cn1_globals.m | 98 +++++++++++-------- .../src/com/bench/GcSteadyState.java | 90 +++++++++-------- .../GcSteadyStateIntegrationTest.java | 65 ++++++++++++ 3 files changed, 171 insertions(+), 82 deletions(-) diff --git a/vm/ByteCodeTranslator/src/cn1_globals.m b/vm/ByteCodeTranslator/src/cn1_globals.m index 5a4a3061eda..9adf63b02c0 100644 --- a/vm/ByteCodeTranslator/src/cn1_globals.m +++ b/vm/ByteCodeTranslator/src/cn1_globals.m @@ -4136,14 +4136,19 @@ + (long long)atomic_load_explicit(&cn1LegacyBytesSinceGc, memory_order_relaxed); // // peak footprint smallest headroom throughput // no reserve 1271MB, x4 63MB, x4 1.00 -// reserve limit>>2 1015-1027MB 306-308MB 0.97-1.05, median 0.99 +// reserve limit>>2 1022-1064MB 272-304MB 0.875-1.035, median 0.90 // // The first two columns are that repeatable because neither is an accident: without the // bound, admission converges on ceiling minus CN1_PACING_HEADROOM_MARGIN by construction; -// with it, the control loop holds the reserve. Throughput is a wash -- two of the four -// pairs came out faster with the bound on -- which is what a bound that engages only -// inside the reserve, rather than taxing every allocation, should look like. Note -// volumeParks in the [PACING] report reads 0 for a run that never enters the reserve. +// with it, the control loop holds the reserve. volumeParks in the [PACING] report reads 0 +// for a run that never enters the reserve, which is what "engages only inside it" means +// as a number. +// +// The throughput column is the third figure this comment has carried, and the earlier two +// were both apparatus and not signal: a racy shared node counter, and then a synchronized +// publication inside the search -- monitorEnter is a GC safepoint here, so the instrument +// was letting the collector stop the workers and the runaway stopped reproducing at all. +// About a tenth is the honest cost, measured with a driver that does neither. // // A single repetition each of the tighter reserves put >> 3 at 1183MB of peak and 150MB // of headroom, and >> 4 at 1207MB/127MB: a smaller reserve engages later and closer to @@ -4177,6 +4182,43 @@ static long long cn1PacingReserveBytes(long long limitBytes) { } #endif +/** + * Whether this thread may proceed under the reserve's volume bound. + * + * ONE definition, called from both the admission test and the wait loop. They started as + * two copies and drifted: the loop's copy was guarded on the thread having already been + * refused, so it could only ever transition refused->allowed. A thread that parked on + * BUDGET while outside the reserve then held a stale "allowed" for the whole wait, and + * could be admitted on headroom alone after other mutators had pushed the uncollected + * total past the cap and the process into the reserve. + * + * Returns true where the bound is compiled out, so callers need no #if. + */ +#if !defined(CN1_DISABLE_BIBOP) && !defined(CN1_PACING_NO_RESERVE) +static JAVA_BOOLEAN cn1PacingVolumeOk(long procHeadroom) { + long long footprint = cn1PacingFootprintNow(); + if(footprint <= 0) { + return JAVA_TRUE; // no probe on this platform; nothing to bound against + } + if((long long)procHeadroom >= cn1PacingReserveBytes(footprint + (long long)procHeadroom)) { + return JAVA_TRUE; // outside the reserve: full speed, this costs nothing + } + // Inside the reserve: clamp the mutator to the STATIC cap so the collector gets ahead + // and the footprint falls back out of it. + { + long long trigger = (long long)atomic_load_explicit(&bibopGcTriggerBytes, + memory_order_relaxed); + return cn1PacingUncollectedBytes() <= trigger * CN1_BIBOP_GC_HARD_CAP_MULTIPLIER + ? JAVA_TRUE : JAVA_FALSE; + } +} +#else +static JAVA_BOOLEAN cn1PacingVolumeOk(long procHeadroom) { + (void)procHeadroom; + return JAVA_TRUE; +} +#endif + // Atomically admit this thread if the live budget, minus what other threads have already // been admitted to dirty, still covers this block plus the margin. Test and claim must be // one step: a plain check followed by a separate add lets every waiter observe the same @@ -4284,26 +4326,10 @@ static void cn1PacingPark(CODENAME_ONE_THREAD_STATE, int which, long long pendin // drops inside the reserve. That is what keeps the footprint proportional to the // COLLECTOR'S WORK rather than to the device's budget, and it is the bound the // unbudgeted branch has always had and this one dropped. - JAVA_BOOLEAN volumeOk = JAVA_TRUE; -#if !defined(CN1_DISABLE_BIBOP) && !defined(CN1_PACING_NO_RESERVE) - { - long long footprint = cn1PacingFootprintNow(); - if(footprint > 0 - && (long long)procHeadroom - < cn1PacingReserveBytes(footprint + (long long)procHeadroom)) { - // Inside the reserve: clamp the mutator to the STATIC cap so the collector - // gets ahead and the footprint falls back out of it. Gating on HEADROOM - // rather than on footprint is what makes this cost nothing until it is - // needed -- a process using three quarters of its budget and holding still - // is not in danger; one with no headroom left is. - long long trigger = (long long)atomic_load_explicit(&bibopGcTriggerBytes, - memory_order_relaxed); - volumeOk = cn1PacingUncollectedBytes() - <= trigger * CN1_BIBOP_GC_HARD_CAP_MULTIPLIER - ? JAVA_TRUE : JAVA_FALSE; - } - } -#endif + // Gating on HEADROOM rather than on footprint is what makes this cost nothing until it + // is needed -- a process using three quarters of its budget and holding still is not in + // danger; one with no headroom left is. + JAVA_BOOLEAN volumeOk = cn1PacingVolumeOk(procHeadroom); // Short-circuit deliberately: cn1PacingTryAdmit CLAIMS on success, so it must not run // while the volume bound is refusing. JAVA_BOOLEAN admitted = volumeOk && cn1PacingTryAdmit(procHeadroom, need, pendingBytes); @@ -4343,22 +4369,12 @@ static void cn1PacingPark(CODENAME_ONE_THREAD_STATE, int which, long long pendin if(headroomNow < 0) { break; // budget disappeared under us; nothing to honour } -#if !defined(CN1_DISABLE_BIBOP) && !defined(CN1_PACING_NO_RESERVE) - if(!volumeOk) { - // Re-test the volume bound too, or a thread refused by it would spin out its - // whole wait against a budget test that was never the thing blocking it. - // bibopBytesSinceGc is exchanged to 0 at cycle START, so this clears as soon - // as the collection this park requested begins. - long long footprintNow = cn1PacingFootprintNow(); - long long trigger = (long long)atomic_load_explicit(&bibopGcTriggerBytes, - memory_order_relaxed); - volumeOk = (footprintNow <= 0 - || headroomNow >= cn1PacingReserveBytes(footprintNow + headroomNow) - || cn1PacingUncollectedBytes() - <= trigger * CN1_BIBOP_GC_HARD_CAP_MULTIPLIER) - ? JAVA_TRUE : JAVA_FALSE; - } -#endif + // Recomputed EVERY iteration and in both directions. A thread refused by the + // volume bound has to see it clear -- bibopBytesSinceGc is exchanged to 0 at cycle + // START, so it clears as soon as the collection this park requested begins -- and a + // thread that parked on budget alone has to start honouring it if other mutators + // push the process into the reserve while it waits. + volumeOk = cn1PacingVolumeOk(headroomNow); if(volumeOk && cn1PacingTryAdmit(headroomNow, need, pendingBytes)) { admitted = JAVA_TRUE; break; diff --git a/vm/benchmarks/src/com/bench/GcSteadyState.java b/vm/benchmarks/src/com/bench/GcSteadyState.java index e562cc2dfca..dfb9f356457 100644 --- a/vm/benchmarks/src/com/bench/GcSteadyState.java +++ b/vm/benchmarks/src/com/bench/GcSteadyState.java @@ -83,21 +83,37 @@ public class GcSteadyState { static final Object SUM_LOCK = new Object(); static long checksum = 0; /** - * Per-worker node counts, one slot each, so counting costs no synchronisation and - * loses no increments. A single shared counter cannot be used for this: an - * unsynchronised read-modify-write from four workers drops updates at a rate that - * depends on CONTENTION, and contention is precisely what differs between the builds - * this benchmark compares -- a build whose threads park more would lose fewer - * increments and so report a throughput advantage it does not have. + * Per-worker node counts, one holder each, so counting costs no synchronisation and + * loses no increments. A single shared counter cannot be used: an unsynchronised + * read-modify-write from four workers drops updates at a rate that depends on + * CONTENTION, and contention is precisely what differs between the builds this + * benchmark compares -- a build whose threads park more would lose fewer increments + * and report a throughput advantage it has not got. * - * Each worker republishes its slot once per round under SUM_LOCK, which the sampler - * also holds to read them, so the live series is properly published rather than - * merely hoped for. NODES= at the end is still the authoritative figure -- it is - * summed after join(), which orders it against every worker's last write regardless - * -- and the live series remains a progress indicator rather than a measurement, - * because a worker that is mid-round has not published yet. + * NODES= at the end is the authoritative figure: it is summed after join(), which + * orders it against every worker's last write regardless. */ - static long[] nodeCounts; + static Progress[] progress; + + /** + * One worker's published node count. + * + *

A volatile long, and NOT a synchronized publication, which is what this was + * first. monitorEnter is a GC SAFEPOINT in this VM, so taking a lock inside the search + * -- even once per million nodes -- let the collector stop the workers far more often + * than the workload otherwise would, and the runaway this driver exists to reproduce + * stopped happening: peak footprint fell from 1271MB to 126MB with the reserve + * compiled out. That is the instrument destroying the experiment, which is the exact + * failure this whole change is about, so it is recorded here rather than fixed + * quietly.

+ * + *

A volatile store is neither a safepoint nor a lock, and JLS 17.7 makes volatile + * long reads and writes atomic -- so this answers the visibility and tearing a plain + * long[] element had, without touching the workload's shape.

+ */ + static final class Progress { + volatile long nodes; + } /** One node of the search: small, short-lived, and REFERENCE-CARRYING. Only a non-leaf * object has a mark function, and only such an object is eligible for maturation into @@ -165,7 +181,10 @@ public static void main(String[] args) { // table walk costs nothing and this workload cannot tell a cheap drain from an // O(heap) one. Reference-carrying, because the rescan skips objects with no mark // function. - nodeCounts = new long[threads]; + progress = new Progress[threads]; + for (int i = 0; i < threads; i++) { + progress[i] = new Progress(); + } legacyLiveSet = new Object[legacyBlocks][]; for (int i = 0; i < legacyBlocks; i++) { Object[] block = new Object[LEGACY_BLOCK_REFS]; @@ -192,7 +211,7 @@ public void run() { Thread[] workers = new Thread[threads]; for (int t = 0; t < threads; t++) { final int seed = t * 7919; - final int slot = t; + final Progress mine = progress[t]; workers[t] = new Thread(new Runnable() { public void run() { long sum = 0; @@ -200,28 +219,19 @@ public void run() { int[] root = new int[BOARD_CELLS]; int round = 0; while (!stop) { - sum += search(root, depth, seed + round, counter, slot); - // Publish once per round so the SAMPLE series is a live progress - // indicator rather than a row of zeroes. A worker that stalls - // stops publishing, and its slot going flat IS the signal. - // - // Under the lock sumNodes() reads: a plain long[] element write has - // no visibility guarantee against a concurrent reader and Java 8 - // permits a 64-bit element to be observed torn, so the series could - // sit stale or jump nonsensically -- precisely when a stalled - // worker is what it is meant to show. Once per round is roughly - // once a second per worker, so the lock costs nothing. - synchronized (SUM_LOCK) { - nodeCounts[slot] = counter[0]; - } + sum += search(root, depth, seed + round, counter, mine); + // Publish between rounds as well as inside the search, so a worker + // that stalls stops publishing and its count going flat IS the + // signal. + mine.nodes = counter[0]; round++; if (sleepMs > 0) { sleep(sleepMs); } } + mine.nodes = counter[0]; // Order-independent, so the checksum does not depend on scheduling. synchronized (SUM_LOCK) { - nodeCounts[slot] = counter[0]; checksum += sum; } } @@ -272,16 +282,16 @@ public void run() { System.out.println("GC_STEADY_STATE_DONE"); } - private static int search(int[] board, int d, int seed, long[] c, int slot) { + private static int search(int[] board, int d, int seed, long[] c, Progress p) { if (stop) { return 0; } // Thread-private: this array belongs to one worker for the whole run. c[0]++; if ((c[0] & (PUBLISH_EVERY_NODES - 1)) == 0) { - synchronized (SUM_LOCK) { - nodeCounts[slot] = c[0]; - } + // A volatile store, NOT a lock: monitorEnter is a GC safepoint here, and one + // inside the search changes the workload this driver exists to reproduce. + p.nodes = c[0]; } if (d == 0) { int s = 0; @@ -307,7 +317,7 @@ private static int search(int[] board, int d, int seed, long[] c, int slot) { chain = mv; } // chain is null when movesPerNode is 0, which is the leaf-only ablation. - int v = search(child, d - 1, seed + b + (chain == null ? 0 : chain.to), c, slot); + int v = search(child, d - 1, seed + b + (chain == null ? 0 : chain.to), c, p); if (v > best) { best = v; } @@ -330,13 +340,11 @@ private static int scrub(int d) { } private static long sumNodes() { - synchronized (SUM_LOCK) { - long total = 0; - for (int i = 0; i < nodeCounts.length; i++) { - total += nodeCounts[i]; - } - return total; + long total = 0; + for (int i = 0; i < progress.length; i++) { + total += progress[i].nodes; } + return total; } private static long footprintKb() { diff --git a/vm/tests/src/test/java/com/codename1/tools/translator/GcSteadyStateIntegrationTest.java b/vm/tests/src/test/java/com/codename1/tools/translator/GcSteadyStateIntegrationTest.java index b8cb8a4a30b..2eb13a6b09d 100644 --- a/vm/tests/src/test/java/com/codename1/tools/translator/GcSteadyStateIntegrationTest.java +++ b/vm/tests/src/test/java/com/codename1/tools/translator/GcSteadyStateIntegrationTest.java @@ -116,6 +116,12 @@ class GcSteadyStateIntegrationTest { /** Cycles needed before the comparison means anything. Anti-vacuousness. */ private static final int MIN_CYCLES = 24; + /** + * Wall-clock samples needed before the second series means anything. The emitter runs + * at 1Hz and the workload is tens of seconds, so this is a floor and not a target. + */ + private static final int MIN_WALL_ROWS = 10; + /** * Synthetic per-process budget for the ceiling scenario. * @@ -229,6 +235,25 @@ private void runGate(List tempDirs) throws Exception { describe("The page heap is still compounding in the second half of the run", good)); + // The per-cycle series above is blind to the shape this whole gate is really + // about: a collector that completes its early cycles and then never finishes + // another. [GCPROBE] stops emitting at that point, so its rows can end while the + // heap is still growing, and the generated main returns as soon as the workers do + // -- the process exits cleanly, prints the marker, and the stall goes unrecorded. + // [GCPROBE-T] is 1Hz off atomics and keeps sampling through exactly that state, + // which is why it was added; checking it here is what makes it a gate rather than + // a convenience. + int wallRows = wallSampleCount(clean.output); + assertTrue(wallRows >= MIN_WALL_ROWS, + "Only " + wallRows + " [GCPROBE-T] samples: the wall-clock emitter did not" + + " run, so the stalled-collector check below measured nothing."); + double wallGrowth = wallSecondHalfPageGrowth(clean.output); + assertTrue(wallGrowth <= MAX_SECOND_HALF_PAGE_GROWTH, + "The page heap is still compounding on the WALL-CLOCK series (second-half" + + " growth " + String.format("%.3f", wallGrowth) + " over " + wallRows + + " samples), which the per-cycle series cannot see if the collector" + + " stopped completing cycles."); + // ---- 2. proof that the gate can fail ---------------------------------- // CN1_SATB_LOG_FRESH is the escape hatch that restores the pre-fix barrier, so it // doubles as the fault injection: without this half, a build in which the probe or @@ -417,6 +442,46 @@ static Series parse(String output) { } } + /** pgTotal from the 1Hz [GCPROBE-T] series, in emission order. */ + private static List wallPages(String output) { + List pages = new ArrayList<>(); + for (String line : output.split("\\R")) { + if (!line.startsWith("[GCPROBE-T] v=1")) { + continue; + } + for (String token : line.split("\\s+")) { + if (token.startsWith("pgTotal=")) { + try { + pages.add(Long.parseLong(token.substring("pgTotal=".length()))); + } catch (NumberFormatException ignored) { + // a malformed row is not a measurement; skip it + } + } + } + } + return pages; + } + + private int wallSampleCount(String output) { + return wallPages(output).size(); + } + + /** Second-half page growth measured on wall-clock time rather than on cycles. */ + private double wallSecondHalfPageGrowth(String output) { + List pages = wallPages(output); + if (pages.size() < MIN_WALL_ROWS) { + return Double.MAX_VALUE; + } + // Same windowing as the per-cycle series: drop the first fifth as start-up. + int from = pages.size() / 5; + int mid = (from + pages.size()) / 2; + long atMid = pages.get(mid); + if (atMid == 0) { + return Double.MAX_VALUE; + } + return (double) (pages.get(pages.size() - 1) - atMid) / atMid; + } + private String describe(String what, Series s) { return what + ": cycles=" + s.cycles + " satbRefs/cycle/liveObject=" + String.format("%.3f", s.satbRefsPerLiveObject()) From d312fbab51fec630ce04f751300064fe9ccdedc8 Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Tue, 25 Aug 2026 14:09:10 +0300 Subject: [PATCH 20/21] Attach evidence to the ceiling assertions The first vm-tests run that ever completed on this branch failed scenario 3 -- "the smallest headroom seen was 62MB" under a 768MB budget -- and reported nothing else. Every other assertion in this gate appends the run's output; this one, the only one that has actually failed, did not. The probe rows that would explain it were captured and then discarded. Both ceiling assertions now carry the [PACING] counters, the last [GCPROBE] footprint partition and the wall-clock summary. That partition is the whole point of the probe: it says whether a footprint the reserve did not defend is even in the Java heap. No behaviour change, and the gate still passes locally on macOS -- which is itself the open question, since the failure is on the Linux runner and the two measure different quantities (phys_footprint against RSS). Co-Authored-By: Claude Opus 5 (1M context) --- .../GcSteadyStateIntegrationTest.java | 33 +++++++++++++++++-- 1 file changed, 31 insertions(+), 2 deletions(-) diff --git a/vm/tests/src/test/java/com/codename1/tools/translator/GcSteadyStateIntegrationTest.java b/vm/tests/src/test/java/com/codename1/tools/translator/GcSteadyStateIntegrationTest.java index 2eb13a6b09d..a5dda462a5a 100644 --- a/vm/tests/src/test/java/com/codename1/tools/translator/GcSteadyStateIntegrationTest.java +++ b/vm/tests/src/test/java/com/codename1/tools/translator/GcSteadyStateIntegrationTest.java @@ -291,7 +291,8 @@ private void runGate(List tempDirs) throws Exception { assertTrue(boundedHeadroomMb >= HEADROOM_THRESHOLD_MB, "Under a " + CEILING_MB + "MB budget the collector should defend about " + RESERVE_MB + "MB of headroom, but the smallest seen was " - + boundedHeadroomMb + "MB -- the process is riding the kill line."); + + boundedHeadroomMb + "MB -- the process is riding the kill line." + + evidence(bounded)); // ---- 4. proof that scenario 3 can fail --------------------------------- Path noReserve = build(distDir, tempDirs, "noreserve", @@ -304,7 +305,7 @@ private void runGate(List tempDirs) throws Exception { assertTrue(unboundedHeadroomMb < HEADROOM_THRESHOLD_MB, "Compiling the reserve out did NOT put the process back on the admission " + "margin (smallest headroom " + unboundedHeadroomMb + "MB), so this " - + "scenario is inert."); + + "scenario is inert." + evidence(unbounded)); } /** @@ -442,6 +443,34 @@ static Series parse(String output) { } } + /** + * The evidence a failing ceiling assertion needs: what the pacing tracer counted, and + * the last footprint partition the probe emitted. + * + *

Without this the only ceiling assertion that can fail reports a single number and + * nothing to explain it -- which is how the first CI failure of this gate arrived, and + * the probe rows it would have needed were captured and then discarded.

+ */ + private String evidence(Run r) { + StringBuilder sb = new StringBuilder("\n--- evidence ---\n"); + String pacing = null; + String lastProbe = null; + for (String line : r.output.split("\\R")) { + if (line.startsWith("[PACING]")) { + pacing = line; + } else if (line.startsWith("[GCPROBE] v=1")) { + lastProbe = line; + } + } + sb.append(pacing == null ? "(no [PACING] line)" : pacing).append('\n'); + sb.append(lastProbe == null ? "(no [GCPROBE] rows)" : lastProbe).append('\n'); + sb.append("wall samples=").append(wallSampleCount(r.output)) + .append(" secondHalfPageGrowth=") + .append(String.format("%.3f", wallSecondHalfPageGrowth(r.output))).append('\n'); + sb.append(tail(r.output)); + return sb.toString(); + } + /** pgTotal from the 1Hz [GCPROBE-T] series, in emission order. */ private static List wallPages(String output) { List pages = new ArrayList<>(); From 3f03a7f1d00e00da86e155f1ebc1e238a229c016 Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Tue, 25 Aug 2026 14:53:31 +0300 Subject: [PATCH 21/21] Assert the reserve's mechanism, report its outcome The first vm-tests run that completed on this branch failed scenario 3 on the Linux runner: 62MB of headroom under a 768MB budget. With the evidence attached, the diagnosis is not what I guessed. I expected allocator retention -- glibc arenas holding freed legacy blocks, which RSS counts and phys_footprint would not. Wrong: residKb was 7MB of a 518MB footprint, so the footprint was the Java heap almost exactly. That is the residual bucket earning its place; it killed the hypothesis in one line. What the runner actually shows is a collector that cannot keep up, with the bound working: volumeParks=879, so it engaged and parked repeatedly, while mark ran 407-545ms per cycle -- 235ms of conservative stack scan, 122-252ms waiting for mutators to reach a safepoint -- against ~170MB of allocation per cycle. With the grace rule holding a cycle's allocation two more cycles, the smallest working set that machine can hold is already above the reserve line at that budget. satbMs was 0 throughout, so the earlier fix is holding and the stack scan is simply the next cost. So an absolute headroom assertion was testing the runner rather than the collector. Scenario 3 now asserts the contract, which is true on any machine: either the process never entered its reserve, or the bound engaged when it did. The headroom achieved is printed either way, so the outcome stays visible without being asserted. A regression that stops the bound engaging fails here; a machine that is merely slow does not. Scenario 4 gains a second half for the same reason -- with the reserve compiled out the process must land on the bare admission margin, or the ceiling is not pressuring the workload and scenario 3's "never entered" branch would pass for the wrong reason -- plus volumeParks == 0, since the bound is not in that build at all. Locally: headroom 161MB inside a 192MB reserve with volumeParks=350, against 63MB and volumeParks=0 with the reserve compiled out. Co-Authored-By: Claude Opus 5 (1M context) --- .../GcSteadyStateIntegrationTest.java | 85 +++++++++++++++---- 1 file changed, 70 insertions(+), 15 deletions(-) diff --git a/vm/tests/src/test/java/com/codename1/tools/translator/GcSteadyStateIntegrationTest.java b/vm/tests/src/test/java/com/codename1/tools/translator/GcSteadyStateIntegrationTest.java index a5dda462a5a..4c2ecb2e739 100644 --- a/vm/tests/src/test/java/com/codename1/tools/translator/GcSteadyStateIntegrationTest.java +++ b/vm/tests/src/test/java/com/codename1/tools/translator/GcSteadyStateIntegrationTest.java @@ -144,14 +144,14 @@ class GcSteadyStateIntegrationTest { private static final long RESERVE_MB = CEILING_MB / 4; /** - * The line between the two regimes: defending a reserve, or converging on the bare - * admission margin. Twice CN1_PACING_HEADROOM_MARGIN, i.e. an ABSOLUTE figure rather - * than a share of the budget -- the margin does not scale with the budget, so a - * proportional threshold silently stops separating the regimes as the budget shrinks - * (at a 400MB budget the reserve is 100MB and the margin still 64MB, and half the - * reserve falls below it). The two regimes here are 192MB and 63MB, so this sits - * clear of both; a tolerance tight enough to distinguish 192 from 180 would be - * measuring the runner. + * The line below which a process is on the bare admission margin rather than + * defending anything. Twice CN1_PACING_HEADROOM_MARGIN, i.e. an ABSOLUTE figure -- + * the margin does not scale with the budget, so a proportional threshold silently + * stops separating the regimes as the budget shrinks. + * + *

Used only for the no-reserve build, to establish that the environment really + * does pressure the process. See the scenario-3 comment for why the reserve build is + * NOT held to an absolute headroom figure.

*/ private static final long HEADROOM_THRESHOLD_MB = 128; @@ -288,11 +288,36 @@ private void runGate(List tempDirs) throws Exception { assertTrue(boundedHeadroomMb >= 0, "No [PACING] report under a simulated ceiling -- the budgeted path never " + "ran, so this scenario measured nothing. Output: " + tail(bounded.output)); - assertTrue(boundedHeadroomMb >= HEADROOM_THRESHOLD_MB, - "Under a " + CEILING_MB + "MB budget the collector should defend about " - + RESERVE_MB + "MB of headroom, but the smallest seen was " - + boundedHeadroomMb + "MB -- the process is riding the kill line." - + evidence(bounded)); + // ASSERT THE MECHANISM, REPORT THE OUTCOME. + // + // The first version of this demanded an absolute headroom figure and failed on the + // Linux runner with 62MB. The evidence said the bound was working exactly as + // designed -- volumeParks=879, so it engaged and parked repeatedly -- and that the + // footprint it could not claw back was entirely the Java heap (residKb=7MB of a + // 518MB footprint, so no allocator retention involved). What that runner cannot do + // is COLLECT fast enough for the reserve line to be reachable: mark ran 407-545ms + // per cycle, of which 235ms was the conservative stack scan and 122-252ms was + // waiting for mutators to reach a safepoint, while the mutator allocated ~170MB + // per cycle. With the grace rule holding a cycle's allocation for two more cycles, + // the smallest working set that machine can hold is already above the reserve line + // at this budget. + // + // So an absolute headroom assertion tests the runner, not the collector. What is + // true on every machine is the contract itself: either the process never entered + // the reserve, or the bound engaged when it did. Both halves are checked, and the + // headroom actually achieved is printed either way, so a regression that stops the + // bound engaging fails here and a machine that is merely slow does not. + long volumeParks = pacingCounter(bounded.output, "volumeParks="); + assertTrue(volumeParks >= 0, + "No [PACING] volumeParks counter -- the tracer did not run, so this " + + "scenario measured nothing." + evidence(bounded)); + assertTrue(boundedHeadroomMb >= RESERVE_MB || volumeParks > 0, + "The process spent time inside its " + RESERVE_MB + "MB reserve (smallest " + + "headroom " + boundedHeadroomMb + "MB) and the volume bound never " + + "engaged -- volumeParks=" + volumeParks + "." + evidence(bounded)); + System.err.println("[GcSteadyState] ceiling: budget=" + CEILING_MB + "MB reserve=" + + RESERVE_MB + "MB smallestHeadroom=" + boundedHeadroomMb + "MB volumeParks=" + + volumeParks); // ---- 4. proof that scenario 3 can fail --------------------------------- Path noReserve = build(distDir, tempDirs, "noreserve", @@ -302,10 +327,20 @@ private void runGate(List tempDirs) throws Exception { long unboundedHeadroomMb = minHeadroomMb(unbounded.output); assertTrue(unboundedHeadroomMb >= 0, "No [PACING] report from the no-reserve build. Output: " + tail(unbounded.output)); + // The fault twin, and what keeps scenario 3 non-vacuous: with the bound compiled + // out the process must end up on the bare admission margin. If it does not, the + // environment is not pressuring it at all and scenario 3's "never entered the + // reserve" branch would be passing for the wrong reason. assertTrue(unboundedHeadroomMb < HEADROOM_THRESHOLD_MB, "Compiling the reserve out did NOT put the process back on the admission " - + "margin (smallest headroom " + unboundedHeadroomMb + "MB), so this " - + "scenario is inert." + evidence(unbounded)); + + "margin (smallest headroom " + unboundedHeadroomMb + "MB), so the " + + "ceiling is not pressuring this workload and scenario 3 proved " + + "nothing." + evidence(unbounded)); + assertEquals(0, pacingCounter(unbounded.output, "volumeParks="), + "The reserve was compiled out, so nothing may have parked on it." + + evidence(unbounded)); + System.err.println("[GcSteadyState] ceiling/no-reserve: smallestHeadroom=" + + unboundedHeadroomMb + "MB"); } /** @@ -334,6 +369,26 @@ private void assertHealthy(Run r, String which, String expectedResult) { + tail(r.output)); } + /** A counter from the [PACING] line, or -1 if the tracer never reported. */ + private long pacingCounter(String output, String key) { + for (String line : output.split("\\R")) { + int at = line.indexOf(key); + if (at < 0 || !line.startsWith("[PACING]")) { + continue; + } + String rest = line.substring(at + key.length()); + int end = 0; + if (end < rest.length() && rest.charAt(end) == '-') { + end++; + } + while (end < rest.length() && Character.isDigit(rest.charAt(end))) { + end++; + } + return end > 0 ? Long.parseLong(rest.substring(0, end)) : -1; + } + return -1; + } + /** Smallest headroom the pacing tracer saw, in MB, or -1 if it never reported. */ private long minHeadroomMb(String output) { for (String line : output.split("\\R")) {