Skip to content

Merge train: #9762, #9768–#9797 (11 PRs) + gate follow-ups - #9804

Merged
proggeramlug merged 22 commits into
mainfrom
train125
Sep 5, 2026
Merged

Merge train: #9762, #9768–#9797 (11 PRs) + gate follow-ups#9804
proggeramlug merged 22 commits into
mainfrom
train125

Conversation

@proggeramlug

Copy link
Copy Markdown
Contributor

Merge train: 11 PRs#9762, #9768, #9769, #9770, #9771, #9773, #9777, #9779, #9793, #9797 — plus gate follow-ups.

#9762 and a correction

#9762 was held out of train #9798 because it failed tdz_numeric_const_read_is_not_constant_folded, and I reported it as a regression. That was wrong. The emitted IR is call i64 @js_box_get_bits_named(i64 %r3, double %r4) — the TDZ check is intact. #9721 routes the read through a named helper so the thrown ReferenceError can identify the binding, which is the PR's purpose and strictly better. The test hard-codes the old spelling, so a better error message read as a lost guard. My A/B was sound but I inferred the cause from the assertion's wording instead of reading the IR printed directly beneath it. Corrected on the PR.

The test now asserts the property — the read goes through a box, either helper — and additionally that the value is not folded, which it never checked. Sabotage-verified: strip the box call and substitute the fold, it still fails.

The same shape, caught a second time

shape_descriptor_census required family_push_back after slab_mut().insert. #9768 adds family_append_fresh: the same append minus a membership scan that is dead work for an id alloc_shape_id just minted and never reuses. The ordering invariant holds — insert at line 59, append at 65 — only the spelling changed. The census now accepts either and still enforces that the by-id descriptor exists before the reverse accelerator points at it. Also sabotage-verified by inverting the order.

Other gate work

Validation

64/64 lint gates; release build; perry-codegen, perry-runtime, perry-stdlib, perry-hir, perry-transform — all green (RUST_TEST_THREADS=1).

Not in this train

#9755, #9756, #9774, #9775, #9776, #9780, #9794, #9795, #9796 conflict with train #9798 and are awaiting rebase. Several are independent rewrites of the same code (#9774 vs #9750 on gc_runtime_root_holders.py; #9796 vs the regex.rs split), which is where a mechanical merge goes quietly wrong — better resolved by their authors.

Ralph Küpper added 22 commits September 5, 2026 09:26
…mily membership scan

`shape_descriptor_ensure_with_holes` ends with `facts_push_back` +
`family_push_back`, and both answer "is this id already in the list?" with a
linear scan. A family holds every descriptor ever created for one keys array,
so interning the n-th descriptor for a keys array cost O(n) and a render loop
that keeps bumping a shape's semantic generation paid quadratic time in that
history. On the compiled claude-code TUI `IdList::contains` was 6.2 % of
main-thread leaf samples during a streamed reply and 5.9 % in the window after
it, 95 % of it under `ShapeTableInner::family_push_back`.

The scan is dead work at those sites: `alloc_shape_id` handed the id out two
statements earlier and never reuses a value (it parks at `SHAPE_ID_END` rather
than wrapping), so an id allocated after a list was built cannot be in it.

* `IdList::append_unchecked`, with the invariant that licenses it.
* `ShapeTableInner::family_append_fresh` / `facts_append_fresh` use it; the two
  sites that append a just-allocated id call those. The metadata rekey, which
  re-files EXISTING ids under a moved keys address, keeps `push_back`.

Test: interning_appends_each_new_descriptor_to_the_family_exactly_once.

Claude-Session: https://claude.ai/code/session_01YPfnmWZmSpSWpmnoXvH8z2
…ne per record

`make_segment_record` built its result property-by-property with `set_field`,
which allocates a fresh `StringHeader` for the key name on every call and
routes through `js_object_set_field_by_name`, which clones the object's key
list before writing. Every record therefore got 3-4 throwaway key strings, a
keys array cloned and regrown once per property, and — because
`shape_id_for_keys_ensure` keys the shape table on the keys array's ADDRESS —
its own ShapeId.

A fresh ShapeId per record makes every read of `.segment` / `.index` /
`.input` a guaranteed inline-cache miss (the PIC token is the ShapeId) and
adds one descriptor to the shape table per record. This is the defect #7564
fixed for `{ value, done }` iterator results; `Intl.Segmenter` was not
covered, and grapheme-aware text measurement (`string-width`, and so every
terminal UI built on ink) segments every string it renders. One 400-character
reply in the compiled claude-code TUI produces 175,797 segment records;
`PERRY_IC_DIAG` attributes 175,797 of that turn's 2,589,696 IC misses to the
`.segment` read site alone.

