Skip to content

perf(gc,codegen): collect dead lazy JSON arrays and stop re-classifying indexed reads - #10136

Closed
proggeramlug wants to merge 6 commits into
mainfrom
perf/json-lazy-movable-brand
Closed

proggeramlug wants to merge 6 commits into
mainfrom
perf/json-lazy-movable-brand

Conversation

@proggeramlug

@proggeramlug proggeramlug commented Sep 12, 2026

Copy link
Copy Markdown
Contributor

Closes #10098. Closes #10118.

Two independent costs on the same object, split into reviewable commits. Both were
measured on the JSON benchmark matrix; numbers below are from the quiet bench mini.

#10098 — a dead lazy JSON array was uncollectable by a minor

A lazy cluster was born into old-gen and pinned there, so a dead one held its whole
element graph live through the remembered set until a full collection. On
records_array_16k:scan that full collection never arrived: the arena rebaselined its own
trigger 134M->268M->536M->1073M while old_in_use climbed past 48 MB, and every minor
reported survival_permille=996, copied_objects=0, freed_bytes=0.

GC_TYPE_LAZY_ARRAY was pinned for two concrete reasons, both removed the way
GC_TYPE_REGEXP removed its own:

  • json_tape_store keys a tape by its owner's address, so a moved header orphaned it.
    Added json_tape_store::owner_moved + GcMoveHookKind::LazyArrayTape, mirroring
    GcMoveHookKind::RegExpSideTables.
  • the copying minor's flip runs no per-object finalize hook, so a header dying young
    leaked its tape. Added finalize_dead_copied_minor_from_space_lazy_tapes — the twin of
    the sweep-entry collect_owners pass — wired into the flip beside map/set/errors/regex
    and reported in the diag line.

The cluster's generation is still decided once, by cache size against the
pointer-bearing threshold, so #7546's rule that header, cache and bitmap share a generation
holds. Large clusters stay old exactly as before.

Dead-owner tape release for a nursery header is a separate concern — a dead nursery
header can carry a stale GC_FLAG_MARKED from an earlier cycle, so the full trace's
dead-owner predicate, which assumed old-gen residency, never reported it dead. That is
mark-bit semantics rather than movability, and it is its own commit here.

#10118 — indexed reads re-classified the receiver on every access

The indexed inline cache's brand check rejected GC_TYPE_LAZY_ARRAY outright, forcing
every read through four layers of re-classification. The brand test now decides on the
tag alone, with the kind and index guards moved into their own tav.get.kind_guard
block, and the shared pointer proof is computed once in the entry block instead of per
tier.

The IC slot is placed in arrlike.ic.header rather than the entry block: the entry
placement dominates all uses but is not array-only, and cost string_a:parse +4.98%.
arrlike.ic.header also dominates every use of key_cache and is array-only (+0.03%).

Results

row before after vs Node vs Bun
records_array_16k:scan peak RSS 205 MiB 51 MiB below below
records_array_1m:scan peak RSS 159 MiB 75 MiB below below
records_array_16k:scan CPU -10.1%
records_array_1m:scan CPU -5.5%
records_array_20m:repeat 0.93x

The access window showed zero separated regressions.

Validation

  • cargo test --release -p perry-runtime --lib: 3627 passed, 0 failed.
  • scripts/run_lint_gates.sh: 82 green, 1 red — benchmarks/ci_public_baseline_check.py,
    which fails identically on a clean origin/main tree (exit 2, same message). No
    harness path is touched by this branch.
  • Raw-handle ratchet back to baseline (944/944); the one new site in
    json_tape_owned.rs is a real scoped-read conversion, not an || () wrapper.

Note for the reviewer

This branch touches collector behaviour (gc_type_is_movable, a new move hook, a new
from-space finalizer). It is opened as a draft because the gc-ratchet corpus has not
been run before/after by someone with that setup — that should gate the merge, not the
runtime suite alone.

Summary by CodeRabbit

  • Performance
    • Improved memory usage and collection performance for lazy JSON arrays.
    • Small lazy JSON arrays can now be reclaimed and relocated during minor garbage collection.
    • Large lazy-array clusters continue to use long-lived storage.
    • Optimized indexed access checks to reach fallback handling faster for incompatible values.

Ralph Küpper added 5 commits September 12, 2026 15:45
The brand test is the first thing every indexed read on an unknown receiver
executes, and it computed the entire guard set before finding out the
receiver was not a typed array: the element kind load, its range test, both
index range checks and three ANDs. A JSON.parse array -- and any ordinary
Array behind an erased receiver -- is not a typed array, so it paid all of
that on every element read, forever, to reach a branch it was always going
to take.

