perf(gc): census-free budgeted cycles + RSS floor (#6179, #6180 Stage 2)#6239
Merged
Conversation
added 3 commits
July 10, 2026 11:57
…et (#6179 groundwork) PERRY_GC_VERIFY_CLASSIFIER=1 asserts, on every positive contains() query against a CENSUS-BUILT set, that the page-metadata classifier (current_heap_header_for_user_ptr) accepts the object - the SUPERSET property that must hold before the exact BTreeSet can be skipped on precise cycles. Reverse divergence (classifier accepts, census lacks) is expected: objects born after the census; safe over-approximation under allocate-black. Verification carve-outs, each principled: - built_by_census: tests fabricating sets bypass verification. - CLASSIFIER_VERIFY_SUPPRESSED: test-only page-metadata wipes (old_arena_page_index_clear_for_tests) make real objects unclassifiable; production never clears the index. - FORWARDED headers: censused-then-moved objects are rejected by the plausible-header check BY DESIGN (the copying machinery owns the redirect). Full suite under verification: 1227/0.
Budgeted (precise, non-moving) cycles no longer build the O(heap) BTreeSet census held across the whole sliced cycle - membership resolves through classifier_valid_object_start: the UNION of exact malloc-registry membership and a plausible arena header on an arena-classified page. Union, not the exclusive generation branch of current_heap_header_for_user_ptr: malloc allocations can sit inside ranges the page metadata attributes to an arena generation, and the exclusive branch left live malloc roots unmarked. Safety anchors: - classifier-mode cycles NEVER conservative-scan (traced conservative words cannot tolerate heuristic false positives) - even if a ManualGcScanGuard appears mid-cycle via drain-before-manual-gc. - maybe_contains passes everything through in classifier mode (no census, no range; call sites prefilter separately). - Synchronous cycles keep the exact set: they force the conservative scan, which requires exactness. - The census walk still runs for budgeted cycles (evacuation policy reads tenured_nursery_bytes); only the set inserts are skipped. Differentially verified: PERRY_GC_VERIFY_CLASSIFIER=1 across the full suite asserts the classifier is a superset of the census on every census-built set. 1227/0 with and without verification.
…ycles (#6180) Measurement redirect: removing the exact census barely moved RSS (929 vs 921 MB on the churn benchmark) - the ~1.8x gap over the synchronous collector is pacing equilibrium (floating garbage across the debt window + allocate-black retention) and allocator pages never returned to the OS, not the BTreeSet. Two levers: - GC_ASSIST_DEBT_BYTES_PER_WORK_UNIT 64 -> 32: doubles the pacing gain, halving equilibrium debt and with it the per-cycle floating-garbage window. - run_malloc_trim no longer skips budgeted cycles (2026-07-09 audit finding): a long-lived incremental process never returned freed allocator pages. Trim runs at Reclaim, outside the atomic tail. Telemetry/reclaim tests updated: budgeted trim outcome is platform- dependent (executed on glibc, unsupported elsewhere), never the old ordinary_budgeted skip.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughBudgeted GC cycles now use classifier-based pointer membership, skip conservative stack scanning, perform allocator trimming, avoid minor-cycle age bumps, and adjust assist pacing. Differential classifier verification, weak-target scanning, and related telemetry tests are also updated. ChangesGC membership classifier
Estimated code review effort: 4 (Complex) | ~50 minutes Sequence Diagram(s)sequenceDiagram
participant GC Cycle
participant ValidPointerSetBuilder
participant ValidPointerSet
participant Root Scanner
participant Allocator Maintenance
GC Cycle->>ValidPointerSetBuilder: select classifier builder for budgeted cycle
ValidPointerSetBuilder->>ValidPointerSet: create classifier-mode pointer set
GC Cycle->>Root Scanner: skip conservative stack scan
GC Cycle->>Allocator Maintenance: run platform-specific malloc trimming
Possibly related issues
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 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 |
…aim (#6180) The budgeted-trim lever only worked on glibc; darwin returned Unsupported, so macOS incremental processes never returned clean allocator pages (B2 churn RSS pinned at ~928MB with healthy pacing). NULL zone = all zones, goal 0 = release everything releasable. Same placement as glibc malloc_trim: Reclaim, outside the atomic tail. Tests updated for the platform matrix.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/perry-runtime/src/gc/tests/incremental_sweep_reclaim.rs (1)
559-583: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
test_malloc_trim_call_count()should not stay at0on glibc.record_test_malloc_trim_call()wraps the samelibc::malloc_trim(0)path that reportsstatus: "executed", so this assertion should expect1here or be gated to non-glibc builds.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/perry-runtime/src/gc/tests/incremental_sweep_reclaim.rs` around lines 559 - 583, Update the assertions in the budgeted GC test around complete_budgeted_gc_cycle: since budgeted reclaim now invokes malloc_trim on glibc, expect test_malloc_trim_call_count() to be 1 there, while gating the expectation appropriately for non-glibc builds if needed. Keep the trace assertions consistent with the allocator_maintenance malloc_trim status.
🧹 Nitpick comments (1)
crates/perry-runtime/src/gc/cycle.rs (1)
751-782: 🚀 Performance & Scalability | 🔵 TrivialConfirm per-cycle
malloc_trim(0)frequency is acceptable.Removing the budgeted skip means every budgeted reclaim now calls
libc::malloc_trim(0). Unlike the surrounding reclaim work,malloc_trimis not budget-bounded — glibc walks all arena free-lists andmadvise/munmaps, with cost driven by fragmentation. For a long-lived incremental process completing budgeted cycles frequently, running it every cycle can add non-trivial CPU. Consider throttling (e.g., pressure-gated or every N completed cycles) so the RSS floor is preserved without paying full trim cost on every reclaim.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/perry-runtime/src/gc/cycle.rs` around lines 751 - 782, Throttle calls to malloc_trim(0) from run_malloc_trim instead of executing on every budgeted reclaim cycle; add pressure-based or periodic gating (such as every N completed cycles) while retaining immediate trimming for explicit or emergency paths, and update the related allocator-maintenance status/reason handling and tests to cover skipped versus executed trims.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@crates/perry-runtime/src/gc/tests/incremental_sweep_reclaim.rs`:
- Around line 559-583: Update the assertions in the budgeted GC test around
complete_budgeted_gc_cycle: since budgeted reclaim now invokes malloc_trim on
glibc, expect test_malloc_trim_call_count() to be 1 there, while gating the
expectation appropriately for non-glibc builds if needed. Keep the trace
assertions consistent with the allocator_maintenance malloc_trim status.
---
Nitpick comments:
In `@crates/perry-runtime/src/gc/cycle.rs`:
- Around line 751-782: Throttle calls to malloc_trim(0) from run_malloc_trim
instead of executing on every budgeted reclaim cycle; add pressure-based or
periodic gating (such as every N completed cycles) while retaining immediate
trimming for explicit or emergency paths, and update the related
allocator-maintenance status/reason handling and tests to cover skipped versus
executed trims.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 6a1c51ac-3ba0-4fa6-8bef-28e474c3406c
📒 Files selected for processing (6)
crates/perry-runtime/src/arena/page_meta.rscrates/perry-runtime/src/gc/cycle.rscrates/perry-runtime/src/gc/progress.rscrates/perry-runtime/src/gc/tests/incremental_sweep_reclaim.rscrates/perry-runtime/src/gc/tests/telemetry_verifier.rscrates/perry-runtime/src/gc/trace.rs
added 2 commits
July 10, 2026 12:34
…rvival (#6180) Whole-cycle allocate-black (#6224 soundness) marks every mid-cycle birth; the sweep age-bump read MARKED as survived, so dead churn tenured into old-gen two cycles later where minors never reclaim it. Measured on the churn benchmark: ~700 MB of tenured garbage (928 MB RSS vs 524 MB synchronous, identical 203-block arena commitment). Budgeted sweeps skip aging; promotion under incremental flows through the copied-minor path, which ages genuinely-surviving objects only.
…ers exist (#6180) The AtomicFinalize WeakProcessing pass walks the entire arena solely to FIND weak holders; the #6182 registry already tracks every live one. Empty registry (the common case) -> the O(heap) pass is a no-op, removing the single largest atomic-finalize cost for weakref-free programs. Registry-iteration for the nonzero case is the tracked refinement.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/perry-runtime/src/gc/cycle.rs (1)
565-573: 🩺 Stability & Availability | 🔴 Critical | ⚡ Quick winRecord whether the conservative scan actually ran.
If a
ManualGcScanGuardappears during a classifier cycle, this branch skips scanning, but Lines 1214-1219 may still seeScanand bypass block persistence. Register-held recent objects can then be swept while live.Make the block-persistence decision depend on an actual-scan flag, or never skip persistence when
valid_ptrs.classifier_modeis true.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/perry-runtime/src/gc/cycle.rs` around lines 565 - 573, Ensure classifier-mode cycles do not bypass block persistence when conservative scanning is skipped. Update the root-scan state around the classifier-mode branch in the cycle collector to record whether a scan actually ran, then make the persistence decision near the Scan handling depend on that flag; alternatively, always preserve blocks when valid_ptrs.classifier_mode is true.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@crates/perry-runtime/src/gc/cycle.rs`:
- Around line 565-573: Ensure classifier-mode cycles do not bypass block
persistence when conservative scanning is skipped. Update the root-scan state
around the classifier-mode branch in the cycle collector to record whether a
scan actually ran, then make the persistence decision near the Scan handling
depend on that flag; alternatively, always preserve blocks when
valid_ptrs.classifier_mode is true.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: df8a0d5b-43b1-42bd-bc35-a2cc2d14ff67
📒 Files selected for processing (3)
crates/perry-runtime/src/gc/cycle.rscrates/perry-runtime/src/gc/tests/incremental_sweep_reclaim.rscrates/perry-runtime/src/gc/tests/telemetry_verifier.rs
🚧 Files skipped from review as they are similar to previous changes (2)
- crates/perry-runtime/src/gc/tests/incremental_sweep_reclaim.rs
- crates/perry-runtime/src/gc/tests/telemetry_verifier.rs
This was referenced Jul 10, 2026
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
1 participant
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.
Part of #6179 and the #6180 Stage-2 (default-on) campaign. Three measured steps:
1. Classifier differential-verify mode (groundwork)
PERRY_GC_VERIFY_CLASSIFIER=1asserts on every positivecontains()query against a census-built set that the page-metadata classifier accepts the object — the superset property gating any replacement of the exact set. Verified 1227/0 across the full suite (single-threaded run is authoritative; parallel runs can observe synthetic cross-test heap states). Three principled carve-outs: test-fabricated sets (built_by_census), test-only page-metadata wipes (CLASSIFIER_VERIFY_SUPPRESSED), and FORWARDED headers (censused-then-moved is by-design divergence).2. Budgeted cycles skip the exact census (#6179)
Budgeted (precise, non-moving) cycles no longer build the O(heap) BTreeSet held across the sliced cycle. Membership resolves via
classifier_valid_object_start: the union of exact malloc-registry membership and plausible-arena-header — not the exclusive generation branch ofcurrent_heap_header_for_user_ptr, which left live malloc roots unmarked when page metadata attributed their range to an arena generation. Safety anchors: classifier-mode cycles never conservative-scan (traced conservative words can't tolerate heuristic false positives, even if aManualGcScanGuardappears mid-cycle via drain-before-manual-gc);maybe_containspasses everything through (call sites prefilter separately); synchronous cycles keep the exact set.3. RSS floor: pacing gain 1/32 + allocator trim on budgeted cycles
Measurement redirect: dropping the census barely moved RSS (929→928 MB on the churn benchmark) — the gap was pacing equilibrium (floating garbage across the debt window) and allocator pages never returned to the OS (audit finding:
run_malloc_trimskipped budgeted cycles). With gain doubled and trim enabled:B1 also holds the latency win: max frame 131 ms vs STW's 636 ms. Differential battery (± manual gc ± force-evacuate): clean on every run. Full suite 1227/0 with and without verification.
PERRY_GC_INCREMENTALremains default-off; this stack is the flip prerequisite (no O(heap) census on the default-to-be path) plus the first RSS-parity result.4. Darwin allocator trim + 5. budgeted sweeps don't age-bump
Two more measured fixes while chasing the churn-benchmark residual:
malloc_zone_pressure_reliefwired as darwin'smalloc_trimcounterpart (the budgeted-trim lever was glibc-only), and budgeted sweeps no longer age-bump — whole-cycle allocate-black marks every mid-cycle birth, and the age-bump read MARKED as "survived", tenuring dead churn into old-gen where minors never reclaim it. Promotion under incremental flows through the copied-minor path only.Final RSS attribution (vmmap, live process)
The churn benchmark's residual 928 MB peak is not a leak: libc malloc ~16 MB, cycle-end arena 203 MB, steady-state footprint 572 MB. The peak is the transient mid-cycle high-water — allocation continuing while a sliced cycle completes — i.e. the structural trade of slicing vs STW's pause wall, bounded by the debt controller. The realistic workload (B1) sits at exact STW parity (521 MB) with max frame 136 ms vs STW's 636 ms.
Summary by CodeRabbit