Merge train 181b: #10189, #10196, #10185 - #10211
Merged
Merged
Conversation
added 10 commits
September 13, 2026 17:17
The monomorphic `this.field` / `obj.field` tower in expr/property_get.rs emitted four runtime call sites per site -- `js_typed_feedback_class_field_get_guard` when the #5093 inline pre-check missed, `js_throw_type_error_property_access` for a nullish receiver, and one or two `js_object_get_field_by_name_f64` by-name lookups -- spread over class_field_get.{fast,fallback,merge,throw_nullish,fallback_lookup}. In @babel/parser that is 13,636 sites and 27 % of the module's IR, and because every call is a statepoint the .perry_gcmap scaled with it too. Keep the pre-check and the fast slot load byte-identical, and replace the four miss arms with ONE call to `js_class_field_get_ic` -- the runtime helper the #5391 path-2 full outline a few lines above already calls. It runs the same guard, reads the same slot at the same header-relative offset on a pass, throws the same nullish TypeError (#7153 put that check there for exactly this equivalence) and falls back to `js_object_get_field_by_name_f64`, recording the fallback itself under the same `typed_feedback_enabled()` gate codegen used. A plain number is self-boxing under NaN-boxing, so a `requires_raw_f64` site reads the helper's return exactly as the old phi read the by-name fallback's -- neither arm converted. The key-handle load/bitcast/mask also sinks into the cold miss block, its only remaining consumer. Measured (fixture with one boxed field, two raw-f64 fields, a read on a bare `Named` parameter, reads on `this`, and a raw-f64 read in a loop): * per site: 88 -> 47 IR instructions, 3 runtime calls -> 1, 7 -> 4 blocks; * the `class_field_inline.deref` pre-check is instruction-for-instruction identical (SSA numbering shifts by 3 because the entry block lost the sunk key load); * the `class_field_get.fast` block LOSES its two RS4GC relocation reloads (14 instructions, 2 `call i64 asm "gc-leaf-function"`): it used to be reachable from the guard call, i.e. across a statepoint, and is now reachable only from the pre-check, which crosses none. The inline hit path is strictly shorter than before; * @babel/parser .text 17,640,473 -> 16,389,647 (-7.09 %), .perry_gcmap 809,047 -> 792,749 (-2.01 %), O0-fallback units unchanged. Runtime behaviour is unchanged: on benchmarks/tls-budget/interp.ts the typed-feedback trace reports the same 273 sites with identical guard_passes (817,155,616), guard_failures (202,359,184) and fallback_calls (202,359,184), zero per-site differences, and identical program output. The raw-f64 site's dynamic-fallback native-value record now names `js_class_field_get_ic`; that pair is ADDED to verify/raw_f64.rs rather than replacing the by-name pair, which verify/tests.rs still pins. Tests: a new cargo-test-visible codegen module pins the one-exit shape (the pre-check branches INTO the fast load, the miss arm is exactly one call, the three retired blocks are gone, and the three retired symbols are absent as CALL FORMS -- a bare substring is satisfied by the `declare` line alone). Four runtime tests cover `js_class_field_get_ic`'s arms, which had none. Five existing assertions that used the retired symbols as the proxy for "a guarded class-field read was emitted" are re-pointed: the positives accept either witness (other lowerings still call the old symbols), and the negatives gain the IC as an excluded symbol so they cannot pass on a still-guarded body. (cherry picked from commit cf7c6ca)
(cherry picked from commit eb4bee5)
Per untyped `obj.prop`, `lower_generic_property_get` expanded 33 basic blocks, 191 pre-RS4GC IR instructions and SIX runtime call sites: an SSO arm, an INT32 class-ref arm, a nullish-throw arm, a non-object-receiver arm, an overflow-slot load, a deleted-slot miss, two Array-subclass named-prefix ladders, and the miss+prime. Every call is a statepoint, so `.text` and `.perry_gcmap` both scale with call sites x live GC values -- on @babel/parser that tower was 29% of all emitted IR across 6,487 sites. The hit is worth its bytes; the arms around it are not. What stays inline is what it was: the receiver-tag test, the small-handle test, the packed header kind/descriptor word, the packed-MRU compare, the overflow-bit test, the inline field load with its hole check, the polymorphic ways (#7753), and the `.length`/`.size` short-circuits. Everything else branches to one of two new runtime entries that reproduce the arms in the same ORDER and with the same cache-priming decisions: js_object_get_field_ic_nonptr(obj_bits, key, site_id) the receiver-tag ladder -- SSO, INT32 class ref, nullish TypeError, and the by-name fallback for every other tag. Takes no cache: none of these arms can prime one. js_object_get_field_ic_slow(obj_handle, key, cache_slot, packed) the heap-pointer arms -- the overflow slot (including its fallback to the THREE-argument miss entry, i.e. WITHOUT republishing the packed word, which is what the old helper did), the deleted-slot miss, the Array-subclass named-prefix proof, and the priming `get_field_ic_miss_impl`. Per site: 33 -> 14 tower blocks, 191 -> 106 IR instructions, 6 -> 2 calls. @babel/parser .text -14.4%, .perry_gcmap -11.7%, O0-fallback units unchanged. TWO exits and not one, measured. With the receiver-tag test and the small-handle test failing to the SAME block, SimplifyCFG folds them into one flat predicate -- `cmp; sete; cmp; setae; test; je` where the chain was `cmp; jne; cmp; ja` -- and every property-read HIT pays +4.00 instructions on a 10M-read monomorphic loop. That is #7883's flat-predicate cost arriving from the optimiser instead of from codegen. Distinct callees keep the chain branchy and let the unmasked receiver bits die in the entry block instead of living across the whole hit path. Three further shapes were needed to finish paying for the hit, each measured on the same 10M-read loop (instructions retired, mine vs base): * the field address is a typed `gep double` rather than `shl 3` + `add`. With an explicit shift the slot is the packed word's last use, so InstCombine folds `(packed >> 32) << 3` into `(packed >> 29) & mask` and isel pays a 10-byte `movabs` plus an `and`; as a GEP index there is no shift to fold and the scaled addressing mode survives. +3.00 -> +1.00 per hit. * the spill-buffer read moved into a `#[cold] #[inline(never)]` `overflow_arm`. Inlined, its `overflow_get` call forced the entry to save callee-saved registers, which gave it a frame and stopped the miss handler from being a sibling call. +49 -> +31 -> +20 per megamorphic miss (the first step was the exit split). * the named-prefix conjunction asks `ObjectMeta` first: that is one load from the header line the entry has already touched, against two dependent loads through the site's cache slot, and an object with no metadata record cannot carry the token. +20 -> +17 per miss. Final micro numbers, instructions retired per read vs base: monomorphic hit +1.00 (one `jmp`, because the hit's and the way's hole checks become congruent after their empty successors fold and SimplifyCFG tail-merges them -- fixing it needs `!prof` branch weights the IR builder cannot emit today), 4-shape polymorphic (the inline ways) -1.24, megamorphic miss +17.02. Registries updated with the new symbols: runtime declarations, the `cold` placement hint, the eh_mode throwing-callee assertion, gc_call_effects' allocating-helper list, and POLL_CAPABLE_RUNTIME in scripts/gc_root_dominance_check.py (both directions of its property-GET self-test fixture now run against both entries -- omitting them would silently re-open the #7154 GET hole, since the calls they replaced no longer appear at those sites). `test-files/test_gap_generic_get_one_exit_arms.ts` is the behavioural witness: every arm that moved out of line, checked against node's own output. PERRY_IC_DIAG counters are identical between the two toolchains on a deterministic fixture (41,216 misses over 5 sites, same per-reason split, same 20,216 primes, same fresh/armed/megamorphic distribution), and the gap-suite subset (10 filters, 290 tests) has byte-identical verdicts. (cherry picked from commit a46769d)
(cherry picked from commit fe2a9cf)
…random access shapes Three additions on top of #10171's shape-keyed arm, all needed together for the JSON access benchmark's two remaining shapes: - a LOOP-CARRIED index (`c = (a*c + b) % m; ... rows[c]`), folded to one affine pair and evaluated as `srem i64`, with the write-back placed at the END of the iteration so a mid-iteration side exit cannot double-apply the recurrence; - K accumulator statements folded into one, so the whole iteration commits once, past every side exit it can take; - `arr[i].prop.length` on a string field and `arr[i].prop ? A : B` on a boolean one, each tag-testing the loaded word and side-exiting otherwise. Plus a shared once-per-iteration element deref/residual check, hung off the body's leading virtual binding, so three reads of one element pay one check. (cherry picked from commit c9ce44b)
…new reads 40 cases in test_gap_json_record_loop_clone.ts, all byte-identical to node 26.5.1: the recurrence with and without its alias, the carried value read after the loop, negative and fractional entry values, a zero and an over-long modulus, a multiplier past the exact-double range, a mid-loop side exit whose sum AND final cursor observe the write-back protocol, SSO and heap names, a non-string and a null name, five non-boolean `active` values, a two-statement fold whose second read side-exits, and records carrying own `name`/`length`/ `size` properties. Plus the changelog fragment with the measured 12-cell table and the instruction counts. (cherry picked from commit bf0904a)
The shape-descriptor census asserts that the element-shape guard reads the authoritative ShapeId at header offset 4. That read moved out of `emit_element_shape_field_load` into the deref helper the per-read path and the shared prologue now share, so the census names the function that actually emits it. (cherry picked from commit 54765a9)
12-cell interleaved A/B against node 26.5.1 and bun 1.3.14, plus instructions retired per iteration, all from one coherent build of this worktree at the base commit and at HEAD. The one cell that does not reach parity (16k fields, 1.029x) is named rather than rounded off. (cherry picked from commit 533b592)
|
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 (46)
📝 WalkthroughWalkthroughChangesCompiler and runtime optimization
Estimated code review effort: 5 (Critical) | ~90 minutes Change: Feature · Severity of issue fixed: Medium Sequence Diagram(s)sequenceDiagram
participant LoopMatcher
participant FastClone
participant ElementGuard
participant RuntimeExit
LoopMatcher->>FastClone: match carried index and folded body
FastClone->>ElementGuard: prefetch element and check shape
ElementGuard->>FastClone: load specialized fields
FastClone->>RuntimeExit: side-exit on failed residual or tag check
Possibly related PRs
✨ 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 181b: lands #10189 (collapse the class-field GET tower to one exit) at
eb4bee5d5d, #10196 (collapse the generic property-get tower to two exits) atfe2a9cfd94and #10185 (element-shape loop clone for the fields and random access shapes) at533b59247b. It adds one test-only fix and the workspace version bump to 0.5.1557. (Named 181b because another merge lane already used "181" for #10202.)The PR commits were cherry-picked onto
26ed55cb74without conflicts.Added commit
test(runtime): mint the prefix-test meta record under a GC suppress scope (#10196): #10196's newic_slow.rstest readobj.get_raw_mut_ptr()inside anacross_mutclosure, and the bareraw_handle_debt.pyrun rejects that in a module with no ceiling. It is #10196's only CI difference from main. The test now mints the meta record withobj.with_mut_ptrunderGcSuppressScope, so the receiver cannot move while the scoped pointer is live, and both ratchet runs pass.Validation (macOS arm64)
Full validation first ran on these PRs over
ad6925b09e. Main has since gained only the regex lane's #10201, #10205 and #10207, and this train changes none of their files.scripts/run_lint_gates.sh: 79 of 83 pass. The only failure beyond main's three was the raw-handle ratchet on the site fixed above.cargo test --release: perry-codegen 2028 passed / 0 failed (all targets, including thetests/integration suites), perry-runtime 3750 / 0, perry-transform 128 / 0.cargo test --release -p perry --test issue_8774_argument_shape_clones: 2 / 0.scripts/gc_root_dominance_corpus.sh --lowering shadowplus the checker: 164/164 sources, 15,001 root stores, 2 violations, both non-movingjs_regexp_site_test_new -> js_regexp_site_test_get_methodintest_gap_gc_regexp_receiver_rooting. The same two fingerprints reproduce with a9b911855f8compiler, so they predate this train (details on [scheduled-gate:gc-root-dominance.yml] GC Root Dominance is failing on main #9925).scripts/run_gap_tests.sh, 779 tests): 768 pass, 10 parity fail, 1 crash. Gated regressions are main's known four (2899_2779_2777_static_helpers,disposablestack_2875,iterator_prototype_next_patch,gc_http2_pending_event_callback_rooting), plus9592_child_timeout_threads, where the Node oracle exits 1 on this host; it fails the same way with the9b911855f8compiler.test_gap_json_record_loop_cloneandtest_gap_generic_get_one_exit_armsmatch Node underPERRY_GC_SCHEDULE_SEED11 and 181 at rate 0.5 with forced evacuation, verification and from-space protection (143–391 copying minors).Confirmation on this exact tree:
RUSTFLAGS=-Dwarnings cargo check -p perry --bins;cargo test --release -p perry-runtime: 3763 passed, 1 failed. The failure isgc::tests::heap_generation::a_free_or_move_outside_every_scope_is_caught_in_debug_builds, added by #10205 and already on main; it expects adebug_assertto fire and is not gated ondebug_assertions, so it fails under--release(noted on #10205).CI attribution (vs main's run at
b5a82cfeae)native_stackfailure, the same four gated gap regressions with identical output, and the same gc-stress failure.Summary by CodeRabbit
Performance
Bug Fixes
Documentation
Chores
0.5.1556to0.5.1557.