perf(gc): move the JSON tape out of the old generation (#7539) - #7553
Merged
Conversation
added 5 commits
August 7, 2026 00:33
`LazyArrayHeader` carried its tape inline, so the whole allocation was as large as the tape — ~2.4 MB on the 10k-record `field_access` fixture. That is over `LARGE_OBJECT_THRESHOLD_BYTES` (16 KB), so `arena_alloc_gc` routed it into the old generation with `GC_FLAG_TENURED`, where only a FULL collection can reclaim it. Per-iteration-dead tapes therefore accumulated at ~2.4 MB per parse until `old_reclaim_pressure_due` fired. The tape is now a `json_tape_store` side allocation, owned by the header and released either deterministically at materialization or by the collector when the owner dies. Claude-Session: https://claude.ai/code/session_019EHcmXKArA7m42SihYCcgH
…bject #7539 shrank `LazyArrayHeader` to ~88 bytes, so it is born in the nursery. #7538/#7546's barrier test exists for the OLD-GEN owner shape — the only one where the in-object/external distinction bites — which production now reaches only by tenuring. `ForceOldGenLazyHeaderGuard` places it there directly, and the test probes cache slot 2048 so header and slot are on different pages by construction rather than by the header having been multi-megabyte. Claude-Session: https://claude.ai/code/session_019EHcmXKArA7m42SihYCcgH
`gc_note_external_side_alloc` feeds `external_side_live_bytes()`, which every `old_reclaim_pressure_due` call site ADDS to old-generation pressure. That is correct for a Map's entries buffer — its owner is typically tenured, so only a full reclaim can free it — and exactly wrong for a tape, whose owner is a nursery object and which materialization frees with no collector at all. Routing tape bytes there would have kept firing the `old_gen_bytes` fulls this change exists to stop, and the fix would have measured as a no-op. Tape bytes now have their own thread-local counter, cross-checked against the registry by the test accessor. Claude-Session: https://claude.ai/code/session_019EHcmXKArA7m42SihYCcgH
Shrinking `LazyArrayHeader` put it in the nursery, which made it MOVABLE for the first time — before #7539 its inline tape made it multi-megabyte, so `arena_alloc_gc`'s large-object arm always parked it in the old generation. Callers outside `json_tape` were written against that: `try_stringify_lazy_array` reads `blob_bytes` off a raw header and then allocates the result string. The copying minor relocated the header out from under them and `field_access` went non-deterministic, emitting a JSON string of NUL bytes for `JSON.stringify(parsed)` on 3 of 60 iterations. The header is now allocated old-gen and born tenured explicitly, so the invariant it always had is stated rather than implied — and the tape registry needs no move hook and no copied-minor from-space pass. `GC_TYPE_LAZY_ARRAY` is marked non-movable to keep old-page defrag from ever changing that; `true` was vacuous before this change anyway. The sparse cache and bitmap move to the same generation. An old-gen header with a nursery cache is a mix nothing covers: the minor treats the old header as a black leaf so it never visits the descriptor that can read the cache, while the cache block is a GC leaf whose contents no walker scans — which lost element identity across a copying minor. Tape bytes go back to `external_side_live_bytes()`. The earlier worry that this re-creates the pathology was wrong: the old cost was dead tape sitting in the old generation at ~2.4 MB per parse, and `field_access` now disowns each tape the moment `materialized` is installed, so the term never accumulates there. `roundtrip`, which genuinely retains its tape, keeps its existing bounded cadence. Claude-Session: https://claude.ai/code/session_019EHcmXKArA7m42SihYCcgH
…counting Pinned quiet host, 11 runs: field_access 1957 -> 1809 ms with sigma 143.8 -> 17.3 and RSS 196 -> 155 MB; roundtrip 201 -> 193 ms with peak old-gen in-use 39.6 -> 14.1 MB. The GC trace goes from 19 cycles / 9 full / 6 old_gen_bytes to 14 / 5 / 2 — the PERRY_JSON_TAPE=0 arm's profile to the cycle. `gc_note_external_side_alloc` moves into `note_allocated` so it is paired with `note_freed` at one site instead of split across two modules, and `live_bytes` becomes test-only. Claude-Session: https://claude.ai/code/session_019EHcmXKArA7m42SihYCcgH
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (4)
📝 WalkthroughWalkthroughLazy JSON-array tape data moves from inline GC-heap storage to registered external buffers. Lazy headers and caches use tenured old-generation allocation, while materialization, sweeping, and thread cleanup release tape ownership. ChangesJSON tape side allocation
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant JSONParser
participant alloc_lazy_array
participant json_tape_store
participant LazyArrayHeader
participant GC
JSONParser->>alloc_lazy_array: create lazy array
alloc_lazy_array->>json_tape_store: allocate and register tape
alloc_lazy_array->>LazyArrayHeader: allocate tenured header
JSONParser->>LazyArrayHeader: materialize array
LazyArrayHeader->>json_tape_store: release tape
GC->>json_tape_store: release dead-owner tapes
Possibly related issues
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 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/json_tape.rs`:
- Around line 1145-1150: Correct the allocation-lifetime statement in the
documentation for the compact header near alloc_lazy_header_bytes: state that
the header is allocated in the old generation with GC_FLAG_TENURED, while
preserving the explanation that json_tape_store owns the tape bytes and the
raw-header stability invariant.
🪄 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: c488bd0d-7cea-4c5e-b3ad-09aa2c7ed9c2
📒 Files selected for processing (15)
changelog.d/7539-json-tape-side-allocation.mdcrates/perry-runtime/src/arena/allocators.rscrates/perry-runtime/src/arena/mod.rscrates/perry-runtime/src/gc/mod.rscrates/perry-runtime/src/gc/oldgen.rscrates/perry-runtime/src/gc/tests/alloc.rscrates/perry-runtime/src/gc/tests/lazy_tape_side_alloc.rscrates/perry-runtime/src/gc/tests/mod.rscrates/perry-runtime/src/gc/tests/runtime_roots/callback_scanners.rscrates/perry-runtime/src/gc/tests/teardown.rscrates/perry-runtime/src/gc/types.rscrates/perry-runtime/src/json_tape.rscrates/perry-runtime/src/json_tape_store.rscrates/perry-runtime/src/json_tape_tests.rscrates/perry-runtime/src/lib.rs
Comment on lines
+1145
to
+1150
| /// #7539: the tape used to be copied INLINE after the header, making this one | ||
| /// allocation as large as the tape (~2.4 MB on a 10 k-record blob). That is | ||
| /// over `LARGE_OBJECT_THRESHOLD_BYTES`, so `arena_alloc_gc` routed it into the | ||
| /// old generation with `GC_FLAG_TENURED` and only a FULL collection could ever | ||
| /// reclaim it. The header is ~88 bytes now and is born in the nursery like any | ||
| /// other short-lived object; `json_tape_store` owns the tape bytes. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Correct the allocation-lifetime documentation.
Line 1149 states that the compact header is born in the nursery. alloc_lazy_header_bytes allocates it in the old generation with GC_FLAG_TENURED. This comment conflicts with the raw-header stability invariant that this function documents.
Proposed fix
-/// reclaim it. The header is ~88 bytes now and is born in the nursery like any
-/// other short-lived object; `json_tape_store` owns the tape bytes.
+/// reclaim it. The header is ~88 bytes now and is allocated in the old
+/// generation with `GC_FLAG_TENURED`; `json_tape_store` owns the tape bytes.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| /// #7539: the tape used to be copied INLINE after the header, making this one | |
| /// allocation as large as the tape (~2.4 MB on a 10 k-record blob). That is | |
| /// over `LARGE_OBJECT_THRESHOLD_BYTES`, so `arena_alloc_gc` routed it into the | |
| /// old generation with `GC_FLAG_TENURED` and only a FULL collection could ever | |
| /// reclaim it. The header is ~88 bytes now and is born in the nursery like any | |
| /// other short-lived object; `json_tape_store` owns the tape bytes. | |
| /// `#7539`: the tape used to be copied INLINE after the header, making this one | |
| /// allocation as large as the tape (~2.4 MB on a 10 k-record blob). That is | |
| /// over `LARGE_OBJECT_THRESHOLD_BYTES`, so `arena_alloc_gc` routed it into the | |
| /// old generation with `GC_FLAG_TENURED` and only a FULL collection could ever | |
| /// reclaim it. The header is ~88 bytes now and is allocated in the old | |
| /// generation with `GC_FLAG_TENURED`; `json_tape_store` owns the tape bytes. |
🤖 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/json_tape.rs` around lines 1145 - 1150, Correct the
allocation-lifetime statement in the documentation for the compact header near
alloc_lazy_header_bytes: state that the header is allocated in the old
generation with GC_FLAG_TENURED, while preserving the explanation that
json_tape_store owns the tape bytes and the raw-header stability invariant.
This was referenced Aug 7, 2026
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.
Closes #7539.
The mechanism, confirmed before it was fixed
The issue's mechanism was a hypothesis from a strong correlation. It is now measured.
A
LazyArrayHeaderwas allocated as ONE arena object with its tape copied inline after the header, so the whole allocation was as large as the tape — ~2.4 MB for the 10 000-record fixture. That is 150×LARGE_OBJECT_THRESHOLD_BYTES(16 KB), soarena_alloc_gc's large-object arm routed it straight into the OLD generation and stampedGC_FLAG_TENUREDon it. Old-generation bytes are reclaimable only by a FULL collection, so a tape that dies at the end of its loop iteration still accumulated at ~2.4 MB per parse untilold_reclaim_pressure_duefired.Being large is not evidence of being old. The header was born tenured on the strength of its size alone, which handed the collector's cheapest question — "did this die in the nursery?" — to its most expensive answer.
PERRY_GC_TRACE=1over the 53 parses ofbench_field_access.tsat the parent commit (cycle counts and byte totals are load-independent, so these were taken on the dev machine):old_gen_bytes-triggeredPERRY_JSON_TAPE=0+ gen-GCPERRY_GEN_GC=0The cleanest attribution is
bench.ts(roundtrip), which never materialises anything: its nursery peaks at 4.1 MB while the old generation peaks at 39.6 MB and fires 5old_gen_bytesfulls, identically under both collectors. In that program there is nothing in the old generation but the tape.That measurement also ruled out the reading the headline RSS numbers first suggested — that the workload was straddling the 192 MB
rss_pressureevacuation threshold.evacuation_policyreportsnot_evaluatedon every cycle of every arm and evacuation moved 0 bytes; old-page defrag is off by default (#6206). The fulls are the whole story.What changed
The tape moves out of the GC heap into a
json_tape_storeside allocation owned by the header. It qualifies on every test already applied toMap/Setentry buffers: pointer-free by construction (TapeEntryis{ offset: u32, kind: u8, link: u32 }; the struct's alignment is 4, so on a 64-bit target no field it has can hold a pointer, and the region has exactly one writer), uniquely owned by one header, immutable and immovable after construction.On top of the collector-driven lifetime, the owner disowns its tape deterministically: the instant
force_materialize_lazyinstallsmaterialized, every subsequent read goes through theArrayHeader, so the tape is freed right there with no collector involved. That is the pathfield_accesstakes after #7537's scan flip, which is why the result does not depend on GC timing for the workload that motivated it.The header stays exactly where it was — and that is load-bearing
The first version of this change let the shrunken ~88-byte header fall into the nursery, which made it movable for the first time in its life; its multi-megabyte inline tape had always parked it in the old generation. Callers outside
json_tapewere written against that —try_stringify_lazy_arrayreadsblob_bytesoff a raw header and then allocates the result string. The copying minor relocated the header out from under them andfield_accesswent non-deterministic:JSON.stringify(parsed)returned a JSON string of NUL bytes on 3 of 60 iterations while every element value stayed correct.So the header is now allocated old-gen and born tenured explicitly, stating the invariant rather than relying on the tape's size to imply it, and
GC_TYPE_LAZY_ARRAYis marked non-movable so old-page defrag can never change it. The sparse cache and bitmap join the same generation: an old-gen header with a nursery cache is a mix nothing covers (a minor treats the old header as a black leaf so it never visits the only descriptor that can read the cache, while the cache block is a GC leaf whose contents no walker scans), and it lost element identity across a copying minor.Result
Pinned quiet host (M1 mini,
taskpolicy -t 0 -l 0, 11 runs, load ≤ 2), same binaries end to end:field_accessPERRY_GEN_GC=0PERRY_GEN_GC=0PERRY_JSON_TAPE=0σ collapses 8.3× — the headline symptom — and RSS drops 41 MB. The decisive row is the last one: turning the tape ON is no longer worse than turning it OFF. The tape-off arm still carries σ 117.2 and 168 MB, so the residual variance and footprint are the generational collector's own behaviour on this workload and have nothing left to do with the tape. The GC trace agrees to the cycle:
field_accessgoes from 19 cycles / 9 full / 6old_gen_bytesto 14 / 5 / 2, which is exactly thePERRY_JSON_TAPE=0profile.roundtrip— the memcpy path this must not regress — improves: 201 → 193 ms (σ 0.5 → 0.7), and its peak old-generation in-use falls 39.6 → 14.1 MB (reserved 58 → 26 MB). It keeps its 5old_gen_bytesfulls by design: it genuinely retains each tape until the lazy array dies, so those bytes should keep escalating the reclaim that frees them.The change also helps the mark-sweep arm (1751 → 1604 ms), which is worth noting because it is independent of GC pacing: an inline multi-megabyte allocation per parse cost real time regardless of collector.
Not claimed. The issue's acceptance number was “~1760 ms at low σ, RSS → ~76 MB”. σ is met decisively and the median lands at 1809 ms, but ~200 ms and ~79 MB still separate the default from
PERRY_GEN_GC=0. The tape-off arm carries the same gap, so that remainder is a separate term, not this one.Validation
Everything below is local. CI is saturated repo-wide, so I am not claiming a green build.
cargo test -p perry-runtime --no-fail-fast— 1807 pass, 0 fail.gc/tests/lazy_tape_side_alloc.rs,gc/tests/teardown.rs,json_tape_tests.rs; each premise-asserts its own subject (that the tape really is over the threshold, that a COPYING minor really ran, that the header really did/didn't move).PERRY_GC_VERIFY_EVACUATION=1clean on both benchmarks and a 60-iteration divergence probe, including withPERRY_GC_FORCE_EVACUATE=1.test-files/*.tsthat build underPERRY_NO_AUTO_OPTIMIZEmatchnode --experimental-strip-types(26.5.1) byte for byte; both benchmark checksums are identical tomainacross repeated runs.raw_handle_debt.py(998, baseline 999 — debt fell, baseline deliberately left alone),gc_store_site_inventory.py,addr_class_inventory.py,check_file_size.sh,cargo fmt --all -- --check.Two existing tests asserted that a copied minor relocates the lazy header — true only because their tiny fixtures made the header small enough to be nursery-resident, which production never was. They now assert the opposite, which is the real invariant, and keep their live half: the materialized array is young, does move, and its handle must still be refreshed.
No version bump (maintainer bumps at merge).
Summary by CodeRabbit
Performance
Bug Fixes
Tests