perf(gc): make the "nothing due" GC check cheap on safepoint polls and trigger checks - #10253
Closed
proggeramlug wants to merge 2 commits into
Closed
proggeramlug wants to merge 2 commits into
proggeramlug wants to merge 2 commits into
Conversation
…d trigger checks Runtime safepoint polls (regex quanta, the microtask pump, the event loop), gc_malloc and the other gc_check_trigger callers spent hundreds to thousands of instructions answering "is a collection due?" when nothing was. Three changes, each leaving every collection decision unchanged: - Runtime polls no longer build a JsGcStepResult. The budgeted step returns a debt-free GcStepReport; the FFI and test entry points attach the GcDebtSnapshot after the step returns, which reads the same values. The cycle start/step machinery moved out of line so a no-trigger poll does not pay its multi-kilobyte frame. - copying_from_space_in_use_bytes() is O(1) between layout changes: the bytes outside Eden's current block are cached, keyed on the heap generation (every reset, detach, evacuation and survivor flip runs inside a HeapChange scope), and every move of an arena's current block goes through Arena::set_current, which invalidates the cache. Debug builds compare every cached answer with the block walk. - gc_check_trigger evaluates the due trigger once instead of up to three times, reusing the answer unless it came from the PerryTS#10169 one-shot leaf priority; the young cap reuses the old-gen pressure the due trigger already read (debug cross-checked) and tests the census-seeded flag first.
|
Warning Review limit reachedNext included review available in 10 minutes. View limit detailsLimit details: You’ve used all 8 included reviews currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (16)
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 |
Contributor
Author
4 tasks
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.
Brief 1 of the #10166 per-call breakdown: the GC safepoint / trigger due-check cost.
What was slow
A runtime safepoint poll that finds nothing due cost about 950 instructions. That covers regex search quanta, the microtask pump and the event loop.
re.execmakes 7 such polls per call, counted with a uprobe.gc_check_triggerpays a similar cost on everygc_mallocand JSON parse, and it evaluates the due trigger up to three times. The biggest pieces:JsGcStepResultwith aGcDebtSnapshot, which evaluates the nursery cap, the arena trigger and the old-reclaim band a second time. All three runtime callers discarded the result.copying_from_space_in_use_bytes()summedblock.offsetover every Eden and active-survivor block on every read.Share of retired instructions in the due-check family before the change (release,
perf record -e instructions:u):re.execJSON.parsere.testawait, 20M object literals, 3M string concat/join, 2M BigInt ops, 1MMap.setWhat changed
No collection decision changes. Each piece is described in its doc comment.
GcStepReport.js_gc_step_*,js_gc_safepointand the test entry pointgc_runtime_safepoint()attach the debt after the step returns, which reads the same values. The runtime polls call the newgc_runtime_safepoint_poll(). Cycle start and stepping moved out of line intogc_budgeted_start_or_step.arena/from_space.rs). The bytes outside Eden's current block are cached, keyed on the heap generation. Every reset, detach, evacuation and survivor flip runs inside aHeapChangescope, which advances the generation. Every move of an arena'scurrentnow goes throughArena::set_current, which invalidates the cache, because the allocator can leave a block and later come back to it. Debug builds compare every cached answer with the block walk.gc_check_trigger.DueTriggerMemoreuses the first answer unless it came from the JSON roundtrip at scale: every minor traces the whole live tree because the large stringify result is malloc-tracked (untraced promotion vetoed) #10169 one-shot leaf priority, which consumes its flag. The young cap reuses the old-gen pressure value the due trigger already read, cross-checked in debug builds. It also tests the census-seeded flag before the two thread-locals it used to read first.Numbers
Shipping release profile (codegen-units=1, thin LTO) on perrymaster.
perf stat -e instructions:u, minimum of 3 interleaved runs; the largest spread between runs was 0.044 %.re.exec(2 captures)JSON.parsere.testawaitMap.setPeak RSS (
/usr/bin/time %M, 3 runs each) is unchanged within run-to-run noise on all eight probes.A build with 16 codegen units showed ±0.5 % swings on unrelated probes, in both directions. Those were partitioning artifacts and disappear at codegen-units=1.
After this change, what remains of the due check in
re.execis about 6.6 % of instructions. The regex engine's own multiplier, seven polls per call, is a separate follow-up.Equivalence
Probes: the 14 GC ratchet probes plus the 8 above. Env arms: each probe's declared env and a 1 MB nursery cap, each with the conservative stack scan on and off. Every run was done twice with
PERRY_GC_TRACE=1 PERRY_GC_DIAG=1. The comparison takes every[gc-trigger]decision line verbatim, which includes the from-space, cap, arena, old-gen and malloc inputs of each decision. It also takes every trace record field except timing.native_stack_maps.frames_visitedand counters that also differ between two base runs: old-page object counts, write-barrier cache hits and layout-scan counters.micro,r14_grow_then_churnandstrsdiverge starting at a collection whose conservative native-stack scan found exactly one root fewer (5 vs 4, 8 vs 7). That collection retained 80 B and 110 KB less, and later decisions follow from the smaller heap. A part-1-only build matched base on these probes. The cache removes a stale stack word the block walk used to leave behind, and the conservative scan sees stack garbage.Fault injections
Each was applied on its own, run, and reverted.
Arena::set_currentcached_occupancy_matches_the_walk_after_returning_to_the_primed_blockfails0as the reused old-gen pressurelarge_presized_array_grows_its_dense_frontier(left 0, right 917552)only_the_leaf_priority_answer_is_unrepeatablefailsDueTriggerMemoreuse every answergc_runtime_safepoint_polla no-opmicrotask_runner_tail_pays_bounded_safepoint_under_pressureandstdlib_pump_and_perry_poll_pay_debt_through_shared_scheduler_surfacesfailallocation_census_seeds_the_first_cap_before_any_minorfailsBefore this PR, nothing tested that the due trigger feeds the tenured-proportional term of the cap: the second injection passed the whole suite. The debug cross-check is what makes it fail now.
Validation
cargo fmt --all -- --checkcargo check -p perry-runtime --no-default-features --features full(regex off)RUST_TEST_THREADS=1 cargo test -p perry-runtime --lib: 3800 passed, 0 failed (macOS, debug, so every cached read is cross-checked)perry-runtime: 0 diagnostics on added linesscripts/gc_runtime_root_holders.py(+--self-test): newnot_a_gc_pointerverdict forSEALED_YOUNG_BYTES;PASS1_MARKEDwindow re-audited and re-pinned forgc/mod.rsandgc/policy.rsscripts/run_lint_gates.sh(compile tier included, macOS): 3 of 83 fail, none from this change.-D warningsall-targets check stops on four pre-existing test-build warnings in untouched files (object/global_this_webassembly.rs,ic_miss/ic_slow.rs).bun-ptymanifest entry that main's docs do not have yet.gc_repsel_matrix.sh --arms pr+ fan-in + instrument smoke), fix and base builds side by side: both 428 pass / 167 unverified / 7 fail, and all 581 distinct cells have the same status. The 7 failures are alltest_gap_gc_http2_pending_event_callback_rooting, an output mismatch even in cells with zero collections, on base too.gc_ratchet.py measure, 14 probes, 3 repeats): all probes pass the Node oracle, and the medians of copied/promoted/freed bytes and objects, heap used/total, minor cycles and step cycles are identical to base on every probe. RSS and wall time in that run are not comparable: base was a codegen-units=16 build and the host load was above 20. The like-for-like RSS check is the table above.GitHub runners are down, so the local replays above are the gate.
Coordination notes:
gc/policy.rsorgc/tenuring.rsand may conflict textually at train time.