* `SEGMENT_RECORD_KEYS`: a per-thread `GC_FLAG_SHAPE_SHARED` keys array for
  each of the two record shapes (`isWordLike` is attached only for word
  granularity, ECMA-402 18.5.1), built at most twice per thread with interned
  names so they are pointer-identical to the ones the read side hashes.
* `make_segment_record` installs the shared array with `js_object_set_keys`
  and writes fields by index: one allocation instead of five to eight, and one
  ShapeId for every segment record in the program.
* `scan_segment_record_keys_roots_mut`, registered beside the iterator-result
  scanner: nothing else references these arrays, and an evacuating collection
  moves them like any other array.

Rooting follows `build_iter_result_ordered`: the caller's two heap values are
rooted first, the keys cache is filled before the record is allocated, and
every pointer used after an allocation is re-read from storage the collector
rewrites.

Tests: the five root-scanner tests `iter_result_keys.rs` holds, mirrored for
this cache (mark, rewrite, empty-cache no-op, registration, build-once).

Claude-Session: https://claude.ai/code/session_01YPfnmWZmSpSWpmnoXvH8z2
…ll sites

The GC census (`gc/census.rs`) accounts for the arena and the side tables.
On the compiled claude-code TUI those two explain ~115 MB of a 300 MB idle
footprint and ~430 MB of a 2 GB peak, and nothing in the runtime could say
where the rest came from — every dirty page in that process is mimalloc's,
and mimalloc only knows totals.

`PERRY_ALLOC_CENSUS=<path>` wraps the `#[global_allocator]` and reports:

  * exact totals and a power-of-two size-class histogram (allocated bytes and
    calls, freed bytes, live bytes, peak live bytes) — every allocation counts;
  * sampled call sites, one per `PERRY_ALLOC_CENSUS_INTERVAL` bytes allocated
    (default 1 MiB). A sample records raw return addresses via `backtrace(3)`
    — no symbolication, no allocation — plus the sampled pointer, so a later
    `dealloc` of that pointer subtracts it again. What remains at dump time is
    LIVE native memory attributed to a call site, not merely churn. Frames are
    symbolised offline with `atos -o <binary> -l <load_address>`, which the
    dump reports.

The switch is read with `getenv(3)` rather than `std::env::var`, so the very
first allocation of the process can decide: `std::env::var` allocates, and an
allocator that allocates to answer "am I recording?" recurses. Startup is
where the most interesting retention is, so waiting for `gc_init` would leave
the largest tables unattributed. A thread-local re-entrancy guard keeps the
sampler's own allocations out of the numbers, and a 1 MiB saturating-counter
presence filter keeps the un-sampled `dealloc` path at one relaxed byte load.

Behind the off-by-default `alloc-census` cargo feature, so a shipped build has
no wrapper at all: `gc_malloc` runs ~1M times/sec and even the disabled
state's relaxed load has no business on that path. The dump rides the existing
`SIGUSR2` heap census and is accompanied by `mi_stats_print`, which says how
much of the committed set is free-but-unpurged.

First result, one 400-character reply on the compiled TUI: 22.5 GB allocated
in 31.7 M calls, peak live 1.70 GB. The largest owners are the GC's own
side-table scanners rebuilding hash maps inside every copying minor
(`descriptor_state::scan_descriptor_roots_mut` 222 MB,
`shapes::scan_shape_table_rekey_mut` 75 MB,
`gc::verify::restore_surviving_dirty_coverage` 29 MB) and regex program
construction (~127 MB) — none of which the heap census could see.

