-
-
Notifications
You must be signed in to change notification settings - Fork 162
perf(codegen): a property read with a numeric shape field vouches as canonical raw f64 (#10777) #10787
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
proggeramlug
wants to merge
16
commits into
PerryTS:main
from
proggeramlug:perf/10777-ptrshape-rawf64
Closed
perf(codegen): a property read with a numeric shape field vouches as canonical raw f64 (#10777) #10787
Changes from all commits
Commits
Show all changes
16 commits
Select commit
Hold shift + click to select a range
efa2050
perf(codegen,hir): stop re-proving a loop-invariant array receiver on…
perry-bot 1246fd9
changelog: fragment for #10718
perry-bot 8d46937
Merge remote-tracking branch 'origin/main' into storebase
perry-bot 2b2b890
perf(codegen): hoist the loop-invariant receiver proof for array elem…
perry-bot 4c0a29b
perf(codegen): let `a[i] += 1` reach the loop tier `a[i] = a[i] + 1` …
2e2fb18
Merge commit '91a566c8af52ff4e310a107b525843184bba4be8' into propfix-…
392b635
Merge commit '4c0a29b838c56c3b69d99fdce07e2a1345884b1b' into numfix-base
0a0af40
perf(transform): re-apply the literal-key member fold after const sub…
perry-bot 70ad5cf
perf(runtime): stop routing plain numbers through the slow arms of th…
perry-bot 1794e99
perf(codegen): stop disabling Ptr<Shape> in entry bodies for a bug th…
perry-bot a51529b
perf(runtime): remove the toFixed cliff at dp >= 7 (#10770)
perry-bot 31efa00
perf(codegen): unary + proves a Number by construction, with no opera…
perry-bot ddf54ad
Merge remote-tracking branch 'origin/perf/10770-tofixed-cliff' into n…
perry-bot 3baab97
Merge remote-tracking branch 'origin/perf/10777-unary-pos-numeric' in…
perry-bot a819742
fix(codegen,runtime): canonicalise NaNs read out of an ArrayBuffer so…
perry-bot f808122
perf(codegen): a property read on a Ptr<Shape> receiver with a numeri…
perry-bot 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,9 @@ | ||
| **Indexed reads on an ordinary `Array` no longer re-prove a loop-invariant receiver on every element.** | ||
|
|
||
| An indexed read cost **87 instructions per element** — against 6 for the same arithmetic on a `Float64Array` and 16 for node — and none of it was a runtime call. 56 of the 87 were loop-invariant receiver revalidation re-executed every iteration: the NaN-box tag and handle-band test, the forwarding-flag follow, and a six-load live-head guard. | ||
|
|
||
| perry already had tiers that hoist that proof into the loop preheader. They were declining at one gate, `array_static_type_excluded` — a *declared static type* test in front of a tier that is otherwise fully runtime-guarded — so `const a: number[]` got it and plain `new Array(400)`, which infers `Array<any>`, did not. Ordinary JavaScript never reached the tier it already had. | ||
|
|
||
| Separately, `a[i] += 1` cost **948** instructions per element, 3.7× the identical `a[i] = a[i] + 1`, and no annotation helped: the compound-assignment spill temporaries were minted as `Type::Any`, erasing the receiver's array-ness and the index's integer-ness before codegen saw the statement. | ||
|
|
||
| Array read **87 → 13.5** (node 16.3), `a[i] += 1` **948 → 273**, `a[i] += b[i]` **1025 → 347**. A particle simulation over four numeric arrays spends **60.9% fewer instructions** and **59% less peak RSS**. The bare loop and both `Float64Array` paths are unchanged to the instruction. |
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,9 @@ | ||
| **Stores to an ordinary `Array` element no longer re-prove a loop-invariant receiver on every element.** | ||
|
|
||
| An indexed write cost **105 instructions per element** — against 8 for the same store to a `Float64Array` and 12 for node — with zero runtime calls. **51 of the 105 were loop-invariant** receiver revalidation, and a further 42 was a write-barrier decision provable away from the value's type. | ||
|
|
||
| This widens the store admission the way #10731 widened reads. The gate was `has_materialization_hazard`, which a trailing `console.log` is enough to set. | ||
|
|
||
| `a[i] = k + i` **105 → 17.4**, `a[i] = a[i] + 1` **256 → 24.5** (node 18.7), `a[i] = a[i] + b[i]` **333 → 35.9**. The bare loop, both `Float64Array` paths and the indexed read are unchanged to the instruction. | ||
|
|
||
| Note this moves none of the five real programs in #10695 — their loop bodies are multi-statement or contain calls, which no current tier admits (#10741) — and `a[i] += 1` is unaffected because its lowering is two statements (#10743). | ||
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 @@ | ||
| **`a[i] += 1` reaches the same loop tier as `a[i] = a[i] + 1`.** | ||
|
|
||
| The two spellings are the same operation and node compiles both to the same cost. perry compiled them **11× apart** — 277 instructions per element against 24 — and the slow one was the idiomatic spelling. | ||
|
|
||
| HIR lowers a compound member assignment into two immutable alias `Let`s plus the store, so the base and the key are each evaluated exactly once and before the right-hand side. The classic range-loop matcher admits exactly ONE statement, so the lowering guaranteed the statement could never reach the tier. Annotating the array changed nothing: the obstacle is the statement count, not type information. | ||
|
|
||
| The temporaries stay. They are load-bearing — an RHS call can reassign the bindings they were read from, and the store must still land at the index evaluated before it ran. Instead the matcher folds them, and only for the guarded fast clones: the slow clone lowers the statements as written, so a failed guard and every side exit still execute the specified evaluation order. Inside the matched subset the fold is exact, because the body walk is a whitelist that admits no call, closure, `await`, update or assignment anywhere in the statement — nothing can write the locals the aliases read. | ||
|
|
||
| `a[i] += 1` **277 → 25.5**, `a[i] -= 1` **208 → 27.5**, `a[i] += b[i]` **347 → 35.9** (identical to `a[i] = a[i] + b[i]`), `a[i] *= 1` **206 → 25.5**, `a[i] |= 0` **236 → 52.5**. The bare loop, both `Float64Array` paths, the indexed read and write, and both expanded spellings are unchanged — their emitted LLVM IR is byte-identical. | ||
|
|
||
| This needs none of #10741's mid-iteration side-exit discipline: the folded-away statements perform no stores, so there is nothing to un-do when a guard fails partway. It also moves none of the five real programs in #10695 — their loop bodies are still multi-statement or contain calls, which no current tier admits. |
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,5 @@ | ||
| **A hoisted `const K = "a"` used as a property key no longer costs 7.3× the same read spelled `o.a`.** | ||
|
|
||
| `o["a"]` in source is already folded to `o.a` by the member lowering. But `module_const_fold` substitutes a hoisted const into the key position *after* that matcher has run, so the node stayed an `IndexGet` and was resolved by name at runtime on every read — UTF-8-validating the key, hashing it for the accessor Bloom summary, classifying the receiver and scanning the shape's key array. | ||
|
|
||
| Re-applying the same fold takes `O[K] + O[J]` from **1236 to 169 instructions**, exactly what `O.a + O.b` costs. It also fixes a spec divergence: `null[K]` and `undefined[K]` read `undefined` before, where node throws. |
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,7 @@ | ||
| **Plain numbers no longer take the slow arms of the string-coercion ladders.** | ||
|
|
||
| `js_string_coerce` and `js_jsvalue_to_string_method` reached their plain-number arm last, through a seven-way jump table, though `is_number()` is a single range test and the exact complement of the arms it skips. `js_number_to_string`'s admission check also forced a 14-instruction saturating `f64`→`u64` cast on a value already proven to be within the 256-entry small-integer cache. | ||
|
|
||
| `n.toString()` **−19.3%** (536.3 → 433.0), `String(n)` **−14.2%** (190.0 → 163.0), template literal −4.7%, large integers −3.2%, floats −0.9%, `"" + n` unchanged. No row regresses. | ||
|
|
||
| `n.toString()` was costing `String(n)` **plus exactly 103 instructions** at every value range — a four-frame dispatch detour through a thread-local one-shot — which is what this removes. |
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,5 @@ | ||
| **`Ptr<Shape>` is no longer disabled in entry bodies for a bug that was fixed in the runtime.** | ||
|
|
||
| `RepselContextFlags::derive`'s `Entry` arm forced `allows_ptr_shape: false` because *"#6991 is an open rooting bug in exactly that position"*. #6991 was closed by #7249, which put `populate_global_this_builtins` inside a `GcSuppressScope` — a runtime fix. The gate has been guarding a bug that no longer exists, and a shape proof in an entry body was being made, counted as a win, then dropped at every access site. | ||
|
|
||
| A module-level `const` with its loop at module level goes **110.00 → 88.99 instructions per iteration (−19.1%)**. The same body inside a function is the control and correctly does not move. |
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,7 @@ | ||
| **`toFixed(7)` no longer costs 10.7× `toFixed(6)`.** | ||
|
|
||
| `spec_to_fixed` asked `format!("{x:.1100}")` on every input — 1100 being the smallest subnormal's worst case — so `(6.0).toFixed(7)` expanded 1100 decimal places through dragon4 and discarded 1093. And `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 rather than a table length. | ||
|
|
||
| dp 7 goes **7125.4 → 633.1** and dp 8 **7159.1 → 645.6**, turning a 7.5× loss against node and bun into a win. A money-shaped `(12.34).toFixed(8)` goes **12637.2 → 596.6, 21.2×**. dp 0–6 gain 4–5% as well, because `10u64.pow(dp)` becomes a table load. | ||
|
|
||
| The table is now `POW10_FIXED` at module scope with 20 entries, and its length *is* the admission bound rather than a second constant that happens to agree. |
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,5 @@ | ||
| **A property read whose shape already records the field as numeric now vouches as canonical raw f64.** | ||
|
|
||
| `expr_produces_canonical_raw_f64` refused every `PropertyGet` on principle, so an arithmetic leaf reading such a field kept a per-iteration tag test — even though the fact was present and already consumed by codegen, which emits `load(DOUBLE, …)` with `rep: F64`. It was then re-derived *syntactically* from the Expr node at the add. That is also why `o.a * 1` reaches 9 where `o.a` does not: `Binary` is a shape the predicate recognises, and the multiply normalises nothing. | ||
|
|
||
| `h += o.a` goes 35 → 33, `h += o.a + o.b` 50 → 43, `o.a = o.a + 1; h += o.a` 97 → 94 — 2–7 instructions per iteration where the receiver is a class instance with proven provenance. |
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,7 @@ | ||
| **Unary `+` now proves a Number by construction.** | ||
|
|
||
| `expr_numeric_by_construction` required `rec(operand)` for `Pos` as though it were a soundness guard. It is not: unary `+` is ToNumber, which either completes holding a Number or throws — BigInt and Symbol throw, an object re-enters ToNumber after ToPrimitive, `undefined` is NaN. A throw stores no value, so the store-universe question the fixpoint asks is vacuous there. | ||
|
|
||
| `Neg` and `BitNot` keep their condition, because ToNumeric is BigInt-preserving (`-1n` is `-1n`). | ||
|
|
||
| The missed proof left the *accumulator* unproven, so its add kept a per-iteration tag test: `const v = +o.a; … h += v` goes **20 → 9 instructions per iteration**, the same figure `o.a * 1` and `o.a - 0` already reached. |
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,7 @@ | ||
| **A double read out of an `ArrayBuffer` can no longer forge a boxed value.** | ||
|
|
||
| A bit pattern falling inside the NaN-box tag window read back as its payload integer rather than the NaN it is, and `Number.isNaN` reported `false` on it. This is memory safety rather than arithmetic: 80 of 144 probe patterns diverge on base and **32 SIGSEGV** — `0x7FF9…` reads back with `typeof === "string"`, `0x7FFD…` as `[object Object]`, a pointer forged out of user-controlled bytes. Seeded GC stress survives **0 of 10 seeds** on base and 10 of 10 after. | ||
|
|
||
| Canonicalising float reads out of an `ArrayBuffer` is sufficient, because a field is a conduit rather than a source — perry already enforces the same invariant for `Array<number>` on the store side. The `Float64Array` element read costs **−0.006%**, and no row where perry beats node regresses. | ||
|
|
||
| NaN payload bits are no longer preserved through a JS number, matching JSC and SpiderMonkey rather than node. Spec-permitted, and unavoidable under any sound design. |
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 | 🟡 Minor | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
Repository: PerryTS/perry
Length of output: 14070
🏁 Script executed:
Repository: PerryTS/perry
Length of output: 10748
🤖 get_repo_knowledge executed:
get_repo_knowledge PerryTS/perry /tmp/coderabbit-repo-knowledge/perryts-perry-d4a878bc/learnings /tmp/coderabbit-repo-knowledge/perryts-perry-d4a878bc/conventionsLength of output: 19410
Describe the final behavior of
a[i] += 1.The release script concatenates all numbered fragments. This entry says that
a[i] += 1is unaffected, whilechangelog.d/10743-compound-assign-alias-fold.mdsays it reaches the loop tier and improves from 277 to 25.5 instructions per element. Replace the stale claim with the final behavior.🤖 Prompt for AI Agents