Adapt BiBOP GC policy to survivor-heavy allocation - #5436
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates ParparVM’s BiBOP collector to adapt its GC trigger and pacing behavior to survivor-heavy allocation patterns (as reported in issue #5425), while reducing mark-phase amplification on normal cycles. It also adds a targeted regression/benchmark harness to validate correctness, policy behavior, and performance/RSS against QA baselines.
Changes:
- Add adaptive GC trigger growth/contraction, per-thread high-throughput promotion, and a bounded legacy bypass for survivor-heavy size classes.
- Replace the unconditional “mark belt” with an overflow-only belt and optimize grace-subtree marking by scanning only pages that received fresh allocations.
- Add a dedicated issue-shaped benchmark (
BiBopAdaptive) plus a runner script and documentation entry for CI/QA-style gating.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| vm/ByteCodeTranslator/src/nativeMethods.m | Initializes new per-thread BiBOP adaptive policy state on thread creation. |
| vm/ByteCodeTranslator/src/cn1_globals.m | Implements adaptive trigger/bypass policy, fresh-page grace scanning, and overflow-only belt behavior in the GC. |
| vm/ByteCodeTranslator/src/cn1_globals.h | Extends thread/page structures and inline allocation paths to support adaptive policy and fresh-page tracking. |
| vm/benchmarks/src/com/bench/BiBopAdaptive.java | New deterministic retained-small-array workload that validates correctness across multiple GC epochs. |
| vm/benchmarks/run-bibop-adaptive.sh | Builds QA variants and enforces correctness/policy/perf/RSS gates using instrumented diagnostics. |
| vm/benchmarks/README.md | Documents the new adaptive BiBOP regression gate and what it validates. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
✅ ByteCodeTranslator Quality ReportTest & Coverage
Benchmark Results
Static Analysis
Generated automatically by the PR CI workflow. |
Contributor
✅ Continuous Quality ReportTest & Coverage
Static Analysis
Generated automatically by the PR CI workflow. |
Contributor
Cloudflare Preview
|
Collaborator
Author
|
Compared 146 screenshots: 146 matched. Benchmark ResultsDetailed Performance Metrics
|
Collaborator
Author
|
Compared 146 screenshots: 146 matched. Benchmark ResultsDetailed Performance Metrics
|
Collaborator
Author
|
Compared 147 screenshots: 147 matched. |
Collaborator
Author
|
Compared 147 screenshots: 147 matched. |
Collaborator
Author
|
Compared 146 screenshots: 146 matched. Benchmark ResultsDetailed Performance Metrics
|
Collaborator
Author
|
Compared 181 screenshots: 181 matched. |
Collaborator
Author
|
Compared 148 screenshots: 148 matched. Benchmark Results
Detailed Performance Metrics
|
Collaborator
Author
|
Compared 217 screenshots: 217 matched. |
Collaborator
Author
|
Compared 143 screenshots: 143 matched. Benchmark Results
Build and Run Timing
Detailed Performance Metrics
|
Collaborator
Author
|
Compared 149 screenshots: 149 matched. Benchmark Results
Build and Run Timing
Detailed Performance Metrics
|
Collaborator
Author
|
Compared 144 screenshots: 144 matched. |
Collaborator
Author
|
Compared 151 screenshots: 151 matched. Native Android coverage
✅ Native Android screenshot tests passed. Native Android coverage
Benchmark ResultsDetailed Performance Metrics
|
Contributor
|
Quick work, but you may not be out of the woods yet. I built my app against master and I'm getting |
shai-almog
added a commit
that referenced
this pull request
Jul 23, 2026
#5442) * Fix GC freeing live objects referenced by untraced fresh BiBOP objects The fresh-page-stack grace scheme introduced in #5436 queued a page onto an alternating per-epoch stack on its FIRST allocation in a GC epoch and consumed both stacks once, mid-mark. The queue-once-per-epoch dedup left a wide uncovered window: every allocation into an already-consumed page for the REST of that epoch -- the remainder of the mark plus the entire unbarriered inter-cycle gap -- was skipped, and if the page received no next-epoch allocation before the next grace pass, its fresh (gcMark==-1) slots were never grace-traced. The sweep then freed any object reachable only through such an untraced fresh object while the fresh object itself survived via grace: a dangling reference inside a surviving object. With compact strings inlining the byte payload into the String's BiBOP slot, recycling those slots rewrites word bytes in place -- the corrupted dictionary entries and impossible NPE reported in issue 5425. Replace the queue with a full-registry walk pruned by the existing gcAllocedSinceSweep flag. The pruning invariant is exact and race-free: a mark==-1 slot can only exist on a page allocated into since that page's last sweep (the sweep converts every -1 it sees), every allocation path already sets the flag, pre-mark stores are published by the mark-start thread sync, and only the sweep -- which never touches an owned page -- clears it. Flag-FALSE pages (the retained-survivor bulk on exactly the workloads #5436 targets) are skipped without touching their slots, so the pause win of #5436 is preserved while the whole fresh-page queueing machinery (two page-header fields, the epoch mirror check and queue call in three allocation paths) is deleted from the hot path. Add a QA-only grace-completeness gate (-DCN1_GRACE_AUDIT): snapshot each page's bump cursor at mark start and, right before the sweep, full-walk the registry tracing any pre-snapshot slot still fresh. It reports missedFresh (fresh slots the grace pass never visited) and doomedChildren (objects that became marked ONLY through them -- each one would otherwise be swept while still referenced; any nonzero value is a collector bug). The new GraceAudit driver allocates dropped fresh nodes holding sole references to older objects WHILE the concurrent mark runs (System.gc is asynchronous), then goes quiet a cycle: against the fresh-page stacks it reports 100-370 missed / 100-250 doomed per cycle (12,178 doomed in one run); with this fix doomedChildren is zero across the suite. StormAB and LoadLoop are the matching perf A/B drivers: wall time and RSS are unchanged vs the pre-fix tree (storm 5-8x faster than pre-#5436, repeated dictionary loads flat), run-bibop-adaptive.sh stays green (adaptive 0.94x time / 0.62x RSS vs legacy), and the full gauntlet passes byte-identical in both thread-stop modes. Fixes the corruption regression reported in #5425. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Address review: relaxed atomics on the grace flag, audit docs, dead code - gcAllocedSinceSweep: the concurrent pair (mutator set on allocation / grace-pass read) now uses relaxed __atomic ops -- identical machine code to the previous plain access, but removes the formal C11 data race. Sweep/format keep plain access: they only touch retired or pooled pages no mutator holds. Comments spell out why relaxed suffices: pre-mark stores are ordered ahead of the grace pass by the mark-start thread pause, a store the pass can still miss is by definition a during-mark allocation (SATB-covered this cycle), and only the sweep clears the flag so a missed store is re-observed next cycle. - Document that the CN1_GRACE_AUDIT mark-start snapshot deliberately under-approximates: boundary slots racing mark start are during-mark allocations, the class the grace guarantee does not cover this cycle, and excluding them keeps the audit free of false positives. - GraceAudit: drop the unused LCG helper and vestigial sink field, reword stale fresh-stack phrasing to be scheme-agnostic. Revalidated: GraceAudit doomedChildren=0, run-gauntlet.sh GREEN (both stop modes), run-bibop-adaptive.sh GREEN (0.93x time / 0.60x RSS vs legacy), StormAB wall time unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Address review: atomics on the registry-visible reformat path cn1BibopAcquirePage reformats a FREE-pool page that is already in the page registry, on a mutator thread, possibly during a concurrent mark -- so its resets of gcAllocedSinceSweep (and gcAuditSnapshot in audit builds) can overlap the grace pass / audit reads. Convert that pair to relaxed __atomic ops: the flag store is value-identical (the sweep already reset it before pooling), so this only removes the formal race. The new-page format path is untouched by observers (it runs before registry insertion) but shares the same code, and format is cold either way. The audit snapshot field now uses relaxed atomics at all three sites (format reset, mark-start snapshot, pre-sweep read). The sweep's three plain accesses remain intentionally plain and the header comment now states the precise reason: the sweep runs on the GC thread after mark completes (program-ordered against the grace pass) on retired pages no mutator holds, and the pool-handoff mutex orders it against the next owner's stores. Revalidated: GraceAudit doomedChildren=0, gauntlet GREEN in both stop modes, run-bibop-adaptive GREEN (0.95x time / 0.61x RSS vs legacy), StormAB unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Address review: clarify that grace-audit silence is success The audit prints a [GRACE-AUDIT] line only when a cycle misses something; the README implied a line per cycle. State that an empty stderr is a fully clean run and the gate is that no line reports doomedChildren != 0. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
shai-almog
added a commit
that referenced
this pull request
Jul 26, 2026
#5471) * Trace grace subtrees on the legacy heap, and gate heap integrity in CI The grace-subtree pass added in #5442 covers BiBOP pages only. The legacy sweep grants a fresh (gcMark == -1) legacy object exactly the same one-cycle grace -- codenameOneGCSweep promotes it to the current epoch instead of freeing it -- but nothing traced its subtree, so an older object reachable ONLY through such an object was freed while it was still referenced. That is the same defect #5442 fixed, on the other heap. Everything above CN1_BIBOP_MAX_OBJECT lands there (the retained large byte[] blocks and Hashtable bucket arrays of issue 5425), as does every allocation the adaptive survivor-heavy bypass diverts off the page heap, and every matured survivor, whose table entry is what the sweep consults. Measured with the audit half added below: ~65,800 untraced fresh legacy objects per cycle on a bypass-heavy workload, with children reachable only through them. Mirror the page walk over allObjectsInHeap. Only entries already migrated into the table can be fresh at that point -- pending allocations are not swept until the mark that migrates them, and migration happens with the owning thread paused, upstream of this pass -- so the pass is exact. Cost is one extra walk of an array the sweep already walks in full, and only fresh entries are traced: StormAB and LoadLoop wall time and RSS are unchanged and LargeArrayLoad still collects in 5 cycles. CN1_DISABLE_LEGACY_GRACE is the A/B escape hatch, mirroring CN1_DISABLE_SATB. Add the gate that would have caught both halves. Checksums are structurally blind to this failure: when a sweep frees memory a survivor still references, nothing diverges at the point of the bug -- the dangling reference reads whatever object recycled the slot, so the damage surfaces later, elsewhere, as corrupted data. That is how #5436's regression reached a user as "non word" dictionary entries and an impossible NPE instead of as a failing test. -DCN1_GC_VERIFY makes the invariant observable by destroying the plausible replacement: - POISON every reclaimed page slot and legacy block. This includes the O(1) all-dead page reclaim, which is where nearly all page memory is actually reclaimed and which normally drops a page without writing a single slot, leaving every dead object with an intact-looking header -- the reason a dangling read in this VM finds plausible data rather than crashing. - QUARANTINE freed legacy blocks in a ring instead of returning them to the C allocator, so a poisoned block stays mapped and recognizable. - VERIFY after every sweep: walk each survivor through its own generated mark function with the collector in verify mode, classifying every reference field against the page registry, the live-extent index and the quarantine set. A field pointing into reclaimed memory is reported with the holder's class, the victim's class and the field's mark call site, then aborts at the cycle that created it. The gate holds CURRENT-EPOCH survivors to the invariant, where a dangling field is unambiguous: the sweep either marked the object reachable (marking traces children) or promoted it by the grace rule (tracing the subtree was the grace pass's job). References it cannot place are skipped, so a violation is never a false alarm. CN1_GC_VERIFY_AGING extends it to previous-epoch survivors as a census rather than a gate. run-gc-verify.sh runs it over nine drivers and then re-injects the #5442 defect (CN1_GC_FAULT=nograce disables the grace pass) and REQUIRES the verifier to catch it -- a gate nobody has watched fail is not a gate, and a build where the verification silently compiled out would otherwise report a permanent, meaningless pass. GcHeapIntegrityIntegrationTest is the CI twin and asserts both halves. Two supporting pieces, both born from the same investigation: - LegacyGrace is the legacy-path twin of GraceAudit, plus the [GRACE-AUDIT-LEGACY] half of -DCN1_GRACE_AUDIT. Writing it exposed why drivers in this area come back green while the defect is present: the hazard has to be built with no mark in flight (during a mark the SATB barriers cover the very reference move under test) and the driver has to scrub its own native stack afterwards, because the conservative root scan marks whatever a returned frame's leftover word still points at. Both are documented at the driver and in the README. - CN1_GC_TRACE_MARK names the mark pass that keeps a class alive, which is how that retention was identified; CN1_GC_VERIFY_CENSUS reports whether a driver's hazard set actually ages out instead of being pinned. Validation: gauntlet GREEN in both stop modes, run-gc-verify GREEN over all nine drivers with the fault self-test firing, GraceAudit clean under -DCN1_GRACE_AUDIT, and the new integration test green. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Sanitize the collector's env knobs in the heap-integrity gate Both halves of the gate are decided by environment variables, and anyone debugging the collector has exactly those exported: CN1_GC_VERIFY_SOFT downgrades the abort the faulted half asserts on, and CN1_GC_FAULT injects the defect the clean half asserts is absent. An inherited knob would invert a result rather than fail loudly. Drop CN1_* from the child environment in the integration test and unset the knobs at the top of run-gc-verify.sh, so both start from a known state and see only what they set themselves. Verified by running each with CN1_GC_FAULT=nograce and CN1_GC_VERIFY_SOFT=1 exported: the test passes and the script reports GC-VERIFY GREEN. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Report the mark site from the frame that actually has it cn1GcVerifyChild read __builtin_return_address(0) itself, which resolves to the return address in gcMarkObject -- one frame too deep, and in a different object file from the generated mark function the label claimed it pointed into. Anyone following a violation back to a field would have landed in the collector rather than at the field read. Capture the address in gcMarkObject instead, where this frame's return address IS the instruction inside the generated mark function (or gcMarkArrayObject for an element), and pass it down. Report it as an offset from the holder's own mark function so the line is self-verifying without symbols and survives ASLR: markSite= 0x102fa66c0 = markFn+36 (the field read, inside the holder's mark function) A small positive offset means the frame is right; add it to the mark function's symbol to reach the exact field. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Make every driver in the gate carry its weight, and fix the CI self-test on Linux Three problems, all the same shape: a check that reports success without having checked anything. 1. The verifier never reported whether it RAN. MtStress, MapTorture and SbTorture exit before any sweep completes, so their "clean" results in run-gc-verify.sh meant only that nothing was ever verified -- exactly the hollow-gate failure the fault self-test exists to prevent, sitting inside the same script. Count completed passes, print them at exit, and fail any driver reporting zero. Those three now end with one collection over the heap they built (printing nothing, so the gauntlet's byte-identical comparison is unaffected) and contribute real coverage. 2. The CI self-test failed on Linux: the fault was injected and the workload completed, but no dangling reference appeared, so the test declared the gate inert. Reproduced in a linux/amd64 container -- the platform is fine (GraceAudit detects the fault there), the app's hazard was too weak. Its during-mark phase now follows the shape that provably breaks a missing grace pass: refill, kick a mark, then allocate dropped fresh nodes in sleep-separated slices so they land across the mark rather than racing past it, then go quiet for a full cycle so the untraced children age past the free threshold. Verified in the container: clean 0 violations / 23 passes, faulted aborts with 20 reports, three runs, no variance. 3. The integration test now also requires a nonzero pass count, so a workload that stops driving collection fails instead of passing silently. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Add the Codename One header to the three tortures this PR touches MapTorture, MtStress and SbTorture predate the copyright gate and carried no header at all. The gate only inspects files a PR adds or modifies, so giving them a trailing collection in the previous commit is what pulled them into scope. Header text copied verbatim from LegacyGrace.java, added in this PR and already passing. Comment-only: scripts/check-copyright-headers.sh passes over the branch, and the gauntlet still matches the host JVM byte for byte. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Correct the documented contract of cn1GcVerifyQuarantineFree The comment described an earlier design in which the function handed the evicted block back for the caller to release. It returns a boolean and frees the displaced block itself, and the call site in codenameOneGcFree already relies on that boolean -- so the comment was the only thing out of date, and the one part a future caller would have read first. State what the return value means instead: TRUE when the block was quarantined and must NOT be freed, FALSE when the quarantine could not be allocated and the caller should free normally. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Resolve the injected fault before the first grace pass consults it cn1GcFaultNoGrace was initialized only by cn1GcVerifyHeap, which runs after the sweep. The first cycle of every faulted run therefore traced grace subtrees normally, and a workload completing a single GC cycle never had the defect injected at all -- the self-test would have reported a gate that "cannot fail" purely because nothing ever faulted it. Resolve the switch at its first use in the mark instead; cn1GcFaultInit is idempotent and GC-thread only, so the call in cn1GcVerifyHeap stays as a harmless second one. Observable: [GC-FAULT] now prints before the first verify pass rather than after a sweep. Checking that turned up a matching gap in GcStress, which reported 1 verify pass on one run and 0 on the next: its churn triggers collections, but whether the last one reaches its sweep before the process exits is a race, so the new vacuity check would have flaked in CI. It gets the same trailing collection as the other drivers (prints nothing, so the gauntlet comparison is unaffected). Two consecutive gate runs now report identical pass counts across all nine drivers, and the gauntlet stays byte-identical to the host JVM. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * State LegacyGrace's real allocation volume against the trigger The hazard comment still described the driver's first shape -- 256 parents of ~800 bytes -- after it was scaled to KEEP=10000 arrays of 65 references. The number matters rather than being decoration: the window only works while nothing in it starts a collection, so a reader checking that property was being handed the wrong figure by an order of magnitude. Give the real one (about 560 bytes each, 5.6 MB total, against the 24 MB allocation-volume trigger) and say which constants would break the window if raised. Same for the other precondition, that no mark is in flight, since both are equally easy to lose when editing the driver. Comment-only; LegacyGrace still reports 120 clean verify passes with the fault self-test firing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Add the Codename One header to GcStress GcStress is the fourth torture in this tree without a header, and the trailing collection added for the vacuity check pulled it into the copyright gate's scope. Missed on the first pass because the earlier commit added headers to the three drivers touched at that point, and GcStress was edited after that check ran. Audited every source file this branch touches rather than fixing one report at a time: all eight now carry the header and scripts/check-copyright-headers.sh passes over the full branch range. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Root cause and impact
Issue #5425 combines a large retained set of small arrays with continued temporary allocation. The fixed 24 MiB BiBOP trigger and the prior throughput classification can force collections too frequently for that shape, while the old unconditional mark belt magnifies the cost of each cycle.
The production collector now observes allocation rate and survival at runtime. It grows the trigger within bounded available-memory limits, promotes high-throughput threads automatically, and uses a bounded legacy bypass for size classes that repeatedly show high survival. Applications do not need compiler flags or collector selection; the flags in the benchmark only build QA comparison variants.
The fresh-page grace scan and overflow-only belt retain correctness without scanning the grow-only page registry or doing a second full reachable-object traversal on normal cycles.
Validation
mvn -q -B -pl ByteCodeTranslator -am testfromvmwith Java 8./run-bibop-adaptive.sh 1git diff --checkFixes #5425