perf(gc): the first collection's barrier-arming walk skips wholly-nursery blocks - #11668
Conversation
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (2)
Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 5 remain after this review. 📝 WalkthroughWalkthroughThe minor remembered-set reconstruction walk now skips arena blocks covered by a single registered nursery range. It records the number of objects walked. Tests cover normal skipping and a test-only mode that skips every block. ChangesNursery Arming Walk
Priority: ➖ Normal Estimated code review effort: 3 (Moderate) | ~20 minutes Change: Refactor Sequence Diagram(s)sequenceDiagram
participant Rebuild as rebuild_minor_old_to_young_remembered_set
participant Cursor as ArenaObjectCursor
participant SkipCheck as arming_walk_skips_block
participant Generation as uniform_heap_generation
participant Census as RememberedReconstructCensus
Rebuild->>Cursor: Configure skip_blocks_where
Cursor->>SkipCheck: Check block extent
SkipCheck->>Generation: Classify the full interval
Generation-->>SkipCheck: Return generation or None
Cursor->>Rebuild: Visit non-skipped objects
Rebuild->>Census: Record objects walked
Merge Risk: ⚪ Minimal · up to The optimization preserves remembered-set reconstruction while avoiding scans of wholly Nursery blocks. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to The change affects which heap objects are scanned before collection, so a mistaken skip could lose reference tracking. The skip requires a wholly nursery block, and the added tests check that an old parent’s reference is still recovered. No new externally callable control was identified. Retained concerns Security review detailsSecurity Blast Radius
Security Findings and Attack Paths
Trust Boundaries and Controls
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
7601742 to
e26749d
Compare
2 participants
Part of #11549
Split out of the #11645 follow-up; #11645 now stacks on this PR. The change stands alone on
mainand does not depend on the nursery pacing.What changes
At a thread's first collection, the lazily-armed write barrier (#7187) rebuilds the old→young remembered set by walking every arena object (
arm_and_reconstruct_remembered_set_if_unarmed→rebuild_minor_old_to_young_remembered_set). The walk keeps a parent only ifbarrier_parent_needs_rememberingsays so, which means an Old-generation object or a malloc object. An object on a nursery block is neither, so the walk classified each one and threw it away. The first collection is also when the young generation is at its fullest. On gc_ratchet 02 that walk cost main 41 M instructions (12.9% of the program). On binary-trees at n=3, it was ~20 M for 131 k young objects.The walk now skips every arena block that is wholly nursery:
arena::uniform_heap_generation(base, end)returns a generation only when one registered range covers the whole extent. A block is registered as one range, so this answers "what generation is this block" without classifying its objects.ArenaObjectCursor::skip_blocks_where(pred)builds the cursor's existing skip set (Old-gen garbage from untraced-promoted transient JSON trees is never reclaimed inside a parse/scan loop (8m:scan peak RSS 1.5–1.7× Node) #10182) from a predicate over each snapshotted block's[data, data + offset).Nursery".The skip is exact, not a heuristic. Every object it skips would have been rejected. The walk runs to completion synchronously inside the arming call, so no mutator window can change a block's generation mid-walk. The budgeted full's remembered-set rebuild uses the same state type and is untouched.
RememberedReconstructCensusgainsobjects_walked, the live-subject counter the witness test reads.Tests and sabotage
gc::tests::barrier_arming::test_11549_arming_walk_skips_nursery_blocks_and_recovers_the_edge: a born-old parent with a young child (the gc/perf: classify_heap_generation is 19% of batch.ts (57.4% total GC bookkeeping) with ZERO collections running — write-barrier tower needs its own lever (#5094 evidence) #7187 fixture) plus 4,096 young objects. It asserts that the edge is still recovered and that the walk visited fewer objects than the nursery alone holds, i.e. the skip engaged. Sabotage: with the predicate forced tofalse, it fails with "the walk visited 4098 objects while the nursery alone holds 4096 — the nursery-block skip did not engage".…_arming_walk_that_skips_old_blocks_loses_the_edge: a test-only sabotage (verify::arming_walk_sabotage) skips every block, and the edge must be lost. This proves the positive test's recovery comes from the walk under test and not from other coverage.Numbers (perrymaster, Linux x86_64 Zen 4,
--release,PERRY_NO_AUTO_OPTIMIZE=1, under/tmp/perry-bench-lock.d)main =
10ece9958.perf stat -e instructions:u. For loops, a two-N differential per iteration (median of 3 at n1, median of 11 at n2). For fixed-size probes, the total (median of 11)./usr/bin/time %Mon the binary itself (no perf wrapper), median of 11, arms interleaved,[min..max].Two RSS columns, because transparent huge pages matter here. mimalloc's
allow_thpdefaults to on, so on a THP host (enabled = madvisehere) most of the heap sits in 2 MB pages, and peak RSS moves in 2 MB steps that depend on layout. THP on is the host as-is. THP off (MIMALLOC_ALLOW_THP=0) is 4 KiB accounting, i.e. memory actually touched.Noise floor: a no-op build (main plus 4 KiB of unused rodata) moves deterministic rows by ≤0.2% THP-on and ≤1.3% THP-off. On dotenv/moment, THP availability flips on some runs of the same binary: see the min column, 31 MB against a 52 MB median.
Instructions and THP-on peak RSS (A0 = this PR):
Noise: instruction spread on an identical binary is ≤0.1% on every row except validator (~5%), moment (~2%) and qs_stringify (~0.5%), so validator's +1.6% and qs_stringify's +0.13% are inside it. THP-on RSS moves ≤0.2% on a no-op build of the deterministic rows. The bare loop runs no GC at all, so none of this code is active there, and it still moves +0.8%, which bounds the file-page variance.
THP-off peak RSS (4 KiB accounting, 11 runs, interleaved):
THP-off differences of up to ±1.6% are file-backed page variance, not heap. On dotenv, anonymous RSS is 18.01 MB against main's 18.02 MB, and minor faults are 6,118–6,152 against 6,161. The bare loop again moves +1.4% with no GC active.
An alternative I measured and dropped: shrinking the minor's transient lists
The #11645 follow-up's first item was the ~8 MB of collector bookkeeping in binary-trees at n=3's one minor. That bookkeeping is
worklistandmoved_headersgrowing by doubling, withrealloccopies and the allocator holding the old buffers. I built a never-reallocating chunkedmoved_headersand a worklist sized once on promoting cycles. With THP off it cut that minor's peak RSS from 25.3 MB to 22.0 MB when the minor is the peak, and it was instruction-neutral once the rebuild loop stayed non-generic. On main it was a trade, so it is not in this PR.make.The code is on
wip/11549-header-list-experimentfor anyone who picks this up. The finding is that the doubling waste only matters where a minor over a fully-live young generation is the program's peak.Correctness
RUST_TEST_THREADS=1 cargo test --release -p perry-runtime(codegen-units 16), on perf(gc): survival-aware nursery pacing, 4 MB floor on the existing influx ladder (includes #11612) #11645's head, which contains this commit unchanged: 4761 passed, 0 failed, 13 ignored.PERRY_GC_PROTECT_FROMSPACE=1, with[gc-fromspace-protect] retired_setchecked armed on every run and output compared to Node 26.5.1.PERRY_GC_VERIFY_EVACUATION=1: 200/200 each.PERRY_SKIP_BUILD=1 PERRY_NO_AUTO_OPTIMIZE=1, each arm its own-p perry -p perry-runtime-static -p perry-stdlib-staticbuild,--filter test_gap_with gc / array / string. Identical on both arms: gc 63 pass + 4 compile failures, array 114 + 2, string 61 + 2, 0 parity failures, the same compile-failure names on both.cargo fmt --all -- --checkOK.scripts/check_file_size.shOK (arena/page_meta/mod.rsis at 1955 lines).SKIP_COMPILE_GATES=1 scripts/run_lint_gates.sh: 103 of 105 script gates pass. The compile tier was not run. The two failures are known: "Public benchmark evidence freshness" (red on main) and "Type-check Windows" (cargo xwinis not installed on this host).git diff --statwas clean after the gates.Not run
cargo testfor crates other thanperry-runtime.cargo xwin check.cargo testand the seeded sweeps on this branch alone. They ran on perf(gc): survival-aware nursery pacing, 4 MB floor on the existing influx ladder (includes #11612) #11645's head, which carries this commit unchanged on top of HOLD (RSS trade): perf(regex): stop charging operation scratch to GC pressure; grow lent registers (bounded) #11612 and the pacing.Summary by CodeRabbit