Decide on the tag alone and leave. The kind and index guards only mean
anything once the tag says typed array, so they move behind it into
tav.get.kind_guard; the typed-array fast path reaches the same guard set by
the same AND-reduction and is unchanged.

Retired instructions per read, measured against the pre-#10114 compiler on
the JSON access fixtures: -2.4% to -3.5% on all twelve rows. That also
erases #10114's one disclosed cost -- the 20 MiB rows, which are ordinary
Arrays above the lazy admission bound, go from +0.7..+1.8% against that
reference to -0.4..-2.6%, i.e. below it.
A lazy cluster born old is never swept by a minor, so a DEAD one holds its
whole element graph live through the remembered set until a full collection.
On records_array_16k:scan that full collection never arrives -- the arena
rebaselines its own trigger 134M->268M->536M->1073M while old_in_use climbs
past 48 MB -- and every minor reports survival_permille=996,
copied_objects=0, freed_bytes=0. That is 205 MiB peak RSS against Node's
62 MiB.

GC_TYPE_LAZY_ARRAY was pinned for two reasons, both now removed the way
GC_TYPE_REGEXP removed its own:

  - json_tape_store keys a tape by its owner's address, so a moved header
    orphaned it. Added json_tape_store::owner_moved plus
    GcMoveHookKind::LazyArrayTape, mirroring GcMoveHookKind::RegExpSideTables.
  - the copying minor's flip runs no per-object finalize hook, so a header
    dying young leaked its tape. Added
    finalize_dead_copied_minor_from_space_lazy_tapes, the twin of the
    sweep-entry collect_owners pass, wired into the flip beside
    map/set/errors/regex and reported in the diag line.

With both present the type is movable and the cluster's generation is
decided by cache size -- decided ONCE, so #7546's rule that header, cache
and bitmap share a generation still holds. Large clusters stay old exactly
as before.

Four tests that asserted immovability were retargeted: the large-cluster one
still pins old-gen residency on size, and the handle tests now assert the
stronger property -- that the rooted handle resolves to wherever the
collector left the header, and that alloc_lazy_array returns the refreshed
address rather than the stale one. One test was itself holding a raw header
across a forced evacuation and faulting on the 0xDEADBEEFBAADF0DE poison
fill; it is rooted now.

Dead-owner tape release for a NURSERY header is not handled here -- a dead
nursery header can carry a stale GC_FLAG_MARKED from an earlier cycle, so
the full trace's dead-owner predicate, which assumed old-gen residency,
never reports it dead. That is mark-bit semantics rather than movability,
so it is the next commit rather than this one.
Completes the movability change. Two tape-release tests asserted that a full
mark-sweep reclaims a dead owner, which was true only while every owner was
old-gen: the old-gen sweep finalizes an unmarked payload directly. A
nursery-resident owner is reclaimed by a MINOR instead, through the new
finalize_dead_copied_minor_from_space_lazy_tapes, exactly as Map/Set/Error/
RegExp reclaim theirs. A full sweep leaves nursery mark bits to the minor, so
asserting on it was asserting against the wrong pass -- measured directly: six
consecutive full sweeps released nothing, and the first minor released exactly
the dead owner's bytes.

The same probe confirmed GcMoveHookKind::LazyArrayTape works: across that
minor the surviving owner relocated and the registry followed it to the new
address.

Both tests also held raw headers across collections, which only became visible
once headers could move. They re-read through the roots they already had --
the shadow slots, which the collector rewrites -- rather than keeping the
address owned_small returned. A RuntimeHandleScope is NOT a root in this file
unless register_runtime_handle_root_scanner_for_tests ran, which these two do
not call, so the shadow slot is the correct root to read back from.

3611 runtime tests pass, 0 fail.
arena_alloc_gc keeps two large-object lines apart, and its comment says why:
tenuring a POINTER-BEARING object does not cost its own bytes, it costs
"every object it can reach, held live through the remembered set by a
container nothing refers to any more". The sparse cache is a block of
JSValues, so it is exactly that container, and a lazy array is the case the
distinction was drawn for. Use the 128 KB line rather than the flat 16 KB
one -- V8's kMaxRegularHeapObjectSize, inside the copier's own ceilings, so a
cluster admitted by it is always movable.