Claude-Session: https://claude.ai/code/session_01YPfnmWZmSpSWpmnoXvH8z2
`codegen_env_vars_are_build_cache_inputs` scans `crates/perry-codegen/src`
for every `env::var("PERRY_…")` and requires each one to be either a
build-cache input or an explicit, justified exclusion. Since #9514 added the
per-site concat cache, `PERRY_CONCAT_SITE_CACHE` has been neither, so the
`cargo-test` job fails:

    test commands::compile::build_cache::tests::codegen_env_vars_are_build_cache_inputs ... FAILED
    panicked at crates/perry/src/commands/compile/build_cache.rs:410:9
    these codegen env vars key neither the build cache nor an exclusion
    (#6394's rule): ["PERRY_CONCAT_SITE_CACHE"]

This is the guard working, not a stale test. `concat_site_cache.rs:78` reads
the var as a build-time kill switch — its own module doc says
"`PERRY_CONCAT_SITE_CACHE=0` removes the lane at build time" — and
`crates/perry/tests/concat_site_cache.rs:243` compiles with the switch off
and asserts the emitted code differs. So it demonstrably changes generated
code, which makes it a cache *input*, not an exclusion: without this entry a
build with the switch flipped can be served a stale cached object from a
build with it in the other state.

Culprit: 0b68a25 perf(strings): per-site concat cache for "literal" +
proven-small value (#9514)

Claude-Session: https://claude.ai/code/session_01YPfnmWZmSpSWpmnoXvH8z2
…chievable (#9772)

Old-gen memory is released a BLOCK at a time
(`old_arena_reclaim_selected_dead_blocks`), but selection ranked individual
4 KB pages by fragmentation. The emptied pages were scattered across blocks
that kept other live occupants, so the reclaim freed none of them: on the
compiled claude-code TUI the pass chose 10,740 pages, predicted 44 MB of
"releasable block bytes", ran 228 ms and released nothing.

Selection now groups pages by their containing block
(`arena::old_arena_block_ranges` / `old_arena_block_range_index`), skips
blocks holding pinned bytes, ranks the rest cheapest-to-empty and takes whole
blocks until the move budget. Every selected block therefore ends the pass
with no live occupant, which is exactly what the reclaim tests, and
`selected_releasable_block_bytes` becomes the sum of real block sizes instead
of a sum of page granules nothing releases.

Two-arm run, same binary, one env var apart (`PERRY_GC_IDLE_COMPACT_BLOCKS`):

  block-targeted  selection predicted 52.4 MB -> released 46.6 MB,
                  kept_promise=true, 50 of 50 targeted blocks released
                  (`has_live=0`), old-gen in-use 120.4 MB -> 73.8 MB
  page-granular   selection predicted 44.4 MB -> released 0 MB,
                  kept_promise=false, no block reclaimed, 516 ms of pause

Two defects that only became visible once the pass could be judged against
its own prediction are fixed with it:

* A pass that DECLINES to evacuate no longer drops the excluded pages' holes
  first. Filtering them is justified only by "this pass is about to empty and
  release these blocks"; doing it before the all-or-nothing movability check
  meant one immovable occupant anywhere in the selection cost the whole
  old-gen residue and returned nothing — measured `reusable` 40.7 MB ->
  0.87 MB with `released=0` and `freed=296` bytes.
* `IDLE_COMPACT_MOVE_BUDGET_BYTES` 8 MiB -> 1 MiB. That constant is a PAUSE
  budget and was calibrated on the #9644 fixture's ~14 ms per MiB moved; a
  real old generation runs the same pass at ~195 ms per MiB (8.39 MB across
  50 blocks in 1.64 s), because its occupants are far smaller and far more
  numerous. 1 MiB keeps the pause at the 190-230 ms the pass already spent
  while returning nothing, so the change costs no additional pause budget.

Counters, so a barren pass names its own obstacle instead of being silent:
`[gc-old-block-reclaim] targeted/released/released_bytes/kept-by-reason`,
`predicted=` and `kept_promise=` on `[gc-idle-compact] done`, and
`broken_promises=` in the exit line. `[gc-general-reclaim]` does the same for
the general arena's empty-block release, which is what proved the eden
capacity in the census is working set in rotation rather than un-returned
memory (12-16 blocks released per cycle, `has_live` the only real obstacle).

Kill switch `PERRY_GC_IDLE_COMPACT_BLOCKS=0`.

Claude-Session: https://claude.ai/code/session_01YPfnmWZmSpSWpmnoXvH8z2
…surement

The 8 MiB -> 1 MiB change was justified as buying back pause at a measured
~195 ms per MiB moved. Three interleaved pairs on the compiled claude-code TUI
refute that: cutting the budget eightfold moved the selection from ~50 blocks
to ~15 and left the pause unchanged (1,070 ms mean against the page-granular
arm's 1,044 ms, 515-1,375 ms spread tracking machine load, not the arm).

So the pass is dominated by fixed per-pass cost — the old-page meta snapshot,
the walk over the selected blocks' pages, the sweep — and the budget bounds the
moved volume rather than the pause. The claim the measurement supports is the
one the table shows: ~15 MB of whole blocks returned per pass for the same
pause the barren pass already spent. Lowering that fixed cost is separate work.

Claude-Session: https://claude.ai/code/session_01YPfnmWZmSpSWpmnoXvH8z2
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant