Skip to content

Commit a3b3566

Browse files
proggeramlugRalph Küpper
andauthored
fix(codegen): statepoint report counted zero safepoints since #7348 (#7368)
* fix(codegen): statepoint report counted zero safepoints since #7348 #7348 deleted the explicit bridge and with it the only callers of note_statepoint and note_skipped -- they lived in the bridge, which counted safepoints as it emitted them. The methods survived with no callers, so statepoints, relocations, max_live_roots, skipped_non_safepoints, live_roots_histogram and both by-callee maps went structurally zero in production. A real compile printed "0 statepoints emitted" while its binary carried 120. Counting at IR-emission time cannot work any more, and that is the lesson: Perry no longer decides which calls become safepoints -- RewriteStatepointsForGC does, inside LLVM. The only honest source is the compact-map rewrite, which already parses the assembly LLVM emitted and computed these exact numbers before dropping them into log::debug!. The report reads from there now: 120 safepoints across 6 function(s) in 1 module(s) 36 live roots recorded, 0.30 per safepoint An absent measurement no longer renders as a measured zero: gc_map.modules == 0 means "never reported", the text report says UNAVAILABLE rather than printing zeros, and JSON carries gc_map separately from totals so a consumer can tell them apart. schema_version -> 2. The CI gate now asserts the counts, not just the label. --only-backend rs4gc passed throughout the regression -- the label was right, the numbers were fiction. It now also requires records > 0 and roots > 0; verified against a synthetic report with the #7348 shape, where the label check still reports 9 functions green while the count checks exit 1. Second round of dead counters here (#7362 removed four that never had a writer at all). The new test documents why the first invariant missed this one: every_rendered_counter_has_a_writer called the mutators itself, so "has a writer" passed while "is written" was false. Claude-Session: https://claude.ai/code/session_01EaD6yNwoinzdW1JbYNkMMF * gc: admit two provably-leaf helpers, and measure that it buys nothing (#7369) * fix(lint): split index_set.rs, over the 2000-line cap since #7342 (#7366) `scripts/check_file_size.sh` exits 1 on main HEAD: `crates/perry-codegen/src/expr/index_set.rs` is 2035 lines against a 2000 cap. It crossed in #7342. That script runs inside the `lint` job, which is a REQUIRED context -- so this is the second independent way `lint` was red on main today (the first was rustfmt on linker.rs, #7361). A required check that is red on main blocks nothing; it means every merge is a bypass. The split follows the recipe in the script's own failure message: extract a topical group into a sibling module. `lower_inline_dyn_typed_array_set` and its `emit_inline_ta_int_store` helper are the guarded inline typed-array store for a type-erased receiver -- one coherent unit, moved verbatim to `index_set_typed_array.rs`. index_set.rs drops to 1749 lines, leaving real headroom rather than landing one line under the cap. Mechanical move: the two functions are byte-identical, only the imports they need travelled with them and `lower_inline_dyn_typed_array_set` became `pub(super)` so its one caller can still reach it. cargo test -p perry-codegen --lib: 609 passed. Claude-Session: https://claude.ai/code/session_01EaD6yNwoinzdW1JbYNkMMF Co-authored-by: Ralph Küpper <ralph@skelpo.com> * docs(plan): both statepoint adoption gates are closed; record the platform matrix (#7367) The plan still said statepoints were aarch64-only (#7321), that the matrix "therefore runs on macos-14", that `statepoints-refuse-x86` pinned the refusal, and it spelled the knob `PERRY_STATEPOINTS` four times. None of that is true now, and this document is what the adoption decision gets made from. What actually changed: - x86-64 is unblocked. `_Unwind_GetGR(ctx, 7)` does segfault and cannot be fixed as stated -- libgcc tracks only the columns CFI restores and RSP is derived, not tracked. #7349 stopped asking for it and derives the SP-relative base from `_Unwind_GetCFA`, with a per-arch return-address adjustment (x86-64 `call` pushes one, aarch64 `bl` does not). x86-64 Linux is a first-class arm. - Windows works via RtlVirtualUnwind (#7355), the one walker with no Itanium unwinder beneath it. - aarch64+ELF is now covered too (#7360) -- the only shape where LLVM spells 32-bit stack-map fields `.word`. - One mechanism, not two: PERRY_STATEPOINTS and the plain-map bridge are deleted, so the kill-policy line about "a mode that still exists" no longer applies to this pair. - The gate proves something now. Until today the Unix arms reported 7 frames and ZERO locations -- they would have passed with a walker that visited nothing. #7359's deep-collect probe took them to 221 locations. - watchOS/visionOS are not blocked by Perry: they build on stable without `dyn-eval`, and fail three crates away in psm's Mach-O guard. So the remaining adoption gate is `llvm-inprocess` becoming a default cargo feature, plus sequencing step 2 (root density) -- adopting today would regress binary size on root-dense code. Claude-Session: https://claude.ai/code/session_01EaD6yNwoinzdW1JbYNkMMF Co-authored-by: Ralph Küpper <ralph@skelpo.com> * gc: admit two provably-leaf helpers, and measure that it buys nothing js_gc_register_global_root was the most frequent non-leaf callee in the probe suite (148 call sites) and is provably GC-leaf: its whole body is runtime_write_barrier_root_heap_word -- which js_write_barrier_root_heap_word, already CannotCollect, wraps in one line -- plus a TLS Vec::push. The "malloc count threshold" trigger does not apply to that push: the counter is MALLOC_STATE.objects.len(), a registry of Perry GC objects, and the #[global_allocator] is plain mimalloc/System with no GC hook. js_typed_feedback_maybe_dump_trace joins its already-admitted family siblings. Measured A/B on the same tree, and the result is a null: probe safepoints roots total bytes __text 06_string_retention 105 -> 100 27=27 0 -4 B 09_try_catch_roots 343 -> 339 259=259 0 -4 B 11_collect_at_depth 120 -> 117 36=36 0 -4 B Root counts are IDENTICAL. The 40 safepoints removed across the suite were all rootless, and a rootless safepoint costs essentially nothing -- which is what docs/engine-plan.md already says: "the axis is not 'statepoints are bigger', it is 'roots are bigger'". Recording it as evidence: the safepoint-count lever is not the binary-size lever, so sequencing step 2 must attack live-root SETS. Two tests come with it. One pins the wrapper's classification to the barrier it wraps. The other pins js_nanbox_string OUT of the allowlist: at 120 call sites it is the obvious next candidate and reads as pure bit manipulation, but its null guard calls js_string_from_bytes to allocate an empty string. Probe suite 11/11 byte-identical under forced evacuation + verification. Claude-Session: https://claude.ai/code/session_01EaD6yNwoinzdW1JbYNkMMF --------- Co-authored-by: Ralph Küpper <ralph@skelpo.com> * fix(ci): report assertion pinned to a probe Windows cannot compile Three review fixes on #7368. The report assertion ran on 09_try_catch_roots, which contains four `try` blocks. RS4GC cannot rewrite WinEH funclet pads, so linker.rs's rs4gc_funclet_refusal rejects that probe on windows-msvc -- the probe loop above tolerates it by grepping the compile log for "funclet", but this step did not. A gate pinned to a probe that cannot compile on one arm fails for a reason unrelated to its subject. The portable assertion now uses 11_collect_at_depth (no `try`, compiles on all four arms); 09_try_catch_roots keeps its own non-Windows step so the try-specific coverage that justified deleting the bridge is not lost. The gc_map doc claimed records/roots would be ABSENT when unmeasured. They are plain u64 fields on a plain derive and always serialise; `modules` is the sentinel. Fixed to describe what the code actually does -- the same class of comment-vs-code drift this PR exists to clean up. The "map never reported" guard fired for any --require-*/--print, including fields that live in `totals` and are counted at IR-emission time whether or not the rewrite ran. Now scoped to map-backed fields: --require-positive textual_calls is answered from its measured value (verified exit 0) while --require-positive records still fails on an unreported map (exit 1). Claude-Session: https://claude.ai/code/session_01EaD6yNwoinzdW1JbYNkMMF --------- Co-authored-by: Ralph Küpper <ralph@skelpo.com>
1 parent 3056986 commit a3b3566

