-
-
Notifications
You must be signed in to change notification settings - Fork 161
perf(codegen): serve lazy JSON array reads from the indexed inline cache #10114
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Closed
Closed
Changes from all commits
Commits
Show all changes
10 commits
Select commit
Hold shift + click to select a range
6f29fb0
perf(codegen): read materialized lazy JSON arrays from the indexed cache
fd52211
perf(codegen): probe the lazy sparse element cache from the indexed c…
07d6d23
test(json): cover both indexed-cache tiers for lazy JSON arrays
28b8396
perf(codegen): route the lazy tier off the subclass probe's miss edge
be49b9e
chore(json): keep json_tape.rs under the file cap; rustfmt the lazy t…
69002c6
test(json): split the lazy defineProperty gap out of the cache fixture
b3bc4d7
perf(codegen): outline the lazy read proof into one runtime probe
62ef8db
test(json): cover the lazy index probe's decline contract
aeb4163
test(parity): ratchet the lazy defineProperty gap on both platforms
1801680
docs(changelog): record the lazy JSON array indexed-cache tier (#10114)
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| **perf(codegen): serve lazy JSON array reads from the indexed inline cache.** A `JSON.parse` result carries `GC_TYPE_LAZY_ARRAY`, and the indexed inline cache's brand check only admitted `GC_TYPE_ARRAY` — so every `parsed[i]` fell through to `arrlike.ic.miss` and re-classified the same receiver three more times (`js_packed_arraylike_index_get` → `js_array_get_f64` → `json_tape::cached_read::lazy_get`), about **227 retired instructions for `rows[7].id`**. #10050 and #10064 made that last helper allocation-free; nothing had touched the dispatcher chain in front of it, which is why post-parse reads were the worst rows in the JSON matrix. | ||
|
|
||
| The cache now proves a live `GC_TYPE_LAZY_ARRAY` from the GC header and makes one call to `js_lazy_array_index_probe` — `lazy_get`'s two non-allocating branches and nothing else, covering both the sparse per-element cache (an array whose adaptive walk never trips the materialization flip stays tape-backed for the life of the program: the 16 KiB fixture is 120 records) and an installed ordinary array. `TAG_HOLE` is the declined signal, unambiguous because a hole is never a value a read yields and holes already route to the miss helper; cold elements, descriptors, out-of-bounds, growth-forwarding stubs and a stale `cached_length` mirror all take that exit. `lazy_get` refreshes that mirror when it serves a read and a probe cannot write, so it instead requires the mirror to agree and declines otherwise — a grown or shrunk array can never report a stale `.length` through a fast-path read. The probe is classified `CannotCollect` and omits `lazy_get`'s rooted fallback, so the caller needs no extra rooting. | ||
|
|
||
| Quiet M1/8 GiB host, Node 26.5.1, Bun 1.3.14, 7 interleaved repetitions, measured against `main` at `e8f912392`: 1 MiB repeat **−41.7%**, fields **−41.6%**, sequential **−33.4%**, random **−31.9%**; 16 KiB repeat **−41.8%**, fields **−39.8%**, random **−35.4%**, sequential **−22.8%**. `records_array_1m:random` goes from 2.11× Node to **1.47×**. Retired instructions per read drop 37.7–48.9% on those rows. Peak RSS unchanged. | ||
|
|
||
| **Disclosed costs.** The 50-row screen shows nine separated regressions of +0.5–1.2% (`numbers_1m:parse +2.41%`), six of which reproduce across two independent windows, on `parse`/`sparse`/`roundtrip` rows. These come from the benchmark worker's structure rather than from parsing: `worker.ts` runs all five operations inside one `run()`, so its parse loop shares code layout and register allocation with the `scan`/`sparse` loops that do contain indexed reads. A worker whose `run()` has no indexed read at all compiles to a **byte-identical object file on both arms**, so the cost cannot reach such a function. Programs that parse and index in the same hot function will see it; programs that do not, cannot. On the access screen, 20 MiB rows (above the lazy admission bound, so ordinary Arrays that never enter the new path) show `repeat +2.97%` and `fields −2.83%`; before #10074 landed the same pair read +0.08% and +4.94%, so that cost relocates between rows when unrelated runtime changes land and is microarchitectural sensitivity on ~0.026 µs rows, not a property of this change. | ||
|
|
||
| **Why one call rather than inlining the proof.** The inline form was built and measured first, then rejected: `run()` grew 10752 → 11804 bytes and the 50-row screen showed `string_a:parse +5.08%` and `null:parse +3.14%` — rows with no array in them. Outlining cuts the growth to **+56 bytes** and those rows to +0.01% and −0.07%, retaining essentially all of the instruction reduction. | ||
|
|
||
| **Validation.** 234 rows — 13 lazy-array fixtures × native/shadow roots × auto/tape/direct parsers × normal/scheduled/full-GC — byte-identical to the reference on both arms, with a real moving-GC witness on every one of the 78 scheduled rows. `test_gap_json_lazy_indexed_cache.ts` covers identity, growth, shrink, holes, a prototype override and its retirement, and cached zero (all-zero NaN-boxed bits, so the bitmap rather than the element word must prove a slot live); two Rust unit tests pin the probe's decline contract. `Object.defineProperty` on a lazy index is a pre-existing gap failing identically on both arms, split into its own reproducer and ratcheted against #10097. The architectural follow-up that would delete this tier entirely is #10098. |
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
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
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
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
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
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
Oops, something went wrong.
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Use the logical array element base.
shift_denseadvances the dense array’s front offset without moving surviving elements. After materialization and a shift, a rooted lazy read can refreshcached_length, so the probe remains eligible. The probe then reads afterArrayHeader, and index0can return the removed physical slot instead of the current logical element.Use
crate::array::array_elements_ptr(cached)and add a regression test for materialization, shift, length-mirror refresh, and index-zero probing.Proposed fix
📝 Committable suggestion
🤖 Prompt for AI Agents