Peak RSS on records_array_16k:scan, against main: 205 MiB -> 51 MiB, where
Node is 62 MiB and Bun 77 MiB.

test_json_tape_lazy_get_records_its_cache_store_as_an_external_edge needed a
bigger fixture to keep its premise: it wants a born-old cluster, which used
to be free because every lazy header was born old unconditionally. At 4096
elements its cache is 32 KB and would now be nursery-resident, so the test
would still pass its later assertions while exercising none of the
containment branch it exists for. 20 000 elements is ~156 KB, over the line.

3611 runtime tests pass, 0 fail.
@coderabbitai

coderabbitai Bot commented Sep 12, 2026

Copy link
Copy Markdown

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: 12f5258f-2367-4b55-8257-3094b67fbcda

📥 Commits

Reviewing files that changed from the base of the PR and between 50e08e9 and 718239b.

📒 Files selected for processing (10)
  • changelog.d/10098-json-lazy-array-movable-and-brand.md
  • crates/perry-codegen/src/expr/index_get/inline_dyn_typed_array.rs
  • crates/perry-runtime/src/gc/copying_phase.rs
  • crates/perry-runtime/src/gc/tests/alloc.rs
  • crates/perry-runtime/src/gc/tests/lazy_tape_side_alloc.rs
  • crates/perry-runtime/src/gc/tests/runtime_roots/callback_scanners.rs
  • crates/perry-runtime/src/gc/tests/runtime_roots/json_tape_owned.rs
  • crates/perry-runtime/src/gc/types.rs
  • crates/perry-runtime/src/json_tape.rs
  • crates/perry-runtime/src/json_tape_store.rs

Included review availability: Your plan provides up to 8 included reviews per hour; 5 remain after this review.


📝 Walkthrough

Walkthrough

Lazy JSON arrays now use size-based generation and movable headers. Tape registrations follow relocated headers, and dead from-space tapes are finalized during copied-minor collection. Typed-array indexed reads perform a tag-only brand check before other guards.

Changes

Lazy JSON array GC and indexed-read performance

