perf(gc): cut a copying minor's fixed cost (intern young log, skip empty array-tail tables, young-only prunes) - #11634
Conversation
…in a minor Part of #11549
|
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 (11)
Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 5 remain after this review. 📝 WalkthroughWalkthroughMinor-GC scans and pruning now use young-entry logs or an occupancy flag for selected runtime tables. Tests check those tracking rules and their completeness. The changelog reports a reduction in fixed-cost instructions and records allocation-loop measurements. ChangesMinor GC table scans
Priority: ➖ Normal Estimated code review effort: 3 (Moderate) | ~25 minutes Change: Refactor Suggested reviewers: Merge Risk: ⚪ Minimal · up to No identified issue remains that should delay merging after normal checks. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to The changed collection paths rely on new tracking rules, but the inspected production writers record or constrain references before publication, and no reachable security bypass was identified. Some runtime behavior remains unverified without execution. Retained concerns Security review detailsSecurity Blast Radius
Trust Boundaries and Controls
Resilience and Maintainability Implications
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 74.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 50 functions across 10 files. (1 skipped: 1 unsupported.)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 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 |
Keep both sides of string/intern.rs: the atom table beside #11634's young-only intern log. The atom table holds strong heap pointers, so it gets its own young log (arm-before-publish in place(), cleared on rehash, debug-asserted) and is scanned in both minor and full passes.
1 participant
Part of #11549
Cuts the fixed instruction cost of a copying minor, the part that does not depend on how much survives. This is direction 2 of #11549, the half #11612 was waiting for.
Where a minor's fixed cost went (measured first)
I put temporary instruction counters (
perf_event_open, user instructions, not committed) around every phase ofrun_copied_minor_attemptand around each registered root scanner. Per copying minor on main:retain(dead-owner prune)Counters on the same runs: the intern table held 0 live entries on the alloc loop and ~820 on dotenv, of which ~33 (4%) were young. Both array-tail tables were empty on every workload measured: they are only filled by
class X extends Arraytail transitions. The small-int caches only ever hold longlived, pinned strings, so every minor visit there is a no-op.visit_tagged_raw_addrwas hot because of these walks (it ran for every intern and array-tail slot), not because of its own cost.What changed
string/intern.rs), the samegc/young_log.rsmechanism the transition cache, descriptors and shapes already use. Both writers note the slot before publishing. A minor visits only logged slots and re-logs the ones still young. A full collection walks the whole table and rebuilds the log. Rule 2 applies: underdebug_assertions/tests the minor walk first re-derives the young slots from the table and panics if the log misses one.object/array_tail_transition.rs). Anarray_tail_occupiedflag is set before the first publish by the only production writer. Nothing buttest_clearever returns a slot toEMPTY(pruning leaves tombstones), so "flag clear" means every slot is empty. Scan and prune return at once. Under debug/tests, the skip re-checks that both tables really are empty.builtin_closure_metadata.rs). Its young log already existed for the scan, but the dead-owner prune always ran a fullretainover both maps. On a minor it now asks only about logged owners. A minor can only find a minor-collectible owner dead, and every such owner is logged.string/format.rs). Both writers publish only longlived, pinned strings. A minor can neither move nor free those, and does not trace through them. Full-scope passes still walk both caches. Under debug/tests the skip asserts that every entry is pinned and non-young.Every skip is conservative. When the precondition cannot be shown, the old walk runs.
Results (perrymaster,
perf stat -e instructions:u,--releasebuilds,PERRY_NO_AUTO_OPTIMIZE=1, under/tmp/perry-bench-lock.d)Fixed cost per minor on the alloc loop. Measured as the instruction difference between a 1 MB and a 16 MB nursery, divided by the difference in minor count (610 vs 67 minors): main 753k per minor, this branch after its first two changes 98k per minor (−87%). The last two changes do not touch this loop.
Per-iteration instructions. Two-N differential, median of 3. main =
2d1f1d7a9, pr = main + #11612, fix = this branch, pr+fix = both. Every run's output matched Node 26.5.1.Noise: moment and validator vary by about ±1–2% run to run on an identical binary, even with
setarch -R. I measured moment at 4 MB repeatedly: pr ranged 20.03–20.30 G total and pr+fix 19.99–20.47 G. So the moment and validator rows can't resolve differences below about 2%. The alloc, dotenv, qs and date-fns rows are stable.The allocation-loop cost of a 4 MB nursery is gone. It was +3.0% (main 469.2 → 483.1). With this branch, 4 MB costs 466.0, which is below main's default of 469.2.
Does #11612 + a 4 MB nursery now meet "no compute regression, no RSS increase"? No, so the default is unchanged
pr+fix at a 4 MB nursery, compared with main at its default:
The qs rows still regress. That cost is not fixed per-minor cost: qs builds large nested structures, so a smaller nursery copies more survivors (qs/stringify goes from 24 to 105 minors). A global smaller nursery therefore still trades compute for RSS on survivor-heavy workloads, and no nursery default change is included here. Direction 2 still needs survival-aware young pacing, for example shrinking only while observed survival is low. Cutting fixed cost makes that pacing cheap but does not replace it. What remains of the fixed cost on dotenv, per minor: canonical-keys prune (~170k), symbol side tables (~155k), object-cache/canonical-keys scan (~180k), descriptors (~140k), stack/shadow roots (~150k).
Correctness
gc/tests/minor_fixed_cost.rsandbuiltin_closure_metadata.rs:visited == 0).arm_intern_youngpanics withyoung log for string.intern_table does not name ….… without arming the flag first.RUST_TEST_THREADS=1 cargo test --release -p perry-runtime --lib: 4714 passed, 0 failed (CARGO_PROFILE_RELEASE_CODEGEN_UNITS=16).PERRY_GC_SCHEDULE_SEED, rate 0.05,PERRY_GC_PROTECT_FROMSPACE=1, quarantine armed:[gc-fromspace-protect] retired_set=#Nlines present), output compared against Node, on pr+fix:extends Arraypush/pop, small-int strings, a retained window);obj_type=3 size=32in retired from-space). This is a separate latent rooting bug on main that this PR neither causes nor fixes. Reproduce withPERRY_GC_INSTRUMENTS=1 PERRY_GC_SCHEDULE_SEED=1 PERRY_GC_SCHEDULE_RATE=0.05 PERRY_GC_PROTECT_FROMSPACE=1 ./qs_parse_nested 300 50frombenchmarks/packages.PERRY_GC_VERIFY_EVACUATION=1with seeds:--filter test_gap_with gc, array, regex, string): identical on both arms. gc: 61 pass, the same 4 compile failures on both. array: 113 pass, the same 2 compile failures on both. regex: 14 pass, the sametest_gap_regex_replace_dyn_regex_with_httpcompile failure on both. string: 56 pass.scripts/gc_runtime_root_holders.pyis OK on this branch and on main. The policy.rs flag reported earlier did not reproduce on either.scripts/thread_exit_address_globals.pyis OK. File-size is OK.cargo fmt --checkis OK.scripts/run_lint_gates.shwithSKIP_COMPILE_GATES=1: 98 of 101 script gates pass, compile tier not run. The 3 failures:cargo xwinis not installed on this host.scripts/gc_store_site_inventory.pyalone passes.Not run
cargo xwin check.cargo testfor crates other thanperry-runtime.Summary by CodeRabbit