fix(gc): make the moving-loop poll default ON in the code, not just the doc (#7690, #7682) - #7721
Conversation
|
Warning Review limit reached
Next review available in: 11 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe change enables moving-loop GC polls by default in runtime and code generation, optimizes selected GC layout paths, and sets explicit pacing modes in runtime-root tests. ChangesGC moving-loop poll defaults
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Environment
participant RuntimePolicy
participant CodegenLoops
participant TriggerTests
Environment->>RuntimePolicy: provide PERRY_GC_MOVING_LOOP_POLLS
Environment->>CodegenLoops: provide PERRY_GC_MOVING_LOOP_POLLS
RuntimePolicy->>TriggerTests: return runtime setting
CodegenLoops->>TriggerTests: return codegen setting
TriggerTests->>TriggerTests: verify parity
Possibly related PRs
🚥 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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.
Inline comments:
In `@crates/perry-runtime/src/gc/policy.rs`:
- Around line 477-492: The documentation around the moving-loop poll policy
still describes the process default as off. Update the comment associated with
moving_loop_polls_enabled_from_env to state that polls are enabled when
PERRY_GC_MOVING_LOOP_POLLS is unset, matching the implementation at lines
547-548 and the existing default-on behavior.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 209e6a6e-801c-40f8-8b5a-3ae4837ceb1e
📒 Files selected for processing (8)
changelog.d/7721-gc-moving-loop-poll-default.mdcrates/perry-codegen/src/stmt/loops.rscrates/perry-runtime/src/gc/layout.rscrates/perry-runtime/src/gc/policy.rscrates/perry-runtime/src/gc/tests/runtime_roots/fs_options_object.rscrates/perry-runtime/src/gc/tests/runtime_roots/json_shape_template.rscrates/perry-runtime/src/gc/tests/runtime_roots/symbol_description.rscrates/perry-runtime/src/gc/tests/triggers.rs
| /// **DEFAULT ON.** The kill switch is `PERRY_GC_MOVING_LOOP_POLLS=0`/`off`/`false`. | ||
| /// See [`moving_loop_polls_enabled_from_env`] for the decision and its evidence; | ||
| /// #7161's stopgap default-OFF (pending #7154) is discharged there. | ||
| /// | ||
| /// MUST match codegen `moving_safepoint_polls_enabled` (same env) so the deferral | ||
| /// and the polls that drain it stay coherent — a runtime default that disagrees | ||
| /// with the codegen default would defer collections that never drain (or drain | ||
| /// collections that were never deferred). | ||
| /// collections that were never deferred). That disagreement is not hypothetical: | ||
| /// it shipped. #7690 wrote the default-ON argument into the doc below and left | ||
| /// both bodies matching `1|on|true`, so the runtime deferred nursery pressure to | ||
| /// a safepoint codegen never emitted. Combined with #7687 (the alloc-point minor | ||
| /// must not move), the shipped collector had NO nursery evacuation at all — | ||
| /// `churn_alloc` ran 13 whole-arena full collections where it had run 105 copying | ||
| /// minors, and `tree` spent 4.1 s of its 5.1 s wall in GC pause. Both predicates | ||
| /// are now pinned by tests, and `polls_default_matches_codegen_mirror` pins that | ||
| /// they agree. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Correct the stale default description.
Line 496 states that the process default is off. Lines 547-548 enable polls when PERRY_GC_MOVING_LOOP_POLLS is unset. Update that comment to state that the process default is on.
🤖 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/policy.rs` around lines 477 - 492, The
documentation around the moving-loop poll policy still describes the process
default as off. Update the comment associated with
moving_loop_polls_enabled_from_env to state that polls are enabled when
PERRY_GC_MOVING_LOOP_POLLS is unset, matching the implementation at lines
547-548 and the existing default-on behavior.
Audit — the change is right; two corrections, and I am respecting your "do not merge on the partial result"Not merging. You asked for the gap verdict first, and 20/511 is not a verdict. Everything below is review, not a hold-up. The flip is real this time, and in both bodies
The framing is also right: this is not a slower configuration, it is a different collector. With no poll emitted, a compute-only program reaches neither precise collection point, so every nursery collection landed at the register-imprecise allocation point where #7687 had just made it correctly non-moving — leaving no nursery evacuation at all and a trigger falling back to whole-arena full collections.
The costs are stated, not buried — 1.
|
|
Addendum on the attribution point — there is stronger evidence than my baseline, and it settles it. #7494 already documents
So the Your Nothing here affects the PR. Everything else in my review stands, and I'm still holding for the gap verdict. |
Gap suite: complete — no regression attributable to this PRRun on the pinned quiet M1 mini ( 511 tests: 487 pass, 18 parity_fail, 6 crash. The harness reports 9 regressions against
The two GC rooting tests actually PASS, and are evidence for this changeTheir assertion is That is #7684's zeal verdict line breaking parity for any test that enables zeal — a pre-existing defect in the verdict's output channel, not a rooting failure. Read the numbers, though: these are the tests written specifically to catch rooting bugs, and under the new default they ran 9,653 and 19,253 copying minors, moving 115,728 and 231,348 objects, and still reported The five crashes are the host's tokio sandbox panicAll are http/net/fetch socket tests. Unrelated to GC. Caveat, stated plainly
One improvement also recorded: |
…he doc (#7690) #7690 wrote the entire default-ON argument into two doc comments — the runtime's `moving_loop_polls_enabled_from_env` and codegen's `moving_safepoint_polls_enabled` — and changed neither body. Both still matched `1|on|true`, i.e. default OFF, and no test pinned the default in either direction, even though the runtime predicate had been factored out expressly to make it "unit-testable without touching process env". That is not a slower configuration, it is a different collector. Nursery pressure has exactly two precise collection points, the loop back-edge poll and the outermost microtask-pump boundary. With no poll emitted, a compute-only program reaches neither, so every nursery collection happened at the register-imprecise allocation point — where #7687 had just made it correctly non-moving. The shipped result was a collector with no nursery evacuation at all. Measured on the quiet bench host, best-of-3, `PERRY_NO_AUTO_OPTIMIZE=1` with a pinned `PERRY_RUNTIME_DIR`, against `a853135aa` binaries rerun back-to-back on the same host: | bench | main | this | a853135 | |---|--:|--:|--:| | churn | 1.01 | 0.45 | 0.66 | | churn_alloc | 0.90 | 0.42 | 0.36 | | push_cls | 0.89 | 0.40 | 0.34 | | retain | 2.33 | 1.37 | 1.33 | | tree | 5.06 | 1.63 | 5.97 | | tree_wide | 7.26 | 2.11 | 12.38 | | cycles | 0.29 | 0.19 | 0.96 | `churn_alloc` ran 13 whole-arena full collections (0.477 s of pause) where the same program at `a853135aa` ran 105 copying minors (0.016 s). `tree`'s GC pause falls 4.107 s -> 0.626 s and its max pause 266 ms -> 23 ms; `trace_worklist` drops from 2,877 ms out of the top six phases entirely. The #7161 blocker that made polls-off a stopgap is separately discharged: a poll at every back-edge defeated the #7480 element-shape fast clone, and step 4 of that work now refuses to emit a poll inside a call-free-by-construction clone. Measured both ways, `churn_read` is 0.02 s. Costs, measured rather than argued: `deeplist` 0.03 -> 0.33 and `retain1` 0.03 -> 0.42. Both are workloads whose heap stays under the initial 64 MB threshold, so they previously ran ZERO collections and the moving nursery is pure added cost; both still beat `a853135aa` (1.09 / —). `push_num` 0.16 -> 0.17. Three tests pin what was unpinned: `polls_default_is_on` and its codegen mirror `moving_safepoint_poll_default::unset_emits_the_poll` each pin one half against the full spelling table, and `polls_default_matches_codegen_mirror` pins that the two crates agree — the disagreement is silent in both directions, so it needs its own assertion rather than being left to two doc comments claiming they match.
…r a disabled counter `heap_payload_slot_selection` runs once per traced object per GC walk (mark, rewrite, verify). For every GC_TYPE_OBJECT it computed `raw_numeric_object_slots` via `with_typed_descriptor_for_query` — a per-object map probe plus, for every class instance, a `SHAPE_LAYOUTS` hash lookup behind a TLS RefCell borrow. That number has exactly one consumer, `record_layout_raw_numeric_object_field_range_skipped`, which returns on its first line unless PERRY_GC_LAYOUT_SCAN_TRACE armed the counter. So the shipped collector paid a hash lookup per object to produce a number nothing read — the same shape as #7702, where a facility disabled at runtime was still having its arguments evaluated. Gate the computation on `layout_scan_trace_active()`. Second item, same walk: `shape_shared_pointer_mask` returned `shape_shared_descriptor(user_ptr).map(|d| d.pointer_mask)`, cloning the whole `TypedLayoutDescriptor` to keep one of its two masks. `LayoutSlotMask` is `Heap(Vec<u64>)` above 64 slots, so a traced wide object allocated and freed a second vector — the `raw_f64_mask` — on every walk. Borrow through `with_shape_shared_descriptor` and clone only the mask returned; `shape_shared_descriptor` had no other caller and is removed rather than left as dead code.
…ssert at Four `runtime_roots` tests took no pacing guard, so they inherited the process default — which this stack changes. They are not asserting about the default; they are asserting that a specific runtime helper's object survives a collection that happens at the allocation point, and they reach that collection through the direct alloc-point minor. Under moving-loop polls that pressure is deferred to a precise safepoint, and a Rust unit test has no loop back-edge poll to drain it, so no collection runs and `assert_automatic_minor_gc_progressed` reports neither a finished assist nor an ACTIVE budgeted cycle. `force_legacy_gc_pacing` is the wrong repair and the tests say so themselves. Three of them carry an evacuation witness — "the minor did not evacuate, so nothing here was exercised and a green result would be meaningless" — and legacy pacing hands the work to the budgeted stepper, which is deliberately non-moving. Pinning it turns a failed assist assertion into a failed liveness assertion, which is the witness doing its job. `force_alloc_point_minor_pacing` (polls OFF, scavenge ON) is the one combination in which both halves hold, and it is the configuration these tests were written against. `symbol_description` has no evacuation witness and takes `force_legacy_gc_pacing`. The moving default's rooting coverage for these helpers is the gap suite's `test_gap_gc_*_rooting.ts` cases and the zeal + from-space-protect runs, not this vehicle — recorded in each test so the next reader does not mistake a pinned pacing for the default being untested.
70a3523 to
691c664
Compare
Merging as v0.5.1418 — the gap verdict holds, and I checked its mechanism independentlyThe methodology is the right one. The load-bearing control is the same-host, same-binary kill-switch A/B, not the cross-host snapshot — and you say so explicitly, including that part of the snapshot delta (the 10 I verified the And the numbers in those two tests are the best evidence in the PR. They are the tests written specifically to catch rooting bugs; under the new default they ran 9,653 and 19,253 copying minors, moving 115,728 and 231,348 objects, and still reported Everything from my earlier review stands: the flip is real in both bodies, the framing (a different collector, not a slower one) is correct, and the costs are stated rather than buried. Two follow-ups, neither blocking
Gates 19/19. Rebased onto v0.5.1417 (this branch was 5 commits behind after today's merges). |
… no-op crates/perry-runtime/src/gc/tests/runtime_roots/generator_attach_prototype.rs's three tests were red on main: js_generator_attach_prototype and js_generator_attach_closure_prototype no longer moved their receiver under an alloc-point copying minor, and the shipped-default witness never saw its trigger armed. warm_generator_intrinsics() called js_generator_attach_prototype(TAG_UNDEFINED, 0) to pre-build the generator intrinsic tower before the timed call under test. That never worked: js_generator_attach_prototype returns at its very first line for any non-pointer obj, so the "warm-up" touched nothing. It went unnoticed because GENERATOR_FUNCTION_INTRINSIC_PTR and its five siblings were plain process-global AtomicI64s pre-#7723 - some earlier test in the same binary had almost always already built the tower, so the real call under test found it cached regardless of what warm_generator_intrinsics() did. #7723 converted those six statics to per_test_global! specifically so each test starts from a guaranteed first-touch state (crates/perry-runtime/src/gc/tests/lazy_intrinsic_towers.rs's whole point). That is a correct, deliberate change - it took away the accidental cross-test priming these three tests had been relying on. With nothing pre-built, the real call now pays the dozens-of-allocations tower build itself, inside build_generator_tower's GcSuppressScope (#7251's no-move window for that build). That suppression window swallows the arena trigger the test injected via arm_collection_on_next_block for the rest of the call: no copying minor ever runs before the tower build's own scope closes, and by then intermediate's own allocation no longer needs a new arena block, so the trigger is never serviced. Confirmed with instrumented gc_check_trigger / GcSuppressScope traces comparing the last-good commit against #7723: on the last-good commit the real call's first allocation reaches gc_check_trigger unsuppressed and services the trigger directly; on #7723 the entire ~1800-call tower build runs suppressed first and nothing ever re-triggers afterward. Fix warm_generator_intrinsics() to call crate::object::ensure_generator_intrinsics() directly - the same builder lazy_intrinsic_towers.rs uses - so it does what its name and doc comment always claimed. This does not touch the liveness/deferral assertions those tests make; it only repairs the test's own setup helper. Bisected via git checkout of each of today's three merges in an isolated worktree: c907953 (pre-#7721) passes; ca8c0d6 (#7721, moving-loop poll default flip) passes; cbb682d (#7723, no-move window + per_test_global towers) is the first commit where all three fail. #7724 is uninvolved.
… no-op (#7731) * fix(gc): warm_generator_intrinsics must call the tower builder, not a no-op crates/perry-runtime/src/gc/tests/runtime_roots/generator_attach_prototype.rs's three tests were red on main: js_generator_attach_prototype and js_generator_attach_closure_prototype no longer moved their receiver under an alloc-point copying minor, and the shipped-default witness never saw its trigger armed. warm_generator_intrinsics() called js_generator_attach_prototype(TAG_UNDEFINED, 0) to pre-build the generator intrinsic tower before the timed call under test. That never worked: js_generator_attach_prototype returns at its very first line for any non-pointer obj, so the "warm-up" touched nothing. It went unnoticed because GENERATOR_FUNCTION_INTRINSIC_PTR and its five siblings were plain process-global AtomicI64s pre-#7723 - some earlier test in the same binary had almost always already built the tower, so the real call under test found it cached regardless of what warm_generator_intrinsics() did. #7723 converted those six statics to per_test_global! specifically so each test starts from a guaranteed first-touch state (crates/perry-runtime/src/gc/tests/lazy_intrinsic_towers.rs's whole point). That is a correct, deliberate change - it took away the accidental cross-test priming these three tests had been relying on. With nothing pre-built, the real call now pays the dozens-of-allocations tower build itself, inside build_generator_tower's GcSuppressScope (#7251's no-move window for that build). That suppression window swallows the arena trigger the test injected via arm_collection_on_next_block for the rest of the call: no copying minor ever runs before the tower build's own scope closes, and by then intermediate's own allocation no longer needs a new arena block, so the trigger is never serviced. Confirmed with instrumented gc_check_trigger / GcSuppressScope traces comparing the last-good commit against #7723: on the last-good commit the real call's first allocation reaches gc_check_trigger unsuppressed and services the trigger directly; on #7723 the entire ~1800-call tower build runs suppressed first and nothing ever re-triggers afterward. Fix warm_generator_intrinsics() to call crate::object::ensure_generator_intrinsics() directly - the same builder lazy_intrinsic_towers.rs uses - so it does what its name and doc comment always claimed. This does not touch the liveness/deferral assertions those tests make; it only repairs the test's own setup helper. Bisected via git checkout of each of today's three merges in an isolated worktree: c907953 (pre-#7721) passes; ca8c0d6 (#7721, moving-loop poll default flip) passes; cbb682d (#7723, no-move window + per_test_global towers) is the first commit where all three fail. #7724 is uninvolved. * changelog: add fragment for #7731 (generator-attach-pacing) * chore: bump version to 0.5.1422 Claude-Session: https://claude.ai/code/session_01Y1QZ5wUP9gRSwpiweT4Wix --------- Co-authored-by: Ralph Küpper <ralph@skelpo.com>
…oad (#7721) #7721 turned the moving-loop back-edge poll on by default, which was right about the collector and wrong about its price. The poll is emitted at EVERY allocating loop back-edge — 20 M of them in `bench/churn_alloc.ts` — so its no-work path is a per-iteration cost of the language, and that path was an out-of-line call into two `OnceLock` acquire loads, an unconditional atomic increment, and a thread-local read that on Darwin is a CALL to `_tlv_get_addr`. ~3 ns per back-edge: `churn_alloc` 0.367 s -> 0.419, `push_cls` 0.350 -> 0.408, `push_num` 0.131 -> 0.178. `gc/poll_arm.rs` adds `PERRY_GC_POLL_ARMED`, a process-global counter of the reasons the poll must do more than return. Zero is a PROOF the poll is a no-op, so codegen loads it inline and branches around the call (two aarch64 instructions, address hoisted into the preheader) and the runtime entry point re-checks it for modules from any other emission path. `GC_SAFEPOINT_PENDING` now has exactly one writer, `policy::set_safepoint_pending`, which moves the flag and the global together — the word reading zero while a deferral is outstanding is the one unsound direction, and it would strand that collection until an event-loop boundary a compute-only program never reaches. Measured best-of-7 interleaved on the quiet M1 bench host, outputs verified against `node --experimental-strip-types`: | bench | main | this | 0.5.1384 | |---|--:|--:|--:| | churn_alloc | 0.419 | 0.376 | 0.367 | | push_cls | 0.408 | 0.357 | 0.350 | | push_num | 0.178 | 0.144 | 0.131 | | churn | 0.45 | 0.41 | — | | churn_read | 0.02 | 0.02 | — | | cycles | 0.19 | 0.19 | — | | deeplist | 0.31 | 0.31 | — | | tree | 1.64 | 1.64 | — | | tree_wide | 2.10 | 2.12 | — | GC behaviour is unchanged: `churn` runs 105 minors in both arms with positive reclamation every cycle, max pause 3.63 ms -> 1.78 ms. `gc-handoff/apps/iso_miss.ts` prints `checksum 437840 misses 0`.
…oad (#7721) #7721 turned the moving-loop back-edge poll on by default, which was right about the collector and wrong about its price. The poll is emitted at EVERY allocating loop back-edge — 20 M of them in `bench/churn_alloc.ts` — so its no-work path is a per-iteration cost of the language, and that path was an out-of-line call into two `OnceLock` acquire loads, an unconditional atomic increment, and a thread-local read that on Darwin is a CALL to `_tlv_get_addr`. ~3 ns per back-edge: `churn_alloc` 0.367 s -> 0.419, `push_cls` 0.350 -> 0.408, `push_num` 0.131 -> 0.178. `gc/poll_arm.rs` adds `PERRY_GC_POLL_ARMED`, a process-global counter of the reasons the poll must do more than return. Zero is a PROOF the poll is a no-op, so codegen loads it inline and branches around the call (two aarch64 instructions, address hoisted into the preheader) and the runtime entry point re-checks it for modules from any other emission path. `GC_SAFEPOINT_PENDING` now has exactly one writer, `policy::set_safepoint_pending`, which moves the flag and the global together — the word reading zero while a deferral is outstanding is the one unsound direction, and it would strand that collection until an event-loop boundary a compute-only program never reaches. Measured best-of-7 interleaved on the quiet M1 bench host, outputs verified against `node --experimental-strip-types`: | bench | main | this | 0.5.1384 | |---|--:|--:|--:| | churn_alloc | 0.419 | 0.376 | 0.367 | | push_cls | 0.408 | 0.357 | 0.350 | | push_num | 0.178 | 0.144 | 0.131 | | churn | 0.45 | 0.41 | — | | churn_read | 0.02 | 0.02 | — | | cycles | 0.19 | 0.19 | — | | deeplist | 0.31 | 0.31 | — | | tree | 1.64 | 1.64 | — | | tree_wide | 2.10 | 2.12 | — | GC behaviour is unchanged: `churn` runs 105 minors in both arms with positive reclamation every cycle, max pause 3.63 ms -> 1.78 ms. `gc-handoff/apps/iso_miss.ts` prints `checksum 437840 misses 0`.
1 participant
What this is
#7690 wrote the entire default-ON argument for the moving-loop poll into two doc comments — the runtime's
moving_loop_polls_enabled_from_envand codegen'smoving_safepoint_polls_enabled— and changed neither body. Both still matched1|on|true, i.e. default OFF. No test pinned the default in either direction, even though the runtime predicate had been factored out expressly to make it "unit-testable without touching process env". The runtime doc even asserted "Codegen'smoving_safepoint_polls_enabledmirrors this exactly — they MUST agree"; they did agree, at the value the doc said they no longer held.That is not a slower configuration, it is a different collector. Nursery pressure has exactly two precise collection points: the loop back-edge poll and the outermost microtask-pump boundary. With no poll emitted, a compute-only program reaches neither, so every nursery collection happened at the register-imprecise allocation point — where #7687 had just made it correctly non-moving. The shipped collector had no nursery evacuation at all, and the trigger fell back to whole-arena full collections.
churn_allocruns 13 whole-arena full collections (0.477 s of pause) onmainwhere the same program ata853135aaran 105 copying minors (0.016 s).Measurements
Pinned quiet M1 mini, best-of-3 interleaved,
PERRY_NO_AUTO_OPTIMIZE=1with a pinnedPERRY_RUNTIME_DIR,a853135aabinaries rerun back-to-back on the same host.12e48edd6a853135aatreetotal GC pause 4.107 s → 0.550 s, max pause 266 ms → 16 ms;trace_worklistfalls from 2,877 ms out of the top six phases entirely.tree_wide0.549 s of pause, 17 ms max. All 11 benchmark stdouts byte-identical to Node.Costs, measured rather than argued.
deeplist0.03 → 0.31 andretain10.03 → 0.42: both keep their heap under the initial 64 MB threshold, so they previously ran zero collections and a moving nursery is pure added cost there. Both still beata853135aa(1.14 / —).push_num0.16 → 0.17.The earlier objection is discharged
Polls-default-ON was declined once because a poll at every back-edge deleted the #7480 element-shape fast clone — a call inside a clone whose admission rests on being call-free-by-construction does not slow it, it removes it. Step 4 of that work now refuses to emit a poll inside such a clone.
churn_readmeasures 0.02 s with polls either way.Also in this PR
perf(gc):heap_payload_slot_selectionruns once per traced object per GC walk and computedraw_numeric_object_slotsvia aSHAPE_LAYOUTShash lookup behind a TLSRefCell— for a counter that returns on its first line unlessPERRY_GC_LAYOUT_SCAN_TRACEarmed it. Same shape as perf(typed-feedback): stop emitting recording calls into default builds (#7480) #7702. Now computed only when the trace is armed. Second item on the same walk:shape_shared_pointer_maskcloned a wholeTypedLayoutDescriptorto keep one of its two masks, allocating and freeing a throwawayVecper traced wide object; it now borrows and clones only what it returns.test(gc): fourruntime_rootstests took no pacing guard and so inherited the default.force_legacy_gc_pacingis the wrong repair and the tests say so themselves — three carry an evacuation witness ("the minor did not evacuate, so nothing here was exercised") and legacy pacing routes to the budgeted stepper, which is deliberately non-moving.force_alloc_point_minor_pacing(polls OFF, scavenge ON) is the combination they were written against.Validation
PERRY_GC_VERIFY_MARK=1 PERRY_GC_VERIFY_EVACUATION=1clean on six benches.PERRY_GC_ZEAL=1 PERRY_GC_PROTECT_FROMSPACE=1at quarantine depths 8 and 800: 4,837 objects moved, no fault, byte-identical output. Zeal's verdict line is the liveness proof that the new default is live rather than merely untripped:forced_collections=2000005 copying_minors=2000005 moved_objects=8 loop_polls=2000000.cargo fmt --all -- --checkandscripts/check_file_size.shclean.parity_fail, and both (test_gap_2159_defineproperty_class_prototype,test_gap_2514_settracesigint) are already intest-parity/known_failures.json. Zero new failures so far. The run is crawling because the dev host is under load from other work; I will post the final verdict on this PR before it should be merged. Please do not merge on the partial result.Three tests pin what was unpinned
polls_default_is_onand codegen'smoving_safepoint_poll_default::unset_emits_the_polleach pin one half against the full spelling table — including the unrecognised-value arm, which is the one that silently changes meaning if thematches!is inverted back.polls_default_matches_codegen_mirrorpins that the two crates agree; that disagreement is silent in both directions (polls nothing consumes, or a deferral nothing drains), so it needs its own assertion rather than two doc comments claiming they match.Unrelated finding, not fixed here
crates/perry-codegen/tests/typed_shape_descriptors.rs::integer_arithmetic_array_push_omits_inbounds_layout_note_and_barrieris red onmainand it is not this change — it fails identically underPERRY_GC_MOVING_LOOP_POLLS=0. #7702 turnedtyped_feedback_emission_enabled()off by default, sokeep_guarded_numeric_pushis false for a canonical-raw-f64 value and theapush.numeric_fastblock is never emitted. The code change is deliberate; the test was not updated. That file is undercrates/*/tests/, which does not run per-PR, so it landed red.push_num0.13 → 0.16 on bothmainand this branch is the matching wall-clock cost.https://claude.ai/code/session_01KaVWLc4q8Ng9P7mWmbUZpf
Summary by CodeRabbit
New Features
0,off, orfalse.Bug Fixes
Tests