Layer / File(s) Summary
Size-based lazy cluster generation
crates/perry-runtime/src/json_tape.rs, crates/perry-runtime/src/gc/tests/lazy_tape_side_alloc.rs
Lazy array clusters choose nursery or old-generation allocation from their cache size. Large clusters remain old-generation and tenured.
Movable headers and tape cleanup
crates/perry-runtime/src/gc/types.rs, crates/perry-runtime/src/json_tape_store.rs, crates/perry-runtime/src/gc/copying_phase.rs
Lazy headers are movable. A move hook rekeys tape ownership, and copied-minor finalization releases tapes for dead from-space owners while recording diagnostics.
GC metadata and runtime-root validation
crates/perry-runtime/src/gc/tests/alloc.rs, crates/perry-runtime/src/gc/tests/runtime_roots/*
Tests now verify header relocation, refreshed roots, preserved large-cluster generation, materialization behavior, and minor-collection tape release.
Staged typed-array indexed-read guards
crates/perry-codegen/src/expr/index_get/inline_dyn_typed_array.rs, changelog.d/10098-json-lazy-array-movable-and-brand.md
The brand block checks only the typed-array tag. The kind and index guards run in a separate block for tagged receivers.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to 71823

The lazy-array GC and indexed-read changes retain their required ownership, collection, and guard behavior, with no actionable merge risk identified.

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Linked Issues check ⚠️ Warning The PR satisfies the scoped GC changes for #10098: it adds owner-move tape rekeying, dead from-space tape finalization, movable lazy headers, and threshold-based cluster generation. However, #10098 re… Complete the #10098 unification by using GC_TYPE_ARRAY and the unmaterialized slot state, then remove the separate lazy cache/materialization state and update the required consumers and tests. For #10118, provide the observed-kind cache d…
Docstring Coverage ⚠️ Warning Docstring coverage is 72.73% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 22 functions across 9 files. (1 skipped: … Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely identifies both primary changes: lazy JSON array garbage-collection improvements and removal of repeated indexed-read classification.
Description check ✅ Passed The description is detailed and on topic. It explains the two changes, references the related issues, documents benchmark results, and provides validation results. It does not reproduce every template…
Out of Scope Changes check ✅ Passed The changes stay within the linked objectives. GC move hooks, tape finalization, generation selection, codegen guard changes, runtime tests, ratchet corrections, and benchmark documentation support #1
Full details: Linked Issues check

Explanation

The PR satisfies the scoped GC changes for #10098: it adds owner-move tape rekeying, dead from-space tape finalization, movable lazy headers, and threshold-based cluster generation. However, #10098 requires the lazy representation to become GC_TYPE_ARRAY with element-state laziness. The reviewed changes still use GC_TYPE_LAZY_ARRAY; json_tape_store cleanup explicitly filters that type. The separate lazy header, cache, and bitmap model therefore remains. For #10118, the evidence shows staged tag and kind/index guards, but it does not establish observed-kind cache dispatch, full-chain fallback for kind changes, or dedicated transition and emitted-size validation.

Resolution

Complete the #10098 unification by using GC_TYPE_ARRAY and the unmaterialized slot state, then remove the separate lazy cache/materialization state and update the required consumers and tests. For #10118, provide the observed-kind cache dispatch with unchanged fallback behavior and add coverage for lazy-array, ordinary-array, typed-array, and proxy transitions, including emitted-size checks.

Full details: Docstring Coverage

Explanation

Docstring coverage is 72.73% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 22 functions across 9 files. (1 skipped: 1 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 2
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🛠️ Fix failing CI checks 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch perf/json-lazy-movable-brand

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

`json_owned_tape_*`'s post-collection check dereferenced a raw
`get_raw_mut_ptr` across the collection it had just forced, which is one new
raw-handle debt site in a module with no ceiling (#7341) and fails the
per-module ratchet. Read `materialized` inside `with_mut_ptr` instead: the
header may have moved, and the scoped read is the protocol that says so.

This fix was made and verified on this branch before it was first pushed, then
lost to an uncommitted-tree reset while checking an unrelated gate, so the
pushed head still carried the raw site. Ratchet: 944 (baseline 944), exit 0.
@proggeramlug

Copy link
Copy Markdown
Contributor Author

Correction to the PR description. It says the raw-handle ratchet is back to baseline (944/944). That was true of my local tree and not of the head I pushed (628f7ce87b): the with_mut_ptr conversion in gc/tests/runtime_roots/json_tape_owned.rs was lost to an uncommitted-tree reset while I was checking an unrelated gate on clean main, so the pushed head still carried the raw get_raw_mut_ptr site and would have failed the per-module ratchet in lint.

Fixed in 718239be8d: raw_handle_debt.py → 944 (baseline 944), exit 0; json_tape_owned tests 11/11. Nothing else in the PR changed.

@proggeramlug

Copy link
Copy Markdown
Contributor Author

Validation status for taking this out of draft:

  • CI on the current head matches main's run on the same base: cargo-test fails the same single unrelated test (native_stack::tests::stack_top_respects_custom_thread_stack_sizes), and the gap suite's failing set is identical to main's 10 (empty set difference in both directions). The other red jobs (warnings, lint, check, gc-stress) are the ones red on main.
  • Mergeable against current main (9b911855f8) with no conflicts.
  • gc-ratchet corpus (14 probes × 7 repeats, plain archives) run on the combined branch json/parity-combined (this PR together with perf(gc,codegen): collect dead lazy JSON arrays and stop re-classifying indexed reads #10136, perf(gc): keep wide JSON document storage in the nursery (#10123) #10145, perf(gc): batch dead old-object page unregistration per sweep step #10147, perf(json): parse eagerly when this thread's lazy arrays keep being traversed #10150) against its base fd4bcbe647: every gated counter is bit-identical across all 14 probes (minor_cycles, step_cycles, copied_objects/bytes, promoted_objects/bytes, heap_used_bytes); peak RSS within ±0.6 %; all 14 correctness checks pass on both arms. Wall time is not gated in the shared_ci profile and was uniformly higher in the combined arm (1.06–2.36×, including on probes whose GC counters are identical) — the two arms ran ~25 minutes apart on a shared host whose load varied between 8 and 40 during the day, and I am reporting it rather than attributing it. check --profile shared_ci fails identically for the untouched base build (pre-existing drift of the pinned baseline: 01_nursery_churn heap_used +107 %, 02_survivor_promotion copied +8 %, 04_dead_after_deep_stack copied −26 %), so that red predates these PRs.

@proggeramlug

Copy link
Copy Markdown
Contributor Author

Landed via merge train #10188 (rebase-merged; main 5cec2fbbc9, tree identical to the train), cherry-picked onto 6874a9eb73 with the version bump to 0.5.1549. Validation and the CI attribution against main are in #10188.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant