merge train: land #10114, #10117, #10119, and #10120 - #10122
Merged
Conversation
added 19 commits
September 12, 2026 10:49
…ic arrays (#10092) The generic per-element loop routed every candidate through js_jsvalue_equals/js_jsvalue_same_value_zero — both #[no_mangle] extern "C" call boundaries the optimizer cannot inline, re-deriving the element's type on every slot. On a proven-numeric dense array (RawF64 layout: no holes, no NaN-boxed pointers) this collapses to a bounded f64 compare loop, hoisting includes's NaN-equals-NaN check out of the loop. A/B on this host: ~18x faster at n=1M, ~12x at n=100k, ~9x at n=1k, checksums identical. Falls back to the existing generic walk for exotic iteration (index accessors/sparse storage/prototype indices) and mixed-kind arrays. Claude-Session: https://claude.ai/code/session_01SAYSYr7R7CwCF3EuAjSdWu (cherry picked from commit eb2b2ab)
Claude-Session: https://claude.ai/code/session_01SAYSYr7R7CwCF3EuAjSdWu (cherry picked from commit dd360ff)
case_convert ran every input, including pure ASCII, through a scalar wtf8_step decode / per-char to_lowercase()/to_uppercase() iterator / re-encode loop, costing 30-33x Node on a 1M-char all-ASCII string. Gate on a real per-byte bytes.is_ascii() scan (not the is_ascii_string byte_len==utf16_len aggregate proxy, which can lie for malformed WTF-8) and use to_ascii_lowercase()/to_ascii_uppercase() for a vectorizable byte-table transform instead. Non-ASCII input, locale-aware casing, and WTF-8/lone-surrogate handling are untouched. Claude-Session: https://claude.ai/code/session_013naeTjgijAXt8PwpQEKkbu (cherry picked from commit fa80c4a)
Claude-Session: https://claude.ai/code/session_013naeTjgijAXt8PwpQEKkbu (cherry picked from commit aa1e9e1)
js_function_bind built the "bound " + target-name string, allocated a runtime string for it, and inserted two set_builtin_property_attrs records for .name/.length on every call, even when neither property is ever read. The attrs calls were redundant: a closure with no dynamic-prop table entry for name/length already defaults correctly (non-enumerable, non-writable, configurable) at every site that observes them. The name string is now synthesized and cached lazily, on first actual .name read, through bound_function_lazy_name - wired into the general closure property-get path, Object.getOwnPropertyDescriptor, and console.log's function formatter, so the value is correct regardless of whether bind itself ever computed it. Get(Target, "name") still runs synchronously at bind time (only the raw value, no string building) so a throwing name getter on the target still fails bind() itself, matching spec and Test262's bind/instance-name-error.js. Also roots the bind target, bound this, the name snapshot, the partial-args array, and the bound closure through a RuntimeHandleScope across every allocating call in js_function_bind, closing a latent staleness gap across the this-boxing/getter/array/closure-alloc calls. Refs #10084. Claude-Session: https://claude.ai/code/session_011B1Jqq3tKredbFkx4t7yaN (cherry picked from commit b0ae000)
Claude-Session: https://claude.ai/code/session_011B1Jqq3tKredbFkx4t7yaN (cherry picked from commit d24d7a9)
A JSON.parse result carries GC_TYPE_LAZY_ARRAY, so the indexed inline cache's brand check (obj_type == GC_TYPE_ARRAY) rejected it and every element read fell through to arrlike.ic.miss, re-classifying the same receiver three more times: js_packed_arraylike_index_get, then js_array_get_f64, then json_tape::cached_read::lazy_get. R22-R26 made that last helper allocation-free, but the call chain in front of it was untouched, so `rows[7].id` on a parsed array still cost ~237 retired instructions against Node's handful of cycles. Serve the read in the cache instead, once a scan or the random-access flip has installed the ordinary array, on exactly the proof lazy_get already takes: live unforwarded GC_TYPE_ARRAY, no descriptor overrides, length within capacity and its plausibility bound, dense in-bounds index. Prototype invalidation is also honoured, which the ordinary tier checks and lazy_get does not, so the admitted set is no wider. lazy_get refreshes the header's cached_length mirror when it takes this path. A cache cannot write, so it requires the mirror to already agree and routes a disagreement to the miss helper, which refreshes it and lets the next read hit. A grown or shrunk array therefore never reports a stale length through a fast-path read. Holes, sparse tape-backed reads, growth-forwarding stubs and every exotic receiver keep the unchanged dispatcher. (cherry picked from commit 6f29fb0)
…ache The materialized tier only fires once a scan or the adaptive random-access flip has installed an ordinary array. An array small enough that the walk never trips that flip — 120 records in the benchmark's 16 KiB fixture, and any repeated or clustered read pattern — stays tape-backed for the life of the program and kept missing the cache entirely, which is why the 16 KiB field walk was the worst row in the matrix at 10.6x Node. Inline lazy_get's sparse branch for that case: bounds against the header's cached_length, non-null bitmap and element words, the bitmap bit, then the parallel element slot. The bitmap is the liveness test because JSValue::ZERO is a legal cached value, so a zero element word cannot serve as one. Out-of-bounds deliberately does not shortcut to undefined the way lazy_get does — the prototype chain stays the miss helper's job. Cold reads, holes, uncached slots and prototype invalidation all keep the unchanged dispatcher. (cherry picked from commit fd52211)
Exercises what the two new cache tiers are allowed to answer and what they must hand back to the dispatcher: identity and value across repeated reads, growth and shrink against the header's length mirror, holes, an accessor descriptor installed on one index, a prototype index override and its retirement, and cached zero — whose NaN-boxed bits are all zero, so the bitmap rather than the element word has to prove a sparse slot live. The sparse half deliberately never scans its array, since a scan would trip the materialization flip and move it onto the other tier. (cherry picked from commit 07d6d23)
Keep brand's successors exactly as they were and reach the lazy tier from arrlike.elem.kind's miss edge instead, after both the ordinary-Array and elements-subclass probes have declined the receiver. The admitted set and every guard are unchanged; only the position in the chain moves. This is hygiene, not a fix. The 20 MiB access rows regress ~4.8% on field walks either way, and the first placement was not the cause: profiling both arms on that row shows js_packed_arraylike_index_get absent entirely. A 20 MiB document is above the lazy admission bound, so it parses to an ordinary Array whose reads hit arrlike.ic.array_guard inline and never reach the miss chain where these blocks live -- they are present, not executed. The cost is code layout: run() grows 10752 to 11804 bytes, and that row's per-iteration work is ~0.027us and front-end bound (72-74% of samples inside run, 25% in fmod from the workload's own i % length). Retired instructions move +0.85% while CPU moves +4.8%, which is the signature of instruction fetch and prediction rather than executed work. (cherry picked from commit 28b8396)
…iers The three new LazyArrayHeader offset pins pushed json_tape.rs to 2004 lines, over scripts/check_file_size.sh's 2000-line cap. Move all four layout contracts (the existing cached_length pin included) into json_tape/layout.rs, which is only their enforcement; the field doc comments keep saying why each word is load-bearing. json_tape.rs lands at 1968 lines, below where main has it. Also rustfmt's rewrap of the new cond_br calls in the indexed cache. (cherry picked from commit be49b9e)
The indexed-cache fixture asserted that an accessor descriptor installed on an index takes reads off the fast path. main cannot do that at all: in both the sparse and the materialized state the descriptor is installed and then ignored, and only PERRY_JSON_TAPE=0 matches Node. The assertion therefore failed identically on both arms and told us nothing about the new tiers. Move it to its own reproducer, covering both lazy states, and record it in the gap snapshot against #10097. The cache fixture keeps every assertion the tiers are actually responsible for. (cherry picked from commit 69002c6)
Inlining the whole lazy proof at every indexed read site made emitted code measurably worse on rows it never executes on. The 50-row screen against the pre-change compiler showed six separated regressions, worst string_a:parse +5.08% and null:parse +3.14% -- rows with no array in them at all -- and the access screen showed 20 MiB field walks +4.78%, on a receiver that is an ordinary Array and never enters these blocks. run() in the JSON worker grew 10752 to 11804 bytes; retired instructions moved +0.85% while CPU moved +4.8%, the signature of instruction fetch and prediction rather than executed work. Replace both tiers with a single call to js_lazy_array_index_probe, which is lazy_get's two non-allocating branches and nothing else. Emitted code per read site drops from ~87 instructions to a tag test, a call and a result test. The dispatcher chain (js_packed_arraylike_index_get -> js_array_get_f64 -> lazy_get) is still skipped; only the proof moves out of line. 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 stubs and a stale length mirror all come back as that. The probe is classified CannotCollect: it reads headers, bitmap and slots, never allocates a managed value, never enters user code, and deliberately omits lazy_get's rooted fallback. The three pointer-word offset pins go away with the inline form, since the cache no longer emits those offsets; only the pre-existing cached_length contract stays pinned. (cherry picked from commit b3bc4d7)
The probe is the indexed cache's whole lazy fast path, so what it DECLINES matters as much as what it serves: every decline is a read the emitted code must hand to its rooted miss helper, and a decline that wrongly became a value would be a silently wrong read. Covers an uncached index, a warmed sparse hit and its still-cold neighbour, cached zero (whose NaN-boxed bits are all zero, so the bitmap rather than the element word has to prove liveness), out-of-bounds -- which must not shortcut to undefined, since the prototype chain is the caller's job -- indices outside the u32 domain, a null receiver, and a materialized read whose length mirror has gone stale, which must decline until the rooted accessor refreshes it. (cherry picked from commit 62ef8db)
(cherry picked from commit 1801680)
…g stores #10119's rewrite left the bound closure's address bound once, immediately after rooting it, and then used it again ~60 lines later — past `closure_get_own_dynamic_prop` and `closure_length`, either of which can allocate and therefore move it. The raw-handle ratchet refused the two new sites (`closure/dispatch/bound.rs` is a module with no ceiling, so it is locked at zero, and `--no-raise-vs <merge-base>` will not accept a new ceiling on it). This is the hazard the ratchet exists for, not a style question, so the sites are converted rather than recorded: - the four capture stores, none of which allocates, now take the closure and the partial-args array through `with_mut_ptr` for exactly that block; - the `.length` publication and the final return re-read the rooted slot, since the dynamic-property lookups between them can allocate. Debt returns to the recorded 944 with every module inside its ceiling (`--no-raise-vs origin/main`: "none raised").
#10114 routes the lazy-JSON-array tier off the elements-subclass probe's miss edge, which moves that guard's false target from `arrlike.ic.miss` to `arrlike.lazy.kind`. `any_typed_dynamic_key_takes_the_numeric_tiers_when_it_is_an_array_index` asserted the OLD block adjacency, so it failed on the train: only GC_TYPE_OBJECT may reach the ObjectMeta.elements load: %r713 = icmp eq i8 %r705, 2 br i1 %r713, label %arrlike.elem.meta.159, label %arrlike.lazy.kind.164 The safety property the test exists for is intact — the guard is still `icmp eq i8 …, 2` and its true edge is still `arrlike.elem.meta`, so no non-object reaches the ObjectMeta.elements load. What changed is that a non-object now takes one more type test before leaving: `arrlike.lazy.kind` checks `icmp eq i8 …, 9` (GC_TYPE_LAZY_ARRAY) and sends everything else to `arrlike.ic.miss`, which is the same complete dispatcher as before. A declining probe returns TAG_HOLE to that same exit. Native Buffers and other exotic managed cells therefore still leave through the dispatcher. So the fix is to assert the property rather than the adjacency, and to pin BOTH hops — which is strictly stronger than what it replaced, because the lazy tier's own kind guard is now covered too. Sabotage-checked rather than assumed: with the lazy guard's false edge rewired to `lazy_call_label` (so an exotic cell would fall into the probe), the new assertion fails with "only GC_TYPE_LAZY_ARRAY may reach the lazy probe; everything else must still exit through the complete dispatcher". Restored, 12 of 12 pass.
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (28)
📝 WalkthroughWalkthroughThe change adds performance paths for lazy JSON array indexing, numeric array search, ASCII case conversion, and bound-function metadata. It also adds runtime contracts, regression tests, changelog entries, parity records, and a version bump. ChangesLazy JSON array indexing
Bound-function lazy metadata
Numeric array search
ASCII case conversion
Estimated code review effort: 4 (Complex) | ~45 minutes Change: Other Sequence Diagram(s)sequenceDiagram
participant IndexedReadIC
participant js_lazy_array_index_probe
participant LazyArrayCache
IndexedReadIC->>js_lazy_array_index_probe: receiver and numeric index
js_lazy_array_index_probe->>LazyArrayCache: inspect sparse bitmap or materialized array
LazyArrayCache-->>js_lazy_array_index_probe: cached element or TAG_HOLE
js_lazy_array_index_probe-->>IndexedReadIC: element value or miss signal
sequenceDiagram
participant js_function_bind
participant BoundClosure
participant bound_function_lazy_name
participant format_function_for_console
js_function_bind->>BoundClosure: capture target name and bound arguments
BoundClosure->>bound_function_lazy_name: request synthesized name on first read
bound_function_lazy_name-->>BoundClosure: cache bound name
format_function_for_console->>bound_function_lazy_name: resolve name when no stored name exists
bound_function_lazy_name-->>format_function_for_console: synthesized display name
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
This was referenced Sep 12, 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
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.
Merge train landing #10114, #10117, #10119 and #10120 on top of
main323e83ac6b(train164).The individual PRs merge as their own branch, so GitHub's close keywords do not
fire — each original is closed with a pointer comment after this lands, and the
issues it names are swept by hand.
eb2b2ab41d,dd360ff9e3fa80c4ad6f,aa1e9e16d5b0ae0004e4,d24d7a909d6f29fb0d03..1801680434No stacks; no shared commits between any open pair; nothing already upstream; no
cherry-pick conflicts.
#10114 marks a test expected-to-fail — verified against pristine
main#10114 adds
test_gap_json_lazy_defineproperty_indexto bothgap_snapshot.jsonandknown_failures.json, claiming the failure ismain'sgap (#10097) rather than the change's. Recording a test as expected-to-fail is
not something to take on trust, so I built a pristine
mainbinary at323e83ac6bbefore applying any picks and ran the fixture against it:Both of the PR's claims hold: the gap predates it, and the direct-parse route
matches Node byte-for-byte. #10097 is open and was filed earlier the same day.
After the train, the fixture still fails in exactly the recorded way and no
other —
descriptor read bypassed (scan=false)— so the baseline entriesdescribe the real behaviour.
Maintainer fix commits
fix(runtime): scope bind's bound-closure pointer to its non-allocating stores— this is a rooting hazard, not a ratchet formality. perf(runtime): make Function.prototype.bind's name/length metadata lazy #10119 bound the new
closure's raw address once, immediately after rooting it, then used it again
~60 lines later, past
closure_get_own_dynamic_propandclosure_length,either of which can allocate and move it. The raw-handle ratchet refused the
two sites (
closure/dispatch/bound.rshas no ceiling, so it is locked atzero). The four capture stores now take both addresses through
with_mut_ptrfor that block alone, and the
.lengthpublication and the final returnre-read the rooted slot. Debt back to the recorded 944;
--no-raise-vs origin/mainreports "none raised".test(codegen): pin both hops of the indexed-read kind guard— perf(codegen): serve lazy JSON array reads from the indexed inline cache #10114routes the lazy tier off the elements-subclass probe's miss edge, which moves
that guard's false target from
arrlike.ic.misstoarrlike.lazy.kind.any_typed_dynamic_key_takes_the_numeric_tiers_when_it_is_an_array_indexasserted the old block adjacency and failed on the train. The safety property
is intact: the guard is still
icmp eq i8 …, 2with its true edge toarrlike.elem.meta, and a non-object now takes one extra type test(
icmp eq i8 …, 9, GC_TYPE_LAZY_ARRAY) before leaving through the samearrlike.ic.missdispatcher — so Buffers and other exotic managed cells stillexit before any ObjectMeta load. The assertion now pins both hops, which is
strictly stronger than what it replaced. Sabotage-checked: rewiring the
lazy guard's false edge so an exotic cell would fall into the probe makes the
new assertion fail with its own message; restored, 12 of 12 pass.
chore: bump workspace version to 0.5.1539— none of the four PRs bumpedit;
Cargo.lockregenerated with it.Validation
6401 tests, zero failures.
perry-codegenis the rerun after the assertion fix;the first run failed only that one test. Exit codes are captured from each
command itself, not from a pipeline. The single lint failure is public benchmark
evidence freshness, genuinely red on
mainsince 2026-07-29.Gap tests, against the pinned Node 26.5.1, with the static archives rebuilt
first and their mtimes confirmed newer than the last commit:
Validated at head
49d6b8827a9e482fb9de01f30d7de008c44429dc.Summary by CodeRabbit
Performance
Array.prototype.indexOfandincludesperformance for dense numeric arrays.toLowerCase()andtoUpperCase()for ASCII text.Bug Fixes
JSON.parse.Release