Skip to content

perf(vm): numeric fast path for comparisons + integer % [1/4 of #39] - #40

Open
mparrett wants to merge 5 commits into
nooga:mainfrom
mparrett:perf/vm-numeric-compare-rem
Open

perf(vm): numeric fast path for comparisons + integer % [1/4 of #39]#40
mparrett wants to merge 5 commits into
nooga:mainfrom
mparrett:perf/vm-numeric-compare-rem

Conversation

@mparrett

Copy link
Copy Markdown
Contributor

Part 1 of the #39 split.

Collapses the per-opcode number guards into one fast path shared by all 13 arithmetic and comparison opcodes, reading the type tags directly rather than through ToFloat() (too large to inline). Comparisons previously had no fast path at all and paid the full ToPrimitive/helperCallDepth/unwinding bookkeeping on every loop condition.

Semantics. ToPrimitive is the identity on numbers, so the fast path computes what the general path would. NaN needs no special casing: Go's float comparisons are already false in all four relational directions, and l == r is false for NaN, matching StrictlyEquals. For two numbers abstract and strict equality coincide, so OpEqual/OpStrictEqual share an arm.

Integer %. math.Mod is a software frexp/ldexp loop, ~7% of the recursion benchmark's profile. Go's truncated int64 remainder takes the dividend's sign, exactly matching JS, so the integer path is used only when both operands round-trip exactly through int64 (±2^53), the divisor is nonzero, and the result isn't a signed zero — JS requires -0 for a negative dividend with zero remainder, which the integer path would lose. Everything else falls through to math.Mod. tests/scripts/remainder_semantics.ts pins these, including 1 / (-7 % 7) === -Infinity.

The third commit drops the per-op number guards that the unified path made unreachable — the cleanup nit from your review.

The inner switch carries a default arm that degrades to the general path. It's unreachable today (the enclosing case list and the switch cover the same 13 opcodes), but without it, adding an opcode to that list without a fast-path arm would continue with destReg never written — a silently stale register, which is a worse failure than the slow path.

Where this sits in the #39 split

#39 bundled ~6 changes; per review it's split into four independent PRs, all branched off current main:

branch contents
1 perf/vm-numeric-compare-rem comparison fast path, integer %, drop unreachable per-op guards
2 perf/vm-finally-nonalloc non-allocating finally-handler check
3 perf/vm-dispatch-deadcode dead debug blocks, redundant IsNaN cleanup
4 perf/vm-array-index-accessor-atomic arrayIndexAccessorSeenatomic.Bool

The other three pieces you flagged as unmentioned — the IsObject() range check, the ToInteger() fast path, and the arrayIndexAccessorSeen latch itself — already landed on main separately, so they aren't repeated here. Together these four are the remainder of #39.

The four merge onto main in sequence with no conflicts; the union passes TestScripts, pkg/vm, pkg/compiler, and go test -race ./pkg/vm/.

🤖 Generated with Claude Code

mparrett and others added 4 commits July 25, 2026 11:46
Builds on the previous commit's arithmetic fast path in two ways:

- Cover the comparison group (<, >, <=, >=, ==, !=, ===, !==), which
  runs on every loop-bound check. JS NaN semantics need no special
  casing: Go float comparisons yield false when either operand is NaN
  (so the != case correctly makes NaN !== NaN true), matching
  ECMAScript exactly.
- Dispatch on the value tags directly instead of IsNumber()/ToFloat().
  ToFloat is a 12-case switch (inline cost 551 vs budget 80), so each
  call was a real function call; the two-tag check and payload read
  inline to a couple of instructions.

BenchmarkFibPlaceholderRun: -8.4%  vs parent (min of 3 x 3s)
BenchmarkMatrixMult:        -23.7% vs parent (min of 3 x 3s)
Measured under background load with the parent's window less loaded
(ratchet anchor 1.20 vs 1.48 ns/op), so deltas are conservative.
Cumulative with the parent commit, on a quiet machine vs 263757b:
fib 32.15ms -> 27.24ms (-15.3%), matrix 4.95ms -> 2.92ms (-41.0%).

TestScripts green. Test262 language suite: 0 new failures / 0 new
passes. Built-ins suite: 0 new failures across the ~60% that ran
before the runner died of memory pressure (2443 failures, all
pre-existing in baseline.txt).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
math.Mod is a software frexp/ldexp loop and showed up as 7.6% of the
recursion benchmark's CPU profile (workloads use `i % k` constantly).
Go's truncated int64 remainder carries the dividend's sign, which is
exactly ECMAScript's % semantics, so use it when it is provably safe:
both operands integral and within +/-2^53 (float64<->int64 round-trips
exactly; beyond that the conversion can saturate), nonzero divisor
(zero divisor must yield NaN), and a non-signed-zero result (JS
requires -0 for a negative or -0 dividend with zero remainder, which
the integer path would lose - detected via math.Signbit and deferred
to math.Mod).

Adds tests/scripts/remainder_semantics.ts covering sign combinations,
signed zeros (via 1/x), zero divisor, Infinity operands, and the
non-integral fall-through.

BenchmarkFibPlaceholderRun: 25.69ms -> 21.75ms (-15.3%, min of 3 x 2s)
vs the previous commit. TestScripts green.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…t path

The combined tag-direct fast path a few lines above already handles
number-op-number for Add/Subtract/Multiply/Divide/Remainder (and
comparisons). The per-case guards left behind by nooga#38 were therefore
unreachable — remove them so the slow path is only the coercion ladder.

Co-authored-by: Cursor <cursoragent@cursor.com>
The unified numeric fast path continues unconditionally after its inner
switch, so an opcode present in the enclosing case list but absent from
the switch would fall through with destReg never written - leaving a
stale register rather than raising. The list is exhaustive today; this
default arm makes the failure mode graceful if it ever stops being.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@mparrett
mparrett marked this pull request as ready for review July 25, 2026 20:02
Each fast-path arm now continues the dispatch loop directly and the
default falls through to the general path, dropping the fastPathHandled
local and its test from the VM's hottest loop. Same degradation property
as before: an opcode added to the enclosing case list without an arm
takes the slow path rather than continuing with destReg unwritten.

The remainder comment credited the +-2^53 test with proving integrality,
which the float64(li)==l round-trip below already does. Its real job is
keeping the conversion in range: Go leaves an out-of-range float64->int64
result implementation-defined (arm64 saturates, amd64 yields MinInt64).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@mparrett

mparrett commented Aug 4, 2026

Copy link
Copy Markdown
Contributor Author

Measured on a dedicated c7a.2xlarge (EPYC 9R14), go1.26.0, against the shared merge base 3167412e85c2. Targeted A/B: alternating launches, b.N pinned at 8, median of 5–8 launches per arm. Negative is faster.

benchmark Δ
FibPlaceholderRun −23.5%
Arith −21.5%
MatrixMult −19.8%
Add −18.8%
SetIndex −4.0%

This is the one clear win of the six open perf PRs. The pattern is coherent with what the change claims: the four numeric-heavy workloads all move together by roughly a fifth, while SetIndex — array writes rather than arithmetic — barely moves. That internal contrast is most of why I believe it.

One caveat in your favour: the four-round session run understated this at −13 to −18%, because the calibration anchor took a ~4% transient excursion during two of this commit's four rounds and normalising by it ate part of the win. The targeted A/B above avoids that.

Floors from null controls in the same run: two commits compiling to byte-identical binaries measure up to 3.7% apart on ./tests. A layout control (real code added, never executed) is no worse — so anything under ~4% here is not attributable to the change. Full write-up and raw data: perf-session-remeasure-results.md.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant