Merge train 183r: #10218 - #10221
Merged
Merged
Merge train 183r: #10218#10221
Conversation
added 4 commits
September 13, 2026 19:19
The guarded element read for a dynamically-typed receiver emitted ~50 basic
blocks and ~343 pre-RS4GC instructions per `a[i]`: eight typed-array
element-kind arms behind a seven-block kind dispatch, the whole shape-carried
Array-subclass IC tower (identity, dense-tail family token, spilled `length`,
spilled elements), the elements-backed subclass probe, the lazy-JSON-array
probe, four runtime calls and six `js_number_coerce` arms. On
`prettier/plugins/flow.mjs` that tower is 55% of all emitted IR across 10,778
sites, for a program that neither constructs a typed array nor subclasses
`Array`.
The site now keeps four guarded arms and one out-of-line call, 20 blocks and
173 instructions:
* the receiver tag / heap-band and canonical-index checks plus the managed
`GcHeader` load, and the packed ordinary `GC_TYPE_ARRAY` arm — byte
identical to before;
* the typed-array arm, collapsed onto the four ELEMENT WIDTHS the header
already stores instead of the nine element kinds. `tav.w4` resolves
`Int32Array`/`Uint32Array`/`Float32Array` from ONE load with two `select`s;
* the elements-backed Array-subclass probe (`ObjectMeta.elements`) — byte
identical;
* `js_packed_arraylike_index_get`, the same call the old `arrlike.ic.miss`
block made, as the single exit for everything else.
Every removed arm was an acceleration of a decision that helper already makes,
and it is still handed the site's own cache slot, so neither the answer nor the
primed cache words move. The one arm it did NOT already make, #10114's
lazy-JSON-array probe, moved into the helper — a `JSON.parse` result still
skips the `js_array_get_f64` -> `lazy_get` chain without every read site
paying three blocks for the proof.
The shape-carried IC tower (15 of the 50 blocks) is removable because it cannot
hit in the shipped configuration: its hit needs a primed layout cache, and
`build_dense_layout` is reached only when `elements_of(obj)` is null, which the
default elements store makes false for every Array subclass.
Measured on the OpenCode corpus (5 interleaved rounds, quiet host):
prettier-flow `.text` -12.67% (38,113,543 -> 33,284,398), babel-parser -0.68%,
babel-types-validators unchanged; `.perry_gcmap` within +-0.14%; O0-fallback
units unchanged. No workload regressed: every shared row is within -0.43% ..
+0.07% retired instructions and -1.1% .. +0.2% peak RSS, while a dynamically
typed `Float64Array` read loop is -9.98% instructions / -40.6% walltime, a
plain `number[]` -40.03% / -72.0%, an `Array`-subclass -12.10% / -22.9% and a
`Uint8Array` -4.79% / -4.8%.
(cherry picked from commit 3d79647)
(cherry picked from commit ce3708d)
|
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 (13)
📝 WalkthroughWalkthroughDynamic indexed reads now use a smaller inline control-flow shape. Ordinary arrays, width-based typed arrays, and elements-backed subclasses remain inline. Other cases use one runtime exit, which adds lazy-JSON probing. Code-generation and runtime tests validate the new behavior. ChangesDynamic index read optimization
Estimated code review effort: 4 (Complex) | ~45 minutes Change: Refactor Sequence Diagram(s)sequenceDiagram
participant DynamicRead
participant InlineCodegen
participant ArraylikeExit
participant LazyJsonProbe
DynamicRead->>InlineCodegen: lower obj[i]
InlineCodegen->>ArraylikeExit: call on guarded miss
ArraylikeExit->>LazyJsonProbe: probe non-forwarded lazy array
LazyJsonProbe-->>ArraylikeExit: value or TAG_HOLE
ArraylikeExit-->>DynamicRead: return indexed value
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 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 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.
Merge train 183r: lands #10218 (one inline hit and one exit for the dynamic
obj[i]read) atce3708d7ca, plus one census commit and the workspace version bump to 0.5.1560.The two PR commits were cherry-picked onto
5d3bf86823without conflicts.Added commit
lint(census): drop the dynamic index read's retired header-size callsite (#10218): #10218 moved the shape-carried subclass IC tower out ofinline_dyn_typed_array.rs, deleting that file's oneobject_header_size_bytescall.scripts/shape_descriptor_census_baseline.jsonstill expected it, both in the exact callsite multiset and in thecodegen_object_header_size_sitessummary (43 → 42). That was #10218's only CI difference from main. The replacement read lives injs_packed_arraylike_index_get, which is compiled per target and so takes the target's header size.Validation (macOS arm64)
Run on the train minus the census commit, a JSON-only difference:
scripts/run_lint_gates.sh: 79 of 83 pass. The only failure beyond main's three was the census step fixed above, which now passes (python3 scripts/shape_descriptor_census.py: rc 0).raw_handle_debt.py(both invocations),gc_runtime_root_holders.py, fmt, file size: pass.check_thread_locals.py: only main'sregex/perex_owner.rs.cargo test --release: perry-codegen 2029 passed / 0 failed (all targets). perry-runtime 3775 passed / 1 failed; the failure isgc::tests::heap_generation::a_free_or_move_outside_every_scope_is_caught_in_debug_builds, which is already on main from perf(regex): resume JS-level searches on non-ASCII strings from the previous call's position #10205 (noted there).scripts/run_gap_tests.sh, 779 tests): 768 pass, 10 parity fail, 1 crash. The gated set is identical to train 181b's full run: main's known four plus9592_child_timeout_threads, where the Node oracle fails on this host.extends Arraysubclass, an array-like object, and 300 rounds of lazy JSON arrays read by index under allocation churn. Output is byte-identical to Node plainly, and underPERRY_GC_SCHEDULE_SEED5, 183 and 10218 at rate 0.3 with forced evacuation, verification and from-space protection (555–564 copying minors), which exercises theGC_FLAG_FORWARDEDguard perf(codegen): one inline hit and one exit for the dynamic obj[i] read #10218 added for movable lazy arrays.ALLOC_KB=0, forced evacuation, verification, from-space protection) onjson_lazy_indexed_cache,json_lazy_growth_alias,json_record_loop_cloneandjson_owned_tape: all match Node.CI attribution (#10218 run 34771184597 vs main's run at
b5a82cfeae)The same failing check/warnings/cargo-test steps and errors, the same
native_stackfailure, the same four gated gap regressions with identical output, and the same gc-stress failure. lint differs only by the census step fixed here.Review notes
selects (shl/ashrfor signed,bitcast i32 → floatforFloat32Array). Width 8 serves onlyFloat64Arrayinline.js_packed_arraylike_index_getnow performs the lazy-JSON probe itself, only for an unforwardedGC_TYPE_LAZY_ARRAYheader. Nothing between the header read and theCannotCollectprobe can collect, and a forwarded receiver falls through tojs_array_get_f64.Summary by CodeRabbit
Performance
Bug Fixes
Documentation