7 files changed

Lines changed: 451 additions & 139 deletions

File tree

.github/workflows/gc-native-roots.yml

Lines changed: 38 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -294,13 +294,47 @@ jobs:
294294
# every function quietly lowered by the other backend, the arm
295295
# measuring the mode it was not testing. `--only-backend rs4gc`
296296
# rejects a single such fallback.
297-
PERRY_RS4GC=1 ./target/perry-dev/perry \
298-
benchmarks/gc_ratchet/probes/09_try_catch_roots.ts \
299-
-o /tmp/rs4gc-report-probe --statepoint-report=json 2> /tmp/rs4gc-report.json
300297
# windows-latest exposes the toolcache python as `python`, not python3.
301298
py=python3; command -v python3 >/dev/null 2>&1 || py=python
299+
300+
# The PORTABLE assertion, on every arm. `11_collect_at_depth` is
301+
# deliberate: it contains no `try`, so it compiles under RS4GC
302+
# everywhere. `09_try_catch_roots` does NOT — RS4GC cannot rewrite
303+
# WinEH funclet pads, so `linker.rs`'s `rs4gc_funclet_refusal` rejects
304+
# it on windows-msvc, and the probe loop above only tolerates that
305+
# because it greps the compile log for "funclet". A report assertion
306+
# pinned to a probe that cannot compile on one arm is a gate that
307+
# fails for a reason unrelated to its subject.
308+
#
309+
# --only-backend proves the lowering ran on every function; the two
310+
# --require-positive checks prove it PRODUCED something. Those counts
311+
# come from the compact-map rewrite parsing the assembly LLVM
312+
# emitted, which is the only honest source now that RS4GC decides
313+
# what becomes a safepoint. Until #7368 the report counted at
314+
# IR-emission time, #7348 deleted those writers with the bridge, and
315+
# every compile printed `0 statepoints emitted` while its binary
316+
# carried hundreds. A label check could not see that; these can.
317+
PERRY_RS4GC=1 ./target/perry-dev/perry \
318+
benchmarks/gc_ratchet/probes/11_collect_at_depth.ts \
319+
-o /tmp/rs4gc-report-probe --statepoint-report=json 2> /tmp/rs4gc-report.json
302320
"$py" scripts/statepoint_report_assert.py /tmp/rs4gc-report.json \
303-
--only-backend rs4gc
321+
--only-backend rs4gc \
322+
--require-positive records \
323+
--require-positive roots
324+
325+
# The try-specific arm, everywhere RS4GC can compile a `try`. This is
326+
# the coverage the probe above cannot give: 128 of 479 gap tests
327+
# contain `try {}`, and RS4GC being the only backend that handles them
328+
# is the reason the bridge could be deleted (#7339, #7348).
329+
if [ "$RUNNER_OS" != "Windows" ]; then
330+
PERRY_RS4GC=1 ./target/perry-dev/perry \
331+
benchmarks/gc_ratchet/probes/09_try_catch_roots.ts \
332+
-o /tmp/rs4gc-try-probe --statepoint-report=json 2> /tmp/rs4gc-try.json
333+
"$py" scripts/statepoint_report_assert.py /tmp/rs4gc-try.json \
334+
--only-backend rs4gc \
335+
--require-positive records \
336+
--require-positive roots
337+
fi
304338
305339
# Walker liveness, on EVERY arm. A walker that visits zero frames
306340
# still lets most probes print the right answer, because other root
Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
### Fixed
2+
3+
**`--statepoint-report` reported `0 statepoints emitted` for every compile since #7348.**
4+
5+
#7348 deleted the explicit statepoint bridge, and with it the only callers of
6+
`FunctionRecord::note_statepoint` and `note_skipped` — they lived in the bridge,
7+
which counted safepoints as it emitted them. The methods survived with no
8+
callers, so `statepoints`, `relocations`, `max_live_roots`,
9+
`skipped_non_safepoints`, `live_roots_histogram` and both by-callee maps became
10+
structurally zero in production. Measured on a real compile:
11+
12+
```
13+
5 function(s), 5 bound native root slots (5 logical slots reserved)
14+
5 textual calls: 5 with live roots, 0 without
15+
0 statepoints emitted; 0 non-collecting calls skipped <-- binary had 120
16+
0 relocations; maximum 0 live roots at one safepoint
17+
```
18+
19+
Counting at IR-emission time cannot work any more, and that is the real lesson:
20+
**Perry no longer decides which calls become safepoints — `RewriteStatepointsForGC`
21+
does, inside LLVM.** The only honest source is the compact-map rewrite, which
22+
already parses the assembly LLVM actually emitted and computed exactly these
23+
numbers before throwing them at `log::debug!`. The report now reads from there:
24+
25+
```
26+
120 safepoints across 6 function(s) in 1 module(s)
27+
36 live roots recorded, 0.30 per safepoint
28+
```
29+
30+
An absent measurement no longer renders as a measured zero. `gc_map.modules == 0`
31+
means "the rewrite never reported", the text report says
32+
`Safepoint counts UNAVAILABLE` instead of printing zeros, and the JSON carries
33+
`gc_map` separately from `totals` so a consumer can tell the two apart.
34+
`schema_version` is now `2`.
35+
36+
**The CI gate now asserts the counts, not just the label.** `gc-native-roots`
37+
checked `--only-backend rs4gc`, which passed throughout the regression — the
38+
backend label was correct, the numbers were fiction. It now also requires
39+
`records > 0` and `roots > 0`. Verified against a synthetic report with the
40+
#7348 shape: the label check reports 9 functions green while the count checks
41+
exit 1.
42+
43+
Note this is the *second* round of dead counters in this file (#7362 removed four
44+
that never had a writer at all). The new test documents why the first invariant
45+
missed this one: `every_rendered_counter_has_a_writer` called the mutators
46+
itself, so "has a writer" passed while "is written" was false. The structural fix
47+
is that the numbers now have exactly one producer and their absence is loud.
48+
49+
Three review fixes on top of the above:
50+
51+
- The report assertion ran on `09_try_catch_roots`, which contains four `try`
52+
blocks — and RS4GC cannot rewrite WinEH funclet pads, so
53+
`rs4gc_funclet_refusal` rejects that probe on `windows-msvc`. The probe loop
54+
above tolerates it by grepping the compile log for "funclet"; this step did
55+
not. The portable assertion now uses `11_collect_at_depth` (no `try`, compiles
56+
on all four arms) and `09_try_catch_roots` keeps its own non-Windows step, so
57+
the try-specific coverage is not lost.
58+
- The `gc_map` doc claimed `records`/`roots` would be *absent* when unmeasured.
59+
They are plain `u64` fields on a plain derive and always serialise; `modules`
60+
is the sentinel. Corrected to describe what the code does.
61+
- The "map never reported" guard in `statepoint_report_assert.py` fired for any
62+
`--require-*`/`--print`, including fields that live in `totals` and are
63+
counted at IR-emission time regardless of the rewrite. It is now scoped to
64+
map-backed fields, so `--require-positive textual_calls` is answered from its
65+
measured value instead of being failed by an unreported map it does not use.
Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
### Changed
2+
3+
**Two runtime helpers admitted to the GC-effect allowlist — and a measured null result on binary size.**
4+
5+
`js_gc_register_global_root` was the single most frequent non-leaf callee in the
6+
probe suite (148 call sites), and it is provably GC-leaf. Its entire body is:
7+
8+
```rust
9+
runtime_write_barrier_root_heap_word(*root); // shade one header
10+
GLOBAL_ROOTS.with(|r| r.borrow_mut().push(root)); // TLS Vec push
11+
```
12+
13+
The first call is exactly what `js_write_barrier_root_heap_word` — already
14+
`CannotCollect` — wraps in one line. The second is a `Vec::push`, and the
15+
"malloc count threshold" GC trigger does not apply to it: that counter is
16+
`MALLOC_STATE.objects.len()`, a registry of Perry GC objects, and the
17+
`#[global_allocator]` is plain mimalloc/System with no GC hook.
18+
`js_typed_feedback_maybe_dump_trace` joins its already-admitted family siblings
19+
(env read, JSON serialise, file write; empty body without `diagnostics`).
20+
21+
**The result, measured A/B on the same tree, is that this buys nothing:**
22+
23+
| probe | safepoints | roots | total bytes | `__text` |
24+
|---|---:|---:|---:|---:|
25+
| `06_string_retention` | 105 → 100 | 27 → 27 | 0 | −4 B |
26+
| `09_try_catch_roots` | 343 → 339 | 259 → 259 | 0 | −4 B |
27+
| `11_collect_at_depth` | 120 → 117 | 36 → 36 | 0 | −4 B |
28+
29+
Root counts are **identical**. The 40 safepoints removed across the suite were
30+
all rootless, and a safepoint with no live roots costs essentially nothing —
31+
which is precisely what `docs/engine-plan.md` already says ("Statepoints have no
32+
fixed cost… the axis is not 'statepoints are bigger', it is 'roots are bigger'").
33+
34+
This is worth recording as evidence rather than a win: **the safepoint-count
35+
lever is not the binary-size lever.** Sequencing step 2's "reduce root density"
36+
must attack live-root *sets*, not safepoint counts. Anyone reaching for the next
37+
obvious helper should read the second test below first.
38+
39+
Two tests come with it. `register_global_root_tracks_the_barrier_it_wraps` pins
40+
the two classifications together so a future demotion of the barrier cannot
41+
leave its wrapper claiming to be leaf. `allocating_helpers_are_not_cannot_collect`
42+
pins `js_nanbox_string` **out** of the allowlist: at 120 call sites it is the
43+
obvious next candidate and it reads as pure bit manipulation, but its
44+
null-pointer guard calls `js_string_from_bytes` to allocate an empty string
45+
rather than boxing null.
46+
47+
Probe suite: 11/11 byte-identical to the Node oracle under `PERRY_RS4GC=1
48+
PERRY_GC_FORCE_EVACUATE=1 PERRY_GC_VERIFY_EVACUATION=1`.

crates/perry-codegen/src/gc_call_effects.rs

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,28 @@ pub(crate) fn classify_direct_callee(name: &str) -> GcCallEffect {
5656
| "js_write_barrier_slot"
5757
| "js_write_barrier_root_heap_word"
5858
| "js_write_barrier_root_nanbox"
59+
// `gc/roots.rs`: registers one module-level global as a root. Audited
60+
// 2026-08-04 and admitted because its whole body is two calls that are
61+
// already covered:
62+
//
63+
// runtime_write_barrier_root_heap_word(*root) <- `js_write_barrier_
64+
// root_heap_word` immediately above is a ONE-LINE wrapper around
65+
// this exact function, and is already CannotCollect. It shades
66+
// one header and calls `push_mark_seed`, which is a TLS
67+
// `Vec::push` (`gc/trace.rs`) — no trace, no sweep, no trigger.
68+
// GLOBAL_ROOTS.with(|r| r.borrow_mut().push(root)) <- a TLS Vec.
69+
//
70+
// The `Vec::push` is the only thing worth pausing on, because CLAUDE.md
71+
// lists a "malloc count threshold" as a GC trigger. It does not apply:
72+
// that counter is `MALLOC_STATE.objects.len()`, a registry of Perry GC
73+
// objects, and the `#[global_allocator]` is plain mimalloc/System with
74+
// no GC hook. A raw Rust allocation cannot arm a trigger — which is the
75+
// case the module doc above already carves out.
76+
//
77+
// Worth the audit: at 148 call sites across the probe suite this is the
78+
// single most frequent non-leaf callee, all of it module-init code
79+
// registering `@perry_global_*` roots.
80+
| "js_gc_register_global_root"
5981
// `gc/layout.rs`: side-table metadata updates only.
6082
| "js_gc_note_slot_layout"
6183
| "js_gc_note_slot_layout_aware"
@@ -71,6 +93,11 @@ pub(crate) fn classify_direct_callee(name: &str) -> GcCallEffect {
7193
| "js_typed_feedback_class_field_set_guard"
7294
| "js_typed_feedback_observe_property_get"
7395
| "js_typed_feedback_observe_property_set"
96+
// Same family, audited 2026-08-04: under `diagnostics` it reads an env
97+
// var, serialises the counters with serde_json and writes a file;
98+
// without the feature the body is empty. No Perry allocation, no
99+
// re-entry into generated code, no route into collection.
100+
| "js_typed_feedback_maybe_dump_trace"
74101
// Refcount writes and array-layout observations; none enters GC.
75102
| "js_string_addref"
76103
| "js_string_addref_if_heap_string"
@@ -130,6 +157,38 @@ pub(crate) fn classify_direct_callee(name: &str) -> GcCallEffect {
130157
mod tests {
131158
use super::*;
132159

160+
/// `js_gc_register_global_root` is `js_write_barrier_root_heap_word` plus
161+
/// a TLS `Vec::push`, so the two must never be classified differently —
162+
/// if a future audit demotes the barrier, this catches the sibling that
163+
/// would otherwise keep claiming to be leaf.
164+
#[test]
165+
fn register_global_root_tracks_the_barrier_it_wraps() {
166+
assert_eq!(
167+
classify_direct_callee("js_gc_register_global_root"),
168+
classify_direct_callee("js_write_barrier_root_heap_word"),
169+
"js_gc_register_global_root's entire body is that barrier plus a \
170+
TLS Vec::push; they cannot have different GC effects"
171+
);
172+
}
173+
174+
/// The helpers that *do* allocate must stay out of `CannotCollect`, and
175+
/// this pins the two that read as pure but are not.
176+
///
177+
/// `js_nanbox_string` looks like bit manipulation and mostly is — but its
178+
/// null-pointer guard calls `js_string_from_bytes` to allocate an empty
179+
/// string rather than boxing null. At 120 call sites it is the obvious
180+
/// thing to reach for next; it is not admissible.
181+
#[test]
182+
fn allocating_helpers_are_not_cannot_collect() {
183+
for name in ["js_nanbox_string", "js_string_from_bytes", "js_array_alloc"] {
184+
assert_ne!(
185+
classify_direct_callee(name),
186+
GcCallEffect::CannotCollect,
187+
"{name} can allocate and must not be marked gc-leaf"
188+
);
189+
}
190+
}
191+
133192
#[test]
134193
fn audited_runtime_bookkeeping_cannot_collect() {
135194
for name in [

crates/perry-codegen/src/gc_map.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -960,6 +960,11 @@ pub fn compact_and_assemble(
960960
stats.records,
961961
stats.roots,
962962
);
963+
// The statepoint report's safepoint counts come from here and nowhere
964+
// else. Perry does not choose which calls become safepoints — RS4GC
965+
// does, inside LLVM — so this parse of the emitted assembly is the
966+
// only place the real numbers exist.
967+
crate::statepoint_report::note_gc_map(stats.functions, stats.records, stats.roots);
963968
}
964969

965970
assemble(clang, target, asm_path, obj_path)

0 commit comments

Comments
 (0)