perf(vm): make arrayIndexAccessorSeen an atomic.Bool [4/4 of #39]

[mparrett]

Part 4 of the #39 split — the arrayIndexAccessorSeen concern from your review.

The latch is now an atomic.Bool. As you noted it was fail-safe (a stale false just restores the slow path), but it was still a data race under -race or with concurrent VM instances; Store/Load make the publication well-defined.

noteAccessorKey keeps its load-then-store shape rather than a CAS — the only transition is false → true, so a redundant store is harmless.

go test -race ./pkg/vm/ passes.

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	arrayIndexAccessorSeen → atomic.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]

This is a substantial regression, not a neutral cleanup. 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	Δ
MatrixMult	+33.2%
FibPlaceholderRun	+30.9%
Arith	+10.4%
Add	+5.7%
SetIndex	−1.1%

Alternating base-vs-head launches on Fib, ten each:

base  880  909  911  911  913  913  914  916  918  922  926   ms
head 1199 1200 1200 1201 1208 1218 1218 1219 1222 1225 1234   ms

No overlap between the two distributions, and each arm is tight (2.9–5.2% spread). This reproduces across every launch.

The mechanism is named in the commit message: "nothing stops the compiler caching the load across an arr[i] = v loop." That is exactly right, and it is the cost — making the flag an atomic.Bool forces a memory read where the compiler previously hoisted it out of the loop. The race is real and worth fixing, but not at this price.

Worth considering: the flag is only ever set once and never cleared, and Array.prototype is per-realm. A cheaper shape might be a plain read guarded by a one-time initialisation, or hoisting the check out of the hot path entirely, rather than an atomic load per access.

Floors from null controls in the same run: two commits compiling to byte-identical binaries measure up to 3.7% apart on ./tests, and a layout control is no worse — so anything under ~4% is not attributable. These are far outside that. Full write-up and raw data: perf-session-remeasure-results.md.

[mparrett]

Correcting my verdict here: the +30.9% is real, and it is not this change.

The regressing benchmark is Fib, which compiles scripts/factorial.ts — a
fixture containing zero OpSetIndex instructions. The load this PR makes
atomic sits inside the OpSetIndex handler, so it cannot execute once in the
benchmark that moved. SetIndex, which executes that guarded load 200,256 times
per iteration, measured −1.1%.

So the un-hoistable-atomic-load reading I gave predicts the opposite ranking to
the one measured.

What is left is code layout. This change alters (*VM).run's size by zero
bytes yet re-encodes 19.7% of its 65,397 instructions: spill slots shift
(151(RSP) → 159(RSP)) across a 261 KB function. Details and the missing
control in #52.

I would leave this open and unmeasured rather than close it as a regression.