From 24517c5138563bd025f60def958c1deb12e79954 Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Mon, 7 Sep 2026 11:17:36 +0300 Subject: [PATCH 01/30] Give ParparVM real weak and soft references The collector had no notion of a weak root. java.lang.ref.WeakReference held its referent in an ordinary field, so the translator emitted a gcMarkObject for it and the referent was STRONG -- a "weak" reference pinned its referent for the life of the process, and every cache built on CodenameOneImplementation.createSoftWeakRef (the EncodedImage decode cache, Image's scale and RGB caches, Border's round-rect cache, rasterised gradients) was unbounded. There was no SoftReference at all. The referent now lives in Reference and the translator does not trace it (ByteCodeClass.isReferenceReferent): it emits cn1GcDiscoverReference, which hands the collector the field addresses and decides soft retention on the spot. Clearing happens in cn1GcProcessReferences, using the sweep's own liveness test -- both halves of the sweep free on `mark != -1 && mark < currentGcMarkValue - 1` and nothing else may be cleared, because a dangling read on this VM is a native crash no Java catch can see. Three things about the placement, none of them optional: - The clear pass runs INSIDE the SATB termination loop, barrier still armed. A thread scanned and released early can pull a referent out through get() and hold it in a local the collector has walked past, and that referent is then neither marked nor fresh -- the one case the sweep's "already marked or FRESH" invariant does not cover. get() carries a load barrier, so a racing read makes the trial clear of gcSatbActive find a non-empty log, which re-arms and re-runs the fixpoint and this pass with it. - That barrier is FILTERED (CN1_SATB_REF_LOAD). Logging every referent read is not a cost but a failure: cn1SatbEnqueue takes a mutex per accepted reference and get() on a hot cache is called far more often than any store barrier sees. Unfiltered it put over 10,000 entries in the log per cycle and reached CN1_SATB_MAX_REOPENS on EVERY cycle. Skipping referents already marked this epoch or fresh -- exactly the ones the clear pass would refuse to clear -- took passes 32 -> 1 and refMs 0.06 -> 0.005. - Soft retention is ranked by age since the last get(), decided when the mark first reaches the reference so the mark stays single pass. Deciding afterwards would mean a second reachability closure over the retained set, on a mark that already spends most of its time in the grace pass. Measured (RefPolicy + ab-refs.sh, five interleaved reps, checksums identical across every arm). References themselves are unambiguous: 255/256 unreachable referents reclaimed against 0/256, for 1.8-4% of mark time, with a vm/benchmarks geomean of 1.011 over 12 interleaved reps against master. The RANKING is a cheap rider rather than the payoff -- against never-clearing it is the same hit rate for about a megabyte. What it does beat decisively is the pressure-triggered alternative, which gave up 12 points of hit rate for zero footprint saving and was worse on both axes at a 160MB ceiling; that arm is the model of the iOS port's didReceiveMemoryWarning -> flushSoftRefMap. vm/CLAUDE.md carries the table and the three conclusions that were drawn from single runs and turned out to be wrong, including the one that generalises: a pressure-triggered cache policy cannot work on this collector, because the pacing loop defends the reserve by throttling the mutator, so headroom converges on any threshold placed there and never crosses it. This change is confined to the VM. The porting layer, the iOS softReferenceMap override and the core call sites are deliberately untouched and follow separately, since they change behaviour in every iOS app and want their own bisect point. Gates: run-gc-verify.sh green (RefPolicy clean over 47 verify passes, both fault-injection self-tests firing), run-gauntlet.sh bit-identical to the host JVM, vm/tests 552 tests / 0 failures, GC integration leg 10/10 including GcSteadyStateIntegrationTest and ProcessBudgetPacingIntegrationTest. Co-Authored-By: Claude Opus 5 (1M context) --- vm/ByteCodeTranslator/src/cn1_globals.h | 66 +++ vm/ByteCodeTranslator/src/cn1_globals.m | 377 ++++++++++++++++++ .../tools/translator/ByteCodeClass.java | 89 +++++ vm/CLAUDE.md | 82 ++++ vm/JavaAPI/src/java/lang/ref/Reference.java | 99 ++++- .../src/java/lang/ref/SoftReference.java | 58 +++ .../src/java/lang/ref/WeakReference.java | 51 +-- vm/benchmarks/ab-refs.sh | 139 +++++++ vm/benchmarks/src/com/bench/RefPolicy.java | 277 +++++++++++++ .../JavascriptRuntimeSemanticsTest.java | 6 + .../tools/translator/JsWeakReferenceApp.java | 18 +- 11 files changed, 1223 insertions(+), 39 deletions(-) create mode 100644 vm/JavaAPI/src/java/lang/ref/SoftReference.java create mode 100755 vm/benchmarks/ab-refs.sh create mode 100644 vm/benchmarks/src/com/bench/RefPolicy.java diff --git a/vm/ByteCodeTranslator/src/cn1_globals.h b/vm/ByteCodeTranslator/src/cn1_globals.h index 9b90b5d2709..b9613e41a3a 100644 --- a/vm/ByteCodeTranslator/src/cn1_globals.h +++ b/vm/ByteCodeTranslator/src/cn1_globals.h @@ -1414,6 +1414,26 @@ extern void cn1SatbEnqueue(JAVA_OBJECT old); } } while(0) #endif +// ---- java.lang.ref support ------------------------------------------------- +// A WeakReference's referent is NOT traced by the generated mark function. That +// function calls cn1GcDiscoverReference instead (see ByteCodeClass), handing over +// the addresses of the reference's fields, and the collector decides for itself +// whether the referent lives. +// +// Field pointers rather than the object, because this file is a fixed template +// compiled beside whatever the translator emitted: `struct obj__java_lang_ref_Reference` +// does not exist in a program that never uses a reference, so naming it here would +// break the build for those. The layout stays on the generated side. +// +// `strength` is CN1_REF_WEAK or CN1_REF_SOFT, taken from a field the subclass +// constructor sets. Deliberately not a class-pointer comparison: the dead-code pass +// is entitled to remove a class symbol this file would then fail to link against, +// and a user-written subclass of either would compare unequal to both. +#define CN1_REF_WEAK 0 +#define CN1_REF_SOFT 1 +// cn1TouchAge value written by get_field_java_lang_ref_Reference_objReference on +// every read. The collector turns it back into an age in cycles. +#define CN1_REF_TOUCHED (-1) extern const char* volatile cn1LastNamSetter; // diagnosis: last bracket toucher #ifdef CN1_CONSERVATIVE_GC_ROOTS // The bracket's purpose was to suppress GC interaction while native C code @@ -3073,6 +3093,52 @@ void codenameOneGcFree(CODENAME_ONE_THREAD_STATE, JAVA_OBJECT obj); extern int currentGcMarkValue; extern void gcMarkObject(CODENAME_ONE_THREAD_STATE, JAVA_OBJECT obj, JAVA_BOOLEAN force); +extern void cn1GcDiscoverReference(CODENAME_ONE_THREAD_STATE, JAVA_OBJECT ref, JAVA_BOOLEAN force, + JAVA_OBJECT* referentField, JAVA_INT* touchAgeField, + JAVA_INT* agedCycleField, JAVA_INT strength); + + +// ---- the Reference.get() load barrier -------------------------------------- +// Emitted into get_field_java_lang_ref_Reference_objReference, and the reason the +// clear pass is allowed to run with mutators still going. +// +// A thread whose stack was scanned and released early can pull the referent out of a +// reference and hold it in a local the collector has already walked past. That referent +// is then neither marked nor fresh, which is the one case the sweep's "already marked or +// FRESH" invariant does not cover, so without this it is freed under a live pointer. +// Enqueuing puts it in the snapshot: the trial clear of gcSatbActive finds a non-empty +// log, re-arms, marks it, and the reference is left alone. +// +// THE FILTER IS NOT AN OPTIMIZATION, it is what makes this affordable. cn1SatbEnqueue +// takes a mutex per accepted reference, and get() on a hot cache is called far more often +// than anything the per-store barrier sees -- measured on RefPolicy before this filter +// existed, a 400,000-access run put over 10,000 entries into the log per cycle and drove +// the SATB termination loop into its CN1_SATB_MAX_REOPENS cap on every single cycle, +// which is the collector failing to converge rather than a cost. +// +// It skips exactly the referents the clear pass would refuse to clear anyway: already +// marked this epoch, or fresh and therefore kept by the sweep's grace rule. Deliberately +// STRICTER than the clear pass's own test, which also spares mark == epoch - 1 (last +// cycle's slack): bibopGcEpoch is only exactly equal to currentGcMarkValue once +// cn1BibopBeginGcCycle has published it, and a barrier must not depend on a mirror being +// current. Skipping less is always safe; skipping more is not. +// +// A retained soft reference costs nothing here at all, because its referent is marked as +// an ordinary strong edge by cn1GcDiscoverReference before any get() can reach it. +#if defined(CN1_DISABLE_SATB) +#define CN1_SATB_REF_LOAD(fieldAddr) do { } while(0) +#else +#define CN1_SATB_REF_LOAD(fieldAddr) \ + do { if(__builtin_expect(gcSatbActive, 0)) { \ + JAVA_OBJECT cn1__r = *(JAVA_OBJECT volatile*)(fieldAddr); \ + if(cn1__r != JAVA_NULL && !CN1_IS_TAGGED(cn1__r)) { \ + int cn1__m = __atomic_load_n(&cn1__r->__codenameOneGcMark, __ATOMIC_RELAXED); \ + int cn1__e = atomic_load_explicit(&bibopGcEpoch, memory_order_relaxed); \ + if(cn1__m != -1 && cn1__m != cn1__e) cn1SatbEnqueue(cn1__r); \ + } \ + } } while(0) +#endif + extern void gcMarkArrayObject(CODENAME_ONE_THREAD_STATE, JAVA_OBJECT obj, JAVA_BOOLEAN force); extern JAVA_BOOLEAN removeObjectFromHeapCollection(CODENAME_ONE_THREAD_STATE, JAVA_OBJECT o); diff --git a/vm/ByteCodeTranslator/src/cn1_globals.m b/vm/ByteCodeTranslator/src/cn1_globals.m index 4bb0caee0fa..3834b802934 100644 --- a/vm/ByteCodeTranslator/src/cn1_globals.m +++ b/vm/ByteCodeTranslator/src/cn1_globals.m @@ -2166,8 +2166,351 @@ JAVA_LONG GcVerifyApp_gcMarkState___R_long(CODENAME_ONE_THREAD_STATE) { } #endif +// ---- java.lang.ref: discovery, ranking and clearing ------------------------- +// +// The generated mark function for java.lang.ref.Reference does NOT trace its +// referent (see ByteCodeClass.isReferenceReferent); it calls +// cn1GcDiscoverReference below instead. That is the whole reason a WeakReference +// here is weak: until this existed the referent was an ordinary field, traced +// like any other, so a "weak" reference pinned its referent for the life of the +// process and every cache built on Display.createSoftWeakRef was unbounded. +// +// THE ORDER OF EVENTS IN A CYCLE, because the safety argument is entirely about +// order and every step below depends on the one before it: +// +// 1. cn1RefBeginCycle - empties last cycle's list, recomputes the soft budget +// 2. cn1GcDiscoverReference - called from mark functions as the mark reaches each +// reference. AGES it, decides soft retention, and for a +// retained soft marks the referent so it is an ordinary +// strong edge for the rest of the cycle. +// 3. cn1GcProcessReferences - runs inside the SATB termination loop, after the strong +// mark has reached its fixpoint and while the barrier is +// STILL ARMED. Clears every reference whose referent the +// sweep is about to free. +// +// Step 2 is where ranking costs the mark nothing beyond the walk it was already +// doing. Deciding retention in step 3 instead -- which is what a textbook +// soft-reference implementation does, and what HotSpot does -- would mean computing +// a SECOND reachability closure over the retained set, on a mark that already +// spends most of its time in the grace pass. The price of deciding early is that +// the decision uses the age as of the previous cycle. That is self-correcting: a +// soft reference demoted at the start of a cycle and then read during it is caught +// by the touch test in step 3 and comes back hot on the next one. +// +// WHY STEP 3 MUST RUN WITH THE BARRIER ARMED. This collector scans a thread's stack +// and RELEASES the thread before the others are scanned. A thread released early can +// call get() and park the referent in a local the collector has already walked past +// -- and that referent is by then neither marked nor fresh, which is the one case +// the "a reference stored after the fixpoint is already marked or FRESH" invariant +// the sweep relies on does not cover. So get() carries a SATB load barrier (emitted +// into get_field_java_lang_ref_Reference_objReference), and this pass sits INSIDE the +// termination loop: an enqueue from a racing get() makes the trial clear of +// gcSatbActive find a non-empty log, which re-arms the barrier, marks the referent +// and runs the whole fixpoint again. A reference cleared in the losing half of that +// race is cleared while its referent was genuinely unreachable, which the contract +// permits -- and the referent itself survives, because the enqueue marked it. +// +// TWO PROPERTIES THAT FOLLOW, both legal ("may be cleared" is not "must be cleared"), +// and both worth knowing before writing a test that asserts prompt collection: +// - The sweep's grace rule keeps any object allocated since the last sweep, so a +// referent is never cleared in the cycle it dies. +// - Native C stacks are scanned conservatively, so a stale machine word that looks +// like the referent keeps it marked and get() keeps answering it. + +// Ablation arms for the retention policy. The ranked default is arm 2; the other two +// are the baselines it has to beat, kept compilable so the comparison can be re-run +// rather than remembered. See vm/benchmarks/ab-refs.sh. +// 0 - pressure only: soft referents are retained until headroom drops into the +// pacing reserve, then all of them are dropped at once. This is what the iOS +// port's didReceiveMemoryWarning -> flushSoftRefMap did, moved into the +// collector and without the shared-table lifetime bug. +// 1 - never: a soft reference is as strong as a field. The upper bound on hit rate +// and the upper bound on footprint. +// 2 - ranked by use (default). +#ifndef CN1_REF_POLICY +#define CN1_REF_POLICY 2 +#endif + +// How many collections a soft referent may go untouched and still be kept, when there +// is memory to spare. A budget in CYCLES rather than milliseconds because the +// collector is the only clock that matters here: it is what ages the reference, and a +// busy app collects more often, which is exactly when a cache should be trimmed harder. +#ifndef CN1_REF_SOFT_RETAIN_MAX +#define CN1_REF_SOFT_RETAIN_MAX 8 +#endif + +struct CN1RefEntry { + JAVA_OBJECT ref; // the reference object itself; diagnostics only + JAVA_OBJECT* referentField; + JAVA_INT* touchAgeField; + JAVA_INT strength; +}; +static struct CN1RefEntry* cn1RefDiscovered = 0; +static long cn1RefDiscoveredTop = 0; +static long cn1RefDiscoveredCap = 0; +// Discovery runs from generated mark functions, which gcMarkDrainParallel may fan out +// across a worker pool, so the append is locked. It is a cold lock by construction: +// it is taken once per REFERENCE per cycle, where the SATB mutex next door is taken +// per logged store. +static pthread_mutex_t cn1RefMutex = PTHREAD_MUTEX_INITIALIZER; +// Ages, in collections, that a soft referent may reach before it is dropped. +// Recomputed once per cycle by cn1RefBeginCycle. Negative means "drop every soft +// referent, however hot". +// +// ATOMIC because the writer and the readers are different threads: cn1RefBeginCycle +// runs on the GC thread and cn1GcDiscoverReference reads it from however many mark +// workers gcMarkDrainParallel is using. Relaxed is the same instruction on every target +// built here; what it buys is that the pair is not a mixed atomic/non-atomic access, +// which is undefined in C however benign the race looks. Reading a value one cycle stale +// would be harmless anyway -- the budget only decides retention, never safety. +static _Atomic int cn1SoftRetainCycles = CN1_REF_SOFT_RETAIN_MAX; + +#ifdef CN1_GC_CONFORM +_Atomic long cn1RefDiscoveries = 0; // references the mark reached (deduped) +_Atomic long cn1RefWeak = 0; // of those, weak +_Atomic long cn1RefRetained = 0; // soft referents kept by the policy +_Atomic long cn1RefKeptTouched = 0; // kept by the racing-get() rule in the clear pass +_Atomic long cn1RefCleared = 0; // referents handed to the sweep +long long cn1RefPhaseNs = 0; // GC thread only: time in cn1GcProcessReferences +long cn1RefPasses = 0; // clear passes run this cycle (>1 == SATB reopen) +#endif + +// Recompute the soft budget and drop the previous cycle's discoveries. Called from +// codenameOneGCMark before anything can mark. +static void cn1RefBeginCycle(void) { + cn1RefDiscoveredTop = 0; +#ifdef CN1_GC_CONFORM + // PER CYCLE, like every other figure in [GCPROBE]. A running total cannot show + // whether the budget is tracking memory pressure, which is the whole question the + // ranking has to answer. + atomic_store_explicit(&cn1RefDiscoveries, 0, memory_order_relaxed); + atomic_store_explicit(&cn1RefWeak, 0, memory_order_relaxed); + atomic_store_explicit(&cn1RefRetained, 0, memory_order_relaxed); + atomic_store_explicit(&cn1RefKeptTouched, 0, memory_order_relaxed); + atomic_store_explicit(&cn1RefCleared, 0, memory_order_relaxed); + cn1RefPhaseNs = 0; + cn1RefPasses = 0; +#endif +#if CN1_REF_POLICY == 1 + atomic_store_explicit(&cn1SoftRetainCycles, 0x7fffffff, memory_order_relaxed); +#else + { + long headroom = cn1ProcessHeadroom(); + if(headroom < 0) { + // No per-process limit on this platform -- desktop, CI, the simulator. Age + // soft referents out at the full budget anyway rather than keeping them + // forever: an unbounded cache is the defect this replaces, and a host with + // plenty of RAM is precisely where it went unnoticed for years. + atomic_store_explicit(&cn1SoftRetainCycles, CN1_REF_SOFT_RETAIN_MAX, + memory_order_relaxed); + } else { + // Against the BUDGET, never the device's free RAM, and the budget is what is + // already spent plus what is left. Sizing this from the device is the defect + // #5563 fixed for the pacing cap, and it is the same defect whichever + // consumer reads the number. + long long footprint = (long long)cn1ProcFootprintBytes(); + long long budget = (footprint > 0 ? footprint : 0) + (long long)headroom; + // + // BANDS AS EXPLICIT FRACTIONS OF THE BUDGET. An earlier version wrote them as + // multiples of the pacing reserve (budget >> 2) and got both ends wrong in a + // way that reads as correct: `reserve * 4` IS the whole budget, so the top band + // required headroom to equal the budget and could never be entered, while the + // bottom band required headroom below a quarter -- which the pacing loop + // actively prevents, since defending that reserve is its job. Written as + // fractions the reachability of each band is obvious on inspection. +#if CN1_REF_POLICY == 0 + // Pressure only, no ranking: everything, until nothing. This is what the iOS + // port's didReceiveMemoryWarning -> flushSoftRefMap does, and it is the arm + // ranking has to beat. It switches at the SAME point the ranked ladder starts + // trimming, so the two differ in one thing only -- what they drop once they + // have decided to drop something. + atomic_store_explicit(&cn1SoftRetainCycles, + (headroom * 2 >= budget) ? 0x7fffffff : -1, + memory_order_relaxed); +#else + if(headroom <= 0) { + atomic_store_explicit(&cn1SoftRetainCycles, -1, memory_order_relaxed); + } else if(headroom * 2 >= budget) { + // Over half the budget still free: nothing to trim for. + atomic_store_explicit(&cn1SoftRetainCycles, CN1_REF_SOFT_RETAIN_MAX, + memory_order_relaxed); + } else if(headroom * 4 >= budget) { + atomic_store_explicit(&cn1SoftRetainCycles, CN1_REF_SOFT_RETAIN_MAX / 2, + memory_order_relaxed); + } else { + // Inside the quarter the pacing loop defends: keep only what was read + // since the last collection. + atomic_store_explicit(&cn1SoftRetainCycles, 0, memory_order_relaxed); + } +#endif + } + } +#endif +} + +void cn1GcDiscoverReference(CODENAME_ONE_THREAD_STATE, JAVA_OBJECT ref, JAVA_BOOLEAN force, + JAVA_OBJECT* referentField, JAVA_INT* touchAgeField, + JAVA_INT* agedCycleField, JAVA_INT strength) { + if(referentField == 0 || *referentField == JAVA_NULL) { + return; // already cleared: nothing to decide + } +#ifdef CN1_NO_WEAK_REFS + // ABLATION ARM: trace the referent strongly and never clear anything, which is + // exactly what this VM did before references were implemented. It exists so the + // gate can prove it is not inert -- a test that asserts referents get collected + // must FAIL when built this way, or it is asserting something else. + gcMarkObject(threadStateData, *referentField, force); + (void)touchAgeField; (void)agedCycleField; (void)strength; (void)ref; + return; +#else + JAVA_BOOLEAN retain; + pthread_mutex_lock(&cn1RefMutex); + // DEDUPE, and it is not an optimization. Being reached more than once in a cycle is + // the normal case, not a rare one: force-marking re-runs mark functions over + // already-marked objects once per statics pass and again for the constant pool. Left + // undeduped, a popular reference would age several times a cycle and its soft + // referent would be dropped that many times sooner. + if(*agedCycleField == currentGcMarkValue) { + pthread_mutex_unlock(&cn1RefMutex); + return; + } + *agedCycleField = currentGcMarkValue; + { + // AGE IT. CN1_REF_TOUCHED means get() ran since the last cycle aged this + // reference, so the age resets; anything else is one collection older. Saturating, + // because an age that wraps to negative would read as freshly touched and make a + // cold entry immortal. + JAVA_INT age = *touchAgeField; + JAVA_INT aged = (age == CN1_REF_TOUCHED) ? 0 + : (age < 0x7ffffffe ? age + 1 : age); + *touchAgeField = aged; + int budget = atomic_load_explicit(&cn1SoftRetainCycles, memory_order_relaxed); + retain = (strength == CN1_REF_SOFT && budget >= 0 && aged <= budget) + ? JAVA_TRUE : JAVA_FALSE; + } + if(cn1RefDiscoveredTop >= cn1RefDiscoveredCap + && atomic_load_explicit(&cn1GcFreezeHeld, memory_order_relaxed) == 0) { + // NEVER GROW WHILE A THREAD IS SIGNAL-FROZEN -- the same rule cn1MatureObject's + // adoption buffer follows, and the reason cn1GcFreezeHeld exists. A frozen thread + // halts at an arbitrary instruction and can own the libc allocator lock, so a + // realloc here would block the collector until a thread that only resumes when the + // collector lets it. Declining costs a deferred reclaim (see below) and nothing else. + // + // This is belt and braces rather than a live hazard: discovery runs from mark + // functions, which run in the drain, which runs after every freeze is released. + // Belt and braces is right for a rule whose violation is a whole-process hang that + // reproduces on one thread in one interleaving. + long nc = cn1RefDiscoveredCap == 0 ? 256 : cn1RefDiscoveredCap * 2; + struct CN1RefEntry* grown = + (struct CN1RefEntry*)realloc(cn1RefDiscovered, (size_t)nc * sizeof(struct CN1RefEntry)); + if(grown != 0) { + cn1RefDiscovered = grown; + cn1RefDiscoveredCap = nc; + } + } + if(cn1RefDiscoveredTop < cn1RefDiscoveredCap) { + struct CN1RefEntry* e = &cn1RefDiscovered[cn1RefDiscoveredTop++]; + e->ref = ref; + e->referentField = referentField; + e->touchAgeField = touchAgeField; + e->strength = strength; + } + // A DROPPED ENTRY IS SAFE, and safe in the direction that matters -- whether it was + // dropped because realloc failed or because the growth above was declined. The clear + // pass never sees this reference, so the referent stays reachable through a field + // nothing cleared: a missed reclaim, never a freed object under a live pointer. Note + // the reference was still AGED above, so a dropped cycle does not make it immortal; + // the next cycle discovers it again and the list has usually grown by then. + pthread_mutex_unlock(&cn1RefMutex); +#ifdef CN1_GC_CONFORM + atomic_fetch_add_explicit(&cn1RefDiscoveries, 1, memory_order_relaxed); + if(strength != CN1_REF_SOFT) { + atomic_fetch_add_explicit(&cn1RefWeak, 1, memory_order_relaxed); + } else if(retain) { + atomic_fetch_add_explicit(&cn1RefRetained, 1, memory_order_relaxed); + } +#endif + if(retain) { + // A retained soft reference is an ordinary strong edge for the rest of this + // cycle, traced with whatever `force` the caller had -- a reference held by a + // static is force-marked like anything else it points at. + gcMarkObject(threadStateData, *referentField, force); + } +#endif +} + +// Clear every discovered reference whose referent this cycle's sweep is about to free. +// Runs on the GC thread only, inside the SATB termination loop, barrier armed. +// Returns JAVA_TRUE if it marked anything, so the caller knows to drain. +static JAVA_BOOLEAN cn1GcProcessReferences(CODENAME_ONE_THREAD_STATE) { + JAVA_BOOLEAN marked = JAVA_FALSE; +#ifdef CN1_GC_CONFORM + long long __r0 = cn1GcNowNs(); + cn1RefPasses++; +#endif + // No lock. Discovery only ever appends, and every mark thread has been drained to a + // fixpoint before the caller reaches this, so the prefix walked here is stable. A + // re-run after a SATB reopen walks the list again from the start, which is what picks + // up anything discovered by the re-opened fixpoint. + long n = cn1RefDiscoveredTop; + for(long i = 0 ; i < n ; i++) { + struct CN1RefEntry* e = &cn1RefDiscovered[i]; + JAVA_OBJECT r = *e->referentField; + if(r == JAVA_NULL || CN1_IS_TAGGED(r)) { + continue; + } + if(*e->touchAgeField == CN1_REF_TOUCHED) { + // A get() landed AFTER this cycle aged this reference, so a mutator may be + // holding the referent in a local the collector has already walked past. + // Keeping it is not an optimization, it is the reason this pass is allowed + // to clear anything at all. Cheaper than trusting the SATB enqueue alone, + // and correct even if the log overflowed. + gcMarkObject(threadStateData, r, JAVA_FALSE); + marked = JAVA_TRUE; +#ifdef CN1_GC_CONFORM + atomic_fetch_add_explicit(&cn1RefKeptTouched, 1, memory_order_relaxed); +#endif + continue; + } +#ifdef CN1_CONSERVATIVE_GC_ROOTS + // The SAME guard gcMarkObject applies, and for the same reason: a reference + // object kept alive by a stale native-stack word can be dead with a referent + // field that dangles into memory unmapped in an earlier cycle, and reading even + // the mark word of that faults. A pointer that does not resolve is either such + // garbage or an object allocated after this cycle's extent snapshot -- which is + // FRESH, so the grace rule keeps it and there is nothing to clear either way. + if(cn1ConservativeResolve((void*)r) != r && !cn1GcImmortalObjContains(r)) { + continue; + } +#endif + // EXACTLY THE SWEEP'S OWN LIVENESS TEST, and it has to be: clearing a reference + // the sweep then keeps only wastes a cache entry, but FAILING to clear one the + // sweep frees hands get() a dangling pointer, and a dangling read on ParparVM is + // a native crash no Java catch can see. Both halves of the sweep -- the BiBOP + // per-slot walk and the legacy table scan -- free on `mark != -1 && mark < + // currentGcMarkValue - 1`; -1 is the one-cycle grace and currentGcMarkValue - 1 + // is last cycle's slack, and both mean the object survives. + int mark = __atomic_load_n(&r->__codenameOneGcMark, __ATOMIC_ACQUIRE); + if(mark == -1 || mark >= currentGcMarkValue - 1) { + continue; + } + *e->referentField = JAVA_NULL; +#ifdef CN1_GC_CONFORM + atomic_fetch_add_explicit(&cn1RefCleared, 1, memory_order_relaxed); +#endif + } +#ifdef CN1_GC_CONFORM + cn1RefPhaseNs += cn1GcNowNs() - __r0; +#endif + return marked; +} + void codenameOneGCMark() { currentGcMarkValue++; + // Drop the previous cycle's reference list and recompute the soft-retention budget + // from the memory still available. Must precede anything that can mark, because + // cn1GcDiscoverReference reads the budget to decide retention as it goes. + cn1RefBeginCycle(); #ifdef CN1_GC_VERIFY atomic_store_explicit(&cn1GcVerifyMarkActive, 1, memory_order_release); #endif @@ -3099,6 +3442,15 @@ void codenameOneGCMark() { gcMarkDrain(d); if(gcMarkNewObjectCount == before) break; // marked nothing new -> closed } + // REFERENCES, here and nowhere else. The strong mark has reached its fixpoint, so + // "unmarked" now means what the sweep will mean by it -- and the barrier is still + // armed, so a get() racing this pass logs its referent and forces the trial clear + // below to find a non-empty log, which re-arms and runs the whole fixpoint (and + // this pass) again. Moving it after the barrier goes down would remove exactly + // that protection and let the sweep free an object a mutator is holding. + if(cn1GcProcessReferences(d)) { + gcMarkDrain(d); + } // Trial clear. A store racing it either logged already (caught just below) or // adds an already-marked or fresh reference, which the sweep keeps either way. __atomic_store_n(&gcSatbActive, 0, __ATOMIC_SEQ_CST); @@ -12066,6 +12418,31 @@ void cn1GcProbeCycle(double markMs, double sweepMs, int threw) { atomic_load_explicit(&cn1MonitorEntries, memory_order_relaxed), cn1ImmortalRootsN, cn1FVLive, sideBytes / 1024, residKb); + // java.lang.ref, on its own line so it can be grepped and joined on cyc without + // parsing the wall of fields above. + // + // refMs against graceMs and drainMs is the answer to "does ranking cost more than it + // is worth" on the COLLECTOR side; hit rate against fpKb (which the workload prints) + // answers it on the mutator side. Neither means anything alone. + // + // softBudget is the age, in collections, a soft referent may reach untouched: it is + // CN1_REF_SOFT_RETAIN_MAX with memory to spare, 0 inside the reserve, -1 out of + // budget, and 0x7fffffff means the policy is not trimming at all. Read it FIRST when + // a hit rate looks wrong -- a budget pinned at 0x7fffffff on a host with no + // per-process limit is a measurement of nothing, and is the shape a desktop A/B + // silently takes. + fprintf(stderr, + "[GCREF] v=1 cyc=%d tMs=%lld discovered=%ld weak=%ld retained=%ld" + " keptTouched=%ld cleared=%ld passes=%ld refMs=%.3f softBudget=%d listCap=%ld\n", + currentGcMarkValue, cn1GcProbeElapsedMs(), + atomic_load_explicit(&cn1RefDiscoveries, memory_order_relaxed), + atomic_load_explicit(&cn1RefWeak, memory_order_relaxed), + atomic_load_explicit(&cn1RefRetained, memory_order_relaxed), + atomic_load_explicit(&cn1RefKeptTouched, memory_order_relaxed), + atomic_load_explicit(&cn1RefCleared, memory_order_relaxed), + cn1RefPasses, cn1RefPhaseNs / 1e6, + atomic_load_explicit(&cn1SoftRetainCycles, memory_order_relaxed), + cn1RefDiscoveredCap); fflush(stderr); // Per-CYCLE, so reset after reporting. A running total cannot show a trend. cn1GcProbeResetPhases(); diff --git a/vm/ByteCodeTranslator/src/com/codename1/tools/translator/ByteCodeClass.java b/vm/ByteCodeTranslator/src/com/codename1/tools/translator/ByteCodeClass.java index 9d973069d90..0e4a4178203 100644 --- a/vm/ByteCodeTranslator/src/com/codename1/tools/translator/ByteCodeClass.java +++ b/vm/ByteCodeTranslator/src/com/codename1/tools/translator/ByteCodeClass.java @@ -37,6 +37,43 @@ */ public class ByteCodeClass { + /** + * The one class whose reference field the collector owns rather than traces. + * + *

{@code java.lang.ref.Reference.objReference} is the referent of every + * WeakReference and SoftReference in the program. Emitting the ordinary + * {@code gcMarkObject} for it would make it a STRONG edge -- which is exactly + * what ParparVM did until this opt-out existed, so that a "weak" reference + * pinned its referent for the life of the process and every cache built on + * {@code Display.createSoftWeakRef} was unbounded. The two emission sites + * below replace that with a weak edge and the load barrier that makes reading + * one safe while a concurrent mark is running.

+ * + *

Matched by name rather than by any annotation because the class is part + * of the VM's own {@code java.lang} surface: there is nowhere to hang an + * annotation that {@code vm/JavaAPI} and {@code Ports/CLDC11} would both + * accept, and a marker interface would be one more thing to keep in step. + * {@code Reference} is final in practice -- its constructor is package + * private -- so the set of classes this can apply to is closed.

+ */ + static final String REFERENCE_CLASS = "java_lang_ref_Reference"; + + /** The referent field within {@link #REFERENCE_CLASS}. */ + static final String REFERENCE_REFERENT_FIELD = "objReference"; + + /** + * True for the one field whose GC treatment and read accessor are special + * cased below. Both call sites must agree, hence the shared predicate: + * suppressing the mark without adding the barrier produces a collector that + * frees a referent a mutator is holding, and adding the barrier without + * suppressing the mark produces a weak reference that is still strong. + */ + private static boolean isReferenceReferent(String owningClass, ByteCodeField fld) { + return REFERENCE_CLASS.equals(owningClass) + && REFERENCE_REFERENT_FIELD.equals(fld.getFieldName()) + && REFERENCE_CLASS.equals(fld.getClsName()); + } + /** * @param isAnonymous the isAnonymous to set */ @@ -1000,6 +1037,37 @@ public String generateCCode(List allClasses) { b.append("_"); b.append(fld.getFieldName()); b.append("(JAVA_OBJECT __cn1T) {\n ").append(nullCheck).append(" "); + if(isReferenceReferent(clsName, fld)) { + // Reference.get() compiles into this accessor, and reading a weak + // referent while a concurrent mark is running needs a barrier that an + // ordinary field read does not. + // + // The collector clears a reference only after the strong mark has + // reached its fixpoint, but it does so with the mutators still running + // and with the SATB barrier still armed. Without the enqueue below, a + // thread whose stack was scanned and released early could take the + // referent out of here, hold it in a local the collector has already + // walked past, and watch the same cycle's sweep free it -- the object + // is by then neither marked nor fresh, which is the one case the + // "already marked or FRESH" invariant the sweep relies on does not + // cover. Enqueuing makes the referent part of the snapshot, so the + // fixpoint loop marks it and the clear pass then sees it live and + // leaves the reference alone. + // + // CN1_SATB_REF_LOAD rather than the plain CN1_SATB_DELETE next door: a + // referent that is already marked this epoch, or fresh, is one the clear + // pass would refuse to clear, so logging it is pure cost -- and on a hot + // cache that cost is enough to stop the SATB termination loop converging. + // Off-mark both are one predicted-not-taken load of gcSatbActive. + b.append("CN1_SATB_REF_LOAD(&((struct obj__").append(clsName).append("*)__cn1T)->") + .append(fld.getClsName()).append("_").append(fld.getFieldName()).append(");\n "); + // The touch stamp, and the entire per-read cost of ranking soft + // references by use: a store of an immediate. Unconditional rather than + // guarded by a "did it change" test, because the branch would cost more + // than the store it saves. + b.append("((struct obj__").append(clsName).append("*)__cn1T)->") + .append(REFERENCE_CLASS).append("_cn1TouchAge = CN1_REF_TOUCHED;\n "); + } if (fld.isVolatile()) { b.append("return atomic_load_explicit(&((struct obj__"); b.append(clsName); @@ -1086,6 +1154,27 @@ public String generateCCode(List allClasses) { b.append("*)objToMark;\n"); for(ByteCodeField fld : fullFieldList) { if(!fld.isStaticField() && fld.isObjectType() && fld.getClsName().equals(clsName)) { + if(isReferenceReferent(clsName, fld)) { + // THE REFERENT IS NOT TRACED. Handing it to gcMarkObject here is + // what made every WeakReference strong; instead the collector is + // told the reference exists and is given the addresses it needs to + // decide, once the strong mark has closed, whether to keep the + // referent or clear the field. + // + // Addresses rather than the object, deliberately: cn1_globals.m is a + // fixed template compiled beside whatever the translator emitted, and + // it cannot name `struct obj__java_lang_ref_Reference` -- the class is + // absent from any program that never uses a reference, and including + // its generated header would make the runtime fail to build for those. + // Passing field pointers keeps the layout knowledge on this side, + // where it is generated from the layout itself. + b.append(" cn1GcDiscoverReference(threadStateData, objToMark, force, &objInstance->"); + b.append(fld.getClsName()).append("_").append(fld.getFieldName()); + b.append(", &objInstance->").append(REFERENCE_CLASS).append("_cn1TouchAge"); + b.append(", &objInstance->").append(REFERENCE_CLASS).append("_cn1AgedCycle"); + b.append(", objInstance->").append(REFERENCE_CLASS).append("_cn1Strength);\n"); + continue; + } b.append(" gcMarkObject(threadStateData, "); if (fld.isVolatile()) { b.append("atomic_load_explicit(&objInstance->"); diff --git a/vm/CLAUDE.md b/vm/CLAUDE.md index 175916114f0..caaeb6e40f6 100644 --- a/vm/CLAUDE.md +++ b/vm/CLAUDE.md @@ -560,6 +560,88 @@ 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. +## java.lang.ref: what it cost, and what the ranking did not buy + +The collector clears references itself. The referent lives in `java.lang.ref.Reference` +and the translator does NOT emit a `gcMarkObject` for it +(`ByteCodeClass.isReferenceReferent`): it emits `cn1GcDiscoverReference`, which hands the +collector the field addresses and decides soft retention on the spot. Clearing happens in +`cn1GcProcessReferences`, inside the SATB termination loop, using the sweep's own liveness +test -- `mark != -1 && mark < currentGcMarkValue - 1`, both halves of the sweep agree on +it. Clearing a reference the sweep keeps wastes a cache entry; failing to clear one it +frees is a dangling read, which on this VM is a native crash no Java catch can see. + +**The clear pass must run with the SATB barrier still ARMED.** A thread scanned and +released early can pull a referent out through `get()` and hold it in a local the +collector has already walked past, and that referent is then neither marked nor fresh -- +the one case the sweep's "already marked or FRESH" invariant does not cover. `get()` +therefore carries a load barrier, emitted into +`get_field_java_lang_ref_Reference_objReference`, and a racing read makes the trial clear +of `gcSatbActive` find a non-empty log, which re-arms and re-runs the fixpoint and this +pass with it. + +**Filter that barrier or the collector stops converging.** Logging every referent read is +not a cost, it is a failure: `cn1SatbEnqueue` takes a mutex per accepted reference and +`get()` on a hot cache is called far more often than any store barrier sees. Measured on +`RefPolicy` before the filter existed -- over 10,000 log entries per cycle and +`CN1_SATB_MAX_REOPENS` (32) reached on EVERY cycle. `CN1_SATB_REF_LOAD` skips referents +already marked this epoch or fresh, which are exactly the ones the clear pass would refuse +to clear: passes 32 -> 1, keptTouched ~10,000 -> ~330, refMs 0.06 -> 0.005. + +### The measurement, and the three ways it lied first + +`vm/benchmarks/src/com/bench/RefPolicy.java` + `ab-refs.sh`, arms `-DCN1_NO_WEAK_REFS` +(references strong, what this VM did before) and `-DCN1_REF_POLICY=0|1|2` +(pressure-triggered all-or-nothing / never clear / ranked by age). Five interleaved reps, +`CN1_SIMULATE_PROC_MEMORY_LIMIT`, checksums identical across every arm: + +| ceiling | arm | hit rate | footprint | refMs | % of mark | weak cleared | +|---|---|---|---|---|---|---| +| 128MB | noweak | 97.44% | 63.5MB | 0.002 | 0.00% | 0/256 | +| 128MB | pressure | 84.99% | 63.1MB | 2.760 | 2.46% | 255/256 | +| 128MB | never | 97.44% | 63.8MB | 1.289 | 1.88% | 255/256 | +| 128MB | ranked | 96.77% | 62.6MB | 1.197 | 1.79% | 255/256 | +| 160MB | pressure | 87.99% | 91.0MB | 2.611 | 16.32% | 255/256 | +| 160MB | ranked | 97.44% | 82.1MB | 0.421 | 3.63% | 255/256 | + +Read it in this order. **References themselves are unambiguous**: 255/256 unreachable +referents reclaimed against 0/256, for 1.8-4% of mark time and a `vm/benchmarks` geomean +of 1.011 over 12 interleaved reps against master. **The pressure-triggered arm is strictly +dominated** -- it gives up 12 points of hit rate and saves no footprint at all, and at +160MB it is worse on BOTH axes. That arm is the model of the iOS port's +`didReceiveMemoryWarning -> flushSoftRefMap`, so it is the thing being replaced, not a +strawman. **The ranking buys nothing over never-clearing here**: same hit rate, ~1MB less. +It is defensible because it costs almost nothing and because it dominates the pressure +arm, not because this measurement shows it winning. + +Three wrong conclusions were drawn from single runs before that table existed, and each +survived until the data contradicted it: + +- **"Ranking is the difference between finishing and not."** True of the outcome, wrong + about the cause: the arms that did not finish were not out of memory. `sample` on a + wedged process put the mutator 100% in `cn1PacingPark` at 44MB of a 96MB ceiling. The + chain is retain-everything -> the collector cannot shrink the live set -> the pacing loop + parks the mutator to hold the budget. Reach for the stacks first, as the demand-signal + note above already says. +- **"The pressure arm fails because all-or-nothing thrashes."** It never fired at all. + Its trigger was below the pacing reserve, and defending that reserve is what the pacing + loop DOES, so headroom converges on the trigger and stops falling. **Any + pressure-triggered cache policy on this collector has that trap waiting: it waits for a + signal the collector exists to suppress.** The second attempt then wrote the bands as + multiples of the reserve, where `reserve * 4` IS the whole budget, so the top band was + unreachable and the arm fired always. Write bands as explicit fractions; reachability is + then visible on inspection. +- **"The non-trimming arms collapse."** `never` was 4x FASTER than `noweak` at 2,000 + accesses and 100x slower at 6,000. Below roughly 1.8x the cache size this workload is + **bistable** -- once pacing engages, throughput drops two orders of magnitude, and + whether a run falls in is timing-sensitive. Single runs there measure the coin. If that + regime is what you want, count how many of N runs complete; do not time one. + +**What is still not measured.** Nothing here separates ranking by RECENCY from "trims at +all" -- there is no random-eviction arm at a matched rate, so the LRU claim is unproven, +only the trimming claim. And the 64MB-cache-against-a-128MB-budget shape is a choice made +to stress the policy, not a measured property of any app. + ## GC latency: the mutator's clock, not the collector's Everything above measures MEMORY. The reporter of #5537 ended up passing all of it and still diff --git a/vm/JavaAPI/src/java/lang/ref/Reference.java b/vm/JavaAPI/src/java/lang/ref/Reference.java index 33521cb931d..07bc9263b9d 100644 --- a/vm/JavaAPI/src/java/lang/ref/Reference.java +++ b/vm/JavaAPI/src/java/lang/ref/Reference.java @@ -25,26 +25,107 @@ /** * Abstract base class for reference objects. This class defines the operations common to all reference objects. Because reference objects are implemented in close cooperation with the garbage collector, this class may not be subclassed directly. * Since: JDK1.2, CLDC 1.1 + * + *

The four fields below are a contract with the collector, not ordinary + * state. Three separate places know their names literally, and renaming one + * without the others produces a build that compiles and silently stops + * collecting -- or, worse, one that clears a reference whose referent is still + * in use:

+ * + *
    + *
  • {@code ByteCodeClass} suppresses the usual {@code gcMarkObject} for + * {@code objReference} in {@code __GC_MARK_java_lang_ref_Reference} and + * emits a {@code cn1GcDiscoverReference} call in its place, handing the + * collector the addresses of these fields. That suppression is what makes + * the referent a weak edge instead of a strong one.
    • + *
  • The same class adds the SATB load barrier and the touch stamp to + * {@code get_field_java_lang_ref_Reference_objReference}, which is the + * accessor every {@code get()} below compiles into.
    • + *
  • {@code cn1GcProcessReferences} in {@code cn1_globals.m} reads and writes + * all four through those addresses.
    • + *
+ * + *

Consequently {@code objReference} must stay the ONLY object-typed field in + * this class: the translator's opt-out is keyed on the class and field name, and + * a second reference field would be traced strongly with nothing to say so.

*/ public abstract class Reference{ + /** + * The referent. Deliberately package private and declared HERE rather than in + * WeakReference, so that one translator opt-out and one collector pass cover + * every subclass. + */ + Object objReference; + + /** + * Cycles since the last {@link #get()}, maintained by the collector, and the + * "hot" input to the soft-reference retention policy. + * + *

{@code TOUCHED} (-1) is written by the field accessor on every read -- + * a store of an immediate, which is the whole per-get cost of ranking, and + * why the ranking is done this way rather than by reading a clock or an + * epoch counter. The collector converts a -1 back to 0 and increments + * everything else once per cycle, so the value is an age in collections.

+ * + *

It also carries the safety property that lets the collector clear a + * reference at all while mutators run: see the discussion of + * {@code cn1GcProcessReferences}.

+ */ + int cn1TouchAge = TOUCHED; + + /** + * {@link #STRENGTH_WEAK} or {@link #STRENGTH_SOFT}, set by the subclass + * constructor. + * + *

The collector needs to tell the two apart and deliberately does NOT do + * it by comparing class pointers: that would make the runtime depend on a + * generated class symbol that the dead-code pass is entitled to remove, and + * would answer wrongly for a user-written subclass of either.

+ */ + int cn1Strength; + + /** + * The mark value of the cycle that last aged this reference, so that a + * reference reached more than once in a cycle ages exactly once. + * + *

Being reached twice is normal rather than exceptional: force-marking + * re-runs mark functions over already-marked objects once per statics pass + * and again for the constant pool, so the collector sees popular references + * several times per cycle.

+ */ + int cn1AgedCycle; + + /** {@link #cn1TouchAge} value meaning "read since the collector last aged this". */ + static final int TOUCHED = -1; + + /** A {@link #cn1Strength} that is never retained once the referent is unreachable. */ + static final int STRENGTH_WEAK = 0; + + /** A {@link #cn1Strength} that is retained while recently used and memory allows. */ + static final int STRENGTH_SOFT = 1; + + Reference(Object ref, int strength) { + this.objReference = ref; + this.cn1Strength = strength; + } + /** * Clears this reference object. */ public void clear(){ - clearImpl(); + objReference = null; } /** * Returns this reference object's referent. If this reference object has been cleared, either by the program or by the garbage collector, then this method returns null. + * + *

This compiles to {@code get_field_java_lang_ref_Reference_objReference}, + * which the translator gives a SATB load barrier and the touch stamp. Both + * belong on the ACCESSOR rather than here: every read of the field goes + * through it, including any the optimizer generates, whereas a barrier + * written in Java would cover only the one call site below.

*/ public java.lang.Object get(){ - return getImpl(); - } - - Object getImpl() { - return null; - } - - void clearImpl() { + return objReference; } } diff --git a/vm/JavaAPI/src/java/lang/ref/SoftReference.java b/vm/JavaAPI/src/java/lang/ref/SoftReference.java new file mode 100644 index 00000000000..6bf8080d369 --- /dev/null +++ b/vm/JavaAPI/src/java/lang/ref/SoftReference.java @@ -0,0 +1,58 @@ +/* + * 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 java.lang.ref; +/** + * A reference the collector keeps while the referent is being used and memory + * allows, and clears before the process runs out. + * + *

This is the reference a CACHE wants, and the difference from + * {@link WeakReference} is not a detail: a weak referent is dropped by the first + * collection after the last ordinary reference to it, which for a decoded bitmap + * behind an {@code EncodedImage} means the cache is emptied faster than it can be + * filled and never hits. A soft reference survives that collection, and the one + * after it, for as long as something is still asking for it.

+ * + *

Retention is ranked by use. The collector ages every reference by one + * on each cycle and resets the age to zero when {@code get()} is called, then + * keeps a soft referent while that age is within a budget it recomputes each + * cycle from the memory still available to the process. So a cache under memory + * pressure loses its cold entries first rather than all of them at once, and an + * entry read on every frame is the last thing to go.

+ * + *

The whole per-read cost of that ranking is a store of a constant into the + * reference, which is why it is ranked by age rather than by reading a clock. + * The decision itself is made once per cycle, when the collector first reaches + * the reference, so ranking costs the mark nothing beyond the walk it was + * already doing -- deciding it afterwards would mean computing a second + * reachability closure over the retained set, on a mark that already spends most + * of its time in the grace pass.

+ */ +public class SoftReference extends java.lang.ref.Reference{ + /** + * Creates a new soft reference that refers to the given object. + */ + public SoftReference(java.lang.Object ref){ + super(ref, STRENGTH_SOFT); + } +} diff --git a/vm/JavaAPI/src/java/lang/ref/WeakReference.java b/vm/JavaAPI/src/java/lang/ref/WeakReference.java index dbaef8f6b68..a1af9be37e1 100644 --- a/vm/JavaAPI/src/java/lang/ref/WeakReference.java +++ b/vm/JavaAPI/src/java/lang/ref/WeakReference.java @@ -25,36 +25,37 @@ /** * This class provides support for weak references. Weak references are most often used to implement canonicalizing mappings. Suppose that the garbage collector determines at a certain point in time that an object is weakly reachable. At that time it will atomically clear all the weak references to that object and all weak references to any other weakly- reachable objects from which that object is reachable through a chain of strong and weak references. * Since: JDK1.2, CLDC 1.1 + * + *

The referent lives in {@link Reference}, and the collector clears it once + * the object is reachable no other way. Two properties of THIS collector make + * that best-effort rather than prompt, and both are legal -- the contract says a + * reference "may" be cleared, never that it must be:

+ * + *
    + *
  • A newly allocated object is kept unconditionally for one cycle by the + * sweep's grace rule, so a referent is never cleared in the cycle it dies.
    • + *
  • The root scan reads native C stacks conservatively, so a stale machine + * word that happens to look like the referent keeps it marked. {@code get()} + * will occasionally keep answering an object nothing references any more.
    • + *
+ * + *

Historical note worth keeping, because both halves were real shipped bugs. + * This class first held its referent in an ordinary field, which the translator + * traced like any other -- so a "weak" reference was strong and the caches built + * on {@code CodenameOneImplementation.createSoftWeakRef} pinned every decoded + * bitmap for the life of the process. Before that, the constructor assigned the + * field to itself ({@code this.objReference = objReference}) and dropped the + * argument, so every reference was born empty and {@code get()} was hardwired to + * null: the same caches could then never hit. Note the failure modes are exact + * opposites, which is why this class needs tests that pin BOTH ends -- that the + * referent is answered while it is reachable, and that it stops being answered + * once it is not.

*/ public class WeakReference extends java.lang.ref.Reference{ - private Object objReference; - /** * Creates a new weak reference that refers to the given object. - *

- * Note that ParparVM's collector has no notion of a weak root: it never - * clears this field, so the referent lives exactly as long as the - * reference object does and {@link #get()} keeps answering it until - * {@link Reference#clear()} is called by hand. That is a legal (if - * pessimistic) implementation of the contract -- "may be cleared" is not - * "must be cleared" -- and it is what the callers need. What is NOT legal - * is the reverse: this constructor used to assign the field to itself - * ({@code this.objReference = objReference}) and drop {@code ref} on the - * floor, so every reference was born empty and {@code get()} was hardwired - * to null. Everything built on - * {@code CodenameOneImplementation.createSoftWeakRef} -- the EncodedImage - * decode cache, Image's scale cache, Border's round-rect cache -- was then - * a cache that could never hit. */ public WeakReference(java.lang.Object ref){ - this.objReference = ref; - } - - Object getImpl() { - return objReference; - } - - void clearImpl() { - objReference = null; + super(ref, STRENGTH_WEAK); } } diff --git a/vm/benchmarks/ab-refs.sh b/vm/benchmarks/ab-refs.sh new file mode 100755 index 00000000000..b2922112daa --- /dev/null +++ b/vm/benchmarks/ab-refs.sh @@ -0,0 +1,139 @@ +#!/bin/bash +# A/B the java.lang.ref retention policies (CN1_REF_POLICY) against the behaviour +# they replaced. +# +# noweak -DCN1_NO_WEAK_REFS references are traced strongly and never cleared. +# This is what ParparVM did before references existed, +# and what the iOS port's soft-reference table still +# does between memory warnings. +# pressure -DCN1_REF_POLICY=0 keep every soft referent until headroom drops into +# the reserve, then drop all of them at once -- the +# GC-integrated form of didReceiveMemoryWarning -> +# flushSoftRefMap. +# never -DCN1_REF_POLICY=1 never clear a soft referent. The upper bound on hit +# rate and on footprint. +# ranked -DCN1_REF_POLICY=2 clear by age since the last get(). The default. +# +# THE POINT OF ALL FOUR IS THAT NEITHER AXIS MEANS ANYTHING ALONE. "never" wins the +# hit rate by keeping everything and "pressure" wins the footprint by keeping nothing; +# ranking is worth its one int field and one store per get() only if it holds a higher +# hit rate than "pressure" AT A COMPARABLE FOOTPRINT. The table below prints both for +# every arm at every ceiling so that comparison cannot be made one column at a time. +# +# ./ab-refs.sh [reps] [ceilingMB ...] +# +# REF_WORKLOAD passes argv to the driver ("keys payloadBytes accesses churn"), and +# REF_TIMEOUT bounds a single run. +# +# CHOOSE CEILINGS WHERE EVERY ARM COMPLETES RELIABLY. Below roughly 1.8x the cache +# size this workload becomes BISTABLE: once the retained set stops the collector +# freeing enough, the process-budget pacing loop parks the mutator and throughput +# collapses by two orders of magnitude, and whether a given run falls into that state +# is timing-sensitive. Measured on this host, the same arm took 936ms and 391s on +# neighbouring workload sizes, and the arms ordered differently each time. Single runs +# in that regime measure the coin, not the policy -- if the tight regime is what you +# want to characterise, count how many of N runs complete rather than timing one. +# +# Requirements: JDK_8_HOME, Maven, clang. +set -e +cd "$(dirname "$0")" +REPS="${1:-5}"; shift || true +CEILINGS="${*:-96 128 192}" +LTO="${CN1_BENCH_LTO--flto=thin}" +mkdir -p target/ab-refs + +# CN1_GC_CONFORM is in every arm. It changes no allocator behaviour -- unlike +# CN1_GC_VERIFY, which forces cn1BibopReleaseOffset() to 0 and compiles out page +# release and the major sweep, so a footprint measured in a verifier build is a +# measurement of the verifier. It is what supplies [GCREF], and it is present in ALL +# arms so the arms differ in one thing only. +build() { # name, flags + CN1_BENCH_CFLAGS="$LTO -DCN1_GC_CONFORM $2" ./translate-and-build.sh RefPolicy \ + "target/ab-refs/$1" >"target/ab-refs/$1.build.log" 2>&1 \ + || { echo "BUILD FAILED: $1"; tail -25 "target/ab-refs/$1.build.log"; exit 1; } + echo "built $1" +} +build noweak "-DCN1_NO_WEAK_REFS" +build pressure "-DCN1_REF_POLICY=0" +build never "-DCN1_REF_POLICY=1" +build ranked "-DCN1_REF_POLICY=2" + +REPS="$REPS" CEILINGS="$CEILINGS" python3 - <<'EOF' +import subprocess, re, os, sys, statistics + +ARMS = ["noweak", "pressure", "never", "ranked"] +reps = int(os.environ["REPS"]) +ceilings = [int(c) for c in os.environ["CEILINGS"].split()] + +def run(arm, ceiling_mb): + env = dict(os.environ) + # Scrub every CN1_* the caller may have exported. An inherited CN1_GC_PROBE or + # CN1_SIMULATE_PROC_MEMORY_LIMIT would silently make two arms incomparable, which + # reads as a policy difference rather than as a mistake. + for k in [k for k in env if k.startswith("CN1_")]: + del env[k] + env["CN1_SIMULATE_PROC_MEMORY_LIMIT"] = str(ceiling_mb * 1024 * 1024) + env["CN1_GC_PROBE"] = "1" + p = subprocess.run([f"target/ab-refs/{arm}"] + os.environ.get("REF_WORKLOAD", "").split(), + capture_output=True, text=True, env=env, + timeout=float(os.environ.get("REF_TIMEOUT", "600"))) + out = p.stdout + def num(key, default=None): + m = re.search(rf'^{key}=(-?\d+)', out, re.M) + if m: return int(m.group(1)) + if default is not None: return default + raise SystemExit(f"{arm}@{ceiling_mb}MB: no {key} in output\n{out}\n{p.stderr[-2000:]}") + refms, cleared, retained = [], 0, 0 + for m in re.finditer(r'\[GCREF\].*?cleared=(\d+).*?refMs=([\d.]+)', p.stderr): + cleared += int(m.group(1)); refms.append(float(m.group(2))) + for m in re.finditer(r'\[GCREF\].*?retained=(\d+)', p.stderr): + retained += int(m.group(1)) + markms = [float(m.group(1)) for m in re.finditer(r'markMs=([\d.]+)', p.stderr)] + return { + "hit_ppm": num("HIT_RATE_PPM"), "fp_kb": num("FINAL_FOOTPRINT_KB"), + "checksum": num("RESULT"), + "weak_cleared": out.split("WEAK_DEAD_CLEARED=")[1].split("\n")[0] if "WEAK_DEAD_CLEARED=" in out else "?", + "refms": sum(refms), "markms": sum(markms), "cleared": cleared, "retained": retained, + } + +results = {(a, c): [] for a in ARMS for c in ceilings} +for rep in range(reps): + # INTERLEAVED. Physical footprint moves with the host's own memory pressure, so two + # soaks taken minutes apart measure the machine; every arm has to see the same + # machine state, which only holds if they alternate inside one session. + for c in ceilings: + for a in ARMS: + results[(a, c)].append(run(a, c)) + print(f"rep {rep+1}/{reps}", flush=True) + +# Checksum parity. A retention policy decides WHEN a payload is rebuilt, never what it +# contains, and the driver accumulates what it read rather than what it rebuilt -- so a +# checksum that moves across arms is a correctness bug, not a policy difference. +sums = {(a, c): {r["checksum"] for r in rs} for (a, c), rs in results.items()} +allsums = set().union(*sums.values()) +if len(allsums) != 1: + print("\nCHECKSUM MISMATCH across arms (correctness bug):") + for k, v in sorted(sums.items()): + print(f" {k}: {sorted(v)}") + sys.exit(1) + +med = lambda vals: statistics.median(vals) +print(f"\nchecksum {allsums.pop()} identical across every arm and ceiling" + f" ({reps} interleaved reps, medians below)") +for c in ceilings: + print(f"\n--- process ceiling {c} MB " + "-" * 46) + # refMs and markMs are TOTALS over the run's cycles, so their ratio is the share + # of collector time the reference phase costs -- which is the question. The + # absolute ms is kept beside it only so a suspiciously round ratio can be checked. + print(f"{'arm':<10}{'hit rate %':>12}{'footprint MB':>14}{'refMs total':>13}" + f"{'% of mark':>11}{'softCleared':>12}{'weak':>10}") + for a in ARMS: + rs = results[(a, c)] + hit = med([r["hit_ppm"] for r in rs]) / 10000.0 + fp = med([r["fp_kb"] for r in rs]) / 1024.0 + ref = med([r["refms"] for r in rs]) + mark = med([r["markms"] for r in rs]) + share = (100.0 * ref / mark) if mark > 0 else 0.0 + print(f"{a:<10}{hit:>11.2f}%{fp:>14.1f}{ref:>13.3f}{share:>10.2f}%" + f"{med([r['cleared'] for r in rs]):>12.0f}{rs[0]['weak_cleared']:>10}") +EOF diff --git a/vm/benchmarks/src/com/bench/RefPolicy.java b/vm/benchmarks/src/com/bench/RefPolicy.java new file mode 100644 index 00000000000..8d0b1167a92 --- /dev/null +++ b/vm/benchmarks/src/com/bench/RefPolicy.java @@ -0,0 +1,277 @@ +/* + * Copyright (c) 2012, Codename One and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. Codename One designates this + * particular file as subject to the "Classpath" exception as provided + * by Oracle in the LICENSE file that accompanied this code. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Codename One through http://www.codenameone.com/ if you + * need additional information or have any questions. + */ +package com.bench; + +import java.lang.ref.Reference; +import java.lang.ref.SoftReference; +import java.lang.ref.WeakReference; + +/** + * The driver behind ParparVM's java.lang.ref support: one half asserts that + * references behave, the other half is the workload the retention policy is + * chosen from. + * + *

Why both halves are here

+ * + *

They fail in opposite directions, and a driver that only had one of them + * would pass while the VM was badly wrong. A reference that is never cleared is + * the bug this feature replaced -- ParparVM traced the referent like any other + * field, so every cache built on {@code Display.createSoftWeakRef} pinned its + * contents for the life of the process. A reference cleared too eagerly is the + * bug BEFORE that one -- the constructor dropped its argument, so {@code get()} + * always answered null and the same caches could never hit. Phase A pins the + * first end, phase B the second: a cache with a zero hit rate is a cache that is + * being emptied faster than it is filled.

+ * + *

Reading the output

+ * + * 
+ * WEAK_LIVE_KEPT=n/n        phase A: referents still strongly held; must be all
+ * WEAK_DEAD_CLEARED=n/m     phase A: unreachable referents the collector cleared
+ * HITS / MISSES / HIT_RATE  phase B: what the cache actually bought
+ * FINAL_FOOTPRINT_KB        phase B: what it cost
+ * CHECKSUM                  policy invariant -- see below
+ *
+ * + *

HIT_RATE and FINAL_FOOTPRINT_KB are the pair the policy is chosen on, and + * NEITHER MEANS ANYTHING ALONE. A policy that never clears wins the hit rate by + * keeping everything, and a policy that clears everything wins the footprint by + * keeping nothing; the question ranking has to answer is whether it holds a + * higher hit rate than the clear-everything arm at the same footprint.

+ * + *

CHECKSUM is deliberately independent of the policy: a rebuilt payload is + * byte-for-byte what it replaced, and the checksum accumulates what was READ + * rather than what was rebuilt. So it is a parity check across arms (and against + * a host JVM) that a policy change cannot legitimately move, while the hit rate + * is free to move as much as it likes.

+ * + *

The trap this driver exists inside

+ * + *

ParparVM scans native C stacks conservatively, so a machine word left behind + * by a returned frame keeps whatever it points at marked. A driver that drops + * references and then sleeps pins them with its own dead frames and reports every + * referent as retained -- the measurement comes back green having measured + * nothing. {@link #scrub(int)} overwrites that region on purpose before any + * assertion about collection; every phase-A step goes through + * {@link #quiesce()}.

+ */ +public class RefPolicy { + + // Phase B's shape, from argv: keys, payload bytes, accesses, churn per access. + // + // SIZE THE CACHE AGAINST THE BUDGET, not against convenience. The defaults are 2048 + // keys of 32KB, so a fully retained cache is 64MB -- most of the ceiling the A/B runs + // under (CN1_SIMULATE_PROC_MEMORY_LIMIT). That is the only regime in which the + // retention policy is the variable: an earlier 512x4KB cache was 2MB against a 96MB + // ceiling, every policy retained all of it, and the run reported a 99.9% hit rate for + // all three arms while measuring nothing but the collector's reaction to the ceiling. + static int keys = 2048; + /** Payload bytes. Well over CN1_BIBOP_MAX_OBJECT (512), so these take the legacy path. */ + static int payload = 32768; + static int accesses = 400000; + /** Objects allocated per access, to drive the collector rather than wait for it. */ + static int churnPerAccess = 24; + /** Referents phase A drops. */ + static final int WEAK_SAMPLES = 256; + + static final int SCRUB_DEPTH = 400; + static long scrubSink; + + /** Cache of SoftReference tokens; index is the key. */ + static Object[] cache; + /** Somewhere for churn to land so the optimizer cannot delete it. */ + static Object[] churnSink = new Object[8]; + + static long hits; + static long misses; + static long checksum; + + public static void main(String[] args) throws Exception { + if (args != null) { + if (args.length > 0) { keys = Integer.parseInt(args[0]); } + if (args.length > 1) { payload = Integer.parseInt(args[1]); } + if (args.length > 2) { accesses = Integer.parseInt(args[2]); } + if (args.length > 3) { churnPerAccess = Integer.parseInt(args[3]); } + } + cache = new Object[keys]; + System.out.println("CONFIG keys=" + keys + " payloadBytes=" + payload + + " accesses=" + accesses + " churnPerAccess=" + churnPerAccess + + " cacheBytes=" + ((long) keys * payload)); + weakPhase(); + cachePhase(); + System.out.println("RESULT=" + checksum); + } + + // ---------------------------------------------------------------- phase A + + /** + * Both ends of the weak-reference contract, in one pass over one array. + * + *

Half the referents stay strongly reachable through {@code live} and must + * still be answered; the other half are dropped and must eventually stop + * being answered. Asserting only the second half would pass on a VM that + * cleared every reference unconditionally.

+ * + *

"Eventually" is two collections, not one, and that is not slop: the + * sweep's grace rule keeps anything allocated since the last sweep, so a + * referent is never cleared in the cycle it dies.

+ */ + private static void weakPhase() throws Exception { + Object[] live = new Object[WEAK_SAMPLES]; + Object[] refsLive = new Object[WEAK_SAMPLES]; + Object[] refsDead = new Object[WEAK_SAMPLES]; + + for (int i = 0; i < WEAK_SAMPLES; i++) { + byte[] kept = build(i); + live[i] = kept; + refsLive[i] = new WeakReference(kept); + + byte[] doomed = build(i + WEAK_SAMPLES); + refsDead[i] = new WeakReference(doomed); + // The only strong path to `doomed` ends here. Nothing else in this frame + // may keep it: no array slot, no local that outlives the iteration. + } + + quiesce(); + quiesce(); + + int liveKept = 0; + for (int i = 0; i < WEAK_SAMPLES; i++) { + if (((Reference) refsLive[i]).get() != null) { + liveKept++; + } + } + int deadCleared = 0; + for (int i = 0; i < WEAK_SAMPLES; i++) { + if (((Reference) refsDead[i]).get() == null) { + deadCleared++; + } + } + // `live` is read after the counting loop so it cannot be optimized away + // before it, which would turn the retention half into a tautology. + for (int i = 0; i < WEAK_SAMPLES; i++) { + checksum += ((byte[]) live[i])[0]; + } + + System.out.println("WEAK_LIVE_KEPT=" + liveKept + "/" + WEAK_SAMPLES); + System.out.println("WEAK_DEAD_CLEARED=" + deadCleared + "/" + WEAK_SAMPLES); + } + + // ---------------------------------------------------------------- phase B + + /** + * A cache of rebuildable payloads under a skewed access distribution, which + * is the shape every real caller of this feature has: a decoded bitmap behind + * an EncodedImage, the int[] behind Image.getRGB, a rasterized gradient. + * + *

Skewed rather than uniform on purpose. Under a uniform distribution + * there is no such thing as a cold entry, so every retention policy that + * keeps the same NUMBER of entries scores the same and ranking cannot show a + * difference even if it has one. The skew is what makes "which entries" a + * question with an answer.

+ */ + private static void cachePhase() { + long seed = 0x2545F4914F6CDD1DL; + for (int i = 0; i < accesses; i++) { + seed = seed * 6364136223846793005L + 1442695040888963407L; + int r = (int) ((seed >>> 33) % keys); + // Squaring a uniform draw concentrates it near zero: a few hot keys, a + // long cold tail. + int key = (int) (((long) r * r) / keys); + + byte[] buf = null; + Object token = cache[key]; + if (token != null) { + buf = (byte[]) ((Reference) token).get(); + } + if (buf == null) { + misses++; + buf = build(key); + cache[key] = new SoftReference(buf); + } else { + hits++; + } + // Accumulate what was READ. A rebuild reproduces the same bytes, so this + // is identical across policies while the hit rate is not. + checksum += buf[0] + buf[payload - 1]; + + for (int c = 0; c < churnPerAccess; c++) { + churnSink[c & 7] = new byte[64]; + } + churnSink[0] = null; + } + + long total = hits + misses; + System.out.println("HITS=" + hits); + System.out.println("MISSES=" + misses); + System.out.println("HIT_RATE_PPM=" + (total == 0 ? 0 : (hits * 1000000L) / total)); + System.out.println("FINAL_FOOTPRINT_KB=" + footprintKb()); + } + + // ---------------------------------------------------------------- helpers + + /** Deterministic content, so a rebuild is byte-for-byte what it replaced. */ + private static byte[] build(int key) { + byte[] b = new byte[payload]; + b[0] = (byte) key; + b[payload - 1] = (byte) (key * 31); + return b; + } + + /** One full collection with the driver's own stack overwritten first. */ + private static void quiesce() throws Exception { + scrub(SCRUB_DEPTH); + System.gc(); + Thread.sleep(250); + } + + /** + * Overwrites the native C stack the loop above ran on. See the class comment: + * without this the conservative root scan keeps the dropped referents marked + * and phase A reports zero collections while claiming success. + */ + private static long scrub(int depth) { + long a = depth * 0x5DEECE66DL; + long b = a ^ 0x1234567890ABCDEFL; + long c = b + 0x0F0F0F0F0F0F0F0FL; + long d = c ^ 0x7FFFFFFFFFFFFFFFL; + if (depth > 0) { + a += scrub(depth - 1); + } + scrubSink = a ^ b ^ c ^ d; + return scrubSink; + } + + /** + * Physical footprint, read in process. On Apple platforms totalMemory() is + * physical RAM and freeMemory() is RAM minus phys_footprint, which is the + * number Apple's own limits are enforced against -- and the reason not to use + * RSS, which counts shared clean pages and moves with whatever else the + * machine is doing. + */ + private static long footprintKb() { + Runtime r = Runtime.getRuntime(); + return (r.totalMemory() - r.freeMemory()) / 1024; + } +} diff --git a/vm/tests/src/test/java/com/codename1/tools/translator/JavascriptRuntimeSemanticsTest.java b/vm/tests/src/test/java/com/codename1/tools/translator/JavascriptRuntimeSemanticsTest.java index fe097c97754..368591c0dec 100644 --- a/vm/tests/src/test/java/com/codename1/tools/translator/JavascriptRuntimeSemanticsTest.java +++ b/vm/tests/src/test/java/com/codename1/tools/translator/JavascriptRuntimeSemanticsTest.java @@ -271,6 +271,12 @@ void weakReferenceHoldsTheReferentItWasConstructedWith(CompilerHelper.CompilerCo // // This runs the real vm/JavaAPI class through the translator and the // worker runtime, which is the only place the bug was observable. + // + // Every referent in the fixture stays strongly reachable, so this pins the + // reference's own behaviour and is unaffected by the collector now clearing + // referents. The opposite end -- that an unreachable referent stops being + // answered -- is RefPolicy in vm/benchmarks, which needs a driver that scrubs + // its native stack and therefore cannot live here. WorkerRunResult result = translateAndRunFixture(config, "JsWeakReferenceApp.java", "JsWeakReferenceApp"); assertEquals(511, result.result, diff --git a/vm/tests/src/test/resources/com/codename1/tools/translator/JsWeakReferenceApp.java b/vm/tests/src/test/resources/com/codename1/tools/translator/JsWeakReferenceApp.java index 37bf34aab01..f58b661b2c2 100644 --- a/vm/tests/src/test/resources/com/codename1/tools/translator/JsWeakReferenceApp.java +++ b/vm/tests/src/test/resources/com/codename1/tools/translator/JsWeakReferenceApp.java @@ -33,10 +33,18 @@ * built on CodenameOneImplementation.createSoftWeakRef -- the EncodedImage * decode cache above all -- into a cache that can never hit. * - * The collector has no weak roots, so a reference here holds its referent until - * it is cleared by hand. That is the pessimistic half of the contract and is - * what these assertions pin down; what they exist to catch is the other half - * going missing again. + * Every referent below is strongly reachable through a local for as long as it + * is asserted on, so these assertions are about the REFERENCE, not about the + * collector: a reference must answer the object it was handed, distinguish its + * referent from another reference's, and empty only when cleared. They hold + * whether or not weak roots are implemented, which is the point -- the + * collector clearing referents is covered by RefPolicy in vm/benchmarks, where + * the referent is deliberately dropped and the driver scrubs its own native + * stack first. + * + * Note this fixture runs on the JavaScript backend, whose gcMarkSweep is a + * no-op: the host JS GC collects, and ParparVM's reference clearing (which + * lives in the C collector) does not run here at all. */ public class JsWeakReferenceApp { static int result; @@ -57,7 +65,7 @@ public static void main(String[] args) throws Exception { mask |= 2; } - // clear() is the only thing that empties a reference on this VM. + // clear() empties a reference by hand, independently of the collector. ref.clear(); if (ref.get() == null) { mask |= 4; From 8a28f0eb5014504e9d416f2076bf7be59ecb2356 Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Mon, 7 Sep 2026 13:13:51 +0300 Subject: [PATCH 02/30] Make reference field access atomic, and decide aliases together Both from review of the previous commit, and both premises hold. ATOMICITY. The collector cleared the referent with a plain store while the generated accessor read it with a plain load, on threads that run concurrently by design -- a data race, undefined in C however benign the emitted instruction is on the targets built here. It was also an inconsistency within the same function: the line below the clear already read the MARK WORD with an acquire atomic, because cn1_globals.m converted its hand-written stores for exactly this reason. Relaxed atomics now at every site that touches the referent: the generated getter and setter (so Reference.clear() and the constructor are covered), CN1_SATB_REF_LOAD, and the three collector paths. cn1TouchAge gets the same treatment -- the mutator stamps it from the accessor while the collector reads and ages it, which is the identical defect one field over. ALIASES. Two references to one referent were decided at different instants, so a get() landing mid-pass could stamp the second as recently read after the first had been cleared -- one alias answering null while another answers the object. The contract says all references to a weakly reachable object are cleared atomically. For a cache a split is a spurious miss; for the callers that use a reference as a LIFETIME ORACLE, reading a null get() as proof the referent died, it is a false death report on one alias while the object is alive through another. That is the failure mode that makes the iOS soft-reference table dangerous today. The clear pass is now two sub-passes: A marks every referent read this cycle and drains, B then clears on the referent's mark word alone. Liveness is a property of the referent, so every alias reads the same answer and they are cleared together or kept together. This removes the possibility rather than narrowing the window -- a get() racing sub-pass B still gets a non-null referent and still enqueues it, so the object survives and every alias is cleared, which is a legal spurious clear and is what "atomically" asks for. RefPolicy grows an alias phase with a concurrent reader. Read the comment on it before citing it: it is NOT a self-test. Built with the new -DCN1_REF_NO_ALIAS_ATOMICITY arm, which restores the single-loop form that has the bug, it still reports ALIAS_SPLIT=0/256 -- the window is microseconds and could not be opened from a driver. Two earlier versions of that phase were worse and are documented so they are not rebuilt: one read the aliases only after quiescing, where every alias carries the same stamp and nothing can fail; the other kept every referent marked, so ALIAS_CLEARED_GROUPS was 0/256 and the thing being tested never happened. The fix ships because the two-sub-pass form is unconditionally correct and simpler, not because a test proved the old one broken -- the same footing as CN1_NO_BULK_INSERTION_BARRIER. Gates: run-gc-verify.sh green (RefPolicy clean over 71 verify passes, both fault-injection self-tests firing), run-gauntlet.sh green with all eleven tortures bit-identical and both GC stress modes. Co-Authored-By: Claude Opus 5 (1M context) --- vm/ByteCodeTranslator/src/cn1_globals.h | 2 +- vm/ByteCodeTranslator/src/cn1_globals.m | 120 +++++++++++++++--- .../tools/translator/ByteCodeClass.java | 27 +++- vm/benchmarks/src/com/bench/RefPolicy.java | 111 ++++++++++++++++ 4 files changed, 237 insertions(+), 23 deletions(-) diff --git a/vm/ByteCodeTranslator/src/cn1_globals.h b/vm/ByteCodeTranslator/src/cn1_globals.h index b9613e41a3a..29391c862bb 100644 --- a/vm/ByteCodeTranslator/src/cn1_globals.h +++ b/vm/ByteCodeTranslator/src/cn1_globals.h @@ -3130,7 +3130,7 @@ extern void cn1GcDiscoverReference(CODENAME_ONE_THREAD_STATE, JAVA_OBJECT ref, J #else #define CN1_SATB_REF_LOAD(fieldAddr) \ do { if(__builtin_expect(gcSatbActive, 0)) { \ - JAVA_OBJECT cn1__r = *(JAVA_OBJECT volatile*)(fieldAddr); \ + JAVA_OBJECT cn1__r = __atomic_load_n((JAVA_OBJECT*)(fieldAddr), __ATOMIC_RELAXED); \ if(cn1__r != JAVA_NULL && !CN1_IS_TAGGED(cn1__r)) { \ int cn1__m = __atomic_load_n(&cn1__r->__codenameOneGcMark, __ATOMIC_RELAXED); \ int cn1__e = atomic_load_explicit(&bibopGcEpoch, memory_order_relaxed); \ diff --git a/vm/ByteCodeTranslator/src/cn1_globals.m b/vm/ByteCodeTranslator/src/cn1_globals.m index 3834b802934..6cc706d4a8d 100644 --- a/vm/ByteCodeTranslator/src/cn1_globals.m +++ b/vm/ByteCodeTranslator/src/cn1_globals.m @@ -2351,7 +2351,13 @@ static void cn1RefBeginCycle(void) { void cn1GcDiscoverReference(CODENAME_ONE_THREAD_STATE, JAVA_OBJECT ref, JAVA_BOOLEAN force, JAVA_OBJECT* referentField, JAVA_INT* touchAgeField, JAVA_INT* agedCycleField, JAVA_INT strength) { - if(referentField == 0 || *referentField == JAVA_NULL) { + // RELAXED ATOMIC on referentField and touchAgeField everywhere they are touched. + // Both are written by the generated accessors on mutator threads while this runs on + // the collector, which is the concurrency this design exists to support -- so a plain + // access on either side is a data race and undefined in C, however benign the emitted + // instruction is on the targets built here. Same treatment the mark word already gets. + if(referentField == 0 + || __atomic_load_n(referentField, __ATOMIC_RELAXED) == JAVA_NULL) { return; // already cleared: nothing to decide } #ifdef CN1_NO_WEAK_REFS @@ -2359,7 +2365,7 @@ void cn1GcDiscoverReference(CODENAME_ONE_THREAD_STATE, JAVA_OBJECT ref, JAVA_BOO // exactly what this VM did before references were implemented. It exists so the // gate can prove it is not inert -- a test that asserts referents get collected // must FAIL when built this way, or it is asserting something else. - gcMarkObject(threadStateData, *referentField, force); + gcMarkObject(threadStateData, __atomic_load_n(referentField, __ATOMIC_RELAXED), force); (void)touchAgeField; (void)agedCycleField; (void)strength; (void)ref; return; #else @@ -2380,10 +2386,10 @@ void cn1GcDiscoverReference(CODENAME_ONE_THREAD_STATE, JAVA_OBJECT ref, JAVA_BOO // reference, so the age resets; anything else is one collection older. Saturating, // because an age that wraps to negative would read as freshly touched and make a // cold entry immortal. - JAVA_INT age = *touchAgeField; + JAVA_INT age = __atomic_load_n(touchAgeField, __ATOMIC_RELAXED); JAVA_INT aged = (age == CN1_REF_TOUCHED) ? 0 : (age < 0x7ffffffe ? age + 1 : age); - *touchAgeField = aged; + __atomic_store_n(touchAgeField, aged, __ATOMIC_RELAXED); int budget = atomic_load_explicit(&cn1SoftRetainCycles, memory_order_relaxed); retain = (strength == CN1_REF_SOFT && budget >= 0 && aged <= budget) ? JAVA_TRUE : JAVA_FALSE; @@ -2434,14 +2440,14 @@ void cn1GcDiscoverReference(CODENAME_ONE_THREAD_STATE, JAVA_OBJECT ref, JAVA_BOO // A retained soft reference is an ordinary strong edge for the rest of this // cycle, traced with whatever `force` the caller had -- a reference held by a // static is force-marked like anything else it points at. - gcMarkObject(threadStateData, *referentField, force); + gcMarkObject(threadStateData, __atomic_load_n(referentField, __ATOMIC_RELAXED), force); } #endif } // Clear every discovered reference whose referent this cycle's sweep is about to free. // Runs on the GC thread only, inside the SATB termination loop, barrier armed. -// Returns JAVA_TRUE if it marked anything, so the caller knows to drain. +// Returns JAVA_TRUE if it marked anything; it has already drained by then. static JAVA_BOOLEAN cn1GcProcessReferences(CODENAME_ONE_THREAD_STATE) { JAVA_BOOLEAN marked = JAVA_FALSE; #ifdef CN1_GC_CONFORM @@ -2453,23 +2459,103 @@ static JAVA_BOOLEAN cn1GcProcessReferences(CODENAME_ONE_THREAD_STATE) { // re-run after a SATB reopen walks the list again from the start, which is what picks // up anything discovered by the re-opened fixpoint. long n = cn1RefDiscoveredTop; + + // TWO SUB-PASSES, AND THE SPLIT IS THE POINT. + // + // Two reachable references can share one referent, and the contract says all + // references to a weakly reachable object are cleared ATOMICALLY -- not one at a + // time. A single loop that re-read the touch stamp per ENTRY could not honour that: + // clear alias A, then a mutator's get() on alias B stamps it TOUCHED before the loop + // reaches B, and B is kept. One alias reads null and the other answers the object. + // + // For a cache that is only a spurious miss. For the callers that use a reference as a + // LIFETIME ORACLE -- read a null get() as proof the referent died, and then release + // something on that basis -- it is a false death report on one alias while the object + // is demonstrably alive through the other, which is the failure mode that made the + // iOS soft-reference table dangerous in the first place. + // + // Splitting the loop removes the possibility rather than narrowing the window, + // because after sub-pass A the decision depends only on the REFERENT's mark word, + // which every alias reads identically. Nothing marks between A's drain and B, so all + // aliases of one referent are cleared together or kept together, whatever a mutator + // does meanwhile. A get() racing sub-pass B still gets a non-null referent and still + // enqueues it, so the object survives -- it is a spurious clear of every alias at + // once, which the contract permits (the referent was weakly reachable when the + // decision was taken) and which is exactly what "atomically" is asking for. + +#ifdef CN1_REF_NO_ALIAS_ATOMICITY + // ABLATION ARM: the single-loop form this replaced, which re-read the touch stamp per + // ENTRY and could therefore clear one alias of a referent and keep another. + // + // IT HAS NEVER BEEN SEEN TO FAIL, and that is recorded rather than hidden. The split + // needs a get() to land between the pass reaching one alias and reaching another, and + // that window is microseconds; RefPolicy's alias phase with a burst reader reported + // ALIAS_SPLIT=0/256 built THIS way, identical to the fixed build, while both collected + // 255 of 256 groups. The defect is real by inspection -- the loop plainly re-reads a + // stamp a mutator can change mid-pass -- and the two-sub-pass form removes the + // possibility rather than narrowing the window, which is why it is the shipped one. + // The arm stays so a future attempt at a reproducer has something to aim at, on the + // same footing as CN1_NO_BULK_INSERTION_BARRIER. Not a supported configuration. + for(long i = 0 ; i < n ; i++) { + struct CN1RefEntry* e = &cn1RefDiscovered[i]; + JAVA_OBJECT r = __atomic_load_n(e->referentField, __ATOMIC_RELAXED); + if(r == JAVA_NULL || CN1_IS_TAGGED(r)) { + continue; + } + if(__atomic_load_n(e->touchAgeField, __ATOMIC_RELAXED) == CN1_REF_TOUCHED) { + gcMarkObject(threadStateData, r, JAVA_FALSE); + marked = JAVA_TRUE; + gcMarkDrain(threadStateData); + continue; + } +#ifdef CN1_CONSERVATIVE_GC_ROOTS + if(cn1ConservativeResolve((void*)r) != r && !cn1GcImmortalObjContains(r)) { + continue; + } +#endif + int mark = __atomic_load_n(&r->__codenameOneGcMark, __ATOMIC_ACQUIRE); + if(mark == -1 || mark >= currentGcMarkValue - 1) { + continue; + } + __atomic_store_n(e->referentField, JAVA_NULL, __ATOMIC_RELAXED); + } +#ifdef CN1_GC_CONFORM + cn1RefPhaseNs += cn1GcNowNs() - __r0; +#endif + return marked; +#endif + + // SUB-PASS A: anything read since this cycle aged it is kept, and its referent + // marked. This is not an optimization, it is the reason the pass may clear anything + // at all: a mutator may be holding that referent in a local the collector has already + // walked past. It backs up the SATB load barrier in the accessor rather than + // duplicating it -- the barrier's log can be dropped on an allocation failure, this + // cannot. for(long i = 0 ; i < n ; i++) { struct CN1RefEntry* e = &cn1RefDiscovered[i]; - JAVA_OBJECT r = *e->referentField; + JAVA_OBJECT r = __atomic_load_n(e->referentField, __ATOMIC_RELAXED); if(r == JAVA_NULL || CN1_IS_TAGGED(r)) { continue; } - if(*e->touchAgeField == CN1_REF_TOUCHED) { - // A get() landed AFTER this cycle aged this reference, so a mutator may be - // holding the referent in a local the collector has already walked past. - // Keeping it is not an optimization, it is the reason this pass is allowed - // to clear anything at all. Cheaper than trusting the SATB enqueue alone, - // and correct even if the log overflowed. + if(__atomic_load_n(e->touchAgeField, __ATOMIC_RELAXED) == CN1_REF_TOUCHED) { gcMarkObject(threadStateData, r, JAVA_FALSE); marked = JAVA_TRUE; #ifdef CN1_GC_CONFORM atomic_fetch_add_explicit(&cn1RefKeptTouched, 1, memory_order_relaxed); #endif + } + } + // Close sub-pass A before deciding anything, so sub-pass B reads settled mark words. + // Without this the entries marked above would still look dead to the loop below. + if(marked) { + gcMarkDrain(threadStateData); + } + + // SUB-PASS B: clear on the referent's liveness alone. + for(long i = 0 ; i < n ; i++) { + struct CN1RefEntry* e = &cn1RefDiscovered[i]; + JAVA_OBJECT r = __atomic_load_n(e->referentField, __ATOMIC_RELAXED); + if(r == JAVA_NULL || CN1_IS_TAGGED(r)) { continue; } #ifdef CN1_CONSERVATIVE_GC_ROOTS @@ -2494,7 +2580,7 @@ static JAVA_BOOLEAN cn1GcProcessReferences(CODENAME_ONE_THREAD_STATE) { if(mark == -1 || mark >= currentGcMarkValue - 1) { continue; } - *e->referentField = JAVA_NULL; + __atomic_store_n(e->referentField, JAVA_NULL, __ATOMIC_RELAXED); #ifdef CN1_GC_CONFORM atomic_fetch_add_explicit(&cn1RefCleared, 1, memory_order_relaxed); #endif @@ -3448,9 +3534,9 @@ void codenameOneGCMark() { // below to find a non-empty log, which re-arms and runs the whole fixpoint (and // this pass) again. Moving it after the barrier goes down would remove exactly // that protection and let the sweep free an object a mutator is holding. - if(cn1GcProcessReferences(d)) { - gcMarkDrain(d); - } + // Drains internally between its two sub-passes, which it must -- sub-pass B + // reads the mark words sub-pass A settled -- so there is nothing to drain here. + cn1GcProcessReferences(d); // Trial clear. A store racing it either logged already (caught just below) or // adds an already-marked or fresh reference, which the sweep keeps either way. __atomic_store_n(&gcSatbActive, 0, __ATOMIC_SEQ_CST); diff --git a/vm/ByteCodeTranslator/src/com/codename1/tools/translator/ByteCodeClass.java b/vm/ByteCodeTranslator/src/com/codename1/tools/translator/ByteCodeClass.java index 0e4a4178203..230fd68a715 100644 --- a/vm/ByteCodeTranslator/src/com/codename1/tools/translator/ByteCodeClass.java +++ b/vm/ByteCodeTranslator/src/com/codename1/tools/translator/ByteCodeClass.java @@ -1065,10 +1065,20 @@ public String generateCCode(List allClasses) { // references by use: a store of an immediate. Unconditional rather than // guarded by a "did it change" test, because the branch would cost more // than the store it saves. - b.append("((struct obj__").append(clsName).append("*)__cn1T)->") - .append(REFERENCE_CLASS).append("_cn1TouchAge = CN1_REF_TOUCHED;\n "); - } - if (fld.isVolatile()) { + b.append("__atomic_store_n(&((struct obj__").append(clsName).append("*)__cn1T)->") + .append(REFERENCE_CLASS).append("_cn1TouchAge, CN1_REF_TOUCHED, __ATOMIC_RELAXED);\n "); + // RELAXED ATOMIC, not a plain load, and the same everywhere this field is + // touched -- cn1GcProcessReferences clears it from the collector thread + // while mutators are running, which is the whole point of the design, so a + // plain access on either side is a data race and undefined in C however + // benign the generated instruction looks. Relaxed is the same instruction + // on every target built here; what it buys is that the write is one the + // reader is allowed to observe. Note the mark word two lines down in + // gcMarkObject is already handled this way for exactly this reason. + b.append("return __atomic_load_n(&((struct obj__").append(clsName).append("*)__cn1T)->") + .append(fld.getClsName()).append("_").append(fld.getFieldName()) + .append(", __ATOMIC_RELAXED);\n}\n\n"); + } else if (fld.isVolatile()) { b.append("return atomic_load_explicit(&((struct obj__"); b.append(clsName); b.append("*)__cn1T)->"); @@ -1106,7 +1116,14 @@ public String generateCCode(List allClasses) { } else { b.append(" __cn1Val, JAVA_OBJECT __cn1T) {\n ").append(nullCheck).append(" "); } - if (fld.isVolatile()) { + if(isReferenceReferent(clsName, fld)) { + // Reference.clear() and the constructor both land here, and the collector + // stores JAVA_NULL into the same word concurrently. Atomic for the reason + // spelled out on the getter above. + b.append("__atomic_store_n(&((struct obj__").append(clsName).append("*)__cn1T)->") + .append(fld.getClsName()).append("_").append(fld.getFieldName()) + .append(", __cn1Val, __ATOMIC_RELAXED);\n}\n\n"); + } else if (fld.isVolatile()) { b.append("atomic_store_explicit(&((struct obj__"); b.append(clsName); b.append("*)__cn1T)->"); diff --git a/vm/benchmarks/src/com/bench/RefPolicy.java b/vm/benchmarks/src/com/bench/RefPolicy.java index 8d0b1167a92..5650e2d29b6 100644 --- a/vm/benchmarks/src/com/bench/RefPolicy.java +++ b/vm/benchmarks/src/com/bench/RefPolicy.java @@ -107,6 +107,9 @@ public class RefPolicy { static long misses; static long checksum; + /** Aliases per referent in the alias phase. */ + static final int ALIASES = 4; + public static void main(String[] args) throws Exception { if (args != null) { if (args.length > 0) { keys = Integer.parseInt(args[0]); } @@ -119,6 +122,7 @@ public static void main(String[] args) throws Exception { + " accesses=" + accesses + " churnPerAccess=" + churnPerAccess + " cacheBytes=" + ((long) keys * payload)); weakPhase(); + aliasPhase(); cachePhase(); System.out.println("RESULT=" + checksum); } @@ -178,6 +182,113 @@ private static void weakPhase() throws Exception { System.out.println("WEAK_DEAD_CLEARED=" + deadCleared + "/" + WEAK_SAMPLES); } + /** Set by the racing reader so the collector's clear pass sees fresh touch stamps. */ + static volatile boolean aliasRacing; + static volatile Object aliasSink; + static Object[] aliasRefs; + + /** + * Several references over ONE referent must agree, INCLUDING while a mutator is + * reading them. + * + *

The contract is that all references to a weakly reachable object are cleared + * atomically -- not one at a time -- and a collector that clears them entry by entry + * cannot honour it: clear the first alias, let a mutator's {@code get()} on the second + * stamp it as recently read before the loop arrives there, and the second is kept. One + * alias then answers null while another answers the object.

+ * + *

For a cache that is a spurious miss. For the callers that use a reference as a + * LIFETIME ORACLE -- reading a null {@code get()} as proof the referent died, and + * releasing something on that basis -- it is a false death report on one alias while + * the object is demonstrably alive through another.

+ * + *

This is NOT a self-test for the split, and the distinction matters. The + * split needs a {@code get()} to land between the clear pass reaching one alias of a + * group and reaching another, and that window is microseconds wide. Measured here: + * with {@code -DCN1_REF_NO_ALIAS_ATOMICITY} restoring the single-loop form that has + * the bug, three runs reported {@code ALIAS_SPLIT=0/256} -- the same as the fixed + * build -- while both collected 255 of 256 groups. So the phase exercises the path + * with real concurrent readers and asserts a real invariant, but it cannot be cited + * as evidence that the invariant holds: it has never been seen to fail. Do not read + * a green {@code ALIAS_SPLIT} as proof.

+ * + *

Two earlier versions were worse and are worth not rebuilding. Reading the + * aliases only after quiescing asserted something that could not fail at all, since + * with no {@code get()} in flight every alias carries the same stamp. Hammering the + * FIRST alias without pausing was hollow in the other direction: it kept every + * referent marked, so nothing was ever condemned and {@code ALIAS_CLEARED_GROUPS} was + * 0/256 -- a test in which the thing being tested never happens. Check that number + * before trusting the one above it.

+ * + *

The assertion is about AGREEMENT, not about collection: all cleared and all kept + * both pass, split does not. Whether a group is collected at all depends on the + * conservative root scan and on whether the reader happened to be holding it.

+ */ + private static void aliasPhase() throws Exception { + final int groups = 256; + aliasRefs = new Object[groups * ALIASES]; + for (int g = 0; g < groups; g++) { + byte[] doomed = build(g + 4096); + for (int a = 0; a < ALIASES; a++) { + aliasRefs[g * ALIASES + a] = new WeakReference(doomed); + } + // `doomed` dies with this iteration; only the aliases above refer to it. + } + + aliasRacing = true; + Thread reader = new Thread(new Runnable() { + public void run() { + // The LAST alias of each group, and only in bursts. + // + // Last, because the split needs the read to land after the pass has + // already cleared the group's earlier aliases -- a read that lands on the + // first entry finds the group not yet condemned and changes nothing. + // + // In bursts, because a reader that never pauses keeps every referent + // marked, so nothing is ever condemned and no split is possible. The pause + // lets a group become collectable between bursts. + while (aliasRacing) { + for (int g = 0; g < groups; g++) { + aliasSink = ((Reference) aliasRefs[g * ALIASES + ALIASES - 1]).get(); + } + aliasSink = null; + try { + Thread.sleep(1); + } catch (InterruptedException e) { + return; + } + } + } + }); + reader.start(); + + for (int i = 0; i < 12; i++) { + quiesce(); + } + + aliasRacing = false; + reader.join(); + scrub(SCRUB_DEPTH); + + int split = 0; + int clearedGroups = 0; + for (int g = 0; g < groups; g++) { + int cleared = 0; + for (int a = 0; a < ALIASES; a++) { + if (((Reference) aliasRefs[g * ALIASES + a]).get() == null) { + cleared++; + } + } + if (cleared == ALIASES) { + clearedGroups++; + } else if (cleared != 0) { + split++; + } + } + System.out.println("ALIAS_SPLIT=" + split + "/" + groups); + System.out.println("ALIAS_CLEARED_GROUPS=" + clearedGroups + "/" + groups); + } + // ---------------------------------------------------------------- phase B /** From b5f53d8df99665f0dbed08da6f45b5ce1003473e Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Mon, 7 Sep 2026 16:59:51 +0300 Subject: [PATCH 03/30] Release soft referents on allocation failure, and stop losing touches Both from review, and both premises hold. ALLOCATION FAILURE. The retention ladder reads cn1ProcessHeadroom(), which answers -1 wherever there is no per-process budget to probe -- every desktop build and the simulator. There it could only ever say "plenty", so a soft referent read at least once every CN1_REF_SOFT_RETAIN_MAX cycles was never dropped however tight memory actually was, while codenameOneGcMalloc's failure path only asked for a collection and retried. That breaks the one guarantee SoftReference makes -- every soft reference cleared before the VM gives up -- and turns recoverable pressure into a retry loop that collects nothing. cn1RefDropAllSoftReferents() raises a latch on the failure path which the next cycle consumes, overriding every policy including "never clear". A latch rather than a direct write, because cn1RefBeginCycle recomputes the budget at the top of each cycle and would erase one. Proven, not argued: with failures injected while soft referents are live, the emergency cycles report retained=0 and cleared=1097 against retained>0 on the same workload uninjected. Getting that proof needed CN1_SIMULATE_ALLOC_FAILURES to grow a ":" form -- it could only fail the FIRST n allocations, which is startup, and a state that exists only at startup cannot exercise anything the program builds later. Every attempt without it produced real emergency cycles that all reported discovered=0. LOST TOUCHES. Ageing was a load, a compute and a store, so a get() landing between the load and the store was erased outright -- the referent looked cold with no record it had been read at all, and the age was not even reset. Worse, it falsified a claim: consuming CN1_REF_TOUCHED at DISCOVERY meant the clear pass's "was it read?" fallback only ever covered reads landing after discovery, not the whole cycle as its comment implied. The ageing therefore moves out of discovery to the end of the clear pass, and is a compare-exchange. Any read anywhere in the cycle is now still visible to sub-pass A, so the fallback covers what it says it does, and a racing touch can no longer be lost -- a failed exchange reloads, sees CN1_REF_TOUCHED and resets the age to 0, which is what the touch means. Two consequences worth naming. The CN1_REF_NO_ALIAS_ATOMICITY arm had to start ageing inline, or with discovery no longer doing it the arm would silently have become "retain everything" rather than the shape it exists to reproduce. And Reference.cn1TouchAge now starts at 0 rather than TOUCHED: TOUCHED means "read since the collector last aged this", which is false for a reference nobody has called get() on, and starting there made the clear pass mark the referent of every newly discovered reference for a cycle, weak ones included. Gates: run-gc-verify.sh green (RefPolicy clean over 71 verify passes, both fault-injection self-tests firing), run-gauntlet.sh green with all eleven tortures bit-identical and both GC stop modes. Co-Authored-By: Claude Opus 5 (1M context) --- vm/ByteCodeTranslator/src/cn1_globals.h | 5 + vm/ByteCodeTranslator/src/cn1_globals.m | 155 +++++++++++++++++--- vm/JavaAPI/src/java/lang/ref/Reference.java | 8 +- 3 files changed, 150 insertions(+), 18 deletions(-) diff --git a/vm/ByteCodeTranslator/src/cn1_globals.h b/vm/ByteCodeTranslator/src/cn1_globals.h index 29391c862bb..5d3365a10b8 100644 --- a/vm/ByteCodeTranslator/src/cn1_globals.h +++ b/vm/ByteCodeTranslator/src/cn1_globals.h @@ -3093,6 +3093,11 @@ void codenameOneGcFree(CODENAME_ONE_THREAD_STATE, JAVA_OBJECT obj); extern int currentGcMarkValue; extern void gcMarkObject(CODENAME_ONE_THREAD_STATE, JAVA_OBJECT obj, JAVA_BOOLEAN force); +// Drop every soft referent at the next collection, whatever the retention policy would +// otherwise have decided. Called when an allocation has actually failed: SoftReference's +// one hard guarantee is that all of them are cleared before the VM gives up, and the +// retention ladder cannot see that coming on a platform with no per-process budget probe. +extern void cn1RefDropAllSoftReferents(void); extern void cn1GcDiscoverReference(CODENAME_ONE_THREAD_STATE, JAVA_OBJECT ref, JAVA_BOOLEAN force, JAVA_OBJECT* referentField, JAVA_INT* touchAgeField, JAVA_INT* agedCycleField, JAVA_INT strength); diff --git a/vm/ByteCodeTranslator/src/cn1_globals.m b/vm/ByteCodeTranslator/src/cn1_globals.m index 6cc706d4a8d..c8a0ba29aa2 100644 --- a/vm/ByteCodeTranslator/src/cn1_globals.m +++ b/vm/ByteCodeTranslator/src/cn1_globals.m @@ -2265,6 +2265,35 @@ JAVA_LONG GcVerifyApp_gcMarkState___R_long(CODENAME_ONE_THREAD_STATE) { // would be harmless anyway -- the budget only decides retention, never safety. static _Atomic int cn1SoftRetainCycles = CN1_REF_SOFT_RETAIN_MAX; +// Raised when an allocation has actually FAILED, and consumed by the next cycle. +// +// The retention ladder is driven by cn1ProcessHeadroom(), which answers -1 wherever there +// is no per-process budget to probe -- every desktop build, and the simulator. There the +// ladder can only ever say "plenty", so a soft referent read at least once every +// CN1_REF_SOFT_RETAIN_MAX cycles is never dropped however tight memory actually is. That +// is a hole rather than a conservative default, because the one guarantee SoftReference +// makes is precisely about this case: every soft reference is cleared before the VM gives +// up. codenameOneGcMalloc's failure path only asks for a collection and retries, so a heap +// held entirely through live soft references would have turned recoverable pressure into an +// allocation loop that collects nothing. +// +// A LATCH rather than a direct write of the budget, because cn1RefBeginCycle recomputes +// that budget from scratch at the top of every cycle and would erase a direct write before +// a single reference was reached. +static _Atomic int cn1RefEmergencyDrop = 0; + +// currentGcMarkValue of the cycle whose clear pass has already aged the discovered set. +// GC thread only. The pass can run more than once in a cycle -- every SATB reopen runs it +// again -- and ageing on each of those would count one collection as several and evict a +// cache by however many times the mutator happened to storm the reference log. +static int cn1RefAgedCycle = 0; + +// Called from the allocation-failure path. Idempotent, allocation-free and safe from a +// thread that is about to park -- a relaxed store and nothing else. +void cn1RefDropAllSoftReferents(void) { + atomic_store_explicit(&cn1RefEmergencyDrop, 1, memory_order_relaxed); +} + #ifdef CN1_GC_CONFORM _Atomic long cn1RefDiscoveries = 0; // references the mark reached (deduped) _Atomic long cn1RefWeak = 0; // of those, weak @@ -2291,6 +2320,14 @@ static void cn1RefBeginCycle(void) { cn1RefPhaseNs = 0; cn1RefPasses = 0; #endif + // THE EMERGENCY LATCH OUTRANKS EVERY POLICY, including "never clear" -- an arm that + // exists to bound the measurement, not to promise a soft reference outlives an + // out-of-memory. Exchange rather than load-then-clear so a failure raised while this + // runs is either honoured now or survives to the next cycle, never dropped between. + if(atomic_exchange_explicit(&cn1RefEmergencyDrop, 0, memory_order_relaxed)) { + atomic_store_explicit(&cn1SoftRetainCycles, -1, memory_order_relaxed); + return; + } #if CN1_REF_POLICY == 1 atomic_store_explicit(&cn1SoftRetainCycles, 0x7fffffff, memory_order_relaxed); #else @@ -2382,16 +2419,22 @@ void cn1GcDiscoverReference(CODENAME_ONE_THREAD_STATE, JAVA_OBJECT ref, JAVA_BOO } *agedCycleField = currentGcMarkValue; { - // AGE IT. CN1_REF_TOUCHED means get() ran since the last cycle aged this - // reference, so the age resets; anything else is one collection older. Saturating, - // because an age that wraps to negative would read as freshly touched and make a - // cold entry immortal. + // READ the age; do NOT consume it. The ageing write lives at the end of the clear + // pass instead, and that placement is a safety property rather than tidiness. + // + // Consuming CN1_REF_TOUCHED here would erase every get() that happened between the + // start of the cycle and this moment, so the clear pass's "was it read?" fallback + // would only ever have covered reads that landed AFTER discovery. It would also + // lose the touch outright under a race -- read 5, a mutator stamps TOUCHED, write 6 + // -- leaving a just-used referent looking cold with no record that it was used at + // all. Deferring the write means any read anywhere in the cycle is still visible to + // sub-pass A. + // + // CN1_REF_TOUCHED is negative, so it compares as hotter than any real age and a + // reference read this cycle is retained by the test below without a special case. JAVA_INT age = __atomic_load_n(touchAgeField, __ATOMIC_RELAXED); - JAVA_INT aged = (age == CN1_REF_TOUCHED) ? 0 - : (age < 0x7ffffffe ? age + 1 : age); - __atomic_store_n(touchAgeField, aged, __ATOMIC_RELAXED); int budget = atomic_load_explicit(&cn1SoftRetainCycles, memory_order_relaxed); - retain = (strength == CN1_REF_SOFT && budget >= 0 && aged <= budget) + retain = (strength == CN1_REF_SOFT && budget >= 0 && age <= budget) ? JAVA_TRUE : JAVA_FALSE; } if(cn1RefDiscoveredTop >= cn1RefDiscoveredCap @@ -2502,11 +2545,21 @@ static JAVA_BOOLEAN cn1GcProcessReferences(CODENAME_ONE_THREAD_STATE) { if(r == JAVA_NULL || CN1_IS_TAGGED(r)) { continue; } - if(__atomic_load_n(e->touchAgeField, __ATOMIC_RELAXED) == CN1_REF_TOUCHED) { - gcMarkObject(threadStateData, r, JAVA_FALSE); - marked = JAVA_TRUE; - gcMarkDrain(threadStateData); - continue; + { + // Ages inline, as the single-loop form did. Discovery no longer ages, so + // without this the arm would never age anything, every reference would read as + // permanently touched, and the arm would silently become "retain everything" + // rather than the shape it exists to reproduce. + JAVA_INT age = __atomic_load_n(e->touchAgeField, __ATOMIC_RELAXED); + JAVA_INT aged = (age == CN1_REF_TOUCHED) ? 0 + : (age < 0x7ffffffe ? age + 1 : age); + __atomic_store_n(e->touchAgeField, aged, __ATOMIC_RELAXED); + if(age == CN1_REF_TOUCHED) { + gcMarkObject(threadStateData, r, JAVA_FALSE); + marked = JAVA_TRUE; + gcMarkDrain(threadStateData); + continue; + } } #ifdef CN1_CONSERVATIVE_GC_ROOTS if(cn1ConservativeResolve((void*)r) != r && !cn1GcImmortalObjContains(r)) { @@ -2551,7 +2604,12 @@ static JAVA_BOOLEAN cn1GcProcessReferences(CODENAME_ONE_THREAD_STATE) { gcMarkDrain(threadStateData); } - // SUB-PASS B: clear on the referent's liveness alone. + // SUB-PASS B: clear on the referent's liveness alone, then age. + // + // The ageing happens HERE, once per cycle, rather than at discovery -- see the comment + // there. It runs after the clear decision for the same reason: sub-pass A's reading of + // the stamp must not be undone by this pass before the decision that depends on it. + JAVA_BOOLEAN doAge = (cn1RefAgedCycle != currentGcMarkValue) ? JAVA_TRUE : JAVA_FALSE; for(long i = 0 ; i < n ; i++) { struct CN1RefEntry* e = &cn1RefDiscovered[i]; JAVA_OBJECT r = __atomic_load_n(e->referentField, __ATOMIC_RELAXED); @@ -2585,6 +2643,30 @@ static JAVA_BOOLEAN cn1GcProcessReferences(CODENAME_ONE_THREAD_STATE) { atomic_fetch_add_explicit(&cn1RefCleared, 1, memory_order_relaxed); #endif } + + // AGE, by compare-exchange, so a get() racing this cannot be erased. A plain + // read-modify-write here would reintroduce exactly what deferring the write was meant + // to remove: read 5, a mutator stamps TOUCHED, write 6, and the read has vanished with + // the age not even reset. On a failed exchange the reload sees the mutator's + // CN1_REF_TOUCHED and resets the age to 0, which is what the touch means. + if(doAge) { + for(long i = 0 ; i < n ; i++) { + JAVA_INT* f = cn1RefDiscovered[i].touchAgeField; + JAVA_INT age = __atomic_load_n(f, __ATOMIC_RELAXED); + for(;;) { + // Saturating: an age that wrapped to negative would compare as hotter than + // anything real and make a cold entry immortal. + JAVA_INT aged = (age == CN1_REF_TOUCHED) ? 0 + : (age < 0x7ffffffe ? age + 1 : age); + if(__atomic_compare_exchange_n(f, &age, aged, 0, + __ATOMIC_RELAXED, __ATOMIC_RELAXED)) { + break; + } + // age now holds what the mutator wrote; recompute against it. + } + } + cn1RefAgedCycle = currentGcMarkValue; + } #ifdef CN1_GC_CONFORM cn1RefPhaseNs += cn1GcNowNs() - __r0; #endif @@ -4220,8 +4302,8 @@ JAVA_BOOLEAN removeObjectFromHeapCollection(CODENAME_ONE_THREAD_STATE, JAVA_OBJE // Something that CHANGES when a collection starts, for callers that need to wait for the // one they just asked for rather than for a handshake that may never reach them. #ifdef CN1_GC_CONFORM -// TEST HOOK. CN1_SIMULATE_ALLOC_FAILURES= makes the next n legacy allocations return -// NULL. The out-of-memory retry path is the one place in this allocator that cannot be +// TEST HOOK. CN1_SIMULATE_ALLOC_FAILURES=[:] makes n legacy allocations return +// NULL, after letting the first through. The out-of-memory retry path is the one place in this allocator that cannot be // reached on a developer machine -- macOS ignores `ulimit -v`, so there is no way to make // calloc fail on demand -- and it has now been the subject of two review findings that // could only be reasoned about. Gated on CN1_GC_CONFORM, like the rest of the QA @@ -4235,14 +4317,45 @@ JAVA_BOOLEAN removeObjectFromHeapCollection(CODENAME_ONE_THREAD_STATE, JAVA_OBJE static _Atomic long cn1SimulatedAllocFailures = 0; _Atomic long cn1AllocRetries = 0; // times the OOM path went round again +// Allocations still to be let through before the failures start. ":" fails n +// allocations AFTER the first skip have succeeded. +// +// Without a skip the hook can only ever fail the FIRST allocations a program makes, which +// is startup -- and a state that exists only at startup cannot exercise anything the +// program builds later. Reaching cn1RefDropAllSoftReferents' effect needs a failure while +// soft referents are actually live, and every attempt without this knob spent its whole +// budget before the first SoftReference existed: the emergency cycles were real and +// visible, and every one of them reported discovered=0. +static _Atomic long cn1SimulatedAllocSkip = 0; + static void cn1AllocFailInit(void) { const char* e = getenv("CN1_SIMULATE_ALLOC_FAILURES"); - long n = e ? atol(e) : 0; + long n = 0, skip = 0; + if(e != 0) { + n = atol(e); + const char* colon = strchr(e, ':'); + if(colon != 0) { + skip = atol(colon + 1); + } + } atomic_store_explicit(&cn1SimulatedAllocFailures, n < 0 ? 0 : n, memory_order_relaxed); + atomic_store_explicit(&cn1SimulatedAllocSkip, skip < 0 ? 0 : skip, memory_order_relaxed); } static JAVA_BOOLEAN cn1ShouldFailAllocation(void) { pthread_once(&cn1AllocFailOnce, cn1AllocFailInit); + // Burn the skip budget first, with the same decrement-only-while-positive discipline + // the failure budget uses and for the same reason. + { + long sk = atomic_load_explicit(&cn1SimulatedAllocSkip, memory_order_relaxed); + while(sk > 0) { + if(atomic_compare_exchange_weak_explicit(&cn1SimulatedAllocSkip, &sk, sk - 1, + memory_order_relaxed, + memory_order_relaxed)) { + return JAVA_FALSE; + } + } + } // Decrement only while positive, so the count cannot go below zero however many // threads race here. long v = atomic_load_explicit(&cn1SimulatedAllocFailures, memory_order_relaxed); @@ -9741,6 +9854,14 @@ JAVA_OBJECT codenameOneGcMalloc(CODENAME_ONE_THREAD_STATE, int size, struct claz goto cn1GcMallocRetry; } invokedGC = YES; + // An allocation has genuinely failed, so every soft referent goes at the next + // cycle regardless of what the retention ladder thinks. SoftReference's one hard + // guarantee is that all of them are cleared before the VM gives up, and the ladder + // cannot anticipate this on a platform whose headroom probe answers -1 -- it would + // keep re-arming a comfortable budget while this loop collected nothing and + // retried. Raised BEFORE the collection is asked for, so the cycle that request + // starts is the one that honours it. + cn1RefDropAllSoftReferents(); java_lang_System_gc__(getThreadLocalData()); CN1_GC_PARK_CAPTURE(threadStateData); // this park can now last seconds; be scannable threadStateData->threadActive = JAVA_FALSE; diff --git a/vm/JavaAPI/src/java/lang/ref/Reference.java b/vm/JavaAPI/src/java/lang/ref/Reference.java index 07bc9263b9d..3dd585c77ab 100644 --- a/vm/JavaAPI/src/java/lang/ref/Reference.java +++ b/vm/JavaAPI/src/java/lang/ref/Reference.java @@ -71,7 +71,13 @@ public abstract class Reference{ * reference at all while mutators run: see the discussion of * {@code cn1GcProcessReferences}.

*/ - int cn1TouchAge = TOUCHED; + // Starts at 0 -- "read this cycle", not TOUCHED. TOUCHED means "read since the + // collector last aged this", and a reference nothing has called get() on yet has not + // been; starting there would make the clear pass treat every newly discovered + // reference as freshly used and mark its referent for a cycle, weak ones included. + // Zero already reads as maximally hot to the soft-retention test, which is what a new + // cache entry should be. + int cn1TouchAge; /** * {@link #STRENGTH_WEAK} or {@link #STRENGTH_SOFT}, set by the subclass From d012f912768f3aa9410384181d6fc3d26b77ad35 Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Mon, 7 Sep 2026 18:31:03 +0300 Subject: [PATCH 04/30] Retain a referent discovery could not record, and load it once Two accepted review findings and one declined, plus a driver that had gone vacuous again. A DROPPED DISCOVERY MUST RETAIN. When the discovery list could not grow -- the realloc failed, or growth was declined because a thread is signal-frozen -- the entry was dropped AND the referent left unmarked. The comment above it claimed that was safe because "the referent stays reachable through a field nothing cleared", which is exactly backwards and is the kind of sentence that reads as obviously true: nothing else marks a weak referent, that being the point of a weak edge, so the sweep frees it and the uncleared field becomes a dangling pointer inside a perfectly reachable Reference, handed to the next get(). On this VM that is a native crash no Java catch can see, on a path only ever taken when memory is already short. The drop path now marks the referent, which costs one deferred reclaim. ONE LOAD IN THE ACCESSOR. CN1_SATB_REF_LOAD only read the field INSIDE its gcSatbActive branch, so with the barrier down the accessor loaded again afterwards and the gap between the two was a hole: a thread that read the flag as 0, was SIGUSR2-frozen with the referent not yet in any register, scanned, released, and only then loaded, came away holding an unmarked referent nothing had enqueued. The accessor now loads once into a local and the barrier (CN1_SATB_REF_KEEP) acts on that same value, so what get() returns is what the barrier saw -- and with the load first the value is in a register before any freeze, where the conservative root scan finds it. ATOMIC PUBLICATION OF ALIAS CLEARING IS DECLINED, and the reasoning is in the code at the store rather than only here. Making N stores visible as one step needs a lock that Reference.get() also takes, and get() is the single hot path this design exists to keep free of one; HotSpot does not do it either, clearing referents one at a time without synchronising against get(). The contract's requirement is that the DECISION covers every alias together, which the two sub-passes provide and which was the part genuinely broken before them. The residual window is transient and self-healing -- a get() inside it arms the barrier and resurrects the object. The alias phase had gone hollow again after ageing moved to the clear pass: with the stamp surviving the whole cycle, a reader touching one alias of EVERY group kept every group alive and ALIAS_CLEARED_GROUPS read 0/256, which is the vacuum that phase exists to detect. It now reads one group in four, and collection is back to 191/256. Gates: run-gc-verify.sh green (RefPolicy clean over 71 verify passes, both fault-injection self-tests firing), run-gauntlet.sh green with all eleven tortures bit-identical and both GC stop modes. Co-Authored-By: Claude Opus 5 (1M context) --- vm/ByteCodeTranslator/src/cn1_globals.h | 27 +++++--- vm/ByteCodeTranslator/src/cn1_globals.m | 45 +++++++++++-- .../tools/translator/ByteCodeClass.java | 63 ++++++++----------- vm/benchmarks/src/com/bench/RefPolicy.java | 10 ++- 4 files changed, 93 insertions(+), 52 deletions(-) diff --git a/vm/ByteCodeTranslator/src/cn1_globals.h b/vm/ByteCodeTranslator/src/cn1_globals.h index 5d3365a10b8..5d9250c5f93 100644 --- a/vm/ByteCodeTranslator/src/cn1_globals.h +++ b/vm/ByteCodeTranslator/src/cn1_globals.h @@ -3130,17 +3130,26 @@ extern void cn1GcDiscoverReference(CODENAME_ONE_THREAD_STATE, JAVA_OBJECT ref, J // // A retained soft reference costs nothing here at all, because its referent is marked as // an ordinary strong edge by cn1GcDiscoverReference before any get() can reach it. +// KEEPS A VALUE THE CALLER HAS ALREADY LOADED, rather than loading its own. +// +// The accessor loads the referent ONCE, into a local, and hands that same value here -- +// so what the barrier acts on is exactly what get() returns. The previous shape checked +// gcSatbActive and only then loaded, leaving the accessor to load again afterwards, and +// the gap between the two was a real hole: a thread that read the flag as 0, was then +// SIGUSR2-frozen, scanned (with the referent not yet in any register or stack slot), +// released, and only then performed its load, came away holding an unmarked referent that +// nothing had enqueued. Loading first closes it without a barrier, because the value is +// in a register before the freeze can happen and the conservative root scan covers +// registers and the native stack. #if defined(CN1_DISABLE_SATB) -#define CN1_SATB_REF_LOAD(fieldAddr) do { } while(0) +#define CN1_SATB_REF_KEEP(refVal) do { (void)(refVal); } while(0) #else -#define CN1_SATB_REF_LOAD(fieldAddr) \ - do { if(__builtin_expect(gcSatbActive, 0)) { \ - JAVA_OBJECT cn1__r = __atomic_load_n((JAVA_OBJECT*)(fieldAddr), __ATOMIC_RELAXED); \ - if(cn1__r != JAVA_NULL && !CN1_IS_TAGGED(cn1__r)) { \ - int cn1__m = __atomic_load_n(&cn1__r->__codenameOneGcMark, __ATOMIC_RELAXED); \ - int cn1__e = atomic_load_explicit(&bibopGcEpoch, memory_order_relaxed); \ - if(cn1__m != -1 && cn1__m != cn1__e) cn1SatbEnqueue(cn1__r); \ - } \ +#define CN1_SATB_REF_KEEP(refVal) \ + do { JAVA_OBJECT cn1__r = (refVal); \ + if(__builtin_expect(gcSatbActive, 0) && cn1__r != JAVA_NULL && !CN1_IS_TAGGED(cn1__r)) { \ + int cn1__m = __atomic_load_n(&cn1__r->__codenameOneGcMark, __ATOMIC_RELAXED); \ + int cn1__e = atomic_load_explicit(&bibopGcEpoch, memory_order_relaxed); \ + if(cn1__m != -1 && cn1__m != cn1__e) cn1SatbEnqueue(cn1__r); \ } } while(0) #endif diff --git a/vm/ByteCodeTranslator/src/cn1_globals.m b/vm/ByteCodeTranslator/src/cn1_globals.m index c8a0ba29aa2..7740d10990f 100644 --- a/vm/ByteCodeTranslator/src/cn1_globals.m +++ b/vm/ByteCodeTranslator/src/cn1_globals.m @@ -2457,20 +2457,35 @@ void cn1GcDiscoverReference(CODENAME_ONE_THREAD_STATE, JAVA_OBJECT ref, JAVA_BOO cn1RefDiscoveredCap = nc; } } + JAVA_BOOLEAN recorded = JAVA_FALSE; if(cn1RefDiscoveredTop < cn1RefDiscoveredCap) { struct CN1RefEntry* e = &cn1RefDiscovered[cn1RefDiscoveredTop++]; e->ref = ref; e->referentField = referentField; e->touchAgeField = touchAgeField; e->strength = strength; + recorded = JAVA_TRUE; } - // A DROPPED ENTRY IS SAFE, and safe in the direction that matters -- whether it was - // dropped because realloc failed or because the growth above was declined. The clear - // pass never sees this reference, so the referent stays reachable through a field - // nothing cleared: a missed reclaim, never a freed object under a live pointer. Note - // the reference was still AGED above, so a dropped cycle does not make it immortal; - // the next cycle discovers it again and the list has usually grown by then. pthread_mutex_unlock(&cn1RefMutex); + // A DROPPED ENTRY MUST BE RETAINED, NOT IGNORED -- whether it was dropped because the + // realloc failed or because the growth above was declined while a thread is frozen. + // + // An earlier version of this comment claimed the opposite, that dropping was safe + // because "the referent stays reachable through a field nothing cleared". That is + // exactly backwards, and it is worth spelling out because it reads as obviously true: + // not clearing the field is not the same as keeping the referent ALIVE. Nothing else + // marks it -- that is the whole point of a weak edge -- so the sweep frees it, and the + // field nothing cleared is then a dangling pointer inside a perfectly reachable + // Reference, handed to the next get(). On this VM that is a native crash no Java catch + // can see, which is the worst possible outcome for a path taken only when memory is + // already short. + // + // Marking is the conservative direction: the referent survives one more cycle, the + // reference is rediscovered next time, and by then the list has usually grown. + if(!recorded) { + gcMarkObject(threadStateData, __atomic_load_n(referentField, __ATOMIC_RELAXED), force); + return; + } #ifdef CN1_GC_CONFORM atomic_fetch_add_explicit(&cn1RefDiscoveries, 1, memory_order_relaxed); if(strength != CN1_REF_SOFT) { @@ -2638,6 +2653,24 @@ static JAVA_BOOLEAN cn1GcProcessReferences(CODENAME_ONE_THREAD_STATE) { if(mark == -1 || mark >= currentGcMarkValue - 1) { continue; } + // THE STORES ARE SEQUENTIAL, AND DELIBERATELY SO. Review asked for the clearing of + // a referent's aliases to be PUBLISHED atomically as well as decided atomically, + // on the grounds that a mutator landing between two iterations can see one alias + // already null and another not. + // + // It can, and that is not fixable at an acceptable price. Making N stores visible + // as one step needs a lock the reader also takes, and the reader is + // Reference.get() -- the single hot path this whole design is built to keep free + // of one. HotSpot does not do it either: its reference processing clears referents + // one at a time and get() is not synchronised against it. + // + // What the contract actually requires is that the DECISION covers every alias + // together, so the collector never leaves one alias cleared and another live once + // it is finished. That is what the two sub-passes above provide, and it is the + // part that was genuinely broken before them. The residual window is transient and + // self-healing: it lasts only until this loop reaches the other alias, and a get() + // inside it returns a referent that is still valid, because the same read arms the + // barrier and resurrects the object for this cycle. __atomic_store_n(e->referentField, JAVA_NULL, __ATOMIC_RELAXED); #ifdef CN1_GC_CONFORM atomic_fetch_add_explicit(&cn1RefCleared, 1, memory_order_relaxed); diff --git a/vm/ByteCodeTranslator/src/com/codename1/tools/translator/ByteCodeClass.java b/vm/ByteCodeTranslator/src/com/codename1/tools/translator/ByteCodeClass.java index 230fd68a715..a1a449d6bd4 100644 --- a/vm/ByteCodeTranslator/src/com/codename1/tools/translator/ByteCodeClass.java +++ b/vm/ByteCodeTranslator/src/com/codename1/tools/translator/ByteCodeClass.java @@ -1038,46 +1038,37 @@ public String generateCCode(List allClasses) { b.append(fld.getFieldName()); b.append("(JAVA_OBJECT __cn1T) {\n ").append(nullCheck).append(" "); if(isReferenceReferent(clsName, fld)) { - // Reference.get() compiles into this accessor, and reading a weak - // referent while a concurrent mark is running needs a barrier that an - // ordinary field read does not. + // Reference.get() compiles into this accessor, and reading a weak referent + // while a concurrent mark is running needs a barrier an ordinary field read + // does not. // - // The collector clears a reference only after the strong mark has - // reached its fixpoint, but it does so with the mutators still running - // and with the SATB barrier still armed. Without the enqueue below, a - // thread whose stack was scanned and released early could take the - // referent out of here, hold it in a local the collector has already - // walked past, and watch the same cycle's sweep free it -- the object - // is by then neither marked nor fresh, which is the one case the - // "already marked or FRESH" invariant the sweep relies on does not - // cover. Enqueuing makes the referent part of the snapshot, so the - // fixpoint loop marks it and the clear pass then sees it live and - // leaves the reference alone. + // ONE LOAD, and everything below acts on that value. The collector clears a + // reference after the strong mark reaches its fixpoint but with mutators + // still running, so a thread whose stack was scanned and released early can + // take the referent out of here and hold it in a local the collector has + // walked past -- neither marked nor fresh, the one case the sweep's "already + // marked or FRESH" invariant does not cover. Enqueuing puts it back in the + // snapshot, and the trial clear of gcSatbActive then finds a non-empty log, + // re-arms, marks it, and leaves the reference alone. // - // CN1_SATB_REF_LOAD rather than the plain CN1_SATB_DELETE next door: a - // referent that is already marked this epoch, or fresh, is one the clear - // pass would refuse to clear, so logging it is pure cost -- and on a hot - // cache that cost is enough to stop the SATB termination loop converging. - // Off-mark both are one predicted-not-taken load of gcSatbActive. - b.append("CN1_SATB_REF_LOAD(&((struct obj__").append(clsName).append("*)__cn1T)->") - .append(fld.getClsName()).append("_").append(fld.getFieldName()).append(");\n "); - // The touch stamp, and the entire per-read cost of ranking soft - // references by use: a store of an immediate. Unconditional rather than - // guarded by a "did it change" test, because the branch would cost more - // than the store it saves. + // Loading BEFORE the barrier check rather than inside it is what makes that + // sound. Checking the flag first and loading afterwards leaves a gap: a + // thread that read the flag as 0, was SIGUSR2-frozen with the referent not + // yet in any register, scanned, released, and only then loaded, came away + // with an unmarked referent nothing had enqueued. With the load first the + // value is in a register before any freeze, where the conservative root scan + // finds it. + b.append(fld.getCDefinition()).append(" __cn1Ref = __atomic_load_n(&((struct obj__") + .append(clsName).append("*)__cn1T)->") + .append(fld.getClsName()).append("_").append(fld.getFieldName()) + .append(", __ATOMIC_RELAXED);\n "); + b.append("CN1_SATB_REF_KEEP(__cn1Ref);\n "); + // The touch stamp, and the entire per-read cost of ranking soft references + // by use: a store of an immediate. Unconditional rather than guarded by a + // "did it change" test, because the branch would cost more than the store. b.append("__atomic_store_n(&((struct obj__").append(clsName).append("*)__cn1T)->") .append(REFERENCE_CLASS).append("_cn1TouchAge, CN1_REF_TOUCHED, __ATOMIC_RELAXED);\n "); - // RELAXED ATOMIC, not a plain load, and the same everywhere this field is - // touched -- cn1GcProcessReferences clears it from the collector thread - // while mutators are running, which is the whole point of the design, so a - // plain access on either side is a data race and undefined in C however - // benign the generated instruction looks. Relaxed is the same instruction - // on every target built here; what it buys is that the write is one the - // reader is allowed to observe. Note the mark word two lines down in - // gcMarkObject is already handled this way for exactly this reason. - b.append("return __atomic_load_n(&((struct obj__").append(clsName).append("*)__cn1T)->") - .append(fld.getClsName()).append("_").append(fld.getFieldName()) - .append(", __ATOMIC_RELAXED);\n}\n\n"); + b.append("return __cn1Ref;\n}\n\n"); } else if (fld.isVolatile()) { b.append("return atomic_load_explicit(&((struct obj__"); b.append(clsName); diff --git a/vm/benchmarks/src/com/bench/RefPolicy.java b/vm/benchmarks/src/com/bench/RefPolicy.java index 5650e2d29b6..6c22669db23 100644 --- a/vm/benchmarks/src/com/bench/RefPolicy.java +++ b/vm/benchmarks/src/com/bench/RefPolicy.java @@ -248,7 +248,15 @@ public void run() { // marked, so nothing is ever condemned and no split is possible. The pause // lets a group become collectable between bursts. while (aliasRacing) { - for (int g = 0; g < groups; g++) { + // ONE GROUP IN FOUR. Touching every group keeps every group alive: + // the stamp now survives until the end of the clear pass, so any read + // anywhere in a cycle makes sub-pass A mark that referent, and with + // one alias of each group read per burst nothing is ever condemned. + // That was measured -- ALIAS_CLEARED_GROUPS=0/256 -- and it is the + // vacuum this phase exists to detect, not to fall into. Leaving three + // groups in four untouched keeps the collection half honest while the + // touched quarter still exercises the interleaving. + for (int g = 0; g < groups; g += 4) { aliasSink = ((Reference) aliasRefs[g * ALIASES + ALIASES - 1]).get(); } aliasSink = null; From 67b1bdf1ccb726d8a887c933f63adcfc4c554e2b Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Mon, 7 Sep 2026 20:19:12 +0300 Subject: [PATCH 05/30] Process references to a fixpoint, and register the get() barrier Two accepted review findings, both real, both about the same thing: the reference pass assumed it could see the whole picture at one instant. DISCOVERY IS NOT DONE WHEN THE PASS STARTS. cn1GcProcessReferences snapshotted cn1RefDiscoveredTop once and cleared against that snapshot -- but sub-pass A DRAINS, and a drain discovers references. Marking a touched referent traces it, and an object kept alive only by that reference can itself hold weak references whose mark functions call cn1GcDiscoverReference; those landed past the snapshot and sub-pass B never looked at them. The result is the failure this whole design exists to prevent: a reachable Reference nothing cleared, holding a referent the sweep freed. Sub-pass A now repeats until a drain adds nothing new, which terminates because the set only grows and is bounded by the live set. THE BARRIER HAD TO JOIN THE TERMINATION HANDSHAKE. Checking gcSatbActive and then calling cn1SatbEnqueue is not enough here: the enqueue takes a mutex, so a thread can pass the check, be delayed acquiring it, and land its entry in a log the collector has already stopped draining -- after which the referent it is about to return gets swept. The per-store barrier accepts precisely that window, and the argument it accepts it on does NOT extend to this path: "a reference stored after the fixpoint is already marked or FRESH" is true of a store and false of a weak referent handed out by get(), which is neither. cn1SatbBulkBegin already registers before it answers, and gcSatbTerminating stays raised across the whole termination loop including reference processing, so reusing that handshake closes it with the machinery already present. Off-GC the fast path is unchanged: two predicted-not-taken flag loads. Also restores the CN1_REF_NO_ALIAS_ATOMICITY arm, which the sub-pass rewrite had silently taken with it -- RefPolicy's alias phase documents that flag, so losing it would have left a comment describing a build that no longer existed. Gates: run-gc-verify.sh green (RefPolicy clean over 71 verify passes, both fault-injection self-tests firing), run-gauntlet.sh green with all eleven tortures bit-identical and both GC stop modes. The emergency soft-drop still fires under injected allocation failure (retained=0 at softBudget=-1). Co-Authored-By: Claude Opus 5 (1M context) --- vm/ByteCodeTranslator/src/cn1_globals.h | 33 +++++++-- vm/ByteCodeTranslator/src/cn1_globals.m | 91 ++++++++++++------------- 2 files changed, 72 insertions(+), 52 deletions(-) diff --git a/vm/ByteCodeTranslator/src/cn1_globals.h b/vm/ByteCodeTranslator/src/cn1_globals.h index 5d9250c5f93..7ab8fefcb14 100644 --- a/vm/ByteCodeTranslator/src/cn1_globals.h +++ b/vm/ByteCodeTranslator/src/cn1_globals.h @@ -3144,12 +3144,37 @@ extern void cn1GcDiscoverReference(CODENAME_ONE_THREAD_STATE, JAVA_OBJECT ref, J #if defined(CN1_DISABLE_SATB) #define CN1_SATB_REF_KEEP(refVal) do { (void)(refVal); } while(0) #else +// REGISTERS BEFORE IT LOGS, via the same handshake the bulk copies use. +// +// Checking gcSatbActive and then enqueuing is not enough on this path, and the reason it +// is enough for the per-store barrier does not carry over. cn1SatbEnqueue takes a mutex, +// so a thread can pass the flag check and then be delayed acquiring it long enough for the +// collector to clear the flag, take an empty final batch and finish termination -- after +// which the entry lands in a log nothing will ever drain. The per-store barrier tolerates +// exactly that window because a reference STORED after the fixpoint is already marked or +// FRESH and the sweep keeps both; a weak REFERENT handed out by get() is neither, which is +// the whole reason this barrier exists. +// +// cn1SatbBulkBegin registers unconditionally and only then reports whether logging is +// needed, so cn1SatbBulkQuiesce cannot complete while this is in flight, and the collector +// cannot finish its final take underneath it. gcSatbTerminating stays raised across the +// whole termination loop -- including reference processing -- so the registration covers +// the window that matters. +// +// The fast path is unchanged off-GC: two predicted-not-taken flag loads. With both flags +// down the mark is over, every reference is either cleared or holds a marked referent, and +// a cycle starting afterwards will scan this thread with the value already in a register. #define CN1_SATB_REF_KEEP(refVal) \ do { JAVA_OBJECT cn1__r = (refVal); \ - if(__builtin_expect(gcSatbActive, 0) && cn1__r != JAVA_NULL && !CN1_IS_TAGGED(cn1__r)) { \ - int cn1__m = __atomic_load_n(&cn1__r->__codenameOneGcMark, __ATOMIC_RELAXED); \ - int cn1__e = atomic_load_explicit(&bibopGcEpoch, memory_order_relaxed); \ - if(cn1__m != -1 && cn1__m != cn1__e) cn1SatbEnqueue(cn1__r); \ + if(cn1__r != JAVA_NULL && !CN1_IS_TAGGED(cn1__r) \ + && (__builtin_expect(gcSatbActive, 0) \ + || __builtin_expect(gcSatbTerminating, 0))) { \ + if(cn1SatbBulkBegin()) { \ + int cn1__m = __atomic_load_n(&cn1__r->__codenameOneGcMark, __ATOMIC_RELAXED); \ + int cn1__e = atomic_load_explicit(&bibopGcEpoch, memory_order_relaxed); \ + if(cn1__m != -1 && cn1__m != cn1__e) cn1SatbEnqueue(cn1__r); \ + } \ + cn1SatbBulkEnd(); \ } } while(0) #endif diff --git a/vm/ByteCodeTranslator/src/cn1_globals.m b/vm/ByteCodeTranslator/src/cn1_globals.m index 7740d10990f..74eb9e2fd42 100644 --- a/vm/ByteCodeTranslator/src/cn1_globals.m +++ b/vm/ByteCodeTranslator/src/cn1_globals.m @@ -2516,44 +2516,23 @@ static JAVA_BOOLEAN cn1GcProcessReferences(CODENAME_ONE_THREAD_STATE) { // fixpoint before the caller reaches this, so the prefix walked here is stable. A // re-run after a SATB reopen walks the list again from the start, which is what picks // up anything discovered by the re-opened fixpoint. - long n = cn1RefDiscoveredTop; - - // TWO SUB-PASSES, AND THE SPLIT IS THE POINT. - // - // Two reachable references can share one referent, and the contract says all - // references to a weakly reachable object are cleared ATOMICALLY -- not one at a - // time. A single loop that re-read the touch stamp per ENTRY could not honour that: - // clear alias A, then a mutator's get() on alias B stamps it TOUCHED before the loop - // reaches B, and B is kept. One alias reads null and the other answers the object. - // - // For a cache that is only a spurious miss. For the callers that use a reference as a - // LIFETIME ORACLE -- read a null get() as proof the referent died, and then release - // something on that basis -- it is a false death report on one alias while the object - // is demonstrably alive through the other, which is the failure mode that made the - // iOS soft-reference table dangerous in the first place. - // - // Splitting the loop removes the possibility rather than narrowing the window, - // because after sub-pass A the decision depends only on the REFERENT's mark word, - // which every alias reads identically. Nothing marks between A's drain and B, so all - // aliases of one referent are cleared together or kept together, whatever a mutator - // does meanwhile. A get() racing sub-pass B still gets a non-null referent and still - // enqueues it, so the object survives -- it is a spurious clear of every alias at - // once, which the contract permits (the referent was weakly reachable when the - // decision was taken) and which is exactly what "atomically" is asking for. + long n; #ifdef CN1_REF_NO_ALIAS_ATOMICITY - // ABLATION ARM: the single-loop form this replaced, which re-read the touch stamp per - // ENTRY and could therefore clear one alias of a referent and keep another. + // ABLATION ARM: the single-loop form the two sub-passes replaced, which re-read the + // touch stamp per ENTRY and could therefore clear one alias of a referent and keep + // another. // // IT HAS NEVER BEEN SEEN TO FAIL, and that is recorded rather than hidden. The split // needs a get() to land between the pass reaching one alias and reaching another, and // that window is microseconds; RefPolicy's alias phase with a burst reader reported // ALIAS_SPLIT=0/256 built THIS way, identical to the fixed build, while both collected - // 255 of 256 groups. The defect is real by inspection -- the loop plainly re-reads a + // the same groups. The defect is real by inspection -- the loop plainly re-reads a // stamp a mutator can change mid-pass -- and the two-sub-pass form removes the // possibility rather than narrowing the window, which is why it is the shipped one. // The arm stays so a future attempt at a reproducer has something to aim at, on the // same footing as CN1_NO_BULK_INSERTION_BARRIER. Not a supported configuration. + n = cn1RefDiscoveredTop; for(long i = 0 ; i < n ; i++) { struct CN1RefEntry* e = &cn1RefDiscovered[i]; JAVA_OBJECT r = __atomic_load_n(e->referentField, __ATOMIC_RELAXED); @@ -2562,9 +2541,8 @@ static JAVA_BOOLEAN cn1GcProcessReferences(CODENAME_ONE_THREAD_STATE) { } { // Ages inline, as the single-loop form did. Discovery no longer ages, so - // without this the arm would never age anything, every reference would read as - // permanently touched, and the arm would silently become "retain everything" - // rather than the shape it exists to reproduce. + // without this the arm would never age anything and would silently become + // "retain everything" rather than the shape it exists to reproduce. JAVA_INT age = __atomic_load_n(e->touchAgeField, __ATOMIC_RELAXED); JAVA_INT aged = (age == CN1_REF_TOUCHED) ? 0 : (age < 0x7ffffffe ? age + 1 : age); @@ -2581,9 +2559,11 @@ static JAVA_BOOLEAN cn1GcProcessReferences(CODENAME_ONE_THREAD_STATE) { continue; } #endif - int mark = __atomic_load_n(&r->__codenameOneGcMark, __ATOMIC_ACQUIRE); - if(mark == -1 || mark >= currentGcMarkValue - 1) { - continue; + { + int mark = __atomic_load_n(&r->__codenameOneGcMark, __ATOMIC_ACQUIRE); + if(mark == -1 || mark >= currentGcMarkValue - 1) { + continue; + } } __atomic_store_n(e->referentField, JAVA_NULL, __ATOMIC_RELAXED); } @@ -2599,24 +2579,39 @@ static JAVA_BOOLEAN cn1GcProcessReferences(CODENAME_ONE_THREAD_STATE) { // walked past. It backs up the SATB load barrier in the accessor rather than // duplicating it -- the barrier's log can be dropped on an allocation failure, this // cannot. - for(long i = 0 ; i < n ; i++) { - struct CN1RefEntry* e = &cn1RefDiscovered[i]; - JAVA_OBJECT r = __atomic_load_n(e->referentField, __ATOMIC_RELAXED); - if(r == JAVA_NULL || CN1_IS_TAGGED(r)) { - continue; - } - if(__atomic_load_n(e->touchAgeField, __ATOMIC_RELAXED) == CN1_REF_TOUCHED) { - gcMarkObject(threadStateData, r, JAVA_FALSE); - marked = JAVA_TRUE; + // TO A FIXPOINT, because the drain below can DISCOVER references. Marking a touched + // referent traces it, and anything it reaches runs its own mark function -- so an + // object kept alive only by a touched reference can carry further references that were + // not in the list when this pass started. Snapshotting the length once and clearing + // against that snapshot left those unprocessed: reachable, never cleared, and holding + // a referent the sweep went on to free. The loop ends when a drain adds nothing new, + // which it must, since the set only grows and is bounded by the live set. + for(;;) { + n = cn1RefDiscoveredTop; + JAVA_BOOLEAN markedThisRound = JAVA_FALSE; + for(long i = 0 ; i < n ; i++) { + struct CN1RefEntry* e = &cn1RefDiscovered[i]; + JAVA_OBJECT r = __atomic_load_n(e->referentField, __ATOMIC_RELAXED); + if(r == JAVA_NULL || CN1_IS_TAGGED(r)) { + continue; + } + if(__atomic_load_n(e->touchAgeField, __ATOMIC_RELAXED) == CN1_REF_TOUCHED) { + gcMarkObject(threadStateData, r, JAVA_FALSE); + markedThisRound = JAVA_TRUE; #ifdef CN1_GC_CONFORM - atomic_fetch_add_explicit(&cn1RefKeptTouched, 1, memory_order_relaxed); + atomic_fetch_add_explicit(&cn1RefKeptTouched, 1, memory_order_relaxed); #endif + } + } + // Close the round before deciding anything, so sub-pass B reads settled mark + // words. Without this the entries marked above would still look dead to it. + if(markedThisRound) { + marked = JAVA_TRUE; + gcMarkDrain(threadStateData); + } + if(cn1RefDiscoveredTop == n) { + break; // the drain found no further references } - } - // Close sub-pass A before deciding anything, so sub-pass B reads settled mark words. - // Without this the entries marked above would still look dead to the loop below. - if(marked) { - gcMarkDrain(threadStateData); } // SUB-PASS B: clear on the referent's liveness alone, then age. From 4cc739b6ea8bdce2b0307a9b1509f47c21dba3f7 Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Mon, 7 Sep 2026 21:16:19 +0300 Subject: [PATCH 06/30] Let the emergency clear soft referents without the discovery list Accepted review finding, and it is two of this branch's own fixes colliding at the one moment both were written for. The emergency retention budget is raised by an allocation FAILURE. An allocation failure is also the likeliest reason the discovery list's realloc cannot grow. So the conservative fallback added for weak-reference safety -- mark anything that could not be recorded -- was marking exactly the soft referents the emergency had just condemned, the collection freed nothing, and codenameOneGcMalloc's retry loop had nothing to make progress against. Each fix is right on its own; together they livelock. A soft reference the emergency has condemned needs no list entry, because the decision is already final rather than deferred. Its field is cleared in place, which is the allocation-free path this situation calls for, and it is safe for the same reason the ordinary clear is: a get() that already loaded the referent enqueued it through the armed barrier and keeps it alive for the cycle, and a get() after the store reads null. Weak references, and anything read since the last ageing, still take the marking branch -- that is where a dangling pointer would actually come from, since a mutator may be holding the referent in a local the collector has walked past. The accepted cost is stated at the code: two aliases of one SOFT referent can disagree when only some of them were recorded. That is confined to soft references, which are caches by definition, and the alternative is an allocator that cannot make progress. The callers that read a null get() as proof of death use weak references, which mark. Gates: run-gc-verify.sh green (RefPolicy clean, both fault-injection self-tests firing), run-gauntlet.sh green with all eleven tortures bit-identical and both GC stop modes, and the emergency drop still fires under injected allocation failure. Co-Authored-By: Claude Opus 5 (1M context) --- vm/ByteCodeTranslator/src/cn1_globals.m | 34 ++++++++++++++++++++++++- 1 file changed, 33 insertions(+), 1 deletion(-) diff --git a/vm/ByteCodeTranslator/src/cn1_globals.m b/vm/ByteCodeTranslator/src/cn1_globals.m index 74eb9e2fd42..3c110871747 100644 --- a/vm/ByteCodeTranslator/src/cn1_globals.m +++ b/vm/ByteCodeTranslator/src/cn1_globals.m @@ -2483,7 +2483,39 @@ void cn1GcDiscoverReference(CODENAME_ONE_THREAD_STATE, JAVA_OBJECT ref, JAVA_BOO // Marking is the conservative direction: the referent survives one more cycle, the // reference is rediscovered next time, and by then the list has usually grown. if(!recorded) { - gcMarkObject(threadStateData, __atomic_load_n(referentField, __ATOMIC_RELAXED), force); + // THE EMERGENCY MUST NOT BE DEFEATED BY THE FALLBACK. Marking unconditionally here + // is right for a weak reference and wrong for a soft one at the exact moment it + // matters: the emergency budget is raised by an allocation FAILURE, and an + // allocation failure is also the most likely reason the list could not grow. So + // the two mechanisms met and the conservative one won -- soft referents past the + // list's capacity were marked and kept, the collection freed nothing, and + // codenameOneGcMalloc's retry loop had nothing to make progress against. + // + // A soft reference the emergency has condemned needs no list: the decision is + // already final, so the field can be cleared here and now, which is exactly the + // allocation-free path this situation calls for. It is safe for the same reason + // the ordinary clear is -- a get() that already loaded the referent enqueued it + // through the armed barrier and keeps it alive for this cycle, and a get() after + // this store reads null. + // + // Everything else still takes the conservative branch: weak references, and + // anything read since the last ageing, where a mutator may be holding the referent + // in a local the collector has walked past. That leaves the possibility of two + // aliases of one soft referent disagreeing when only some were recorded -- accepted + // deliberately, because it is confined to soft references, which are caches by + // definition, and the alternative is an allocator that cannot make progress. The + // lifetime-oracle callers use weak references, which take the marking branch. + JAVA_OBJECT r = __atomic_load_n(referentField, __ATOMIC_RELAXED); + if(strength == CN1_REF_SOFT + && atomic_load_explicit(&cn1SoftRetainCycles, memory_order_relaxed) < 0 + && __atomic_load_n(touchAgeField, __ATOMIC_RELAXED) != CN1_REF_TOUCHED) { + __atomic_store_n(referentField, JAVA_NULL, __ATOMIC_RELAXED); +#ifdef CN1_GC_CONFORM + atomic_fetch_add_explicit(&cn1RefCleared, 1, memory_order_relaxed); +#endif + } else { + gcMarkObject(threadStateData, r, force); + } return; } #ifdef CN1_GC_CONFORM From 67f8b8635ea5557ede9f1cfc39d072a1dc8a69a1 Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Mon, 7 Sep 2026 21:31:45 +0300 Subject: [PATCH 07/30] Hold the SATB registration across the referent load Two accepted review findings. The P1 is this branch's own optimisation undoing its own fix, which is worth naming plainly. REGISTRATION MUST SPAN THE LOAD. The previous commit made the get() barrier register through cn1SatbBulkBegin, whose entire value is that it registers FIRST and reports whether logging is needed afterwards -- and then gated entry to it on an outer read of the same two flags, as a fast path. That puts the race back one level out: a thread can be descheduled between the outer read and the registration, and the collector can clear the field, finish its empty final take, lower gcSatbTerminating and quiesce with the in-flight count still zero. The enqueue is then declined and the sweep frees the referent get() is about to return. Nothing may be sampled before the registration, so the accessor now brackets the whole load: CN1_REF_LOAD_BEGIN, load, CN1_SATB_REF_KEEP, CN1_REF_LOAD_END. The cost is two seq_cst read-modify-writes on every Reference.get(), paid unconditionally, and there is no sound way to skip them: any flag consulted before registering can go stale in exactly the window the registration exists to close. With it held across the load a false answer is safe rather than merely unlikely -- the collector cannot be mid-termination because its quiesce waits for this registration, so either no mark is running and a later one scans this thread with the value already in a register, or reference processing is complete and a field still holding a pointer was not condemned. THE OOM FALLBACK MUST NOT CLEAR A STRONGLY REACHABLE REFERENT. The emergency clear added last commit did not consult the mark word, so an application holding both an ordinary field and a SoftReference to one object could watch get() answer null under allocation pressure for an object that was never softly reachable. It now skips anything already marked. Partial by construction, and said so at the code: the mark is still running there, so a referent a strong edge reaches LATER in the cycle is not yet marked and can still be cleared. Being certain would mean deferring to the clear pass, which is precisely what that path exists because it cannot do. Gates: run-gc-verify.sh green (RefPolicy clean over 69 verify passes, both fault-injection self-tests firing), run-gauntlet.sh green with all eleven tortures bit-identical and both GC stop modes. Both the -DCN1_DISABLE_SATB and default arms build and run; an earlier revision of this commit left the header's conditional nesting unbalanced, which is why that arm is now checked explicitly. Co-Authored-By: Claude Opus 5 (1M context) --- vm/ByteCodeTranslator/src/cn1_globals.h | 70 ++++++++----------- vm/ByteCodeTranslator/src/cn1_globals.m | 21 +++++- .../tools/translator/ByteCodeClass.java | 18 ++--- 3 files changed, 61 insertions(+), 48 deletions(-) diff --git a/vm/ByteCodeTranslator/src/cn1_globals.h b/vm/ByteCodeTranslator/src/cn1_globals.h index 7ab8fefcb14..bef3248da63 100644 --- a/vm/ByteCodeTranslator/src/cn1_globals.h +++ b/vm/ByteCodeTranslator/src/cn1_globals.h @@ -3130,51 +3130,43 @@ extern void cn1GcDiscoverReference(CODENAME_ONE_THREAD_STATE, JAVA_OBJECT ref, J // // A retained soft reference costs nothing here at all, because its referent is marked as // an ordinary strong edge by cn1GcDiscoverReference before any get() can reach it. -// KEEPS A VALUE THE CALLER HAS ALREADY LOADED, rather than loading its own. -// -// The accessor loads the referent ONCE, into a local, and hands that same value here -- -// so what the barrier acts on is exactly what get() returns. The previous shape checked -// gcSatbActive and only then loaded, leaving the accessor to load again afterwards, and -// the gap between the two was a real hole: a thread that read the flag as 0, was then -// SIGUSR2-frozen, scanned (with the referent not yet in any register or stack slot), -// released, and only then performed its load, came away holding an unmarked referent that -// nothing had enqueued. Loading first closes it without a barrier, because the value is -// in a register before the freeze can happen and the conservative root scan covers -// registers and the native stack. -#if defined(CN1_DISABLE_SATB) -#define CN1_SATB_REF_KEEP(refVal) do { (void)(refVal); } while(0) -#else -// REGISTERS BEFORE IT LOGS, via the same handshake the bulk copies use. +// REGISTERS AROUND THE LOAD, via the same handshake the bulk copies use, and the +// registration is what the caller must hold ACROSS its load -- hence the awkward shape: +// the accessor calls cn1RefLoadBegin(), loads, calls this, then cn1RefLoadEnd(). // // Checking gcSatbActive and then enqueuing is not enough on this path, and the reason it // is enough for the per-store barrier does not carry over. cn1SatbEnqueue takes a mutex, -// so a thread can pass the flag check and then be delayed acquiring it long enough for the -// collector to clear the flag, take an empty final batch and finish termination -- after -// which the entry lands in a log nothing will ever drain. The per-store barrier tolerates -// exactly that window because a reference STORED after the fixpoint is already marked or -// FRESH and the sweep keeps both; a weak REFERENT handed out by get() is neither, which is -// the whole reason this barrier exists. +// so a thread can pass a flag check and then be delayed long enough for the collector to +// clear the field, finish its empty final take, lower gcSatbTerminating and quiesce -- and +// the entry then lands in a log nothing will ever drain, or is skipped entirely, while the +// sweep frees the referent the caller is about to return. The per-store barrier tolerates +// that window because a reference STORED after the fixpoint is already marked or FRESH and +// the sweep keeps both; a weak REFERENT handed out by get() is neither. // -// cn1SatbBulkBegin registers unconditionally and only then reports whether logging is -// needed, so cn1SatbBulkQuiesce cannot complete while this is in flight, and the collector -// cannot finish its final take underneath it. gcSatbTerminating stays raised across the -// whole termination loop -- including reference processing -- so the registration covers -// the window that matters. +// AN OUTER "FAST PATH" FLAG CHECK BREAKS THIS, and did: gating entry to +// cn1SatbBulkBegin() on a prior read of the same flags reintroduces the race one level +// out, because the thread can be descheduled between that read and the registration. The +// whole value of cn1SatbBulkBegin is that it registers FIRST and reports afterwards, so +// nothing may be sampled before it. // -// The fast path is unchanged off-GC: two predicted-not-taken flag loads. With both flags -// down the mark is over, every reference is either cleared or holds a marked referent, and -// a cycle starting afterwards will scan this thread with the value already in a register. -#define CN1_SATB_REF_KEEP(refVal) \ +// With the registration held across the load, a false answer is safe rather than merely +// unlikely: the collector cannot be mid-termination (its quiesce waits for this +// registration), so either no mark is running -- and one starting later scans this thread +// with the value already in a register -- or reference processing is complete, in which +// case a field still holding a pointer was not condemned and its referent is marked. +#if defined(CN1_DISABLE_SATB) +#define CN1_REF_LOAD_BEGIN() JAVA_FALSE +#define CN1_REF_LOAD_END() do { } while(0) +#define CN1_SATB_REF_KEEP(active, refVal) do { (void)(active); (void)(refVal); } while(0) +#else +#define CN1_REF_LOAD_BEGIN() cn1SatbBulkBegin() +#define CN1_REF_LOAD_END() cn1SatbBulkEnd() +#define CN1_SATB_REF_KEEP(active, refVal) \ do { JAVA_OBJECT cn1__r = (refVal); \ - if(cn1__r != JAVA_NULL && !CN1_IS_TAGGED(cn1__r) \ - && (__builtin_expect(gcSatbActive, 0) \ - || __builtin_expect(gcSatbTerminating, 0))) { \ - if(cn1SatbBulkBegin()) { \ - int cn1__m = __atomic_load_n(&cn1__r->__codenameOneGcMark, __ATOMIC_RELAXED); \ - int cn1__e = atomic_load_explicit(&bibopGcEpoch, memory_order_relaxed); \ - if(cn1__m != -1 && cn1__m != cn1__e) cn1SatbEnqueue(cn1__r); \ - } \ - cn1SatbBulkEnd(); \ + if((active) && cn1__r != JAVA_NULL && !CN1_IS_TAGGED(cn1__r)) { \ + int cn1__m = __atomic_load_n(&cn1__r->__codenameOneGcMark, __ATOMIC_RELAXED); \ + int cn1__e = atomic_load_explicit(&bibopGcEpoch, memory_order_relaxed); \ + if(cn1__m != -1 && cn1__m != cn1__e) cn1SatbEnqueue(cn1__r); \ } } while(0) #endif diff --git a/vm/ByteCodeTranslator/src/cn1_globals.m b/vm/ByteCodeTranslator/src/cn1_globals.m index 3c110871747..3a645e4f870 100644 --- a/vm/ByteCodeTranslator/src/cn1_globals.m +++ b/vm/ByteCodeTranslator/src/cn1_globals.m @@ -2506,7 +2506,26 @@ void cn1GcDiscoverReference(CODENAME_ONE_THREAD_STATE, JAVA_OBJECT ref, JAVA_BOO // definition, and the alternative is an allocator that cannot make progress. The // lifetime-oracle callers use weak references, which take the marking branch. JAVA_OBJECT r = __atomic_load_n(referentField, __ATOMIC_RELAXED); - if(strength == CN1_REF_SOFT + // NOT IF IT IS ALREADY MARKED. A soft reference may only be cleared when its + // referent is softly reachable, and an object already marked at this point is + // reachable some other way -- through an ordinary strong edge, or as a root. An + // application holding both a field and a SoftReference to one object would + // otherwise watch get() answer null under allocation pressure for an object that + // was never a candidate for collection at all. + // + // Partial, and deliberately so: the mark is still in progress here, so a referent + // that a strong edge reaches LATER in this cycle is not yet marked and can still be + // cleared. Being sure would mean deferring to the clear pass, which is precisely + // what this path exists because it cannot do -- the list is full and cannot grow. + // Under genuine exhaustion the residue is a spurious cache miss on an object that + // stays alive, against an allocator that otherwise cannot make progress. + JAVA_BOOLEAN alreadyLive = JAVA_FALSE; + if(r != JAVA_NULL && !CN1_IS_TAGGED(r)) { + int rm = __atomic_load_n(&r->__codenameOneGcMark, __ATOMIC_ACQUIRE); + alreadyLive = (rm == currentGcMarkValue || rm == -1) ? JAVA_TRUE : JAVA_FALSE; + } + if(!alreadyLive + && strength == CN1_REF_SOFT && atomic_load_explicit(&cn1SoftRetainCycles, memory_order_relaxed) < 0 && __atomic_load_n(touchAgeField, __ATOMIC_RELAXED) != CN1_REF_TOUCHED) { __atomic_store_n(referentField, JAVA_NULL, __ATOMIC_RELAXED); diff --git a/vm/ByteCodeTranslator/src/com/codename1/tools/translator/ByteCodeClass.java b/vm/ByteCodeTranslator/src/com/codename1/tools/translator/ByteCodeClass.java index a1a449d6bd4..bb74cabf0eb 100644 --- a/vm/ByteCodeTranslator/src/com/codename1/tools/translator/ByteCodeClass.java +++ b/vm/ByteCodeTranslator/src/com/codename1/tools/translator/ByteCodeClass.java @@ -1051,23 +1051,25 @@ public String generateCCode(List allClasses) { // snapshot, and the trial clear of gcSatbActive then finds a non-empty log, // re-arms, marks it, and leaves the reference alone. // - // Loading BEFORE the barrier check rather than inside it is what makes that - // sound. Checking the flag first and loading afterwards leaves a gap: a - // thread that read the flag as 0, was SIGUSR2-frozen with the referent not - // yet in any register, scanned, released, and only then loaded, came away - // with an unmarked referent nothing had enqueued. With the load first the - // value is in a register before any freeze, where the conservative root scan - // finds it. + // REGISTERING BEFORE THE LOAD, and holding it across, is what makes that + // sound. Any flag sampled before registering can go stale in the gap: a + // thread that read the flag as 0 -- or read it as 1 and was then descheduled + // before registering -- can come away with an unmarked referent nothing + // enqueued, while the collector finishes termination and sweeps it. + // CN1_REF_LOAD_BEGIN registers first and answers afterwards, so the + // collector's quiesce cannot complete anywhere inside this accessor. + b.append("JAVA_BOOLEAN __cn1RefActive = CN1_REF_LOAD_BEGIN();\n "); b.append(fld.getCDefinition()).append(" __cn1Ref = __atomic_load_n(&((struct obj__") .append(clsName).append("*)__cn1T)->") .append(fld.getClsName()).append("_").append(fld.getFieldName()) .append(", __ATOMIC_RELAXED);\n "); - b.append("CN1_SATB_REF_KEEP(__cn1Ref);\n "); + b.append("CN1_SATB_REF_KEEP(__cn1RefActive, __cn1Ref);\n "); // The touch stamp, and the entire per-read cost of ranking soft references // by use: a store of an immediate. Unconditional rather than guarded by a // "did it change" test, because the branch would cost more than the store. b.append("__atomic_store_n(&((struct obj__").append(clsName).append("*)__cn1T)->") .append(REFERENCE_CLASS).append("_cn1TouchAge, CN1_REF_TOUCHED, __ATOMIC_RELAXED);\n "); + b.append("CN1_REF_LOAD_END();\n "); b.append("return __cn1Ref;\n}\n\n"); } else if (fld.isVolatile()) { b.append("return atomic_load_explicit(&((struct obj__"); From b343356782f706450c45b363cf3a175123dc3b9a Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Mon, 7 Sep 2026 21:46:45 +0300 Subject: [PATCH 08/30] Finish the referent's atomic accesses, and stop clearing after a dropped log entry Two accepted review findings, both leftovers of earlier fixes on this branch rather than new ground. HALF A RACE WAS FIXED. Making the referent's accesses atomic converted the getter and the setter's own store but left CN1_SATB_DELETE, emitted immediately before that store, reading the same field through a plain JAVA_OBJECT volatile*. The collector writes JAVA_NULL there atomically from cn1GcProcessReferences, so the pair stayed a mixed atomic/non-atomic access -- the exact defect the earlier change set out to remove. CN1_SATB_DELETE_REF is the deletion barrier with an atomic load, used for this field only; every other field keeps the generic macro, which is correct for them because nothing else writes them concurrently. A DROPPED LOG ENTRY VOIDS THE CLEAR PASS'S EVIDENCE. cn1SatbEnqueue discards a reference when its stack cannot grow. The comment there argues that is survivable, and for an ordinary STORE it is -- "only re-opens the original race". It is not survivable for a referent Reference.get() has already handed to a mutator: the enqueue was the only record that it escaped, sub-pass B decides purely on mark state, and the final take stays empty so nothing re-opens, leaving the sweep free to reclaim an object a thread is holding. Registering the load in the termination handshake does not help here; it delays the take, it does not stop a drop. Nothing on that path can allocate its way out, so the pass now snapshots the drop count at cycle start and declines to clear at all if it moved. That costs one cycle of reclaim in a process that is already out of memory, and it deliberately does not disable the emergency soft-drop, which clears at DISCOVERY and never touches the log. Gates: run-gc-verify.sh green (RefPolicy clean over 71 verify passes, both fault-injection self-tests firing), run-gauntlet.sh green with all eleven tortures bit-identical and both GC stop modes; the emergency drop still fires under injected allocation failure and the driver's weak, alias and cache assertions are unchanged. Co-Authored-By: Claude Opus 5 (1M context) --- vm/ByteCodeTranslator/src/cn1_globals.h | 19 +++++++++ vm/ByteCodeTranslator/src/cn1_globals.m | 39 ++++++++++++++++++- .../tools/translator/ByteCodeClass.java | 6 ++- 3 files changed, 62 insertions(+), 2 deletions(-) diff --git a/vm/ByteCodeTranslator/src/cn1_globals.h b/vm/ByteCodeTranslator/src/cn1_globals.h index bef3248da63..7da3b264380 100644 --- a/vm/ByteCodeTranslator/src/cn1_globals.h +++ b/vm/ByteCodeTranslator/src/cn1_globals.h @@ -3154,6 +3154,25 @@ extern void cn1GcDiscoverReference(CODENAME_ONE_THREAD_STATE, JAVA_OBJECT ref, J // registration), so either no mark is running -- and one starting later scans this thread // with the value already in a register -- or reference processing is complete, in which // case a field still holding a pointer was not condemned and its referent is marked. +// The deletion barrier for the referent field, with an ATOMIC load. +// +// CN1_SATB_DELETE next door reads through a plain JAVA_OBJECT volatile*, which is right +// for every ordinary field because nothing else writes them concurrently. The referent is +// the exception: cn1GcProcessReferences stores JAVA_NULL into it atomically from the +// collector while Reference.clear() runs here, so the plain read would leave that pair a +// mixed atomic/non-atomic access -- undefined in C, and the same defect that was fixed for +// the getter and for this setter's own store. Making the store atomic and leaving the +// barrier's read plain fixes half a race. +#if defined(CN1_DISABLE_SATB) +#define CN1_SATB_DELETE_REF(fieldAddr) do { } while(0) +#else +#define CN1_SATB_DELETE_REF(fieldAddr) \ + do { if(__builtin_expect(gcSatbActive, 0)) { \ + JAVA_OBJECT cn1__old = __atomic_load_n((JAVA_OBJECT*)(fieldAddr), __ATOMIC_RELAXED); \ + if(cn1__old != JAVA_NULL && !CN1_IS_TAGGED(cn1__old)) cn1SatbEnqueue(cn1__old); \ + } } while(0) +#endif + #if defined(CN1_DISABLE_SATB) #define CN1_REF_LOAD_BEGIN() JAVA_FALSE #define CN1_REF_LOAD_END() do { } while(0) diff --git a/vm/ByteCodeTranslator/src/cn1_globals.m b/vm/ByteCodeTranslator/src/cn1_globals.m index 3a645e4f870..316b6465b98 100644 --- a/vm/ByteCodeTranslator/src/cn1_globals.m +++ b/vm/ByteCodeTranslator/src/cn1_globals.m @@ -1690,6 +1690,10 @@ static void cn1DrainDeadThreadPending() { static long gcSatbTop = 0; // guarded by gcSatbMutex static long gcSatbCap = 0; static pthread_mutex_t gcSatbMutex = PTHREAD_MUTEX_INITIALIZER; +// Entries cn1SatbEnqueue could not record because its stack would not grow. Read by the +// reference pass, which cannot act on a snapshot that may be missing a referent a mutator +// has already been handed. +_Atomic long cn1SatbDrops = 0; // Monotonic count of objects transitioned unmarked->marked this process; the SATB // drain snapshots it around a batch to detect "marked nothing new" (fixpoint). long gcMarkNewObjectCount = 0; @@ -2061,7 +2065,16 @@ void cn1SatbEnqueue(JAVA_OBJECT old) { if(gcSatbTop >= gcSatbCap) { long ncap = gcSatbCap ? gcSatbCap * 2 : 8192; JAVA_OBJECT* n = (JAVA_OBJECT*)realloc(gcSatbStack, (size_t)ncap * sizeof(JAVA_OBJECT)); - if(n == 0) { pthread_mutex_unlock(&gcSatbMutex); return; } // OOM: drop (rare; only re-opens the original race) + if(n == 0) { + // OOM: drop. For an ordinary STORE that only re-opens the original race and is + // survivable. For a weak REFERENT taken out by Reference.get() it is not -- + // the clear pass would decide on stale liveness and the sweep would free an + // object a mutator is holding -- so record that it happened; the reference + // pass reads this and declines to clear anything this cycle. + atomic_fetch_add_explicit(&cn1SatbDrops, 1, memory_order_relaxed); + pthread_mutex_unlock(&gcSatbMutex); + return; + } gcSatbStack = n; gcSatbCap = ncap; } gcSatbStack[gcSatbTop++] = old; @@ -2288,6 +2301,11 @@ JAVA_LONG GcVerifyApp_gcMarkState___R_long(CODENAME_ONE_THREAD_STATE) { // cache by however many times the mutator happened to storm the reference log. static int cn1RefAgedCycle = 0; +// cn1SatbDrops as it stood when this cycle began. If it has moved by the time the clear +// pass runs, some referent handed to a mutator never reached the log and the pass has no +// sound basis for clearing anything. +static long cn1RefDropsAtCycleStart = 0; + // Called from the allocation-failure path. Idempotent, allocation-free and safe from a // thread that is about to park -- a relaxed store and nothing else. void cn1RefDropAllSoftReferents(void) { @@ -2308,6 +2326,7 @@ void cn1RefDropAllSoftReferents(void) { // codenameOneGCMark before anything can mark. static void cn1RefBeginCycle(void) { cn1RefDiscoveredTop = 0; + cn1RefDropsAtCycleStart = atomic_load_explicit(&cn1SatbDrops, memory_order_relaxed); #ifdef CN1_GC_CONFORM // PER CYCLE, like every other figure in [GCPROBE]. A running total cannot show // whether the budget is tracking memory pressure, which is the whole question the @@ -2665,6 +2684,24 @@ static JAVA_BOOLEAN cn1GcProcessReferences(CODENAME_ONE_THREAD_STATE) { } } + // A DROPPED LOG ENTRY VOIDS THIS PASS'S EVIDENCE. cn1SatbEnqueue silently discards a + // reference when its stack cannot grow, which for an ordinary store is survivable -- + // the comment there says so -- but not for a referent Reference.get() has already + // handed to a mutator: the enqueue was the only record that it escaped, sub-pass B + // decides purely on mark state, and the final take stays empty so nothing re-opens. + // The result would be the sweep freeing an object a thread is holding. + // + // Nothing here can allocate its way out of that, so the pass declines to clear for the + // cycle. It costs one cycle of reclaim in a situation where the process is already out + // of memory, and it does NOT disable the emergency drop, which clears at DISCOVERY and + // never touches the log. + if(atomic_load_explicit(&cn1SatbDrops, memory_order_relaxed) != cn1RefDropsAtCycleStart) { +#ifdef CN1_GC_CONFORM + cn1RefPhaseNs += cn1GcNowNs() - __r0; +#endif + return marked; + } + // SUB-PASS B: clear on the referent's liveness alone, then age. // // The ageing happens HERE, once per cycle, rather than at discovery -- see the comment diff --git a/vm/ByteCodeTranslator/src/com/codename1/tools/translator/ByteCodeClass.java b/vm/ByteCodeTranslator/src/com/codename1/tools/translator/ByteCodeClass.java index bb74cabf0eb..5701065e0b8 100644 --- a/vm/ByteCodeTranslator/src/com/codename1/tools/translator/ByteCodeClass.java +++ b/vm/ByteCodeTranslator/src/com/codename1/tools/translator/ByteCodeClass.java @@ -1104,7 +1104,11 @@ public String generateCCode(List allClasses) { b.append("CN1_WRITE_BARRIER(__cn1T, __cn1Val); "); // SATB deletion barrier: preserve the reference being overwritten for the // current mark cycle. No-op (single flag load) outside GC. - b.append("CN1_SATB_DELETE(&((struct obj__").append(clsName).append("*)__cn1T)->") + // The referent takes the ATOMIC deletion barrier: the collector stores + // JAVA_NULL into that field concurrently, so the generic macro's plain + // volatile read would leave the pair a mixed atomic/non-atomic access. + b.append(isReferenceReferent(clsName, fld) ? "CN1_SATB_DELETE_REF" : "CN1_SATB_DELETE") + .append("(&((struct obj__").append(clsName).append("*)__cn1T)->") .append(fld.getClsName()).append("_").append(fld.getFieldName()).append("); "); } else { b.append(" __cn1Val, JAVA_OBJECT __cn1T) {\n ").append(nullCheck).append(" "); From aa8cdafd70cf5193e8e6714bf322ee0bb869ee18 Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Mon, 7 Sep 2026 22:02:10 +0300 Subject: [PATCH 09/30] Count take-side SATB losses, and stop clearing when SATB is compiled out Two accepted review findings, and one pre-existing bug found next to the first. THE DROP COUNTER GUARDED HALF THE LOSS PATH. The previous commit invalidated reference clearing when cn1SatbEnqueue discarded an entry, but cn1SatbTake discards too: it resets gcSatbTop unconditionally and, when its scratch buffer cannot be grown, reports the batch as EMPTY. Entries that were logged successfully are then thrown away and the collector reads "nothing slipped in", which is exactly the signal that lets termination finish. To the referent that gets swept a lost enqueue and a lost batch are the same event, so the take now counts as a drop as well. PRE-EXISTING, in the same function and worth its own paragraph: realloc's result was assigned straight back over scratch, so a failure lost the buffer that was already there, and scratchCap was advanced whether or not the growth succeeded. After a single failure every later take saw n <= scratchCap, skipped the realloc, found scratch NULL and returned 0 -- the barrier logging into a stack nothing would ever drain again, permanently and silently. It now grows through a temporary and advances the cap only on success. -DCN1_DISABLE_SATB WAS UNSOUND, NOT MERELY SLOWER. That arm compiles out the load barrier that makes handing a weak referent to a mutator safe, but reference processing is not part of SATB and kept running: a thread released after its stack scan could load a referent, have the field cleared underneath it and the object swept before it could use the pointer. Reference processing now switches off with the barrier, degrading to the behaviour that preceded this feature -- referents traced strongly and never cleared -- which is the right fallback for an escape hatch and keeps the arm measuring the barrier's cost rather than a different collector. Verified rather than assumed: WEAK_DEAD_CLEARED reads 0/256 in that arm against 255/256 by default. Gates: run-gc-verify.sh green (RefPolicy clean over 70 verify passes, both fault-injection self-tests firing), run-gauntlet.sh green with all eleven tortures bit-identical and both GC stop modes. Co-Authored-By: Claude Opus 5 (1M context) --- vm/ByteCodeTranslator/src/cn1_globals.m | 35 +++++++++++++++++++++++-- 1 file changed, 33 insertions(+), 2 deletions(-) diff --git a/vm/ByteCodeTranslator/src/cn1_globals.m b/vm/ByteCodeTranslator/src/cn1_globals.m index 316b6465b98..d05d4fac632 100644 --- a/vm/ByteCodeTranslator/src/cn1_globals.m +++ b/vm/ByteCodeTranslator/src/cn1_globals.m @@ -2089,10 +2089,26 @@ static long cn1SatbTake(JAVA_OBJECT** out) { static JAVA_OBJECT* scratch = 0; static long scratchCap = 0; if(n > scratchCap) { long nc = n < 8192 ? 8192 : n; - scratch = (JAVA_OBJECT*)realloc(scratch, (size_t)nc * sizeof(JAVA_OBJECT)); - scratchCap = nc; + // Through a TEMPORARY. Assigning realloc's result straight back loses the existing + // buffer on failure, and advancing scratchCap alongside it made that permanent: + // every later take saw n <= scratchCap, skipped the realloc, found scratch NULL and + // returned 0, so the barrier kept logging into a stack nothing ever drained again. + JAVA_OBJECT* grown = (JAVA_OBJECT*)realloc(scratch, (size_t)nc * sizeof(JAVA_OBJECT)); + if(grown != 0) { + scratch = grown; + scratchCap = nc; + } } if(n > 0 && scratch != 0) memcpy(scratch, gcSatbStack, (size_t)n * sizeof(JAVA_OBJECT)); + // TAKE-SIDE LOSS COUNTS AS A DROP TOO. gcSatbTop is reset either way, so entries that + // were successfully logged are discarded here when the scratch buffer could not be + // grown -- and the caller is told the batch was empty, which reads as "termination can + // finish". For a referent Reference.get() has handed out that is the same hazard as a + // failed enqueue and has to invalidate reference clearing the same way; a lost enqueue + // and a lost batch are indistinguishable to the object that gets swept. + if(n > 0 && scratch == 0) { + atomic_fetch_add_explicit(&cn1SatbDrops, 1, memory_order_relaxed); + } gcSatbTop = 0; pthread_mutex_unlock(&gcSatbMutex); *out = scratch; @@ -2240,6 +2256,21 @@ JAVA_LONG GcVerifyApp_gcMarkState___R_long(CODENAME_ONE_THREAD_STATE) { // 1 - never: a soft reference is as strong as a field. The upper bound on hit rate // and the upper bound on footprint. // 2 - ranked by use (default). +// WITHOUT THE SATB BARRIER THERE IS NO SAFE WAY TO CLEAR, so this configuration does not. +// +// -DCN1_DISABLE_SATB is the documented escape hatch for A/B-ing the barrier's cost or +// falling back if it regresses. It compiles out the load barrier that makes handing a weak +// referent to a mutator safe -- but the clear pass is not part of SATB and would keep +// running, so a thread released after its stack scan could load a referent, have the field +// cleared underneath it and the object swept before it could use the pointer. Reference +// processing therefore turns itself off with the barrier, degrading to the behaviour that +// preceded this feature: referents traced strongly and never cleared. That is the right +// fallback for an escape hatch, and it keeps the arm measuring barrier cost rather than +// measuring a different collector. +#if defined(CN1_DISABLE_SATB) && !defined(CN1_NO_WEAK_REFS) +#define CN1_NO_WEAK_REFS 1 +#endif + #ifndef CN1_REF_POLICY #define CN1_REF_POLICY 2 #endif From 99504d075363c858b6e93c182ae32f5efe3cd80e Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Mon, 7 Sep 2026 22:27:59 +0300 Subject: [PATCH 10/30] Stop the verifier being blind to referents, and close a heap overflow Two accepted review findings. One is a heap overflow this branch introduced an hour ago; the other says the gate that was supposed to catch such things could not see the field in question at all. THE OVERFLOW WAS MINE. Growing cn1SatbTake's scratch buffer through a temporary keeps the old allocation when realloc fails -- which was the point, the previous shape leaked it -- and therefore leaves scratch NON-NULL and SMALLER than n. The guard still tested only scratch != 0, so the memcpy wrote n entries into an allocation sized for fewer: heap corruption written by the collector under memory pressure, strictly worse than the leak it replaced. The buffer is usable only if it exists AND scratchCap >= n. THE VERIFIER COULD NOT SEE THE REFERENT. cn1GcVerifyHeap walks survivors through the generated mark functions and relies on every reference field reaching gcMarkObject, whose verify branch classifies it. Suppressing that call for the referent -- the very thing that makes the edge weak -- also took the referent out of the verifier's reach, so a live Reference holding a pointer into reclaimed memory passed with violations=0. Every "clean over N verify passes" recorded on this branch before now was silent about the referent specifically, which is the one thing this work risks. cn1GcDiscoverReference now routes it to cn1GcVerifyChild, ahead of the ageing and the dedupe: a verify walk is not a collection cycle, and letting it age references or consume the dedupe stamp would corrupt what the next real cycle reads and drop exactly the repeat visits a whole-heap walk produces. PROVEN, not assumed. CN1_GC_FAULT=refnoclear leaves a dead referent in its field instead of clearing it, and the verifier reports DANGLING REFERENCE; clean it reports none. run-gc-verify.sh gains self-test3 so this cannot go quietly blind again. Note the obvious-looking fault is the wrong one and was tried first: clearing MORE references than liveness warrants only produces extra nulls, which are safe, and it reported violations=0 -- stopping there would have "confirmed" the hook while proving nothing. The dangling direction is clearing LESS. The fault's use is inside CN1_GC_VERIFY because the cn1GcFault* family is declared there. Unguarded it broke every ordinary build while the verifier build -- the one configuration in which the symbol exists -- kept passing, so all three shapes are now built explicitly: plain, CN1_GC_CONFORM and CN1_GC_VERIFY. Gates: run-gc-verify.sh green with all three self-tests firing, run-gauntlet.sh green with all eleven tortures bit-identical and both GC stop modes. Co-Authored-By: Claude Opus 5 (1M context) --- vm/ByteCodeTranslator/src/cn1_globals.m | 60 +++++++++++++++++++++++-- vm/benchmarks/run-gc-verify.sh | 36 +++++++++++++++ 2 files changed, 93 insertions(+), 3 deletions(-) diff --git a/vm/ByteCodeTranslator/src/cn1_globals.m b/vm/ByteCodeTranslator/src/cn1_globals.m index d05d4fac632..fabfa665069 100644 --- a/vm/ByteCodeTranslator/src/cn1_globals.m +++ b/vm/ByteCodeTranslator/src/cn1_globals.m @@ -104,6 +104,17 @@ // CN1_GC_FAULT=earlyfree restores the pre-fix O(1) page-reclaim bound // (gcLastMarkedEpoch != V), which frees slots the per-slot walk would keep. int cn1GcFaultEarlyFree = 0; +// CN1_GC_FAULT=refnoclear skips the clear for a referent the sweep is about to free, so a +// reachable Reference is left holding a pointer into reclaimed memory. That is the exact +// hazard this feature's whole risk reduces to, and until cn1GcDiscoverReference routed the +// referent to cn1GcVerifyChild the verifier could not see it at all -- such a Reference +// passed with zero violations. This fault is how that hook is shown to have teeth, on the +// same footing as nograce and earlyfree. +// +// Note the obvious-looking fault is the wrong one: clearing MORE references than liveness +// warrants only produces extra nulls, which are safe, and an attempt at it reported +// violations=0 for exactly that reason. The dangling direction is clearing LESS. +int cn1GcFaultRefClear = 0; void cn1GcFaultInitPublic(void); static void cn1GcFaultInit(void) { static int done = 0; @@ -117,6 +128,9 @@ static void cn1GcFaultInit(void) { } else if(strcmp(f, "earlyfree") == 0) { cn1GcFaultEarlyFree = 1; fprintf(stderr, "[GC-FAULT] O(1) page reclaim restored to the pre-fix bound\n"); + } else if(strcmp(f, "refnoclear") == 0) { + cn1GcFaultRefClear = 1; + fprintf(stderr, "[GC-FAULT] dead referents left in place instead of cleared\n"); } else { fprintf(stderr, "[GC-FAULT] unknown fault '%s'\n", f); } @@ -2099,20 +2113,29 @@ static long cn1SatbTake(JAVA_OBJECT** out) { scratchCap = nc; } } - if(n > 0 && scratch != 0) memcpy(scratch, gcSatbStack, (size_t)n * sizeof(JAVA_OBJECT)); + // CAPACITY, NOT JUST NON-NULLNESS. Growing through a temporary keeps the old buffer on + // failure, which is what the leak fix wanted -- and it means a FAILED growth leaves + // scratch non-null but SMALLER than n. Testing only scratch != 0 then memcpy'd n + // entries into an allocation sized for fewer: a heap overflow written by the collector + // under memory pressure, which is a far worse failure than the leak it replaced. The + // buffer is usable only if it exists AND is big enough. + JAVA_BOOLEAN usable = (scratch != 0 && scratchCap >= n) ? JAVA_TRUE : JAVA_FALSE; + if(n > 0 && usable) { + memcpy(scratch, gcSatbStack, (size_t)n * sizeof(JAVA_OBJECT)); + } // TAKE-SIDE LOSS COUNTS AS A DROP TOO. gcSatbTop is reset either way, so entries that // were successfully logged are discarded here when the scratch buffer could not be // grown -- and the caller is told the batch was empty, which reads as "termination can // finish". For a referent Reference.get() has handed out that is the same hazard as a // failed enqueue and has to invalidate reference clearing the same way; a lost enqueue // and a lost batch are indistinguishable to the object that gets swept. - if(n > 0 && scratch == 0) { + if(n > 0 && !usable) { atomic_fetch_add_explicit(&cn1SatbDrops, 1, memory_order_relaxed); } gcSatbTop = 0; pthread_mutex_unlock(&gcSatbMutex); *out = scratch; - return (scratch != 0) ? n : 0; + return usable ? n : 0; } void cn1RefreshFreeMemCache(void); // defined near cn1BibopMaybeGc; drives the dynamic pacing cap @@ -2447,6 +2470,28 @@ void cn1GcDiscoverReference(CODENAME_ONE_THREAD_STATE, JAVA_OBJECT ref, JAVA_BOO || __atomic_load_n(referentField, __ATOMIC_RELAXED) == JAVA_NULL) { return; // already cleared: nothing to decide } +#ifdef CN1_GC_VERIFY + // THE VERIFIER HAS TO SEE THE REFERENT, and it could not. + // + // cn1GcVerifyHeap walks the surviving objects through the SAME generated mark + // functions, relying on every reference field arriving at gcMarkObject, whose verify + // branch classifies it. Suppressing that call for the referent -- which is what makes + // the edge weak -- also took the referent out of the verifier's reach, so a live + // Reference holding a pointer into reclaimed memory passed with zero violations. That + // is precisely the defect this collector work most needs the verifier to catch, and + // every "clean over N verify passes" result recorded on this branch before this hook + // existed was silent about the referent specifically. + // + // Answered here, ahead of the ageing and the dedupe: a verify walk is not a collection + // cycle, and letting it age references or mark cn1AgedCycle would corrupt the state the + // next real cycle reads -- and the dedupe would drop the second and later visits, which + // are exactly the ones a walk of the whole heap produces. + if(cn1GcVerifyActive) { + cn1GcVerifyChild(__atomic_load_n(referentField, __ATOMIC_RELAXED), + __builtin_return_address(0)); + return; + } +#endif #ifdef CN1_NO_WEAK_REFS // ABLATION ARM: trace the referent strongly and never clear anything, which is // exactly what this VM did before references were implemented. It exists so the @@ -2767,6 +2812,15 @@ static JAVA_BOOLEAN cn1GcProcessReferences(CODENAME_ONE_THREAD_STATE) { if(mark == -1 || mark >= currentGcMarkValue - 1) { continue; } +#ifdef CN1_GC_VERIFY + // Fault injection lives with the verifier, and the guard is not decoration: the + // cn1GcFault* family is declared inside CN1_GC_VERIFY, so referencing this one + // unguarded broke every ordinary build while the verifier build -- the only + // configuration where the symbol exists -- went on passing. + if(cn1GcFaultRefClear) { + continue; // fault injection: leave the dead referent in place + } +#endif // THE STORES ARE SEQUENTIAL, AND DELIBERATELY SO. Review asked for the clearing of // a referent's aliases to be PUBLISHED atomically as well as decided atomically, // on the grounds that a mutator landing between two iterations can see one alias diff --git a/vm/benchmarks/run-gc-verify.sh b/vm/benchmarks/run-gc-verify.sh index 5c9600ced85..dd4702b999f 100755 --- a/vm/benchmarks/run-gc-verify.sh +++ b/vm/benchmarks/run-gc-verify.sh @@ -122,4 +122,40 @@ else fail=1 fi +# Third self-test, for java.lang.ref. The referent is the ONE reference field the +# generated mark functions deliberately do not hand to gcMarkObject -- that +# suppression is what makes the edge weak -- and for a while it therefore bypassed +# the verifier completely: a live Reference holding a pointer into reclaimed memory +# passed with violations=0, which is the single defect this collector work most +# needs caught. cn1GcDiscoverReference now routes it to cn1GcVerifyChild, and this +# proves that routing has teeth rather than assuming it. +# +# refnoclear leaves a dead referent in its field instead of clearing it. Note the +# obvious-looking fault is the wrong one: clearing MORE references than liveness +# warrants only produces extra nulls, which are safe, and an attempt at that +# reported violations=0 for exactly that reason. The dangling direction is +# clearing LESS. +printf '%-16s ' "self-test3" +if [ ! -x ./target/bin/RefPolicy-verify ]; then + ./translate-and-build.sh RefPolicy target/bin/RefPolicy-verify -DCN1_GC_VERIFY \ + > target/bin/RefPolicy-selftest-build.log 2>&1 || true +fi +if [ ! -x ./target/bin/RefPolicy-verify ]; then + echo "BROKEN -- could not build RefPolicy for the reference self-test" + fail=1 +else + rcOut="$(CN1_GC_FAULT=refnoclear ./target/bin/RefPolicy-verify 128 8192 1500 24 2>&1)" || true + if printf '%s' "$rcOut" | grep -q 'DANGLING REFERENCE'; then + echo "detected the injected dangling referent ($(printf '%s' "$rcOut" | grep -c 'DANGLING REFERENCE') reports)" + elif ! printf '%s' "$rcOut" | grep -q 'GC-VERIFY. SUMMARY'; then + echo "BROKEN -- faulted run died before the verifier summary" + printf '%s\n' "$rcOut" | tail -20 + fail=1 + else + echo "BROKEN -- an uncleared dead referent was NOT reported; the verifier cannot see referents" + printf '%s\n' "$rcOut" | tail -5 + fail=1 + fi +fi + [ "$fail" -eq 0 ] && echo "GC-VERIFY GREEN" || { echo "GC-VERIFY FAILED"; exit 1; } From be618fc583c8d535ad3a9f452bfdd5887dff885a Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Tue, 8 Sep 2026 05:00:13 +0300 Subject: [PATCH 11/30] Retain discovered referents when a dropped log abandons the clear pass Accepted review finding, and a repeat of a mistake this branch already corrected once. The drop-counter fallback returned without clearing, which leaves every discovered referent unmarked AND its field non-null -- so the sweep frees objects that live References still point at. That is the dangling read the pass exists to prevent, produced by the code meant to prevent it. "Skip the clear" is not "keep the referent alive". Nothing else marks a weak referent; that is what makes the edge weak. The note on the unrecorded-discovery path says exactly this, and the same confusion reappeared here. The fallback now marks every discovered referent and drains before returning. Gates: run-gc-verify.sh green with all three self-tests firing, run-gauntlet.sh green with all eleven tortures bit-identical and both GC stop modes. Co-Authored-By: Claude Opus 5 (1M context) --- vm/ByteCodeTranslator/src/cn1_globals.m | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/vm/ByteCodeTranslator/src/cn1_globals.m b/vm/ByteCodeTranslator/src/cn1_globals.m index fabfa665069..ea50ea079a4 100644 --- a/vm/ByteCodeTranslator/src/cn1_globals.m +++ b/vm/ByteCodeTranslator/src/cn1_globals.m @@ -2772,6 +2772,25 @@ static JAVA_BOOLEAN cn1GcProcessReferences(CODENAME_ONE_THREAD_STATE) { // of memory, and it does NOT disable the emergency drop, which clears at DISCOVERY and // never touches the log. if(atomic_load_explicit(&cn1SatbDrops, memory_order_relaxed) != cn1RefDropsAtCycleStart) { + // RETAIN, do not merely decline to clear. Returning here leaves every discovered + // referent unmarked AND its field non-null, so the sweep frees objects that live + // References still point at -- which is the dangling read this pass exists to + // prevent, produced by the code meant to prevent it. + // + // This is the second time on this branch that "skip the clear" was mistaken for + // "keep the referent alive"; the note on the unrecorded-discovery path says the + // same thing. Not clearing a weak field does not retain anything, because nothing + // else marks a weak referent -- that is what makes the edge weak. + for(long i = 0 ; i < cn1RefDiscoveredTop ; i++) { + JAVA_OBJECT r = __atomic_load_n(cn1RefDiscovered[i].referentField, __ATOMIC_RELAXED); + if(r != JAVA_NULL && !CN1_IS_TAGGED(r)) { + gcMarkObject(threadStateData, r, JAVA_FALSE); + marked = JAVA_TRUE; + } + } + if(marked) { + gcMarkDrain(threadStateData); + } #ifdef CN1_GC_CONFORM cn1RefPhaseNs += cn1GcNowNs() - __r0; #endif From 8bffa85c5770a1fac2f141c31cc6e06dc0fa823e Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Tue, 8 Sep 2026 05:10:44 +0300 Subject: [PATCH 12/30] Measure what the Reference.get() load barrier actually costs The barrier registers in the SATB termination handshake, which is two seq_cst read-modify-writes on every Reference.get() and cannot be skipped soundly -- any flag sampled before registering can go stale in the window the registration exists to close. Whether that price is worth paying is a question for a number, not an intuition, so this adds the means to get one. -DCN1_REF_NO_LOAD_BARRIER compiles the registration and the enqueue out, leaving the load and the touch stamp. It is UNSOUND and is not a shipping configuration; it is the arm the cost is measured against. cn1RefGets counts get() calls, and [GCREF-TOTAL] prints the run total at exit. The per-cycle [GCREF] line cannot answer "how often does this workload call get()", because it only prints when a collection happens -- so a get()-heavy but allocation-light phase, which is exactly the shape that costs the barrier most, leaves the last line stranded early in the run. Costing the barrier off that number understates the calls and overstates the nanoseconds: it read 153,408 for a run that made 6,155,728. Measured on this host, RefPolicy, nine interleaved reps, both arms -DCN1_GC_CONFORM -flto=thin: get-dominated 64 keys x 64B, 6M reads, no churn 6,155,728 gets +0.07% median, +0.35% floor -> +0.45 ns median, +2.14 ns floor per get() image-cache 512 keys x 32KB, 400k reads, churn 556,416 gets +0.10% median, -0.30% floor -> unmeasurable at this call volume So the barrier costs roughly half a nanosecond to two nanoseconds per call, and 0.35% of wall time only at 1.6 MILLION get() calls per second. An image-heavy screen redrawing fifty encoded images at 60fps calls it three thousand times a second, some three orders of magnitude below the rate at which it becomes visible. Co-Authored-By: Claude Opus 5 (1M context) --- vm/ByteCodeTranslator/src/cn1_globals.h | 17 ++++++++++++++++- vm/ByteCodeTranslator/src/cn1_globals.m | 25 +++++++++++++++++++++++-- 2 files changed, 39 insertions(+), 3 deletions(-) diff --git a/vm/ByteCodeTranslator/src/cn1_globals.h b/vm/ByteCodeTranslator/src/cn1_globals.h index 7da3b264380..17195d9b9a7 100644 --- a/vm/ByteCodeTranslator/src/cn1_globals.h +++ b/vm/ByteCodeTranslator/src/cn1_globals.h @@ -3178,10 +3178,25 @@ extern void cn1GcDiscoverReference(CODENAME_ONE_THREAD_STATE, JAVA_OBJECT ref, J #define CN1_REF_LOAD_END() do { } while(0) #define CN1_SATB_REF_KEEP(active, refVal) do { (void)(active); (void)(refVal); } while(0) #else +// -DCN1_REF_NO_LOAD_BARRIER compiles the registration and the enqueue out, leaving the +// touch stamp and the load. It is UNSOUND -- it is the arm that measures what the barrier +// costs, not a configuration to ship -- and exists because "is get() too expensive?" has to +// be answered with a number rather than an intuition. +#if defined(CN1_REF_NO_LOAD_BARRIER) +#define CN1_REF_LOAD_BEGIN() JAVA_FALSE +#define CN1_REF_LOAD_END() do { } while(0) +#else #define CN1_REF_LOAD_BEGIN() cn1SatbBulkBegin() #define CN1_REF_LOAD_END() cn1SatbBulkEnd() +#endif +#ifdef CN1_GC_CONFORM +extern _Atomic long cn1RefGets; +#define CN1_REF_COUNT_GET() atomic_fetch_add_explicit(&cn1RefGets, 1, memory_order_relaxed) +#else +#define CN1_REF_COUNT_GET() do { } while(0) +#endif #define CN1_SATB_REF_KEEP(active, refVal) \ - do { JAVA_OBJECT cn1__r = (refVal); \ + do { CN1_REF_COUNT_GET(); JAVA_OBJECT cn1__r = (refVal); \ if((active) && cn1__r != JAVA_NULL && !CN1_IS_TAGGED(cn1__r)) { \ int cn1__m = __atomic_load_n(&cn1__r->__codenameOneGcMark, __ATOMIC_RELAXED); \ int cn1__e = atomic_load_explicit(&bibopGcEpoch, memory_order_relaxed); \ diff --git a/vm/ByteCodeTranslator/src/cn1_globals.m b/vm/ByteCodeTranslator/src/cn1_globals.m index ea50ea079a4..ce951a34b34 100644 --- a/vm/ByteCodeTranslator/src/cn1_globals.m +++ b/vm/ByteCodeTranslator/src/cn1_globals.m @@ -2372,6 +2372,7 @@ void cn1RefDropAllSoftReferents(void) { _Atomic long cn1RefRetained = 0; // soft referents kept by the policy _Atomic long cn1RefKeptTouched = 0; // kept by the racing-get() rule in the clear pass _Atomic long cn1RefCleared = 0; // referents handed to the sweep +_Atomic long cn1RefGets = 0; // Reference.get() calls, for costing the load barrier long long cn1RefPhaseNs = 0; // GC thread only: time in cn1GcProcessReferences long cn1RefPasses = 0; // clear passes run this cycle (>1 == SATB reopen) #endif @@ -2390,6 +2391,8 @@ static void cn1RefBeginCycle(void) { atomic_store_explicit(&cn1RefRetained, 0, memory_order_relaxed); atomic_store_explicit(&cn1RefKeptTouched, 0, memory_order_relaxed); atomic_store_explicit(&cn1RefCleared, 0, memory_order_relaxed); + // cn1RefGets is NOT reset: it is a whole-run total, because the question it answers + // ("how often does a real workload call get()?") is about the run, not the cycle. cn1RefPhaseNs = 0; cn1RefPasses = 0; #endif @@ -12860,7 +12863,8 @@ void cn1GcProbeCycle(double markMs, double sweepMs, int threw) { // silently takes. fprintf(stderr, "[GCREF] v=1 cyc=%d tMs=%lld discovered=%ld weak=%ld retained=%ld" - " keptTouched=%ld cleared=%ld passes=%ld refMs=%.3f softBudget=%d listCap=%ld\n", + " keptTouched=%ld cleared=%ld passes=%ld refMs=%.3f softBudget=%d listCap=%ld" + " getsTotal=%ld\n", currentGcMarkValue, cn1GcProbeElapsedMs(), atomic_load_explicit(&cn1RefDiscoveries, memory_order_relaxed), atomic_load_explicit(&cn1RefWeak, memory_order_relaxed), @@ -12869,7 +12873,8 @@ void cn1GcProbeCycle(double markMs, double sweepMs, int threw) { atomic_load_explicit(&cn1RefCleared, memory_order_relaxed), cn1RefPasses, cn1RefPhaseNs / 1e6, atomic_load_explicit(&cn1SoftRetainCycles, memory_order_relaxed), - cn1RefDiscoveredCap); + cn1RefDiscoveredCap, + atomic_load_explicit(&cn1RefGets, memory_order_relaxed)); fflush(stderr); // Per-CYCLE, so reset after reporting. A running total cannot show a trend. cn1GcProbeResetPhases(); @@ -13271,6 +13276,20 @@ static void cn1ReportAllocProfile(void) { } #endif +// Reference.get() calls for the WHOLE run, printed at exit. +// +// The per-cycle [GCREF] line cannot answer "how often does this workload call get()": +// it only prints when a collection happens, so a get()-heavy but allocation-light phase +// -- exactly the shape that costs the load barrier the most -- leaves the last line +// stranded early in the run. Costing the barrier off that number understates the call +// count and therefore overstates the nanoseconds per call; measured, it read 153,408 for +// a run that made millions. +static void cn1ReportRefGets(void) { + fprintf(stderr, "[GCREF-TOTAL] gets=%ld\n", + atomic_load_explicit(&cn1RefGets, memory_order_relaxed)); + fflush(stderr); +} + static void cn1ReportStalls(void) { long long wallMs = cn1GcProbeElapsedMs(); // Mutator-only, to match the thread count it is divided by; the per-cause table below @@ -13586,6 +13605,8 @@ void initConstantPool() { #ifdef CN1_GC_CONFORM atexit(cn1ReportAllocProfile); #endif + + atexit(cn1ReportRefGets); #ifdef CN1_CONSERVATIVE_GC_ROOTS // The self test sorts the conservative extent table, which only exists on // this arm. Calling it under CN1_GC_CONFORM alone does not compile, so From 00c02d44b3bb43612343ff8d27c5d74da0318783 Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Tue, 8 Sep 2026 09:05:33 +0300 Subject: [PATCH 13/30] Close the remaining reference-clearing races, and fix the no-BiBOP build Five accepted review findings. -DCN1_DISABLE_BIBOP DID NOT LINK. bibopGcEpoch is defined inside cn1_globals.m's #ifndef CN1_DISABLE_BIBOP block, and the load barrier named it unguarded, so a supported A/B and fallback configuration failed with an undefined _bibopGcEpoch. Confirmed by building it rather than by reading guards. The epoch is only a filter -- dropping it enqueues referents that would have been skipped, which is more work and never less safety -- so the disabled build compares against a value the mark word cannot equal and the barrier keeps its single comparison. DROP RECOVERY DID NOT REACH A FIXPOINT. The same defect already fixed in sub-pass A, reappearing in the recovery loop added a commit earlier: marking a retained referent traces it, and an object kept alive only that way can itself hold references whose mark functions register after the loop has passed them. THE CAPPED SATB EXIT SKIPPED REFERENCES ENTIRELY. The CN1_SATB_MAX_REOPENS branch drains twice on its way out, and those drains can newly mark an object whose graph contains a Reference. It was the one exit that left without a reference pass, so a reachable reference kept an unmarked referent the sweep then freed. THE DROP CHECK COULD NOT SEE A DROP THAT HAD NOT HAPPENED YET. A get() starting after the pre-loop check loads an unmarked referent, and a failed enqueue moves the counter only once clearing has already decided it was safe. The pass now records what each entry cleared, quiesces -- every getter registers for the duration of its load, so an in-flight count of zero means every getter that overlapped has finished and published its drop -- and re-reads the counter, marking what it cleared if it moved. The stores are not undone and need not be: a cleared reference answering null is legal, the object being freed under a mutator is not. THE EMERGENCY PATH HAD NOWHERE TO RECORD. It clears at discovery precisely because the list could not grow, so the recovery above had nothing to consult and a racing get() with a failed enqueue would have been handed a freed pointer. It now remembers what it cleared in a fixed preallocated array -- allocation being the one thing unavailable there -- and REFUSES TO CLEAR when that is full, marking instead: memory the emergency wanted back is retained, which is worse than clearing and far better than a dangling read. Rebased onto master, which landed the parallel-mark collector work (#5717) in the same two files; the conflict was additive on both sides and the resolved tree was compiled before the rebase continued. Gates: run-gc-verify.sh green with all three self-tests firing, run-gauntlet.sh green with all eleven tortures bit-identical and both GC stop modes, and all five build shapes compile -- plain, CN1_GC_CONFORM, CN1_DISABLE_BIBOP, CN1_DISABLE_SATB and CN1_GC_VERIFY. Co-Authored-By: Claude Opus 5 (1M context) --- vm/ByteCodeTranslator/src/cn1_globals.h | 15 +++- vm/ByteCodeTranslator/src/cn1_globals.m | 106 ++++++++++++++++++++++-- 2 files changed, 112 insertions(+), 9 deletions(-) diff --git a/vm/ByteCodeTranslator/src/cn1_globals.h b/vm/ByteCodeTranslator/src/cn1_globals.h index 17195d9b9a7..e64cf488918 100644 --- a/vm/ByteCodeTranslator/src/cn1_globals.h +++ b/vm/ByteCodeTranslator/src/cn1_globals.h @@ -3189,6 +3189,18 @@ extern void cn1GcDiscoverReference(CODENAME_ONE_THREAD_STATE, JAVA_OBJECT ref, J #define CN1_REF_LOAD_BEGIN() cn1SatbBulkBegin() #define CN1_REF_LOAD_END() cn1SatbBulkEnd() #endif +// bibopGcEpoch is DEFINED inside cn1_globals.m's #ifndef CN1_DISABLE_BIBOP block, so +// naming it unguarded here broke -DCN1_DISABLE_BIBOP outright -- a supported A/B and +// fallback configuration -- with an undefined _bibopGcEpoch at link time. The epoch is +// only a filter, and dropping it merely enqueues referents that would have been skipped: +// more work, never less safety. A value the mark word can never equal keeps the single +// comparison below rather than branching on the configuration. +#ifdef CN1_DISABLE_BIBOP +#define CN1_REF_EPOCH_NOW() (-2) +#else +#define CN1_REF_EPOCH_NOW() atomic_load_explicit(&bibopGcEpoch, memory_order_relaxed) +#endif + #ifdef CN1_GC_CONFORM extern _Atomic long cn1RefGets; #define CN1_REF_COUNT_GET() atomic_fetch_add_explicit(&cn1RefGets, 1, memory_order_relaxed) @@ -3199,8 +3211,7 @@ extern _Atomic long cn1RefGets; do { CN1_REF_COUNT_GET(); JAVA_OBJECT cn1__r = (refVal); \ if((active) && cn1__r != JAVA_NULL && !CN1_IS_TAGGED(cn1__r)) { \ int cn1__m = __atomic_load_n(&cn1__r->__codenameOneGcMark, __ATOMIC_RELAXED); \ - int cn1__e = atomic_load_explicit(&bibopGcEpoch, memory_order_relaxed); \ - if(cn1__m != -1 && cn1__m != cn1__e) cn1SatbEnqueue(cn1__r); \ + if(cn1__m != -1 && cn1__m != CN1_REF_EPOCH_NOW()) cn1SatbEnqueue(cn1__r); \ } } while(0) #endif diff --git a/vm/ByteCodeTranslator/src/cn1_globals.m b/vm/ByteCodeTranslator/src/cn1_globals.m index ce951a34b34..bf564018043 100644 --- a/vm/ByteCodeTranslator/src/cn1_globals.m +++ b/vm/ByteCodeTranslator/src/cn1_globals.m @@ -2311,6 +2311,10 @@ JAVA_LONG GcVerifyApp_gcMarkState___R_long(CODENAME_ONE_THREAD_STATE) { JAVA_OBJECT* referentField; JAVA_INT* touchAgeField; JAVA_INT strength; + // What this entry's field held when the clear pass nulled it, so a drop that only + // becomes visible AFTER the store can still keep the object alive. Clearing is a + // destructive publish and cannot be taken back; marking the old value can. + JAVA_OBJECT clearedReferent; }; static struct CN1RefEntry* cn1RefDiscovered = 0; static long cn1RefDiscoveredTop = 0; @@ -2360,6 +2364,19 @@ JAVA_LONG GcVerifyApp_gcMarkState___R_long(CODENAME_ONE_THREAD_STATE) { // sound basis for clearing anything. static long cn1RefDropsAtCycleStart = 0; +// Referents cleared by the EMERGENCY path, which runs at discovery and has no list entry +// to remember them in -- that path exists precisely because the list could not grow. +// Fixed and preallocated, because allocating here is what is unavailable: the emergency is +// raised by an allocation failure. +// +// Overflow is handled by not clearing at all: past this many the emergency marks the +// referent instead, which retains memory it wanted to release but never hands out a +// dangling pointer. 512 is far above the number of references that can reach this path in +// practice, since it needs the discovery list to have failed to grow first. +#define CN1_REF_EMERGENCY_SLOTS 512 +static JAVA_OBJECT cn1RefEmergencyCleared[CN1_REF_EMERGENCY_SLOTS]; +static long cn1RefEmergencyTop = 0; + // Called from the allocation-failure path. Idempotent, allocation-free and safe from a // thread that is about to park -- a relaxed store and nothing else. void cn1RefDropAllSoftReferents(void) { @@ -2382,6 +2399,7 @@ void cn1RefDropAllSoftReferents(void) { static void cn1RefBeginCycle(void) { cn1RefDiscoveredTop = 0; cn1RefDropsAtCycleStart = atomic_load_explicit(&cn1SatbDrops, memory_order_relaxed); + cn1RefEmergencyTop = 0; #ifdef CN1_GC_CONFORM // PER CYCLE, like every other figure in [GCPROBE]. A running total cannot show // whether the budget is tracking memory pressure, which is the whole question the @@ -2562,6 +2580,7 @@ void cn1GcDiscoverReference(CODENAME_ONE_THREAD_STATE, JAVA_OBJECT ref, JAVA_BOO e->referentField = referentField; e->touchAgeField = touchAgeField; e->strength = strength; + e->clearedReferent = JAVA_NULL; recorded = JAVA_TRUE; } pthread_mutex_unlock(&cn1RefMutex); @@ -2622,10 +2641,20 @@ void cn1GcDiscoverReference(CODENAME_ONE_THREAD_STATE, JAVA_OBJECT ref, JAVA_BOO int rm = __atomic_load_n(&r->__codenameOneGcMark, __ATOMIC_ACQUIRE); alreadyLive = (rm == currentGcMarkValue || rm == -1) ? JAVA_TRUE : JAVA_FALSE; } + // Only clear if the referent can be REMEMBERED. A racing get() can load it and + // then fail to log it, and this path has no entry the late-drop recovery could + // consult -- so without a record the sweep would free a pointer already handed to a + // mutator. With nowhere to record it, marking is the answer: it keeps memory the + // emergency wanted back, which is a worse outcome than clearing and a far better + // one than a dangling read. + JAVA_BOOLEAN canRemember = (cn1RefEmergencyTop < CN1_REF_EMERGENCY_SLOTS) + ? JAVA_TRUE : JAVA_FALSE; if(!alreadyLive + && canRemember && strength == CN1_REF_SOFT && atomic_load_explicit(&cn1SoftRetainCycles, memory_order_relaxed) < 0 && __atomic_load_n(touchAgeField, __ATOMIC_RELAXED) != CN1_REF_TOUCHED) { + cn1RefEmergencyCleared[cn1RefEmergencyTop++] = r; __atomic_store_n(referentField, JAVA_NULL, __ATOMIC_RELAXED); #ifdef CN1_GC_CONFORM atomic_fetch_add_explicit(&cn1RefCleared, 1, memory_order_relaxed); @@ -2784,15 +2813,29 @@ static JAVA_BOOLEAN cn1GcProcessReferences(CODENAME_ONE_THREAD_STATE) { // "keep the referent alive"; the note on the unrecorded-discovery path says the // same thing. Not clearing a weak field does not retain anything, because nothing // else marks a weak referent -- that is what makes the edge weak. - for(long i = 0 ; i < cn1RefDiscoveredTop ; i++) { - JAVA_OBJECT r = __atomic_load_n(cn1RefDiscovered[i].referentField, __ATOMIC_RELAXED); - if(r != JAVA_NULL && !CN1_IS_TAGGED(r)) { - gcMarkObject(threadStateData, r, JAVA_FALSE); + // TO A FIXPOINT, for the same reason sub-pass A runs to one: marking a retained + // referent traces it, and an object kept alive only that way can itself hold + // further references whose mark functions register late. Marking the list once and + // draining discovers those AFTER the loop has finished, and the recovery would + // return leaving a newly reachable Reference holding an unmarked referent -- the + // dangling pointer this recovery exists to avoid, one level deeper. + for(;;) { + long before = cn1RefDiscoveredTop; + JAVA_BOOLEAN markedThisRound = JAVA_FALSE; + for(long i = 0 ; i < before ; i++) { + JAVA_OBJECT r = __atomic_load_n(cn1RefDiscovered[i].referentField, __ATOMIC_RELAXED); + if(r != JAVA_NULL && !CN1_IS_TAGGED(r)) { + gcMarkObject(threadStateData, r, JAVA_FALSE); + markedThisRound = JAVA_TRUE; + } + } + if(markedThisRound) { marked = JAVA_TRUE; + gcMarkDrain(threadStateData); + } + if(cn1RefDiscoveredTop == before) { + break; } - } - if(marked) { - gcMarkDrain(threadStateData); } #ifdef CN1_GC_CONFORM cn1RefPhaseNs += cn1GcNowNs() - __r0; @@ -2861,12 +2904,48 @@ static JAVA_BOOLEAN cn1GcProcessReferences(CODENAME_ONE_THREAD_STATE) { // self-healing: it lasts only until this loop reaches the other alias, and a get() // inside it returns a referent that is still valid, because the same read arms the // barrier and resurrects the object for this cycle. + e->clearedReferent = r; __atomic_store_n(e->referentField, JAVA_NULL, __ATOMIC_RELAXED); #ifdef CN1_GC_CONFORM atomic_fetch_add_explicit(&cn1RefCleared, 1, memory_order_relaxed); #endif } + // RE-CHECK AFTER A QUIESCE, because the pre-loop check cannot see a drop that has not + // happened yet. A get() starting after that check loads an unmarked referent, and if + // its enqueue fails the counter only moves once the clear above has already decided it + // was safe. The quiesce is what makes the re-read meaningful: every getter registers + // for the duration of its load, so waiting for the in-flight count to reach zero + // guarantees any getter that overlapped this loop has finished and published its drop. + // + // The stores cannot be undone, and do not need to be -- a cleared reference that + // answers null is legal. What must not happen is the OBJECT being freed while a + // mutator holds it, so the recovery marks what was cleared rather than restoring it. + cn1SatbBulkQuiesce(); + if(atomic_load_explicit(&cn1SatbDrops, memory_order_relaxed) != cn1RefDropsAtCycleStart) { + JAVA_BOOLEAN recovered = JAVA_FALSE; + for(long i = 0 ; i < n ; i++) { + JAVA_OBJECT was = cn1RefDiscovered[i].clearedReferent; + if(was != JAVA_NULL && !CN1_IS_TAGGED(was)) { + gcMarkObject(threadStateData, was, JAVA_FALSE); + recovered = JAVA_TRUE; + } + } + // The emergency path's clears too. They never reached the list -- that is why they + // exist -- so they are remembered separately and recovered on the same signal. + for(long i = 0 ; i < cn1RefEmergencyTop ; i++) { + JAVA_OBJECT was = cn1RefEmergencyCleared[i]; + if(was != JAVA_NULL && !CN1_IS_TAGGED(was)) { + gcMarkObject(threadStateData, was, JAVA_FALSE); + recovered = JAVA_TRUE; + } + } + if(recovered) { + marked = JAVA_TRUE; + gcMarkDrain(threadStateData); + } + } + // AGE, by compare-exchange, so a get() racing this cannot be erased. A plain // read-modify-write here would reintroduce exactly what deferring the write was meant // to remove: read 5, a mutator stamps TOUCHED, write 6, and the read has vanished with @@ -3922,6 +4001,19 @@ void codenameOneGCMark() { gcMarkDrain(d); } } + // REFERENCES ONE LAST TIME. Those drains can newly mark an object whose + // graph contains a Reference, and its generated mark function then appends + // a discovery -- after the only reference pass this cycle has run. Leaving + // through here without another pass means that reachable reference keeps an + // unmarked referent the sweep goes on to free, which is the dangling + // pointer the whole pass exists to prevent, reached by the one exit that + // skipped it. + // + // The barrier is already down on this path, so a get() racing this pass + // cannot log -- which is exactly the weaker invariant the cap falls back + // on, and it is no weaker for references than for anything else the cap + // gives up on. + cn1GcProcessReferences(d); break; } } From a12f741e6f6a8890d41ea2043a6b08214b77ba0a Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Tue, 8 Sep 2026 11:08:15 +0300 Subject: [PATCH 14/30] Define the GC epoch mirror unconditionally instead of working around it The load barrier needed bibopGcEpoch, which was defined inside cn1_globals.m's #ifndef CN1_DISABLE_BIBOP block, so that configuration failed to link. The first fix taught the barrier to compare against a sentinel when BiBOP was compiled out -- a configuration branch on a hot path to accommodate a symbol that had no reason to be conditional. The epoch is a plain mirror of currentGcMarkValue for mutator-side reads. Nothing about it belongs to the page heap; it sat inside that guard by accident of placement. Hoisting it out removes the macro entirely and returns the barrier to one comparison with no configuration in it. Keeping the arm alive rather than dropping it costs nothing now and keeps vm/CLAUDE.md honest: -DCN1_DISABLE_BIBOP is listed there as an ablation, and an ablation that does not link is a trap for whoever reaches for it. With BiBOP off nothing advances the epoch, so the barrier enqueues referents it would otherwise have skipped -- the epoch is a filter, and a stale one only ever declines to skip. Six build shapes verified: plain, CN1_GC_CONFORM, CN1_DISABLE_BIBOP, CN1_DISABLE_SATB, CN1_GC_VERIFY, and CN1_DISABLE_BIBOP with CN1_GC_CONFORM together. Gates: run-gc-verify.sh green with all three self-tests firing, run-gauntlet.sh green with all eleven tortures bit-identical and both GC stop modes. Co-Authored-By: Claude Opus 5 (1M context) --- vm/ByteCodeTranslator/src/cn1_globals.h | 15 ++------------- vm/ByteCodeTranslator/src/cn1_globals.m | 11 ++++++++++- 2 files changed, 12 insertions(+), 14 deletions(-) diff --git a/vm/ByteCodeTranslator/src/cn1_globals.h b/vm/ByteCodeTranslator/src/cn1_globals.h index e64cf488918..17195d9b9a7 100644 --- a/vm/ByteCodeTranslator/src/cn1_globals.h +++ b/vm/ByteCodeTranslator/src/cn1_globals.h @@ -3189,18 +3189,6 @@ extern void cn1GcDiscoverReference(CODENAME_ONE_THREAD_STATE, JAVA_OBJECT ref, J #define CN1_REF_LOAD_BEGIN() cn1SatbBulkBegin() #define CN1_REF_LOAD_END() cn1SatbBulkEnd() #endif -// bibopGcEpoch is DEFINED inside cn1_globals.m's #ifndef CN1_DISABLE_BIBOP block, so -// naming it unguarded here broke -DCN1_DISABLE_BIBOP outright -- a supported A/B and -// fallback configuration -- with an undefined _bibopGcEpoch at link time. The epoch is -// only a filter, and dropping it merely enqueues referents that would have been skipped: -// more work, never less safety. A value the mark word can never equal keeps the single -// comparison below rather than branching on the configuration. -#ifdef CN1_DISABLE_BIBOP -#define CN1_REF_EPOCH_NOW() (-2) -#else -#define CN1_REF_EPOCH_NOW() atomic_load_explicit(&bibopGcEpoch, memory_order_relaxed) -#endif - #ifdef CN1_GC_CONFORM extern _Atomic long cn1RefGets; #define CN1_REF_COUNT_GET() atomic_fetch_add_explicit(&cn1RefGets, 1, memory_order_relaxed) @@ -3211,7 +3199,8 @@ extern _Atomic long cn1RefGets; do { CN1_REF_COUNT_GET(); JAVA_OBJECT cn1__r = (refVal); \ if((active) && cn1__r != JAVA_NULL && !CN1_IS_TAGGED(cn1__r)) { \ int cn1__m = __atomic_load_n(&cn1__r->__codenameOneGcMark, __ATOMIC_RELAXED); \ - if(cn1__m != -1 && cn1__m != CN1_REF_EPOCH_NOW()) cn1SatbEnqueue(cn1__r); \ + int cn1__e = atomic_load_explicit(&bibopGcEpoch, memory_order_relaxed); \ + if(cn1__m != -1 && cn1__m != cn1__e) cn1SatbEnqueue(cn1__r); \ } } while(0) #endif diff --git a/vm/ByteCodeTranslator/src/cn1_globals.m b/vm/ByteCodeTranslator/src/cn1_globals.m index bf564018043..394be2dd7e6 100644 --- a/vm/ByteCodeTranslator/src/cn1_globals.m +++ b/vm/ByteCodeTranslator/src/cn1_globals.m @@ -4909,6 +4909,16 @@ JAVA_INT java_lang_System_identityHashCode___java_lang_Object_R_int(CODENAME_ONE BOOL isAppSuspended = 0; #endif +// DEFINED UNCONDITIONALLY, unlike the rest of the BiBOP state below it. It is a plain +// mirror of currentGcMarkValue for mutator-side reads, and the Reference.get() load +// barrier consults it on every call -- so leaving it inside the BiBOP guard meant +// -DCN1_DISABLE_BIBOP failed to link with an undefined _bibopGcEpoch. Hoisting it here is +// the whole fix, and it removes the configuration-dependent macro the barrier briefly +// carried instead. With BiBOP disabled nothing advances it, which costs the barrier some +// extra enqueues and no correctness: the epoch is a filter, and a stale one only ever +// declines to skip. +_Atomic int bibopGcEpoch = 1; + #ifndef CN1_DISABLE_BIBOP // ========================================================================= // BiBOP: non-moving segregated-fits page heap + mark-sweep for SMALL non-array @@ -5067,7 +5077,6 @@ JAVA_INT java_lang_System_identityHashCode___java_lang_Object_R_int(CODENAME_ONE // Non-static: also read/written by the inlined bump fast path (cn1_globals.h). _Atomic long bibopBytesSinceGc = 0; _Atomic long bibopGcTriggerBytes = CN1_BIBOP_GC_TRIGGER_BYTES; -_Atomic int bibopGcEpoch = 1; _Atomic int bibopBypassGeneration[CN1_BIBOP_NUM_CLASSES]; static long bibopCycleAllocatedBytes = 0; // LEGACY bytes charged to the cycle that is starting -- the twin of From c27e1e7b3248243a88b4ada1fe288fe9157dfa92 Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Tue, 8 Sep 2026 13:48:26 +0300 Subject: [PATCH 15/30] Serialize emergency recovery slots, and stop the epoch mirror going stale Four accepted review findings; three are defects in the recovery paths added a commit earlier. EMERGENCY SLOT RESERVATION WAS RACY. cn1RefEmergencyTop++ runs after cn1RefMutex has been released, and mark functions run on however many workers gcMarkDrainParallel is using -- so increments were lost and two workers could publish into one slot. An emergency clear that goes unrecorded is exactly the case the drop recovery cannot repair, leaving the sweep free to take the referent under a racing get(). The index is now an atomic reserve-then-publish, and a reservation that cannot be satisfied means the clear does not happen at all. THE EARLY DROP FALLBACK DID NOT RETAIN EMERGENCY CLEARS. It returns before the post-clear recovery, and a referent cleared by the emergency path is gone from its field -- it exists only in the recovery array. Retaining cn1RefDiscovered alone therefore left those objects to be swept under a getter that had already been handed one. THE POST-CLEAR RECOVERY DRAINED ONCE. The same fixpoint defect as the other two recovery loops: a referent brought back can hold a further Reference whose mark function registers during the drain, after the loops have run. It now iterates, and retains anything the drain discovered rather than clearing it -- by that point the pass is past where clearing is safe. THE STALE EPOCH WAS NOT HARMLESS. The previous commit hoisted bibopGcEpoch out of the BiBOP guard so -DCN1_DISABLE_BIBOP would link, and its comment claimed a frozen mirror cost only some extra enqueues. It costs convergence: CN1_SATB_REF_KEEP skips a referent whose mark equals the epoch, and against a mirror frozen at 1 that matches nothing from the second collection onward -- the unfiltered shape already measured on this branch to put over 10,000 entries a cycle into the log and reach CN1_SATB_MAX_REOPENS every cycle. A filter that silently stops filtering is a cliff, not a rounding error. The epoch is now published by codenameOneGCMark, which every cycle passes through whether or not the page heap is compiled in, leaving one writer instead of two. Verified rather than argued: with -DCN1_DISABLE_BIBOP -DCN1_GC_CONFORM the termination loop reports passes=1 on all 32 cycles. Gates: run-gc-verify.sh green with all three self-tests firing, run-gauntlet.sh green with all eleven tortures bit-identical and both GC stop modes; plain, CN1_GC_CONFORM, CN1_DISABLE_BIBOP, CN1_DISABLE_SATB and CN1_GC_VERIFY all build. Co-Authored-By: Claude Opus 5 (1M context) --- vm/ByteCodeTranslator/src/cn1_globals.m | 121 ++++++++++++++++++------ 1 file changed, 94 insertions(+), 27 deletions(-) diff --git a/vm/ByteCodeTranslator/src/cn1_globals.m b/vm/ByteCodeTranslator/src/cn1_globals.m index 394be2dd7e6..7385a05a039 100644 --- a/vm/ByteCodeTranslator/src/cn1_globals.m +++ b/vm/ByteCodeTranslator/src/cn1_globals.m @@ -2375,7 +2375,12 @@ JAVA_LONG GcVerifyApp_gcMarkState___R_long(CODENAME_ONE_THREAD_STATE) { // practice, since it needs the discovery list to have failed to grow first. #define CN1_REF_EMERGENCY_SLOTS 512 static JAVA_OBJECT cn1RefEmergencyCleared[CN1_REF_EMERGENCY_SLOTS]; -static long cn1RefEmergencyTop = 0; +// ATOMIC, because this is reached with cn1RefMutex already released and mark functions run +// on however many workers gcMarkDrainParallel is using. A plain post-increment there loses +// increments and lets two workers publish into one slot, so a referent that WAS cleared +// goes unrecorded -- and an unrecorded emergency clear is exactly the case the drop +// recovery cannot repair, leaving the sweep free to take it under a racing get(). +static _Atomic long cn1RefEmergencyTop = 0; // Called from the allocation-failure path. Idempotent, allocation-free and safe from a // thread that is about to park -- a relaxed store and nothing else. @@ -2394,12 +2399,41 @@ void cn1RefDropAllSoftReferents(void) { long cn1RefPasses = 0; // clear passes run this cycle (>1 == SATB reopen) #endif +// Claim a recovery slot and publish the referent into it, or report that there is none. +// Reserving BEFORE the field is cleared is the point: a slot that could not be claimed +// means the clear must not happen, because nothing would be able to undo it. +static JAVA_BOOLEAN cn1RefEmergencyReserve(JAVA_OBJECT r) { + long idx = atomic_fetch_add_explicit(&cn1RefEmergencyTop, 1, memory_order_relaxed); + if(idx >= CN1_REF_EMERGENCY_SLOTS) { + return JAVA_FALSE; // full: caller marks instead of clearing + } + cn1RefEmergencyCleared[idx] = r; + return JAVA_TRUE; +} + +// Mark every referent the emergency path cleared, for the recoveries below. +static JAVA_BOOLEAN cn1RefRecoverEmergency(CODENAME_ONE_THREAD_STATE) { + JAVA_BOOLEAN any = JAVA_FALSE; + long top = atomic_load_explicit(&cn1RefEmergencyTop, memory_order_relaxed); + if(top > CN1_REF_EMERGENCY_SLOTS) { + top = CN1_REF_EMERGENCY_SLOTS; + } + for(long i = 0 ; i < top ; i++) { + JAVA_OBJECT was = cn1RefEmergencyCleared[i]; + if(was != JAVA_NULL && !CN1_IS_TAGGED(was)) { + gcMarkObject(threadStateData, was, JAVA_FALSE); + any = JAVA_TRUE; + } + } + return any; +} + // Recompute the soft budget and drop the previous cycle's discoveries. Called from // codenameOneGCMark before anything can mark. static void cn1RefBeginCycle(void) { cn1RefDiscoveredTop = 0; cn1RefDropsAtCycleStart = atomic_load_explicit(&cn1SatbDrops, memory_order_relaxed); - cn1RefEmergencyTop = 0; + atomic_store_explicit(&cn1RefEmergencyTop, 0, memory_order_relaxed); #ifdef CN1_GC_CONFORM // PER CYCLE, like every other figure in [GCPROBE]. A running total cannot show // whether the budget is tracking memory pressure, which is the whole question the @@ -2647,14 +2681,11 @@ void cn1GcDiscoverReference(CODENAME_ONE_THREAD_STATE, JAVA_OBJECT ref, JAVA_BOO // mutator. With nowhere to record it, marking is the answer: it keeps memory the // emergency wanted back, which is a worse outcome than clearing and a far better // one than a dangling read. - JAVA_BOOLEAN canRemember = (cn1RefEmergencyTop < CN1_REF_EMERGENCY_SLOTS) - ? JAVA_TRUE : JAVA_FALSE; if(!alreadyLive - && canRemember && strength == CN1_REF_SOFT && atomic_load_explicit(&cn1SoftRetainCycles, memory_order_relaxed) < 0 - && __atomic_load_n(touchAgeField, __ATOMIC_RELAXED) != CN1_REF_TOUCHED) { - cn1RefEmergencyCleared[cn1RefEmergencyTop++] = r; + && __atomic_load_n(touchAgeField, __ATOMIC_RELAXED) != CN1_REF_TOUCHED + && cn1RefEmergencyReserve(r)) { __atomic_store_n(referentField, JAVA_NULL, __ATOMIC_RELAXED); #ifdef CN1_GC_CONFORM atomic_fetch_add_explicit(&cn1RefCleared, 1, memory_order_relaxed); @@ -2829,6 +2860,14 @@ static JAVA_BOOLEAN cn1GcProcessReferences(CODENAME_ONE_THREAD_STATE) { markedThisRound = JAVA_TRUE; } } + // AND THE EMERGENCY CLEARS. Those referents are gone from their fields + // already, so cn1RefDiscovered cannot reach them -- they exist only in the + // recovery array. This early return happens before the post-clear recovery + // below, so without this they would be retained nowhere and swept under a + // getter that had already been handed one. + if(cn1RefRecoverEmergency(threadStateData)) { + markedThisRound = JAVA_TRUE; + } if(markedThisRound) { marked = JAVA_TRUE; gcMarkDrain(threadStateData); @@ -2923,26 +2962,43 @@ static JAVA_BOOLEAN cn1GcProcessReferences(CODENAME_ONE_THREAD_STATE) { // mutator holds it, so the recovery marks what was cleared rather than restoring it. cn1SatbBulkQuiesce(); if(atomic_load_explicit(&cn1SatbDrops, memory_order_relaxed) != cn1RefDropsAtCycleStart) { - JAVA_BOOLEAN recovered = JAVA_FALSE; - for(long i = 0 ; i < n ; i++) { - JAVA_OBJECT was = cn1RefDiscovered[i].clearedReferent; - if(was != JAVA_NULL && !CN1_IS_TAGGED(was)) { - gcMarkObject(threadStateData, was, JAVA_FALSE); - recovered = JAVA_TRUE; + // TO A FIXPOINT, like every other recovery here. A referent brought back can hold + // a further Reference, whose mark function registers during the drain -- after + // these loops have run. Draining once would leave that nested reference reachable + // with an unmarked referent, and an empty final take would then let termination + // finish over it. + for(;;) { + long before = cn1RefDiscoveredTop; + JAVA_BOOLEAN recovered = JAVA_FALSE; + for(long i = 0 ; i < n ; i++) { + JAVA_OBJECT was = cn1RefDiscovered[i].clearedReferent; + if(was != JAVA_NULL && !CN1_IS_TAGGED(was)) { + gcMarkObject(threadStateData, was, JAVA_FALSE); + recovered = JAVA_TRUE; + } } - } - // The emergency path's clears too. They never reached the list -- that is why they - // exist -- so they are remembered separately and recovered on the same signal. - for(long i = 0 ; i < cn1RefEmergencyTop ; i++) { - JAVA_OBJECT was = cn1RefEmergencyCleared[i]; - if(was != JAVA_NULL && !CN1_IS_TAGGED(was)) { - gcMarkObject(threadStateData, was, JAVA_FALSE); + // The emergency path's clears too. They never reached the list -- that is why + // they exist -- so they are remembered separately and recovered on the same + // signal. + if(cn1RefRecoverEmergency(threadStateData)) { recovered = JAVA_TRUE; } - } - if(recovered) { - marked = JAVA_TRUE; - gcMarkDrain(threadStateData); + // Anything newly discovered by the drain still has to be retained: this pass + // is past the point where it could safely clear. + for(long i = n ; i < before ; i++) { + JAVA_OBJECT r = __atomic_load_n(cn1RefDiscovered[i].referentField, __ATOMIC_RELAXED); + if(r != JAVA_NULL && !CN1_IS_TAGGED(r)) { + gcMarkObject(threadStateData, r, JAVA_FALSE); + recovered = JAVA_TRUE; + } + } + if(recovered) { + marked = JAVA_TRUE; + gcMarkDrain(threadStateData); + } + if(cn1RefDiscoveredTop == before) { + break; + } } } @@ -2977,6 +3033,17 @@ static JAVA_BOOLEAN cn1GcProcessReferences(CODENAME_ONE_THREAD_STATE) { void codenameOneGCMark() { currentGcMarkValue++; + // PUBLISH THE EPOCH HERE, not only from cn1BibopBeginGcCycle, because that call is + // compiled out under -DCN1_DISABLE_BIBOP and the mirror then stays at 1 forever. + // + // That is not the harmless staleness it looks like, and an earlier comment on this + // branch wrongly called it that. CN1_SATB_REF_KEEP skips a referent whose mark equals + // the epoch; against a frozen epoch it matches nothing from the second collection on, + // so every Reference.get() during a mark enqueues -- which is precisely the unfiltered + // shape measured on this branch to put over 10,000 entries a cycle into the log and + // drive the SATB termination loop into CN1_SATB_MAX_REOPENS every single cycle. A + // filter that silently stops filtering is a performance cliff, not a rounding error. + atomic_store_explicit(&bibopGcEpoch, currentGcMarkValue, memory_order_relaxed); // Drop the previous cycle's reference list and recompute the soft-retention budget // from the memory still available. Must precede anything that can mark, because // cn1GcDiscoverReference reads the budget to decide retention as it goes. @@ -5226,9 +5293,9 @@ static void cn1BibopFormatPage(CN1BibopPage* p, int ci) { } void cn1BibopBeginGcCycle(void) { - // Publish the new GC-owned epoch separately for mutators. They must never - // read currentGcMarkValue while the collector increments it concurrently. - atomic_store_explicit(&bibopGcEpoch, currentGcMarkValue, memory_order_relaxed); + // The epoch is published by codenameOneGCMark, which every cycle passes through + // whether or not the page heap is compiled in. It used to be published here, which + // left the mirror frozen under -DCN1_DISABLE_BIBOP; see the note at that store. // Charge allocations racing this mark to the NEXT cycle. The old sweep-end // store lost those bytes and could delay a collection indefinitely under a // sustained allocator. From d4bbc9b24241326547661591a4820bf02475ad57 Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Tue, 8 Sep 2026 14:01:31 +0300 Subject: [PATCH 16/30] Retain rather than clear on the capped SATB termination path Accepted review finding, and the defect was in a comment before it was in the code. The capped path -- reached when a mutator storms the barrier past CN1_SATB_MAX_REOPENS -- called cn1GcProcessReferences on the way out, justified by "the barrier is already down on this path, so a get() racing this pass cannot log". That sentence is false. cn1SatbBulkBegin answers gcSatbActive OR gcSatbTerminating, and gcSatbTerminating stays raised until after the loop, so a getter there registers, enqueues successfully, and lands its entry in a log this path never takes again. Clearing on that basis can free an object a getter is in the middle of being handed. Rather than correct the reasoning and keep clearing, the path now RETAINS. Nothing is cleared, so nothing can dangle however the race falls, and no argument about flag ordering is load-bearing. The cost is one cycle of reclaim on a path whose own comment records reaching it 0-4 times against a cap of 32. cn1GcRetainAllReferences factors out the retain-to-fixpoint walk that the drop fallback and this path both need -- written three times by hand across this branch, and the fixpoint was missing from two of them. It covers discovered referents, referents already cleared this cycle, and the emergency array, and iterates because marking a retained referent can reach a Reference whose mark function registers during the drain. Gates: run-gc-verify.sh green with all three self-tests firing, run-gauntlet.sh green with all eleven tortures bit-identical and both GC stop modes; plain, CN1_GC_CONFORM, CN1_DISABLE_BIBOP, CN1_DISABLE_SATB and CN1_GC_VERIFY all build. Co-Authored-By: Claude Opus 5 (1M context) --- vm/ByteCodeTranslator/src/cn1_globals.m | 65 ++++++++++++++++++++----- 1 file changed, 53 insertions(+), 12 deletions(-) diff --git a/vm/ByteCodeTranslator/src/cn1_globals.m b/vm/ByteCodeTranslator/src/cn1_globals.m index 7385a05a039..43a331d8150 100644 --- a/vm/ByteCodeTranslator/src/cn1_globals.m +++ b/vm/ByteCodeTranslator/src/cn1_globals.m @@ -2428,6 +2428,39 @@ static JAVA_BOOLEAN cn1RefRecoverEmergency(CODENAME_ONE_THREAD_STATE) { return any; } +// Mark every referent the collector knows about -- discovered, and emergency-cleared -- +// until a drain stops finding more. The safe direction whenever clearing cannot be +// justified: it costs a cycle's reclaim and can never hand out a freed pointer. +static JAVA_BOOLEAN cn1GcRetainAllReferences(CODENAME_ONE_THREAD_STATE) { + JAVA_BOOLEAN marked = JAVA_FALSE; + for(;;) { + long before = cn1RefDiscoveredTop; + JAVA_BOOLEAN round = JAVA_FALSE; + for(long i = 0 ; i < before ; i++) { + JAVA_OBJECT r = __atomic_load_n(cn1RefDiscovered[i].referentField, __ATOMIC_RELAXED); + if(r != JAVA_NULL && !CN1_IS_TAGGED(r)) { + gcMarkObject(threadStateData, r, JAVA_FALSE); + round = JAVA_TRUE; + } + JAVA_OBJECT was = cn1RefDiscovered[i].clearedReferent; + if(was != JAVA_NULL && !CN1_IS_TAGGED(was)) { + gcMarkObject(threadStateData, was, JAVA_FALSE); + round = JAVA_TRUE; + } + } + if(cn1RefRecoverEmergency(threadStateData)) { + round = JAVA_TRUE; + } + if(round) { + marked = JAVA_TRUE; + gcMarkDrain(threadStateData); + } + if(cn1RefDiscoveredTop == before) { + return marked; + } + } +} + // Recompute the soft budget and drop the previous cycle's discoveries. Called from // codenameOneGCMark before anything can mark. static void cn1RefBeginCycle(void) { @@ -4068,19 +4101,27 @@ void codenameOneGCMark() { gcMarkDrain(d); } } - // REFERENCES ONE LAST TIME. Those drains can newly mark an object whose - // graph contains a Reference, and its generated mark function then appends - // a discovery -- after the only reference pass this cycle has run. Leaving - // through here without another pass means that reachable reference keeps an - // unmarked referent the sweep goes on to free, which is the dangling - // pointer the whole pass exists to prevent, reached by the one exit that - // skipped it. + // REFERENCES ONE LAST TIME, and RETAINED rather than cleared. + // + // The drains above can newly mark an object whose graph contains a + // Reference, whose mark function then appends a discovery after the only + // reference pass this cycle has run -- so leaving here without doing + // anything would let the sweep free a referent a reachable Reference still + // points at. + // + // An earlier revision called cn1GcProcessReferences here and justified it + // by saying the barrier was already down so a racing get() could not log. + // THAT WAS WRONG: cn1SatbBulkBegin answers gcSatbActive OR + // gcSatbTerminating, and gcSatbTerminating stays raised until after this + // loop, so a getter here enqueues successfully -- into a log this path then + // never takes again. Clearing on that basis could free an object a getter + // was in the middle of being handed. // - // The barrier is already down on this path, so a get() racing this pass - // cannot log -- which is exactly the weaker invariant the cap falls back - // on, and it is no weaker for references than for anything else the cap - // gives up on. - cn1GcProcessReferences(d); + // Retaining needs none of that reasoning. Nothing is cleared, so nothing + // can dangle however the race falls; the cost is one cycle of reclaim on a + // path reached only when a mutator has stormed the barrier past + // CN1_SATB_MAX_REOPENS, which measures 0-4 against a cap of 32. + cn1GcRetainAllReferences(d); break; } } From 653713da6e0bcb3d42d6a4a22f72d4299dd7b718 Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Tue, 8 Sep 2026 14:45:32 +0300 Subject: [PATCH 17/30] Recover references when the final SATB take loses its batch Accepted review finding, and the last unguarded corner of the same take-side loss. cn1SatbTake reports an empty batch for two different reasons: the log really was empty, or its scratch buffer could not grow and the entries were thrown away. It records the second case in cn1SatbDrops -- but the final catch in codenameOneGCMark runs AFTER cn1GcProcessReferences made its last drop check, and `if(n == 0) break` reads an empty batch as "the mark is closed". Nothing looked at the new counter value, so a Reference.get() that logged its referent successfully, and then had that batch discarded here, kept a pointer the following sweep freed. The break now retains when the counter has moved since the cycle began. Retaining rather than another clear pass, because the barrier is coming down at that point and there is no sound basis left for calling anything dead. Gates: run-gc-verify.sh green with all three self-tests firing, run-gauntlet.sh green with all eleven tortures bit-identical and both GC stop modes. Co-Authored-By: Claude Opus 5 (1M context) --- vm/ByteCodeTranslator/src/cn1_globals.m | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/vm/ByteCodeTranslator/src/cn1_globals.m b/vm/ByteCodeTranslator/src/cn1_globals.m index 43a331d8150..1a0d6849b6a 100644 --- a/vm/ByteCodeTranslator/src/cn1_globals.m +++ b/vm/ByteCodeTranslator/src/cn1_globals.m @@ -4035,6 +4035,21 @@ void codenameOneGCMark() { cn1GcSatbEntries += n; #endif if(n == 0) { + // A ZERO HERE IS NOT ALWAYS "NOTHING SLIPPED IN". cn1SatbTake reports an + // empty batch both when the log was empty and when it could not grow its + // scratch buffer and threw the entries away -- it records the second case + // in cn1SatbDrops, but this catch runs AFTER cn1GcProcessReferences made + // its last drop check, so nothing would otherwise look at the new value. + // A getter that logged successfully and had its batch discarded here would + // then keep a pointer the following sweep frees. + // + // Retaining is the answer rather than another clear pass: the barrier is + // coming down, so there is no sound basis left for deciding anything is + // dead. + if(atomic_load_explicit(&cn1SatbDrops, memory_order_relaxed) + != cn1RefDropsAtCycleStart) { + cn1GcRetainAllReferences(d); + } break; // nothing slipped in: closed, barrier down } // The ONLY way out of this loop is the empty catch above. There is deliberately From ffb8d6fa66cee8fe01e288ad2b35f2d3667834ed Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Tue, 8 Sep 2026 19:59:06 +0300 Subject: [PATCH 18/30] Re-arm the SATB barrier before tracing recovered referents Accepted review finding, and it catches this change breaking a rule stated in capitals a few lines above it. The drop recovery added by the previous commit ran cn1GcRetainAllReferences after gcSatbActive had already been cleared. Retaining MARKS referents that were white and gcMarkDrain then scans them, so those objects are grey at a moment when no barrier is watching -- and a mutator moving an old child out of one of them in that window logs nothing on either side, leaving the child unmarked, not fresh, and reachable only from a grey object the sweep will not protect. That is exactly the hazard the trial-clear comment above describes, and the reason the clear is a TRIAL rather than the end of the mark. Recovery is one more thing that can turn out to mark something new, so it now behaves like the catch it sits next to: re-arm gcSatbActive, retain, and go round the fixpoint again rather than draining underneath a lowered barrier. At the reopen cap it falls through to the same weaker invariant the cap already documents. recoveredDrops is what makes that terminate. Comparing against cn1RefDropsAtCycleStart would re-trigger the recovery on every pass, because that baseline never moves once a drop has happened -- the loop would then re-arm and retain until it hit CN1_SATB_MAX_REOPENS every time a single drop occurred. Recording the count each recovery consumed means another pass happens only if a NEW drop has since been recorded. Gates: run-gc-verify.sh green with all three self-tests firing, run-gauntlet.sh green with all eleven tortures bit-identical and both GC stop modes. Co-Authored-By: Claude Opus 5 (1M context) --- vm/ByteCodeTranslator/src/cn1_globals.m | 33 ++++++++++++++++++++++--- 1 file changed, 30 insertions(+), 3 deletions(-) diff --git a/vm/ByteCodeTranslator/src/cn1_globals.m b/vm/ByteCodeTranslator/src/cn1_globals.m index 1a0d6849b6a..90b7de5d014 100644 --- a/vm/ByteCodeTranslator/src/cn1_globals.m +++ b/vm/ByteCodeTranslator/src/cn1_globals.m @@ -3986,6 +3986,10 @@ void codenameOneGCMark() { // repeats when it marked something NEW, and marks are monotonic and bounded by the live // set. The common case costs one extra empty cn1SatbTake. int reopens = 0; + // The drop count this cycle has already recovered from. Comparing against the + // cycle-start baseline instead would re-trigger the recovery on every pass, because + // that baseline never moves once a drop has happened. + long recoveredDrops = cn1RefDropsAtCycleStart; __atomic_store_n(&gcSatbTerminating, 1, __ATOMIC_SEQ_CST); for(;;) { for(;;) { @@ -4046,9 +4050,32 @@ void codenameOneGCMark() { // Retaining is the answer rather than another clear pass: the barrier is // coming down, so there is no sound basis left for deciding anything is // dead. - if(atomic_load_explicit(&cn1SatbDrops, memory_order_relaxed) - != cn1RefDropsAtCycleStart) { - cn1GcRetainAllReferences(d); + { + long dropsNow = atomic_load_explicit(&cn1SatbDrops, memory_order_relaxed); + if(dropsNow != recoveredDrops) { + // RE-ARM BEFORE TRACING. Retaining marks referents that were white + // and gcMarkDrain then scans them, so with the barrier down those + // objects are grey and unwatched -- and a mutator moving an old + // child out of one in that window logs nothing on either side, which + // is the exact hazard the trial-clear comment above describes. The + // clear is a TRIAL for this reason; recovery is one more thing that + // can turn out to mark something new, so it goes back through the + // fixpoint rather than running underneath a lowered barrier. + recoveredDrops = dropsNow; + __atomic_store_n(&gcSatbActive, 1, __ATOMIC_SEQ_CST); + reopens++; +#ifdef CN1_GC_CONFORM + atomic_fetch_add_explicit(&cn1GcSatbReopens, 1, memory_order_relaxed); +#endif + cn1GcRetainAllReferences(d); + if(reopens < CN1_SATB_MAX_REOPENS) { + continue; // round again with the barrier back up + } + // At the cap: fall through to the same weaker invariant the cap + // documents, with the barrier lowered again below. + __atomic_store_n(&gcSatbActive, 0, __ATOMIC_SEQ_CST); + cn1SatbBulkQuiesce(); + } } break; // nothing slipped in: closed, barrier down } From ba514a7e6e0e5d16e16413c00dabf47049d01c8f Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Tue, 8 Sep 2026 20:31:22 +0300 Subject: [PATCH 19/30] Track emergency slots when deciding the retain fixpoint Accepted review finding. cn1GcRetainAllReferences decided it had converged by comparing cn1RefDiscoveredTop alone, but discovery has two ways to make progress and that counter only sees one of them. During an emergency cycle the drain can reach another soft reference whose referent cn1GcDiscoverReference clears on the spot -- and that path deliberately does NOT append to the discovery list, because it exists precisely for the case where the list could not grow. It fills a cn1RefEmergencyCleared slot instead. So the length could be unchanged while a fresh recovery slot had just been written, the loop would read that as "nothing new" and return, and the referent recorded in that slot would never be marked. Reached from the CN1_SATB_MAX_REOPENS fallback, that leaves the sweep free to take a referent a concurrent Reference.get() is being handed. The loop now watches cn1RefEmergencyTop as well and continues while either counter moves. Gates: run-gc-verify.sh green with all three self-tests firing, run-gauntlet.sh green with all eleven tortures bit-identical and both GC stop modes. Co-Authored-By: Claude Opus 5 (1M context) --- vm/ByteCodeTranslator/src/cn1_globals.m | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/vm/ByteCodeTranslator/src/cn1_globals.m b/vm/ByteCodeTranslator/src/cn1_globals.m index 90b7de5d014..9b14b1ca7e4 100644 --- a/vm/ByteCodeTranslator/src/cn1_globals.m +++ b/vm/ByteCodeTranslator/src/cn1_globals.m @@ -2434,7 +2434,14 @@ static JAVA_BOOLEAN cn1RefRecoverEmergency(CODENAME_ONE_THREAD_STATE) { static JAVA_BOOLEAN cn1GcRetainAllReferences(CODENAME_ONE_THREAD_STATE) { JAVA_BOOLEAN marked = JAVA_FALSE; for(;;) { + // BOTH COUNTERS, because discovery has two ways to make progress. The drain can + // reach a reference whose referent the emergency path clears on the spot, and that + // path deliberately does NOT append to the discovery list -- it exists because the + // list could not grow. Watching only cn1RefDiscoveredTop therefore reads "nothing + // new" while a fresh recovery slot has just been filled, and the loop leaves + // without marking the referent it holds. long before = cn1RefDiscoveredTop; + long beforeEmergency = atomic_load_explicit(&cn1RefEmergencyTop, memory_order_relaxed); JAVA_BOOLEAN round = JAVA_FALSE; for(long i = 0 ; i < before ; i++) { JAVA_OBJECT r = __atomic_load_n(cn1RefDiscovered[i].referentField, __ATOMIC_RELAXED); @@ -2455,7 +2462,8 @@ static JAVA_BOOLEAN cn1GcRetainAllReferences(CODENAME_ONE_THREAD_STATE) { marked = JAVA_TRUE; gcMarkDrain(threadStateData); } - if(cn1RefDiscoveredTop == before) { + if(cn1RefDiscoveredTop == before + && atomic_load_explicit(&cn1RefEmergencyTop, memory_order_relaxed) == beforeEmergency) { return marked; } } From 79a2b662e879b080dfaa39e7fca75b9c3c4484f4 Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Tue, 8 Sep 2026 20:55:32 +0300 Subject: [PATCH 20/30] Consume the touch stamp when the emergency path retains an unrecorded reference Accepted review finding, and it is a livelock inside the path that exists to prevent one. The emergency clear refuses to act on a reference whose stamp reads CN1_REF_TOUCHED and marks the referent instead, which is right for this cycle: a mutator may be holding it. But the ageing loop walks cn1RefDiscovered, and a reference reaching that branch is there precisely BECAUSE it could not be recorded in that list -- so nothing ever resets the stamp. It stays TOUCHED for the life of the process, every later emergency cycle refuses to clear the same referent however long ago it was last read, and if that retained memory is what is blocking the allocation then codenameOneGcMalloc's retry loop never makes progress. The emergency was raised by an allocation failure; this is the failure mode it was added to break. Consuming the stamp after marking closes it. The referent has just been retained, which is the whole of what the stamp was protecting, so the next cycle is free to clear it if nothing reads it again -- and if something does, that get() stamps it afresh. Compare-exchange rather than a plain store so a get() landing between the read and the reset is not silently discarded. Gates: run-gc-verify.sh green with all three self-tests firing, run-gauntlet.sh green with all eleven tortures bit-identical and both GC stop modes. Co-Authored-By: Claude Opus 5 (1M context) --- vm/ByteCodeTranslator/src/cn1_globals.m | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/vm/ByteCodeTranslator/src/cn1_globals.m b/vm/ByteCodeTranslator/src/cn1_globals.m index 9b14b1ca7e4..6e269105dc4 100644 --- a/vm/ByteCodeTranslator/src/cn1_globals.m +++ b/vm/ByteCodeTranslator/src/cn1_globals.m @@ -2733,6 +2733,25 @@ void cn1GcDiscoverReference(CODENAME_ONE_THREAD_STATE, JAVA_OBJECT ref, JAVA_BOO #endif } else { gcMarkObject(threadStateData, r, force); + // CONSUME THE TOUCH, having retained the referent for this cycle. + // + // Nothing else will. The ageing loop walks cn1RefDiscovered, and this reference + // is here precisely because it could not be recorded there -- so a stamp left + // at CN1_REF_TOUCHED stays that way for the life of the process. The condition + // above then refuses to clear on every subsequent emergency cycle, the soft + // referent is retained forever however long ago it was last read, and if that + // memory is what is blocking the allocation, codenameOneGcMalloc's retry loop + // never makes progress. That is the livelock this whole emergency path exists + // to break, reached through the one reference it cannot write down. + // + // Safe because the referent was just MARKED: a mutator holding it is covered + // for this cycle, which is all the stamp was protecting. If it is read again + // the next get() stamps it afresh and it is retained again; if it is not, the + // next emergency cycle is free to clear it. Compare-exchange rather than a + // plain store so a get() landing in between is not silently overwritten. + JAVA_INT expected = CN1_REF_TOUCHED; + __atomic_compare_exchange_n(touchAgeField, &expected, 0, 0, + __ATOMIC_RELAXED, __ATOMIC_RELAXED); } return; } From 9dccb84d90058abfc1aa824f00a59cd8939cf781 Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Tue, 8 Sep 2026 21:11:26 +0300 Subject: [PATCH 21/30] Export SoftReference to the supported API, and retain under the barrier at the cap Two accepted review findings. The first is a hole in this branch's own plan that every gate here was structurally unable to see. SOFTREFERENCE WAS IMPLEMENTED AND UNUSABLE. It existed only in vm/JavaAPI, but maven/java-runtime builds the SUPPORTED API SURFACE from Ports/CLDC11/src and BytecodeComplianceMojo indexes that artifact as the set of types an application may touch. Application code would therefore resolve SoftReference against the host JDK and then be rejected as forbidden API -- a confusing way to discover that a shipped feature was never exported. The plan for this work said to add the CLDC11 stub and it was never done; nothing caught it because every gate here compiles vm/JavaAPI directly, which is exactly the path that bypasses the compliance surface. RETENTION TRACED WITH THE BARRIER DOWN AT THE REOPEN CAP. The drop-recovery path was corrected for this a commit ago and the identical defect was left at CN1_SATB_MAX_REOPENS: cn1GcRetainAllReferences marks referents that were white and gcMarkDrain then traces them, so with gcSatbActive already lowered those objects are grey and unwatched, and a mutator moving an old child out of one logs nothing on either side. Retention now runs BEFORE the barrier is lowered, so everything known at that point is traced under it. What remains after the final take -- retaining references that last drain discovered -- does trace with the barrier down, and that is said plainly at the call rather than glossed: it is the same weaker invariant the cap already relies on for the gcMarkDrain immediately above it, not a new exposure, on a path whose own comment measures 0-4 reopens against a cap of 32. Gates: run-gc-verify.sh green with all three self-tests firing, run-gauntlet.sh green with all eleven tortures bit-identical and both GC stop modes; the CLDC11 java.lang.ref package compiles. Co-Authored-By: Claude Opus 5 (1M context) --- .../src/java/lang/ref/SoftReference.java | 39 ++++++++++++++++++ vm/ByteCodeTranslator/src/cn1_globals.m | 40 ++++++++++--------- 2 files changed, 60 insertions(+), 19 deletions(-) create mode 100644 Ports/CLDC11/src/java/lang/ref/SoftReference.java diff --git a/Ports/CLDC11/src/java/lang/ref/SoftReference.java b/Ports/CLDC11/src/java/lang/ref/SoftReference.java new file mode 100644 index 00000000000..4c242f0d169 --- /dev/null +++ b/Ports/CLDC11/src/java/lang/ref/SoftReference.java @@ -0,0 +1,39 @@ +/* + * 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 java.lang.ref; +/// A reference the collector keeps while the referent is being used and memory allows, and +/// clears before the process runs out. +/// +/// This directory is the SUPPORTED API SURFACE, not an implementation: `maven/java-runtime` +/// compiles it and `BytecodeComplianceMojo` indexes the result as the set of types an +/// application is allowed to touch. A class implemented in `vm/JavaAPI` but missing here is +/// therefore invisible to user code -- javac resolves it against the host JDK and the +/// compliance check then rejects it as forbidden API, which is a confusing way to learn a +/// feature was never exported. The bodies are deliberately inert for the same reason the +/// neighbouring stubs are. +public class SoftReference extends java.lang.ref.Reference{ + /// Creates a new soft reference that refers to the given object. + public SoftReference(java.lang.Object ref){ + } + +} diff --git a/vm/ByteCodeTranslator/src/cn1_globals.m b/vm/ByteCodeTranslator/src/cn1_globals.m index 6e269105dc4..4a872e2ec5c 100644 --- a/vm/ByteCodeTranslator/src/cn1_globals.m +++ b/vm/ByteCodeTranslator/src/cn1_globals.m @@ -4158,6 +4158,13 @@ void codenameOneGCMark() { // Draining is the strictly better of the two -- it loses an object only if // a mutator moves a reference out of one specific object during one // specific scan, where not draining loses it with certainty. + // RETAIN WHILE THE BARRIER IS STILL UP. Retention marks referents that + // were white and gcMarkDrain then traces them, so doing it after the clear + // below would leave those objects grey with nothing watching -- the same + // hazard the trial-clear comment describes, and the same one the drop + // recovery was corrected for. Everything known at this point is covered + // here, under the barrier. + cn1GcRetainAllReferences(d); __atomic_store_n(&gcSatbActive, 0, __ATOMIC_SEQ_CST); cn1SatbBulkQuiesce(); { @@ -4170,27 +4177,22 @@ void codenameOneGCMark() { gcMarkDrain(d); } } - // REFERENCES ONE LAST TIME, and RETAINED rather than cleared. - // - // The drains above can newly mark an object whose graph contains a - // Reference, whose mark function then appends a discovery after the only - // reference pass this cycle has run -- so leaving here without doing - // anything would let the sweep free a referent a reachable Reference still - // points at. + // ANYTHING THAT LAST DRAIN DISCOVERED, retained too. // - // An earlier revision called cn1GcProcessReferences here and justified it - // by saying the barrier was already down so a racing get() could not log. - // THAT WAS WRONG: cn1SatbBulkBegin answers gcSatbActive OR - // gcSatbTerminating, and gcSatbTerminating stays raised until after this - // loop, so a getter here enqueues successfully -- into a log this path then - // never takes again. Clearing on that basis could free an object a getter - // was in the middle of being handed. + // The drain above can newly mark an object whose graph holds a Reference, + // whose mark function then registers a discovery after the retention that + // ran under the barrier. Retaining is all that is left to do with it: + // clearing would need a liveness decision, and the barrier is down. // - // Retaining needs none of that reasoning. Nothing is cleared, so nothing - // can dangle however the race falls; the cost is one cycle of reclaim on a - // path reached only when a mutator has stormed the barrier past - // CN1_SATB_MAX_REOPENS, which measures 0-4 against a cap of 32. - cn1GcRetainAllReferences(d); + // This last call does trace with the barrier lowered, and that is the + // weaker invariant the cap already documents and already relies on for the + // gcMarkDrain immediately above -- it is not a new exposure, and it is + // bounded by a path measured at 0-4 reopens against a cap of 32. The + // retention that matters happened before the barrier came down. + if(cn1RefDiscoveredTop != 0 + || atomic_load_explicit(&cn1RefEmergencyTop, memory_order_relaxed) != 0) { + cn1GcRetainAllReferences(d); + } break; } } From 3065b6abb6867ccb73d1e024c4b0a80ecb35fb20 Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Tue, 8 Sep 2026 21:25:16 +0300 Subject: [PATCH 22/30] Resolve unrecorded referents before reading them, and stop self-test3 going stale Two accepted review findings, both introduced by this branch in the last few commits. A NATIVE CRASH ON THE EMERGENCY PATH. Deciding whether an unrecorded referent was already live read its mark word directly, without the cn1ConservativeResolve guard the clear pass carries. Under conservative roots a dead Reference kept alive by a stale native-stack word can hold a referent swept in an earlier cycle, whose memory is now unmapped -- and gcMarkObject's own comment says reading even the mark word of such a pointer faults. So a collection under memory pressure, which is the only situation that reaches this path, could take the process down. The referent is now resolved before any dereference, and an unresolvable one is treated as neither live nor clearable: nothing can validate it, and it is either garbage or an object allocated after this cycle's extent snapshot that the grace rule keeps anyway. SELF-TEST3 COULD PASS ON A STALE BINARY. It built RefPolicy-verify only when the file was absent, while every driver above it rebuilds unconditionally -- so a regression in the reference-verifier hook could still produce a green self-test by running an old binary. That is the same "a gate that cannot fail" problem this self-test was added to solve, reintroduced in the way the self-test itself is built. It now rebuilds every invocation. Gates: run-gc-verify.sh green with all three self-tests firing, run-gauntlet.sh green with all eleven tortures bit-identical and both GC stop modes; CN1_GC_CONFORM and CN1_DISABLE_BIBOP both build. Co-Authored-By: Claude Opus 5 (1M context) --- vm/ByteCodeTranslator/src/cn1_globals.m | 18 +++++++++++++++++- vm/benchmarks/run-gc-verify.sh | 10 ++++++---- 2 files changed, 23 insertions(+), 5 deletions(-) diff --git a/vm/ByteCodeTranslator/src/cn1_globals.m b/vm/ByteCodeTranslator/src/cn1_globals.m index 4a872e2ec5c..67183662621 100644 --- a/vm/ByteCodeTranslator/src/cn1_globals.m +++ b/vm/ByteCodeTranslator/src/cn1_globals.m @@ -2712,7 +2712,22 @@ void cn1GcDiscoverReference(CODENAME_ONE_THREAD_STATE, JAVA_OBJECT ref, JAVA_BOO // Under genuine exhaustion the residue is a spurious cache miss on an object that // stays alive, against an allocator that otherwise cannot make progress. JAVA_BOOLEAN alreadyLive = JAVA_FALSE; - if(r != JAVA_NULL && !CN1_IS_TAGGED(r)) { + JAVA_BOOLEAN resolvable = (r != JAVA_NULL && !CN1_IS_TAGGED(r)) ? JAVA_TRUE : JAVA_FALSE; +#ifdef CN1_CONSERVATIVE_GC_ROOTS + // RESOLVE BEFORE DEREFERENCING, exactly as the clear pass does. A dead Reference + // kept alive by a stale native-stack word can hold a referent that was swept in an + // earlier cycle and whose memory is now unmapped -- and gcMarkObject's own comment + // says reading even the mark word of such a pointer faults. The clear pass guards + // for this; this fallback read its header first and reached gcMarkObject's + // validation only afterwards, so a collection under memory pressure could take the + // process down. An unresolvable pointer is treated as not-live, which is what it + // is: garbage, or an object allocated after this cycle's extent snapshot and + // therefore kept by the grace rule regardless. + if(resolvable && cn1ConservativeResolve((void*)r) != r && !cn1GcImmortalObjContains(r)) { + resolvable = JAVA_FALSE; + } +#endif + if(resolvable) { int rm = __atomic_load_n(&r->__codenameOneGcMark, __ATOMIC_ACQUIRE); alreadyLive = (rm == currentGcMarkValue || rm == -1) ? JAVA_TRUE : JAVA_FALSE; } @@ -2723,6 +2738,7 @@ void cn1GcDiscoverReference(CODENAME_ONE_THREAD_STATE, JAVA_OBJECT ref, JAVA_BOO // emergency wanted back, which is a worse outcome than clearing and a far better // one than a dangling read. if(!alreadyLive + && resolvable && strength == CN1_REF_SOFT && atomic_load_explicit(&cn1SoftRetainCycles, memory_order_relaxed) < 0 && __atomic_load_n(touchAgeField, __ATOMIC_RELAXED) != CN1_REF_TOUCHED diff --git a/vm/benchmarks/run-gc-verify.sh b/vm/benchmarks/run-gc-verify.sh index dd4702b999f..2514d7e3d1b 100755 --- a/vm/benchmarks/run-gc-verify.sh +++ b/vm/benchmarks/run-gc-verify.sh @@ -136,10 +136,12 @@ fi # reported violations=0 for exactly that reason. The dangling direction is # clearing LESS. printf '%-16s ' "self-test3" -if [ ! -x ./target/bin/RefPolicy-verify ]; then - ./translate-and-build.sh RefPolicy target/bin/RefPolicy-verify -DCN1_GC_VERIFY \ - > target/bin/RefPolicy-selftest-build.log 2>&1 || true -fi +# UNCONDITIONALLY, like every driver above. Reusing a binary left by an earlier build or +# checkout means a regression in the reference-verifier hook can still produce a green +# self-test by exercising stale code -- which is the same "a gate that cannot fail" problem +# this self-test was added to solve, reintroduced in the way the self-test is built. +./translate-and-build.sh RefPolicy target/bin/RefPolicy-verify -DCN1_GC_VERIFY \ + > target/bin/RefPolicy-selftest-build.log 2>&1 || true if [ ! -x ./target/bin/RefPolicy-verify ]; then echo "BROKEN -- could not build RefPolicy for the reference self-test" fail=1 From a64e25a9f625024ea78032d8b59e1bf70d9d585c Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Tue, 8 Sep 2026 21:38:20 +0300 Subject: [PATCH 23/30] Delete the duplicated retain walk, and make the self-test rebuild fail loudly Two accepted review findings, and both are the same mistake: a fix applied to one copy of something and not to its duplicate. THE EARLY DROP RECOVERY WAS A SECOND COPY of the retain-to-fixpoint walk. The shared helper was taught that discovery advances TWO counters -- the emergency path clears a referent into cn1RefEmergencyCleared without touching cn1RefDiscoveredTop, because it exists precisely for when that list cannot grow -- and this copy was left comparing the list length alone. It therefore read "nothing new" over a freshly written recovery slot and returned without marking what that slot held, so a racing get() whose enqueue had also failed could be handed an object the sweep then freed. The copy is deleted and the helper called; patching it would have left a third place to drift. THE SELF-TEST REBUILD STILL ACCEPTED STALE CODE. Rebuilding unconditionally was one of three things needed and the only one done. translate-and-build.sh replaces its output only after the final compiler run succeeds, so a FAILED rebuild leaves the previous binary in place; and `|| true` discarded the status, so the -x test below accepted that binary. The self-test could then run old code and report green -- the "gate that cannot fail" problem it exists to prevent, for the second time in how it is built. It now removes the output first, checks the build status, and fails the gate. Gates: run-gc-verify.sh green with all three self-tests firing, run-gauntlet.sh green with all eleven tortures bit-identical and both GC stop modes. Co-Authored-By: Claude Opus 5 (1M context) --- vm/ByteCodeTranslator/src/cn1_globals.m | 41 ++++++------------------- vm/benchmarks/run-gc-verify.sh | 20 ++++++++---- 2 files changed, 24 insertions(+), 37 deletions(-) diff --git a/vm/ByteCodeTranslator/src/cn1_globals.m b/vm/ByteCodeTranslator/src/cn1_globals.m index 67183662621..76149da1688 100644 --- a/vm/ByteCodeTranslator/src/cn1_globals.m +++ b/vm/ByteCodeTranslator/src/cn1_globals.m @@ -2920,37 +2920,16 @@ static JAVA_BOOLEAN cn1GcProcessReferences(CODENAME_ONE_THREAD_STATE) { // "keep the referent alive"; the note on the unrecorded-discovery path says the // same thing. Not clearing a weak field does not retain anything, because nothing // else marks a weak referent -- that is what makes the edge weak. - // TO A FIXPOINT, for the same reason sub-pass A runs to one: marking a retained - // referent traces it, and an object kept alive only that way can itself hold - // further references whose mark functions register late. Marking the list once and - // draining discovers those AFTER the loop has finished, and the recovery would - // return leaving a newly reachable Reference holding an unmarked referent -- the - // dangling pointer this recovery exists to avoid, one level deeper. - for(;;) { - long before = cn1RefDiscoveredTop; - JAVA_BOOLEAN markedThisRound = JAVA_FALSE; - for(long i = 0 ; i < before ; i++) { - JAVA_OBJECT r = __atomic_load_n(cn1RefDiscovered[i].referentField, __ATOMIC_RELAXED); - if(r != JAVA_NULL && !CN1_IS_TAGGED(r)) { - gcMarkObject(threadStateData, r, JAVA_FALSE); - markedThisRound = JAVA_TRUE; - } - } - // AND THE EMERGENCY CLEARS. Those referents are gone from their fields - // already, so cn1RefDiscovered cannot reach them -- they exist only in the - // recovery array. This early return happens before the post-clear recovery - // below, so without this they would be retained nowhere and swept under a - // getter that had already been handed one. - if(cn1RefRecoverEmergency(threadStateData)) { - markedThisRound = JAVA_TRUE; - } - if(markedThisRound) { - marked = JAVA_TRUE; - gcMarkDrain(threadStateData); - } - if(cn1RefDiscoveredTop == before) { - break; - } + // THE SAME RETAIN-TO-FIXPOINT the capped path uses, called rather than copied. + // + // This was a second hand-written copy of that walk, and it drifted exactly where a + // copy does: the helper was taught that discovery advances TWO counters -- the + // emergency path clears a referent into cn1RefEmergencyCleared without touching + // cn1RefDiscoveredTop, because it exists for when that list cannot grow -- and this + // copy was left comparing the list length alone. It therefore read "nothing new" + // over a freshly written recovery slot and returned without marking what it held. + if(cn1GcRetainAllReferences(threadStateData)) { + marked = JAVA_TRUE; } #ifdef CN1_GC_CONFORM cn1RefPhaseNs += cn1GcNowNs() - __r0; diff --git a/vm/benchmarks/run-gc-verify.sh b/vm/benchmarks/run-gc-verify.sh index 2514d7e3d1b..d6324e7012e 100755 --- a/vm/benchmarks/run-gc-verify.sh +++ b/vm/benchmarks/run-gc-verify.sh @@ -136,12 +136,20 @@ fi # reported violations=0 for exactly that reason. The dangling direction is # clearing LESS. printf '%-16s ' "self-test3" -# UNCONDITIONALLY, like every driver above. Reusing a binary left by an earlier build or -# checkout means a regression in the reference-verifier hook can still produce a green -# self-test by exercising stale code -- which is the same "a gate that cannot fail" problem -# this self-test was added to solve, reintroduced in the way the self-test is built. -./translate-and-build.sh RefPolicy target/bin/RefPolicy-verify -DCN1_GC_VERIFY \ - > target/bin/RefPolicy-selftest-build.log 2>&1 || true +# REMOVE FIRST, THEN BUILD, AND CHECK THE STATUS. Three things are needed and only the +# third is obvious. Rebuilding unconditionally is not enough on its own: translate-and-build +# replaces its output only after the final compiler run succeeds, so a failed rebuild leaves +# the PREVIOUS binary in place. Nor is `|| true` harmless: it discards the status, and the +# -x test below then accepts that stale executable. Either way the self-test runs old code +# and reports green -- the "gate that cannot fail" problem this self-test exists to prevent, +# reintroduced in how the self-test is built. +rm -f ./target/bin/RefPolicy-verify +if ! ./translate-and-build.sh RefPolicy target/bin/RefPolicy-verify -DCN1_GC_VERIFY \ + > target/bin/RefPolicy-selftest-build.log 2>&1; then + echo "BROKEN -- could not build RefPolicy for the reference self-test" + tail -25 target/bin/RefPolicy-selftest-build.log + fail=1 +fi if [ ! -x ./target/bin/RefPolicy-verify ]; then echo "BROKEN -- could not build RefPolicy for the reference self-test" fail=1 From a41b4a4c2070e71692c9163bcd6be8f19a69acf6 Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Tue, 8 Sep 2026 21:53:26 +0300 Subject: [PATCH 24/30] Let drop recovery reclaim what the emergency condemned, and delete the last copy Two accepted review findings. DROP RECOVERY DEADLOCKED THE ALLOCATOR. Retaining everything on an SATB loss is the safe reflex and it is wrong under the emergency budget: that budget is raised by an allocation FAILURE, sustained exhaustion is exactly what keeps the SATB stack from growing, and if soft-referenced data is what exhausted memory then every retry cycle loses a batch, retains the same data, and codenameOneGcMalloc spins on collections that free nothing. The emergency exists to break that deadlock and this recovery was reinstating it. The distinction that resolves it: the emergency decision never depended on the log. "Drop every soft referent" is a policy choice taken from the memory budget at cycle start, not an inference from liveness, so a lost log entry does not invalidate it. What the log would have protected is a referent a mutator is mid-read of -- and the touch stamp records that independently and allocation-free, which is the signal sub-pass A already trusts. So cn1GcRecoverAfterDrop retains touched referents, still clears condemned soft ones, and retains everything else. The residual, stated at the code, is a get() that loaded a soft referent and was descheduled before stamping: the window the emergency path already accepts, against an allocator that otherwise cannot progress. A THIRD COPY OF THE FIXPOINT. The previous commit deleted one hand-written copy of the retain walk and said a third would drift again; there already was one, in the post-clear recovery, and it was not looked for. It had drifted the same way -- watching cn1RefDiscoveredTop while the emergency path advances cn1RefEmergencyTop -- so it read "nothing new" over a freshly written recovery slot. There is now one implementation and no copies. Gates: run-gc-verify.sh green with all three self-tests firing, run-gauntlet.sh green with all eleven tortures bit-identical and both GC stop modes; the emergency still reclaims under injected allocation failure (retained=0 at softBudget=-1). Co-Authored-By: Claude Opus 5 (1M context) --- vm/ByteCodeTranslator/src/cn1_globals.m | 119 ++++++++++++++++-------- 1 file changed, 80 insertions(+), 39 deletions(-) diff --git a/vm/ByteCodeTranslator/src/cn1_globals.m b/vm/ByteCodeTranslator/src/cn1_globals.m index 76149da1688..96b34a25b05 100644 --- a/vm/ByteCodeTranslator/src/cn1_globals.m +++ b/vm/ByteCodeTranslator/src/cn1_globals.m @@ -2469,6 +2469,74 @@ static JAVA_BOOLEAN cn1GcRetainAllReferences(CODENAME_ONE_THREAD_STATE) { } } +// Recovery after an SATB loss, which must keep racing loads alive WITHOUT undoing the +// emergency's reclaim. +// +// Retaining everything is the safe reflex and it deadlocks the allocator: the emergency +// budget is raised by an allocation failure, sustained exhaustion keeps the SATB stack from +// growing, and if soft-referenced data is what exhausted memory then every retry cycle +// loses a batch, retains the same data, and codenameOneGcMalloc spins forever on a +// collection that frees nothing. +// +// The emergency decision does not depend on the log. "Drop every soft referent" is a policy +// choice made from the memory budget at cycle start, not an inference from liveness, so a +// lost log entry does not invalidate it. What the log WOULD have protected is a referent a +// mutator is mid-read of -- and the touch stamp records that independently, allocation-free, +// which is the same signal sub-pass A relies on. +// +// So: touched referents are retained, condemned soft referents are still cleared, and +// everything else is retained. The residual is a get() that loaded a soft referent and was +// descheduled before stamping; that window is the one the emergency path already accepts +// and documents, and the alternative to accepting it is an allocator that cannot progress. +static JAVA_BOOLEAN cn1GcRecoverAfterDrop(CODENAME_ONE_THREAD_STATE) { + int budget = atomic_load_explicit(&cn1SoftRetainCycles, memory_order_relaxed); + if(budget >= 0) { + return cn1GcRetainAllReferences(threadStateData); // no emergency: retain freely + } + JAVA_BOOLEAN marked = JAVA_FALSE; + for(;;) { + long before = cn1RefDiscoveredTop; + long beforeEmergency = atomic_load_explicit(&cn1RefEmergencyTop, memory_order_relaxed); + JAVA_BOOLEAN round = JAVA_FALSE; + for(long i = 0 ; i < before ; i++) { + struct CN1RefEntry* e = &cn1RefDiscovered[i]; + JAVA_OBJECT r = __atomic_load_n(e->referentField, __ATOMIC_RELAXED); + if(r != JAVA_NULL && !CN1_IS_TAGGED(r)) { + if(e->strength == CN1_REF_SOFT + && __atomic_load_n(e->touchAgeField, __ATOMIC_RELAXED) != CN1_REF_TOUCHED) { + // Condemned by the emergency, and not being read: clear it, which is + // the whole point of the emergency. Not recorded in clearedReferent -- + // this recovery IS the last word, and recording it would only cause + // the retention below to undo it. + __atomic_store_n(e->referentField, JAVA_NULL, __ATOMIC_RELAXED); +#ifdef CN1_GC_CONFORM + atomic_fetch_add_explicit(&cn1RefCleared, 1, memory_order_relaxed); +#endif + } else { + gcMarkObject(threadStateData, r, JAVA_FALSE); + round = JAVA_TRUE; + } + } + JAVA_OBJECT was = e->clearedReferent; + if(was != JAVA_NULL && !CN1_IS_TAGGED(was)) { + gcMarkObject(threadStateData, was, JAVA_FALSE); + round = JAVA_TRUE; + } + } + if(cn1RefRecoverEmergency(threadStateData)) { + round = JAVA_TRUE; + } + if(round) { + marked = JAVA_TRUE; + gcMarkDrain(threadStateData); + } + if(cn1RefDiscoveredTop == before + && atomic_load_explicit(&cn1RefEmergencyTop, memory_order_relaxed) == beforeEmergency) { + return marked; + } + } +} + // Recompute the soft budget and drop the previous cycle's discoveries. Called from // codenameOneGCMark before anything can mark. static void cn1RefBeginCycle(void) { @@ -2920,7 +2988,9 @@ static JAVA_BOOLEAN cn1GcProcessReferences(CODENAME_ONE_THREAD_STATE) { // "keep the referent alive"; the note on the unrecorded-discovery path says the // same thing. Not clearing a weak field does not retain anything, because nothing // else marks a weak referent -- that is what makes the edge weak. - // THE SAME RETAIN-TO-FIXPOINT the capped path uses, called rather than copied. + // RECOVERY, not blanket retention: see cn1GcRecoverAfterDrop. Retaining every + // condemned soft referent here is what would let the allocator spin forever when + // soft-referenced data is the thing exhausting memory. // // This was a second hand-written copy of that walk, and it drifted exactly where a // copy does: the helper was taught that discovery advances TWO counters -- the @@ -2928,7 +2998,7 @@ static JAVA_BOOLEAN cn1GcProcessReferences(CODENAME_ONE_THREAD_STATE) { // cn1RefDiscoveredTop, because it exists for when that list cannot grow -- and this // copy was left comparing the list length alone. It therefore read "nothing new" // over a freshly written recovery slot and returned without marking what it held. - if(cn1GcRetainAllReferences(threadStateData)) { + if(cn1GcRecoverAfterDrop(threadStateData)) { marked = JAVA_TRUE; } #ifdef CN1_GC_CONFORM @@ -3017,43 +3087,14 @@ static JAVA_BOOLEAN cn1GcProcessReferences(CODENAME_ONE_THREAD_STATE) { // mutator holds it, so the recovery marks what was cleared rather than restoring it. cn1SatbBulkQuiesce(); if(atomic_load_explicit(&cn1SatbDrops, memory_order_relaxed) != cn1RefDropsAtCycleStart) { - // TO A FIXPOINT, like every other recovery here. A referent brought back can hold - // a further Reference, whose mark function registers during the drain -- after - // these loops have run. Draining once would leave that nested reference reachable - // with an unmarked referent, and an empty final take would then let termination - // finish over it. - for(;;) { - long before = cn1RefDiscoveredTop; - JAVA_BOOLEAN recovered = JAVA_FALSE; - for(long i = 0 ; i < n ; i++) { - JAVA_OBJECT was = cn1RefDiscovered[i].clearedReferent; - if(was != JAVA_NULL && !CN1_IS_TAGGED(was)) { - gcMarkObject(threadStateData, was, JAVA_FALSE); - recovered = JAVA_TRUE; - } - } - // The emergency path's clears too. They never reached the list -- that is why - // they exist -- so they are remembered separately and recovered on the same - // signal. - if(cn1RefRecoverEmergency(threadStateData)) { - recovered = JAVA_TRUE; - } - // Anything newly discovered by the drain still has to be retained: this pass - // is past the point where it could safely clear. - for(long i = n ; i < before ; i++) { - JAVA_OBJECT r = __atomic_load_n(cn1RefDiscovered[i].referentField, __ATOMIC_RELAXED); - if(r != JAVA_NULL && !CN1_IS_TAGGED(r)) { - gcMarkObject(threadStateData, r, JAVA_FALSE); - recovered = JAVA_TRUE; - } - } - if(recovered) { - marked = JAVA_TRUE; - gcMarkDrain(threadStateData); - } - if(cn1RefDiscoveredTop == before) { - break; - } + // THE SHARED WALK, not a third hand-written copy. Two earlier copies of this + // fixpoint each drifted from the helper in the same way -- watching only + // cn1RefDiscoveredTop, while the emergency path advances cn1RefEmergencyTop + // instead -- so the loop read "nothing new" over a freshly written recovery slot. + // The helper covers discovered referents, referents already cleared this cycle and + // the emergency array, and iterates on both counters. + if(cn1GcRetainAllReferences(threadStateData)) { + marked = JAVA_TRUE; } } From 4c0292af739884e00c6441649eaf639de699e556 Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Tue, 8 Sep 2026 22:05:18 +0300 Subject: [PATCH 25/30] Quiesce before reading touch stamps in drop recovery, and spare live referents Two accepted review findings, both on the recovery helper added one commit ago. THE TOUCH STAMP WAS READ TOO EARLY. cn1SatbDrops becomes visible when an enqueue FAILS, and that happens before the accessor reaches its stamp -- so entering recovery on that signal and reading touchAgeField immediately can see "not touched" for a getter that is mid-load and about to stamp. Clearing on that reading hands the sweep an object the getter is being given, and this path deliberately does not record clearedReferent, so nothing downstream could recover it. The helper now quiesces first. Every getter registers across its whole load, the stamp included, so an in-flight count of zero means every getter that overlapped has finished and published -- which is what makes the stamp evidence rather than a race. A STRONGLY REACHABLE REFERENT COULD BE CLEARED. The other emergency path consults the mark word before clearing, because a SoftReference may only be cleared when its referent is SOFTLY reachable; this helper omitted that test and would clear an application's cache entry for an object it also holds in an ordinary field. It now applies the same current-epoch-or-fresh check. Both are the same shape: new code on one path missing a guard its sibling already carries. Gates: run-gc-verify.sh green with all three self-tests firing, run-gauntlet.sh green with all eleven tortures bit-identical and both GC stop modes; the emergency still reclaims under injected allocation failure (retained=0 at softBudget=-1). Co-Authored-By: Claude Opus 5 (1M context) --- vm/ByteCodeTranslator/src/cn1_globals.m | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/vm/ByteCodeTranslator/src/cn1_globals.m b/vm/ByteCodeTranslator/src/cn1_globals.m index 96b34a25b05..1b281defe70 100644 --- a/vm/ByteCodeTranslator/src/cn1_globals.m +++ b/vm/ByteCodeTranslator/src/cn1_globals.m @@ -2494,6 +2494,17 @@ static JAVA_BOOLEAN cn1GcRecoverAfterDrop(CODENAME_ONE_THREAD_STATE) { return cn1GcRetainAllReferences(threadStateData); // no emergency: retain freely } JAVA_BOOLEAN marked = JAVA_FALSE; + // QUIESCE BEFORE READING ANY TOUCH STAMP. The drop counter becomes visible when an + // enqueue fails, which is BEFORE the accessor reaches its stamp -- so entering here on + // that signal and reading touchAgeField immediately can see "not touched" for a getter + // that is mid-load and about to stamp. Clearing on that reading hands the sweep an + // object the getter is being given, and this path deliberately does not record + // clearedReferent, so nothing downstream could recover it. + // + // Every getter registers across its whole load, stamp included, so an in-flight count + // of zero means every getter that overlapped has finished and published. That is what + // makes the stamp readable as evidence rather than as a race. + cn1SatbBulkQuiesce(); for(;;) { long before = cn1RefDiscoveredTop; long beforeEmergency = atomic_load_explicit(&cn1RefEmergencyTop, memory_order_relaxed); @@ -2502,7 +2513,16 @@ static JAVA_BOOLEAN cn1GcRecoverAfterDrop(CODENAME_ONE_THREAD_STATE) { struct CN1RefEntry* e = &cn1RefDiscovered[i]; JAVA_OBJECT r = __atomic_load_n(e->referentField, __ATOMIC_RELAXED); if(r != JAVA_NULL && !CN1_IS_TAGGED(r)) { - if(e->strength == CN1_REF_SOFT + // A referent already marked, or fresh, is reachable some other way -- a + // strong edge or a root -- and a SoftReference may only be cleared when + // its referent is SOFTLY reachable. The other emergency path applies this + // same test; omitting it here would clear an application's cache entry for + // an object it also holds in an ordinary field. + int rm = __atomic_load_n(&r->__codenameOneGcMark, __ATOMIC_ACQUIRE); + JAVA_BOOLEAN reachableOtherwise = + (rm == currentGcMarkValue || rm == -1) ? JAVA_TRUE : JAVA_FALSE; + if(!reachableOtherwise + && e->strength == CN1_REF_SOFT && __atomic_load_n(e->touchAgeField, __ATOMIC_RELAXED) != CN1_REF_TOUCHED) { // Condemned by the emergency, and not being read: clear it, which is // the whole point of the emergency. Not recorded in clearedReferent -- From a11dae2e52752b6094b54692793c76a002c5e079 Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Tue, 8 Sep 2026 22:20:27 +0300 Subject: [PATCH 26/30] Guard the recovery's header read, and record what it clears Two accepted review findings, both on the helper this branch added three commits ago, and the first is the previous fix creating the next defect. THE MARK-WORD READ HAD NO RESOLVE GUARD. It was added one commit back to stop the recovery clearing strongly reachable referents -- and it dereferences the referent without cn1ConservativeResolve, which the clear pass and the unrecorded emergency path both apply before the same access. A Reference kept alive by a stale native-stack word can hold a referent swept in an earlier cycle whose memory is unmapped, and reading even its mark word faults. So a fix for a correctness gap introduced a native crash, in code whose two siblings show the right pattern. THE QUIESCE CANNOT HOLD. It drains the getters in flight when recovery starts and cannot stop a new one registering immediately afterwards, while gcSatbActive and gcSatbTerminating are both still raised. That getter can load the referent, have its enqueue fail, and be descheduled before stamping, so this loop reads the old stamp and clears a field whose referent is being handed out -- with nothing saved, no later pass could mark it. The clear is now recorded. The post-clear recovery therefore marks it whenever a drop is visible, which gives the emergency's reclaim back in exactly the case where safety is uncertain, and keeps it in the common case where no further drop occurs. That is the right way round: reclaim is the goal, not being right. This is the fourth consecutive round on this one helper, each fix producing the next finding. The oscillation is between two poles this path genuinely sits between -- reclaiming under memory pressure, and staying safe against an unreliable log -- which is an argument about the shape of the design rather than about any of the individual fixes. Gates: run-gc-verify.sh green with all three self-tests firing, run-gauntlet.sh green with all eleven tortures bit-identical and both GC stop modes. The first gauntlet run was killed at IbpTest and re-run from the start; a partial gauntlet is not a result. Co-Authored-By: Claude Opus 5 (1M context) --- vm/ByteCodeTranslator/src/cn1_globals.m | 49 +++++++++++++++++++------ 1 file changed, 37 insertions(+), 12 deletions(-) diff --git a/vm/ByteCodeTranslator/src/cn1_globals.m b/vm/ByteCodeTranslator/src/cn1_globals.m index 1b281defe70..c7317d8e8ef 100644 --- a/vm/ByteCodeTranslator/src/cn1_globals.m +++ b/vm/ByteCodeTranslator/src/cn1_globals.m @@ -2513,26 +2513,51 @@ static JAVA_BOOLEAN cn1GcRecoverAfterDrop(CODENAME_ONE_THREAD_STATE) { struct CN1RefEntry* e = &cn1RefDiscovered[i]; JAVA_OBJECT r = __atomic_load_n(e->referentField, __ATOMIC_RELAXED); if(r != JAVA_NULL && !CN1_IS_TAGGED(r)) { + // RESOLVE BEFORE READING THE HEADER. A Reference kept alive by a stale + // native-stack word can hold a referent swept in an earlier cycle whose + // memory is unmapped, and reading even its mark word faults. The clear + // pass and the unrecorded emergency path both guard this; the guard was + // omitted here when this mark-word test was added one commit ago. + JAVA_BOOLEAN usable = JAVA_TRUE; +#ifdef CN1_CONSERVATIVE_GC_ROOTS + if(cn1ConservativeResolve((void*)r) != r && !cn1GcImmortalObjContains(r)) { + usable = JAVA_FALSE; + } +#endif // A referent already marked, or fresh, is reachable some other way -- a // strong edge or a root -- and a SoftReference may only be cleared when - // its referent is SOFTLY reachable. The other emergency path applies this - // same test; omitting it here would clear an application's cache entry for - // an object it also holds in an ordinary field. - int rm = __atomic_load_n(&r->__codenameOneGcMark, __ATOMIC_ACQUIRE); - JAVA_BOOLEAN reachableOtherwise = - (rm == currentGcMarkValue || rm == -1) ? JAVA_TRUE : JAVA_FALSE; - if(!reachableOtherwise + // its referent is SOFTLY reachable. Omitting this would clear an + // application's cache entry for an object it also holds in a field. + JAVA_BOOLEAN reachableOtherwise = JAVA_TRUE; + if(usable) { + int rm = __atomic_load_n(&r->__codenameOneGcMark, __ATOMIC_ACQUIRE); + reachableOtherwise = + (rm == currentGcMarkValue || rm == -1) ? JAVA_TRUE : JAVA_FALSE; + } + if(usable + && !reachableOtherwise && e->strength == CN1_REF_SOFT && __atomic_load_n(e->touchAgeField, __ATOMIC_RELAXED) != CN1_REF_TOUCHED) { - // Condemned by the emergency, and not being read: clear it, which is - // the whole point of the emergency. Not recorded in clearedReferent -- - // this recovery IS the last word, and recording it would only cause - // the retention below to undo it. + // RECORDED, even though recording can cost the reclaim. + // + // The quiesce above drains the getters that were in flight when this + // began; it cannot stop a new one registering immediately after, and + // that getter can load r, have its enqueue fail, and be descheduled + // before stamping -- so this loop sees the old stamp and clears a field + // whose referent is being handed out. Without a record nothing later + // could mark it and the sweep would take it. + // + // Saving it means the post-clear recovery marks it when a drop is + // visible, which yields the emergency's reclaim back in exactly the + // situation where safety is uncertain -- the right way round. In the + // common case, where no further drop occurs, the clear stands and the + // reclaim happens. + e->clearedReferent = r; __atomic_store_n(e->referentField, JAVA_NULL, __ATOMIC_RELAXED); #ifdef CN1_GC_CONFORM atomic_fetch_add_explicit(&cn1RefCleared, 1, memory_order_relaxed); #endif - } else { + } else if(usable) { gcMarkObject(threadStateData, r, JAVA_FALSE); round = JAVA_TRUE; } From 6d3cf6362f8d43da92e9dbc749ac3e59d416e363 Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Tue, 8 Sep 2026 22:56:35 +0300 Subject: [PATCH 27/30] Do no reference work in a program that has no references Localises and fixes an intermittent CI regression, and removes a cost every application was paying for a feature most of them never use. BibopPageFloorIntegrationTest began failing on the 4-marker arm64 job -- five times against master's five clean samples -- reporting its pages as released while phys_footprint stayed at ~231MB. The workload contains no Reference at all, which is why it took so long to believe: nothing in this feature can run there. Except one thing did. cn1GcProcessReferences ran on every outer termination pass regardless, and once the drop re-check landed it called cn1SatbBulkQuiesce() unconditionally -- which is not free. It spins in usleep(50) while any BULK ARRAY COPY is in flight, and that app copies object arrays, so the collector could stall inside the termination loop on behalf of a feature the program does not use. It fits the shape that never made sense otherwise: intermittent, only under the configuration where four markers and bulk copies actually overlap, and reporting release while the footprint does not move. Dispatching the workflow on wip/refbisect-base -- master plus only the first commit, weak references with none of this machinery -- passes the same job, which is what localised it to the later commits rather than to the feature. The fix is what should have been there from the start: with nothing discovered and nothing in the emergency array, the pass returns immediately and touches nothing. An earlier hypothesis for this failure was WRONG and is recorded so it is not retried: cn1RefBeginCycle's headroom probe was suspected of adding a per-cycle footprint syscall, but the test sets only CN1_LOG_PAGE_RELEASE, never CN1_SIMULATE_PROC_MEMORY_LIMIT, and cn1SimulatedProcLimitBytes caches -- so on Linux that path returns -1 from a cached atomic read. Gates: run-gc-verify.sh green with all three self-tests firing, run-gauntlet.sh green with all eleven tortures bit-identical and both GC stop modes; RefPolicy's weak, alias and cache assertions unchanged. Co-Authored-By: Claude Opus 5 (1M context) --- vm/ByteCodeTranslator/src/cn1_globals.m | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/vm/ByteCodeTranslator/src/cn1_globals.m b/vm/ByteCodeTranslator/src/cn1_globals.m index c7317d8e8ef..59040b1c5fa 100644 --- a/vm/ByteCodeTranslator/src/cn1_globals.m +++ b/vm/ByteCodeTranslator/src/cn1_globals.m @@ -2905,6 +2905,23 @@ void cn1GcDiscoverReference(CODENAME_ONE_THREAD_STATE, JAVA_OBJECT ref, JAVA_BOO // Runs on the GC thread only, inside the SATB termination loop, barrier armed. // Returns JAVA_TRUE if it marked anything; it has already drained by then. static JAVA_BOOLEAN cn1GcProcessReferences(CODENAME_ONE_THREAD_STATE) { + // NOTHING TO DO IN A PROGRAM WITH NO REFERENCES, and doing nothing has to mean + // touching nothing. Most applications never construct a Reference, and for those this + // pass previously still ran a cn1SatbBulkQuiesce() on every outer termination pass -- + // which is not free: it spins in usleep(50) while any BULK ARRAY COPY is in flight, so + // an allocation-heavy program that arraycopies object arrays pays a collector stall for + // a feature it does not use. + // + // The early exit is also the answer to a CI regression that took a long time to find: + // BibopPageFloorIntegrationTest, whose workload contains no Reference at all, began + // failing intermittently on the 4-marker arm64 configuration once that quiesce landed, + // reporting its pages as released while the footprint stayed put. The base feature + // commit -- weak references with none of this machinery -- passes that job, which is + // what localised it here. + if(cn1RefDiscoveredTop == 0 + && atomic_load_explicit(&cn1RefEmergencyTop, memory_order_relaxed) == 0) { + return JAVA_FALSE; + } JAVA_BOOLEAN marked = JAVA_FALSE; #ifdef CN1_GC_CONFORM long long __r0 = cn1GcNowNs(); From 1c8ab03bf5d6edbd58455bf10491e0a0e0b4bce7 Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Wed, 9 Sep 2026 00:05:48 +0300 Subject: [PATCH 28/30] Nursery promotion, two silent gates, and a baseline the rename broke cn1GcDiscoverReference now defers to gcMarkObject while a thread is inside its own nursery minor collection. cn1PromoteDrain runs the generated mark functions with nurseryPromoting raised, and gcMarkObject's promotion branch is what moves a referenced object out of the block being recycled -- routing the referent past it meant a surviving WeakReference could be promoted alone and left pointing into a reclaimed block. Nothing clears on that path, so the edge costs a promotion and nothing else. The nursery arm did not compile at all, here or on master: nativeMethods' bulk barrier calls cn1SatbBulkBegin unconditionally while the prototype sat in the #else of the CN1_NURSERY split, which clang rejects as an implicit declaration under C99. The load barrier added a second instance. Declared beside the deletion barrier instead, where the callers are. Two gates that could not fail: - RefPolicy printed WEAK_LIVE_KEPT and exited 0 whatever it said. A referent cleared while still strongly reachable is heap-SAFE -- null dangles nothing -- so run-gc-verify cannot see it either, and the checksum reads live[] directly. Both advertised validation paths could stay green while WeakReference silently emptied every cache built on it. It is an assertion now. - ab-refs.sh ignored the return code. Metrics print before exit, so a VM that corrupted its heap and died in an atexit handler still emitted RESULT and the whole table, and the harness published checksum-matched medians from a crashed run. The cast-semantics baseline names anonymous classes as Outer$N, so #5746 adding one to AndroidImplementation renumbered onReceive from $46 to $47 and left the entry stale. master is red on the gate today; this is the same cast, not a new one. Co-Authored-By: Claude Opus 5 (1M context) --- scripts/cast-semantics-baseline.txt | 2 +- vm/ByteCodeTranslator/src/cn1_globals.h | 12 ++++++++++++ vm/ByteCodeTranslator/src/cn1_globals.m | 17 +++++++++++++++++ vm/benchmarks/ab-refs.sh | 7 +++++++ vm/benchmarks/src/com/bench/RefPolicy.java | 12 ++++++++++++ 5 files changed, 49 insertions(+), 1 deletion(-) diff --git a/scripts/cast-semantics-baseline.txt b/scripts/cast-semantics-baseline.txt index f1754f94c10..abc1f2534b1 100644 --- a/scripts/cast-semantics-baseline.txt +++ b/scripts/cast-semantics-baseline.txt @@ -54,7 +54,7 @@ com/codename1/impl/android/AndroidImplementation#scheduleBackgroundWork(Lcom/cod com/codename1/impl/android/AndroidImplementation#signingCertificatesViaReflection(Landroid/content/pm/PackageManager;Ljava/lang/String;)[Landroid/content/pm/Signature;|cast to [Landroid.content.pm.Signature; inside catch(java.lang.Throwable) com/codename1/impl/android/AndroidImplementation#signingCertificatesViaReflection(Landroid/content/pm/PackageManager;Ljava/lang/String;)[Landroid/content/pm/Signature;|cast to java.lang.Boolean inside catch(java.lang.Throwable) com/codename1/impl/android/AndroidImplementation#vibrate(I)V|cast to android.os.Vibrator inside catch(java.lang.Throwable) -com/codename1/impl/android/AndroidImplementation$46#onReceive(Landroid/content/Context;Landroid/content/Intent;)V|cast to android.content.ComponentName inside catch(java.lang.Throwable) +com/codename1/impl/android/AndroidImplementation$47#onReceive(Landroid/content/Context;Landroid/content/Intent;)V|cast to android.content.ComponentName inside catch(java.lang.Throwable) com/codename1/impl/android/AndroidImplementation$SetCurrentFormImpl#run()V|cast to [Ljava.lang.Class; inside catch(java.lang.Throwable) com/codename1/impl/android/AndroidImplementation$SetCurrentFormImpl#run()V|cast to [Ljava.lang.Object; inside catch(java.lang.Throwable) com/codename1/impl/android/AndroidImplementation$SetCurrentFormImpl#run()V|cast to android.graphics.Bitmap inside catch(java.lang.Throwable) diff --git a/vm/ByteCodeTranslator/src/cn1_globals.h b/vm/ByteCodeTranslator/src/cn1_globals.h index 17195d9b9a7..b12ce93136d 100644 --- a/vm/ByteCodeTranslator/src/cn1_globals.h +++ b/vm/ByteCodeTranslator/src/cn1_globals.h @@ -1401,6 +1401,18 @@ static inline JAVA_BOOLEAN cn1InNursery(void* p) { // heap ref store), which thread-pausing structurally cannot. extern volatile int gcSatbActive; extern void cn1SatbEnqueue(JAVA_OBJECT old); +// DECLARED HERE, not beside the write barrier, because that copy sits in the #else of +// the CN1_NURSERY split and these four have callers that are not conditional on it: +// nativeMethods' arraycopy/cloneArray bulk barrier and CN1_REF_LOAD_BEGIN/END. With the +// declarations behind the nursery #else, -DCN1_NURSERY compiled those calls as implicit +// C89 declarations returning int, which clang has rejected outright since C99 became the +// default -- so the nursery build did not compile at all, and nothing noticed because no +// gate builds that arm. Duplicating an extern is legal and keeps the two halves honest. +extern volatile int gcSatbTerminating; +extern JAVA_BOOLEAN cn1SatbBulkBegin(void); +extern void cn1SatbEnqueueRangeLocked(JAVA_ARRAY_OBJECT* refs, int count); +extern void cn1SatbBulkEnd(void); +extern void cn1SatbBulkQuiesce(void); #if defined(CN1_DISABLE_SATB) // Escape hatch to A/B the barrier cost or fall back if a regression appears. When // disabled, gcSatbActive is never armed (see codenameOneGCMark) AND the per-store diff --git a/vm/ByteCodeTranslator/src/cn1_globals.m b/vm/ByteCodeTranslator/src/cn1_globals.m index 59040b1c5fa..1f3af2e9a2f 100644 --- a/vm/ByteCodeTranslator/src/cn1_globals.m +++ b/vm/ByteCodeTranslator/src/cn1_globals.m @@ -2679,6 +2679,23 @@ void cn1GcDiscoverReference(CODENAME_ONE_THREAD_STATE, JAVA_OBJECT ref, JAVA_BOO || __atomic_load_n(referentField, __ATOMIC_RELAXED) == JAVA_NULL) { return; // already cleared: nothing to decide } +#ifdef CN1_NURSERY + // A NURSERY PROMOTION IS NOT A WEAK-REFERENCE DECISION. cn1PromoteDrain runs the + // generated mark functions with nurseryPromoting raised, and gcMarkObject's promotion + // branch is what moves a referenced object out of the block being recycled. Routing the + // referent here instead skips that branch entirely: a surviving WeakReference whose + // referent sits in a different nursery block would be promoted alone, the minor + // collector would recycle the referent's block, and the field -- never cleared, because + // this is not a collection cycle -- would be left pointing into it. + // + // During promotion the referent is therefore treated exactly like any other field. No + // clearing happens on this path in any case, so making the edge strong here costs a + // promotion and nothing else. + if(threadStateData != 0 && threadStateData->nurseryPromoting) { + gcMarkObject(threadStateData, __atomic_load_n(referentField, __ATOMIC_RELAXED), force); + return; + } +#endif #ifdef CN1_GC_VERIFY // THE VERIFIER HAS TO SEE THE REFERENT, and it could not. // diff --git a/vm/benchmarks/ab-refs.sh b/vm/benchmarks/ab-refs.sh index b2922112daa..981295915d0 100755 --- a/vm/benchmarks/ab-refs.sh +++ b/vm/benchmarks/ab-refs.sh @@ -77,6 +77,13 @@ def run(arm, ceiling_mb): p = subprocess.run([f"target/ab-refs/{arm}"] + os.environ.get("REF_WORKLOAD", "").split(), capture_output=True, text=True, env=env, timeout=float(os.environ.get("REF_TIMEOUT", "600"))) + # A NONZERO EXIT INVALIDATES THE SAMPLE, however complete the output looks. The metrics + # are printed before the process ends, so a VM that corrupts its heap and dies in an + # atexit handler still emits RESULT and the whole table -- and without this the harness + # would publish checksum-matched medians from a crashed run and exit 0. + if p.returncode != 0: + raise SystemExit(f"{arm}@{ceiling_mb}MB: exited {p.returncode}; sample rejected\n" + + p.stdout[-2000:] + "\n" + p.stderr[-2000:]) out = p.stdout def num(key, default=None): m = re.search(rf'^{key}=(-?\d+)', out, re.M) diff --git a/vm/benchmarks/src/com/bench/RefPolicy.java b/vm/benchmarks/src/com/bench/RefPolicy.java index 6c22669db23..e55c928f74b 100644 --- a/vm/benchmarks/src/com/bench/RefPolicy.java +++ b/vm/benchmarks/src/com/bench/RefPolicy.java @@ -179,6 +179,18 @@ private static void weakPhase() throws Exception { } System.out.println("WEAK_LIVE_KEPT=" + liveKept + "/" + WEAK_SAMPLES); + // AN ASSERTION, not a printed number. A referent still reachable through `live` + // must never be cleared, and until this exited nonzero the driver reported a + // reduced count and finished successfully: the checksum reads live[] directly so + // it does not move, and run-gc-verify cannot see it either, because a field cleared + // too early is heap-SAFE -- null dangles nothing. Both advertised validation paths + // could therefore stay green while WeakReference was broken in the direction that + // silently empties every cache built on it. + if (liveKept != WEAK_SAMPLES) { + System.out.println("FAIL: " + (WEAK_SAMPLES - liveKept) + + " strongly reachable referent(s) were cleared"); + System.exit(2); + } System.out.println("WEAK_DEAD_CLEARED=" + deadCleared + "/" + WEAK_SAMPLES); } From 7385cc9d9f4026e2fcb0de6801a33897f80a1b60 Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Wed, 9 Sep 2026 01:04:09 +0300 Subject: [PATCH 29/30] The emergency reclaim retained everything it condemned cn1GcRecoverAfterDrop is reached precisely when soft-referenced data is what is exhausting memory, and blanket retention there is what lets the allocator spin -- which is why it clears selectively instead of calling cn1GcRetainAllReferences. It then recorded each condemned referent in clearedReferent and marked that same referent one statement later in the same iteration, so the sweep kept every one of them and the reclaim never happened. Its own comment described the behaviour it did not have: "in the common case, where no further drop occurs, the clear stands and the reclaim happens". The recovery is owed to clears made by an EARLIER pass, which predate the drop that brought this one here; it is not owed to the clears this pass has just decided for itself, after its own quiesce. Entries therefore record which pass cleared them. A drop count cannot make that distinction, and the first version of this fix used one. The getter the record defends against fails its enqueue BEFORE the clear stamps anything, so the count at the clear already includes it, and "the count moved since" is false in exactly the case where the mark is owed -- it would have reintroduced the dangling read the record exists to prevent. The drop count is now read once per pass, after the quiesce that bounds the window, and a drop past it retains through the shared walk. Two harness gates that could not fail: - RefPolicy exits nonzero on ALIAS_SPLIT. This does NOT make the phase a detector for the violation, and the code says so: with -DCN1_REF_NO_ALIAS_ATOMICITY putting the single-loop bug back, three runs still reported 0/256, because catching it needs a get() inside a window microseconds wide. The ablation is what tests that path. - ab-refs.sh requires the weak phase to have cleared something -- and for noweak, nothing. The checksums are independent of retention policy by design, so they agree just as well when an arm keeps every referent strong, which is the regression that matters. Co-Authored-By: Claude Opus 5 (1M context) --- vm/ByteCodeTranslator/src/cn1_globals.m | 61 +++++++++++++++++++++- vm/benchmarks/ab-refs.sh | 19 +++++++ vm/benchmarks/src/com/bench/RefPolicy.java | 13 +++++ 3 files changed, 91 insertions(+), 2 deletions(-) diff --git a/vm/ByteCodeTranslator/src/cn1_globals.m b/vm/ByteCodeTranslator/src/cn1_globals.m index 1f3af2e9a2f..c4a93106b1a 100644 --- a/vm/ByteCodeTranslator/src/cn1_globals.m +++ b/vm/ByteCodeTranslator/src/cn1_globals.m @@ -2315,6 +2315,19 @@ JAVA_LONG GcVerifyApp_gcMarkState___R_long(CODENAME_ONE_THREAD_STATE) { // becomes visible AFTER the store can still keep the object alive. Clearing is a // destructive publish and cannot be taken back; marking the old value can. JAVA_OBJECT clearedReferent; + // WHICH PASS cleared it, which is what makes clearedReferent recoverable without being + // permanently retained. cn1GcRecoverAfterDrop owes a recovery mark to clears made by an + // EARLIER pass -- those predate the drop that brought it here -- and owes nothing to the + // ones it has just decided itself, after its own quiesce. Without this the two are + // indistinguishable, so it marked every referent it had condemned one statement earlier + // and reclaimed nothing, which is the blanket retention it exists to avoid. + // + // A drop count cannot answer this. The getter this record defends against fails its + // enqueue BEFORE the clear stamps anything, so the count at the clear already includes + // it and "the count moved since" is false exactly when the recovery is owed. Pass + // identity is exact, and the drop count is then used once, for the whole pass, after a + // quiesce that bounds the window. + long clearedAtPass; }; static struct CN1RefEntry* cn1RefDiscovered = 0; static long cn1RefDiscoveredTop = 0; @@ -2363,6 +2376,11 @@ JAVA_LONG GcVerifyApp_gcMarkState___R_long(CODENAME_ONE_THREAD_STATE) { // pass runs, some referent handed to a mutator never reached the log and the pass has no // sound basis for clearing anything. static long cn1RefDropsAtCycleStart = 0; +// Identifies one run of the reference machinery, so an entry can say which pass cleared it. +// Bumped once per cn1GcProcessReferences, which is also the only caller of +// cn1GcRecoverAfterDrop -- the two never clear within the same pass, because the recovery +// path returns before the main loop. Read on the collector only. +static long cn1RefPass = 0; // Referents cleared by the EMERGENCY path, which runs at discovery and has no list entry // to remember them in -- that path exists precisely because the list could not grow. @@ -2505,6 +2523,12 @@ static JAVA_BOOLEAN cn1GcRecoverAfterDrop(CODENAME_ONE_THREAD_STATE) { // of zero means every getter that overlapped has finished and published. That is what // makes the stamp readable as evidence rather than as a race. cn1SatbBulkQuiesce(); + // READ AFTER THE QUIESCE, DELIBERATELY. Every getter in flight when this pass began has + // now finished and published, so a drop counted from here belongs to one that started + // afterwards -- the only kind that can still be holding a referent this pass is about + // to condemn. Taken before the quiesce it would also count drops the quiesce has + // already accounted for, and the post-loop check would retain on every one of them. + long dropsAfterQuiesce = atomic_load_explicit(&cn1SatbDrops, memory_order_relaxed); for(;;) { long before = cn1RefDiscoveredTop; long beforeEmergency = atomic_load_explicit(&cn1RefEmergencyTop, memory_order_relaxed); @@ -2553,6 +2577,7 @@ static JAVA_BOOLEAN cn1GcRecoverAfterDrop(CODENAME_ONE_THREAD_STATE) { // common case, where no further drop occurs, the clear stands and the // reclaim happens. e->clearedReferent = r; + e->clearedAtPass = cn1RefPass; __atomic_store_n(e->referentField, JAVA_NULL, __ATOMIC_RELAXED); #ifdef CN1_GC_CONFORM atomic_fetch_add_explicit(&cn1RefCleared, 1, memory_order_relaxed); @@ -2562,8 +2587,21 @@ static JAVA_BOOLEAN cn1GcRecoverAfterDrop(CODENAME_ONE_THREAD_STATE) { round = JAVA_TRUE; } } + // RECOVER ONLY WHAT A LATER DROP CALLED INTO QUESTION. This block used to + // mark every clearedReferent unconditionally, and the clear above records one + // -- so an entry condemned by this very loop was marked one statement later, + // its object survived the sweep, and the emergency reclaimed nothing. That is + // the blanket retention this function exists to avoid: the caller reaches it + // precisely when soft-referenced data is what is exhausting memory, and + // retaining it there is what lets the allocator spin. + // + // The clear is still recoverable, which is the point of recording it. A drop + // that becomes visible after the store means a getter may have taken the + // referent without the collector seeing the keep-alive, and the comparison + // below is what tells that apart from the drop that brought us here -- which + // the clear decision above already accounted for, after its own quiesce. JAVA_OBJECT was = e->clearedReferent; - if(was != JAVA_NULL && !CN1_IS_TAGGED(was)) { + if(was != JAVA_NULL && !CN1_IS_TAGGED(was) && e->clearedAtPass != cn1RefPass) { gcMarkObject(threadStateData, was, JAVA_FALSE); round = JAVA_TRUE; } @@ -2577,9 +2615,22 @@ static JAVA_BOOLEAN cn1GcRecoverAfterDrop(CODENAME_ONE_THREAD_STATE) { } if(cn1RefDiscoveredTop == before && atomic_load_explicit(&cn1RefEmergencyTop, memory_order_relaxed) == beforeEmergency) { - return marked; + break; } } + // THE SAME RE-CHECK THE MAIN PASS MAKES, and for the same reason. The fixpoint above + // exits on the discovery counters, which a drop does not move, so without this a drop + // raised while this pass was clearing would be recovered only if some LATER call + // happened to run -- and the final cn1GcProcessReferences of a cycle has none after + // it. The clears stand either way; what this restores is the mark, so the sweep cannot + // free an object a getter took while the log was dropping. + cn1SatbBulkQuiesce(); + if(atomic_load_explicit(&cn1SatbDrops, memory_order_relaxed) != dropsAfterQuiesce) { + if(cn1GcRetainAllReferences(threadStateData)) { + marked = JAVA_TRUE; + } + } + return marked; } // Recompute the soft budget and drop the previous cycle's discoveries. Called from @@ -2786,6 +2837,7 @@ void cn1GcDiscoverReference(CODENAME_ONE_THREAD_STATE, JAVA_OBJECT ref, JAVA_BOO e->touchAgeField = touchAgeField; e->strength = strength; e->clearedReferent = JAVA_NULL; + e->clearedAtPass = 0; recorded = JAVA_TRUE; } pthread_mutex_unlock(&cn1RefMutex); @@ -2940,6 +2992,10 @@ static JAVA_BOOLEAN cn1GcProcessReferences(CODENAME_ONE_THREAD_STATE) { return JAVA_FALSE; } JAVA_BOOLEAN marked = JAVA_FALSE; + // AFTER the early exit, so the identity only advances on a pass that can actually + // clear something. Bumping it above would be harmless but would let the counter run in + // applications that hold no Reference at all, which is the case the exit exists for. + cn1RefPass++; #ifdef CN1_GC_CONFORM long long __r0 = cn1GcNowNs(); cn1RefPasses++; @@ -3148,6 +3204,7 @@ static JAVA_BOOLEAN cn1GcProcessReferences(CODENAME_ONE_THREAD_STATE) { // inside it returns a referent that is still valid, because the same read arms the // barrier and resurrects the object for this cycle. e->clearedReferent = r; + e->clearedAtPass = cn1RefPass; __atomic_store_n(e->referentField, JAVA_NULL, __ATOMIC_RELAXED); #ifdef CN1_GC_CONFORM atomic_fetch_add_explicit(&cn1RefCleared, 1, memory_order_relaxed); diff --git a/vm/benchmarks/ab-refs.sh b/vm/benchmarks/ab-refs.sh index 981295915d0..5d88b83fcf7 100755 --- a/vm/benchmarks/ab-refs.sh +++ b/vm/benchmarks/ab-refs.sh @@ -84,6 +84,25 @@ def run(arm, ceiling_mb): if p.returncode != 0: raise SystemExit(f"{arm}@{ceiling_mb}MB: exited {p.returncode}; sample rejected\n" + p.stdout[-2000:] + "\n" + p.stderr[-2000:]) + # THE WEAK PHASE MUST ACTUALLY HAVE CLEARED SOMETHING (or, for noweak, nothing). + # The checksums are deliberately independent of retention policy, so they agree just + # as well when an arm silently keeps every referent strong -- which is precisely the + # regression that matters here, and it would have published medians and exited 0. + # Asserting on the numerator being nonzero rather than on 256/256: clearing is + # best-effort by construction, since a stale word on the conservative native stack + # pins a referent, and the observed figure is 255/256. + m = re.search(r'^WEAK_DEAD_CLEARED=(\d+)/(\d+)', p.stdout, re.M) + if not m: + raise SystemExit(f"{arm}@{ceiling_mb}MB: no WEAK_DEAD_CLEARED in output") + got = int(m.group(1)) + if arm == "noweak" and got != 0: + raise SystemExit(f"noweak@{ceiling_mb}MB: cleared {got} referent(s); with " + f"CN1_NO_WEAK_REFS the referent is a strong edge and nothing " + f"may be cleared -- the arm is not the control it is read as") + if arm != "noweak" and got == 0: + raise SystemExit(f"{arm}@{ceiling_mb}MB: cleared 0 of {m.group(2)} dead " + f"referents; this arm never exercised clearing, so its hit rate " + f"and footprint describe no policy") out = p.stdout def num(key, default=None): m = re.search(rf'^{key}=(-?\d+)', out, re.M) diff --git a/vm/benchmarks/src/com/bench/RefPolicy.java b/vm/benchmarks/src/com/bench/RefPolicy.java index e55c928f74b..a8a7f3eac70 100644 --- a/vm/benchmarks/src/com/bench/RefPolicy.java +++ b/vm/benchmarks/src/com/bench/RefPolicy.java @@ -307,6 +307,19 @@ public void run() { } System.out.println("ALIAS_SPLIT=" + split + "/" + groups); System.out.println("ALIAS_CLEARED_GROUPS=" + clearedGroups + "/" + groups); + // A SPLIT IS A FAILURE, not a statistic: all cleared and all kept both satisfy the + // contract, one alias cleared beside a live one does not. + // + // Do NOT read this exit as making the phase a detector for that violation. The + // class javadoc records the measurement: with -DCN1_REF_NO_ALIAS_ATOMICITY putting + // the single-loop bug back, three runs still reported ALIAS_SPLIT=0/256, because + // catching it needs a get() inside a window microseconds wide. The assertion + // costs nothing and is right to make, but a green run remains evidence of nothing, + // and the ablation above is what actually has to be re-run to test this path. + if (split != 0) { + System.out.println("FAIL: " + split + " group(s) left partly cleared"); + System.exit(2); + } } // ---------------------------------------------------------------- phase B From 8d5948313181613704309ce0ea573207be4f0bbd Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Wed, 9 Sep 2026 01:14:00 +0300 Subject: [PATCH 30/30] Record why the porting layer still hands back a WeakReference A review read the unmigrated createSoftWeakRef as the feature being undelivered: the base implementation still returns a WeakReference and the iOS override still pins every entry in a Hashtable until a memory warning replaces the map, so no framework cache constructs the new class. The facts are right and the conclusion is not. Migrating them changes the lifetime of every decoded image, gradient and resource cache in every app on every platform, which wants its own change and its own bisect point -- and each call site needs deciding rather than sweeping, because a lifetime tracker like JavascriptContext reads a null extract as proof of collection and breaks under a reference that outlives its referent. This change is the mechanism and the measurement that justifies it. Noted at both places a reader arrives from, since a PR thread is not somewhere anyone looks later. Co-Authored-By: Claude Opus 5 (1M context) --- .../com/codename1/impl/CodenameOneImplementation.java | 11 +++++++++++ vm/JavaAPI/src/java/lang/ref/SoftReference.java | 9 +++++++++ 2 files changed, 20 insertions(+) diff --git a/CodenameOne/src/com/codename1/impl/CodenameOneImplementation.java b/CodenameOne/src/com/codename1/impl/CodenameOneImplementation.java index 3e413577a56..a766621c9af 100644 --- a/CodenameOne/src/com/codename1/impl/CodenameOneImplementation.java +++ b/CodenameOne/src/com/codename1/impl/CodenameOneImplementation.java @@ -5607,6 +5607,17 @@ public void run() { /// /// a caching object or null if caching isn't supported public Object createSoftWeakRef(Object o) { + // STILL A WeakReference, DELIBERATELY, and not an oversight now that ParparVM has + // a real SoftReference with a ranked retention policy behind it. + // + // Switching this method -- and deleting the iOS override, which pins every entry + // in a Hashtable until a memory warning replaces the whole map -- changes the + // lifetime of every decoded image, gradient and resource cache in every app on + // every platform. That wants its own change and its own bisect point, because + // each call site also needs deciding individually rather than mechanically: a + // rebuildable cache wants a SoftReference, while a lifetime tracker such as + // JavascriptContext reads a null extract as PROOF the wrapper was collected and + // would leak, or worse, under one that outlives its referent. return new WeakReference(o); } diff --git a/vm/JavaAPI/src/java/lang/ref/SoftReference.java b/vm/JavaAPI/src/java/lang/ref/SoftReference.java index 6bf8080d369..602fdc7a259 100644 --- a/vm/JavaAPI/src/java/lang/ref/SoftReference.java +++ b/vm/JavaAPI/src/java/lang/ref/SoftReference.java @@ -49,6 +49,15 @@ * of its time in the grace pass.

*/ public class SoftReference extends java.lang.ref.Reference{ + // NOTHING IN THE PORTS CONSTRUCTS ONE YET, and that is the intended state here. + // Display.createSoftWeakRef still answers with a WeakReference, and the iOS port still + // overrides it with a Hashtable that pins entries until a memory warning; migrating + // those is a separate change, for the reasons recorded at + // CodenameOneImplementation.createSoftWeakRef. What this class and the collector work + // beside it deliver is the mechanism and the measurement that justifies it -- an + // application may construct one today and get the ranked behaviour. + + /** * Creates a new soft reference that refers to the given object. */