chore: merge train 234 (v0.5.1613) - #10786
Merged
Merged
Conversation
…stitution (#10761) `o["a"]` written in source is already lowered to `o.a` by the AST->HIR member lowering (the #529 fold in `lower/expr_member/member_tail.rs`). But `module_const_fold` substitutes a hoisted `const K = "a"` into the key position *after* that matcher has run, and nothing re-ran it — so the enclosing node stayed an `IndexGet` and codegen's static-string-key arm resolved it by name at runtime on every read: UTF-8-validate the key, hash it for the accessor Bloom summary, classify the receiver, then scan the shape's key array. Phase 2 re-applies the same rewrite. The produced node is bit-identical to the one `o["name"]` produces in source, so there is no new fast path and no new guard; the read simply reaches the per-site monomorphic inline cache that the dotted spelling already used. O[K] + O[J] on {a:1,b:2,c:3} 1236 -> 169 instructions/iteration (7.31x) which is exactly what the same pair spelled `O.a + O.b` costs. Identical at both fit ranges. It also corrects a spec divergence: `null[K]` and `undefined[K]` silently read `undefined` before this change, where node throws a TypeError. Numeric-index strings are excluded, mirroring the source-level fold verbatim, so `arr["0"]` keeps IndexGet semantics.
…at was fixed in the runtime (#10769) `RepselContextFlags::derive`'s `Entry` arm forced `allows_ptr_shape: false` and a `MODULE_INIT_CONTEXT` denial, on the stated grounds that "#6991 is an open rooting bug in exactly that position". #6991 is closed. It was fixed by #7249 (64c1f56), which placed `populate_global_this_builtins` inside a `GcSuppressScope` — a runtime fix, not a codegen one. The gate has since been guarding against a bug that no longer exists, and the effect was that a shape proof in an entry body was made, counted as a win in the optimiser report, and then dropped at every access site. The `Entry` arm now derives all three flags from their knobs like any other body. module-level const, loop at module level 110.00 -> 88.99 (-19.1%) node is 14.50 on the same fixture, so this does not reach parity; roughly 36 instructions of entry-body cost remain and are not this gate. The same body placed inside a function is the control and correctly does not move. The nine real programs do not move, and the mechanism was checked rather than assumed: `--opt-report` module-init denial mentions are identical on both arms for all nine, because none of them has a `Ptr<Shape>` candidate in its entry body. `validate` and `resolve` do hold module-level const records, but they are read from inside functions, which globalizes them and puts them behind the separate storage limitation tracked as #7109.
…e string-coercion ladders (#10762) Four edits, all runtime, no codegen: `js_string_coerce` and `js_jsvalue_to_string_method` reached their plain-number arm last, through a seven-way jump table; `is_number()` is one range test and the exact complement of the arms it skips, so the number arm is hoisted ahead of them. `js_number_to_string`'s admission check forced LLVM to emit a 14-instruction saturating f64->u64 cast on a value already proven to be in 0..256, plus a redundant second bound check; the cheaper admission lets it emit a 4-instruction cast, and the cache-fill arm is outlined `#[cold]` so its inlined `format!` stops costing 15 instructions of prologue in the hit path. `format_number_into` gains a range-proven i32 arm. String(k%100) 190.0 -> 163.0 (-14.2%) n.toString() 536.3 -> 433.0 (-19.3%) `${n}` 433.3 -> 413.0 (-4.7%) String(k%1e6) 558.3 -> 540.3 (-3.2%) float 1146.6 -> 1136.6 (-0.9%) "" + n 264.5 -> 264.5 0.00% control (no conv) 82.0 -> 82.0 0.00% No row regresses. Both arms are flat within 2% across 20k->200k and 500k->5M. This does not reach parity with node or bun, and the remaining distance needs an ABI change rather than another pass: `"" + n` never allocates, because `js_string_concat_value_box` returns an f64 and packs a short result into SHORT_STRING_TAG, while the other three entry points are declared `-> *mut StringHeader` and must allocate a heap string for a three-byte result.
`toFixed(6)` cost 646 instructions and `toFixed(7)` cost 7,125 — a 10.7x jump
for one more decimal place, while node and bun are flat across the range.
Two causes, both of them a bound that had drifted from the thing it bounds:
`spec_to_fixed` asked `format!("{x:.1100}")` on every input. 1100 is the
smallest subnormal's worst case, so `(6.0).toFixed(7)` expanded 1100 decimal
places through dragon4 and discarded 1093 of them. It now asks for the digits
the value actually has.
`POW10` was seven entries local to `fmt_fixed_int`, while the admission bound
read `dp <= 6` a hundred lines away as though it were an overflow limit. It is
now `POW10_FIXED` at module scope with 20 entries, and the doc comment states
that the table's length *is* the bound — they are the same object rather than
two constants that happen to agree.
dp 0 524.2 -> 520.2
dp 2 592.6 -> 564.6
dp 6 646.0 -> 620.9
dp 7 7125.4 -> 633.1
dp 8 7159.1 -> 645.6
(12.34).toFixed(8) 12637.2 -> 596.6 (21.2x)
node is 905-954 and bun 1016-1108 across the same range, so every row is now a
win where dp >= 7 was a 7.5x loss. dp 0-6 also gained 4-5% because `10u64.pow(dp)`
became a table load.
405,828 node-identical results across the fixture set, including 378,000
targeting the newly admitted inexact-product population.
|
Note Currently processing new changes in this PR. This may take a few minutes, please wait... ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (16)
✨ 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 20, 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
2 participants
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 234 — the tail of the perf stack, released as v0.5.1613.
Contents
perf(transform): re-apply the literal-key member fold after const substitutionperf(codegen): stop disablingPtr<Shape>in entry bodies for a bug that was fixed in the runtime — 110 → 89 instructions (−19.1%)perf(runtime): hoist the plain-number arm in the string-coercion laddersperf(runtime): remove thetoFixedcliff at dp ≥ 7 — 7125 → 633 instructionsTwo chains that both fork off #10752 (landed as v0.5.1611). Four unique non-merge commits; the two merge commits and the four already-on-
mainancestors were not picked.#10774's premise was checked, not taken
That PR removes a codegen gate justified by a comment reading "#6991 is an open rooting bug in exactly that position". Everything rests on that no longer being true, so it was verified independently rather than read:
64c1f56fb— "fix(gc): run the globalThis bootstrap in a no-move window (GC: #7207's spread-accessor reproducer still SIGSEGVs on main under the evac_minor arm (allocation-point relocation, not the safepoint route) #7217) (fix(gc): run the globalThis bootstrap in a no-move window (#7217) #7249)" — is an ancestor ofmain.GcSuppressScopeit added is still present today atcrates/perry-runtime/src/object/global_this/populate.rs:78, inside the exact function the gate cites.So the gate was guarding a bug that no longer exists, and its effect was that a shape proof in an entry body was made, counted as a win in the optimiser report, and then dropped at every access site.
Worth keeping from the PR's own writeup: the boundary it identifies is that a top-level binding read only at top level is not globalized, and this gate was its whole blocker — one read from inside a function globalizes it and it becomes #7109 instead. That is exactly why three fixtures move and the nine real programs do not (0.000% to −0.004%), and the PR verified the mechanism rather than reporting the zero:
--opt-reportmodule-init denial mentions are identical on both arms for all nine.Two "missing" insertions that were correct absences
The line-level representation check flagged both, and taking either at face value would have caused damage:
dp <= 6cliff,POW10: [u64; 7], andformat!("{x:.1100}"). perf(runtime): remove the toFixed cliff at dp>=7 — 7125 to 633 instructions, and 21.2x on a money-shaped value (#10770) #10776 deletes them. Superseded inside the stack, exactly like perf(codegen,hir): stop re-proving a loop-invariant array receiver on every element access — Array read 87 to 13.5 instructions (#10718) #10731/perf(codegen): hoist the loop-invariant receiver proof for array element stores — 105 to 17.4 instructions (#10718) #10746 in train 232.scripts/gc_runtime_root_holders.jsonentry forcrates/perry-stdlib/src/commander.rs, a file train 233 removed an hour earlier. "Restoring" it would have registered a root holder for a nonexistent file and reddened the holders gate.gc_runtime_root_holders.pypasses on this tree andmainno longer mentions that file.Validation
Assembled on
1698e7fbc8; four source heads asserted fresh; no attribution trailers (the stack carried one per commit; the rewrite left the tree hash byte-identical). Nine cheap gates,cargo check --workspace --all-targetsunder-D warnings, all five pinned artifacts byte-identical before and after the sweep, and six unit suites with an empty failing set — the baseline stays at zero, so any new failure is a regression rather than a triage question.lintcompleted its full 6-of-6 compile tier with nothing outside the known-red public-baseline step.Gap sweep at
PERRY_RUN_TIMEOUT=30, ten areas, every one asserted live, zero unexplained regressions. The weighting is deliberate: #10774 re-enables a shape optimisation in a position that used to carry a rooting bug, so the GC area is the largest single slice rather than an afterthought.This train touches no
crates/*/tests/*.rs, and the driver now says so explicitly (integration_suites live=[] deleted=[]) rather than leaving an empty derivation indistinguishable from a broken one.