perf(vm): array-destructuring fast path for pristine arrays

[mparrett]

Stacks on #29 (iterator fast path). b0e1462 modifies pkg/vm/array_iterator.go, which #29's commits remove/replace, and shares the vm.go/bytecode.go opcode regions — so this branch is cut on top of #29 and its diff includes those commits until #29 merges. Review/merge after #29; this branch then rebases onto main and its diff shrinks to the single destructuring commit. Net new commit here: b0e1462.

const [k, v] = pair and for-of element patterns ran the full iterator protocol per pattern — build an iterator object, then a next() call per element with a {value, done} allocation each. For a pristine array the whole thing is equivalent to indexed reads.

What this does

• Two opcodes. OpArrayDestructFastCheck identity-checks the source's Symbol.iterator against the recorded canonical array iterator; any instance- or prototype-level override fails the check and takes the generic path. OpArrayRawGetInt reads Arr.Get(idx) — bit-identical to what the built-in array iterator's Step yields, so no accessor/proxy divergence.
• Per-element branch selects fast (index read) vs generic (next()); both converge on the value register, so the existing binding, default, nested-pattern, and elision logic is untouched. On the fast arm done is forced true, which short-circuits the existing iterator-cleanup emit to a no-op — correct, since a pristine array iterator has no return() anyway.
• Rest patterns keep the generic path. Applied at all three destructuring sites (declaration, for-of decl, for-of assignment) via shared helpers.

Behavior preserved: short source → undefined tail, holes → undefined, defaults, nested patterns, and (verified) deopt on a prototype-level Symbol.iterator override.

Design point for review: the Symbol.iterator identity check that guards the fast path.

Numbers

Local (M2, min of 5 interleaved A/B): const [k,v]=pair hot loop 4.8×; for (const [k,v] of map) 3.8×. The perf-label A/B is authoritative.

Verification

TestScripts green + tests/scripts/destructure_array_fast_path.ts (short source, holes, defaults, elisions, nested patterns, rest, Map for-of, prototype-override deopt). Test262 language 0 new failures (differential vs upstream/main control); targeted built-ins 0 new failures.

Aside (filed separately, not part of this PR): while testing, instance-level arr[Symbol.iterator] = ... overrides on arrays turn out to be ignored by the existing generic for-of/spread paths too. The fast path matches that behavior, so no regression — but it's a pre-existing conformance gap worth an issue.

---

Part of a VM micro-optimization series (profile-driven, one slice per PR). A tracking issue with the full map and a reviewer's guide follows.

[nooga]

Thanks for this — the register-cleanup, holes/defaults/rest/nested-pattern handling, and the fast-arm/generic-arm split are all solid. But I found a confirmed correctness regression, so requesting changes before merge.

Bug: subclassed arrays silently bypass a custom Symbol.iterator in the fast path.

isFastDestructureArray approves the fast path via vm.GetSymbolProperty(v, vm.SymbolIterator), which checks the array's own symbol props then falls back unconditionally to vm.ArrayPrototype — it never consults the per-instance arr.prototype override used for class S extends Array {} subclasses. The existing opGetPropSymbol handles this correctly today (checks base.AsArray().prototype first), so the fast-path gate needs the same walk.

Repro:

class MyArray extends Array<number> {
  [Symbol.iterator]() { /* yields value*1000 */ }
}
const a = new MyArray(); a.push(1,2,3);
const [x,y,z] = a;               // destructuring
for (const v of a) out.push(v);  // for-of

• On main (pre-PR): both destructuring and for-of give 1000,2000,3000 (custom iterator honored, consistent).
• On this branch: destructuring gives 1,2,3 (raw values — custom iterator silently skipped), for-of still gives 1000,2000,3000.

This is exactly the "silent correctness regression" failure mode the project treats as worst-case, since it affects a real (if less common) pattern — Array subclassing with custom iteration — and the existing test suite (tests/scripts/destructure_array_fast_path.ts) only covers prototype-level Symbol.iterator reassignment, not subclassing, so it didn't catch this.

Ask: make isFastDestructureArray walk arr.prototype (falling back to vm.ArrayPrototype) the same way opGetPropSymbol does, and add a subclassed-array regression test alongside the existing fast-path tests. Happy to re-review once that's in.

[mparrett]

Fixed in 336ed8c, and thanks — the diagnosis was exactly right.

Root cause. isFastDestructureArray resolved Symbol.iterator through vm.GetSymbolProperty, whose array branch checks own symbol properties and then falls back to vm.ArrayPrototype unconditionally, never consulting the per-instance arr.prototype. That's also why the existing test missed it: it reassigns Array.prototype[Symbol.iterator], mutating the one object that branch does look at, so the deopt fired for the wrong reason.

Fix. Resolves Symbol.iterator the way opGetPropSymbol does — own symbol properties, then arr.prototype falling back to the intrinsic Array.prototype, then that chain. One deliberate deviation: on a non-plain (dictionary-mode) link I return false and take the generic path, where opGetPropSymbol walks past it. Skipping such a link could miss an override and wrongly approve the fast path, and this gate only decides an optimization, so the conservative direction is free.

Verification.

• Your repro now gives 1000,2000,3000 for both destructuring and for-of, matching main.
• The fast path still engages: I probed the gate directly, since a behavior test can't distinguish "correct" from "optimization silently disabled". Pristine array → approved, subclass → generic path. Only the subclass case deopts.
• The regression test fails without the fix, with 1,2,3/1000,2000,3000 — the destructure-vs-for-of disagreement itself. It asserts the two forms agree rather than pinning literal values, since that's the actual invariant.

Also rebased. Three of the four commits (the for-of fast paths over plain arrays, over string/arguments/keys/entries, and over Map/Set) had already landed on main, so the rebase dropped them. This is now two commits: the original fast path, plus the fix as a separate commit so you can review just the delta.

🤖 Generated with Claude Code

[mparrett]

Regression, largest on MatrixMult. 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	+43.8%
FibPlaceholderRun	+17.3%
Add	+8.0%
Arith	+0.6%
SetIndex	−3.9%

MatrixMult was re-run with true per-launch interleaving: +43.8% interleaved against +44.0% grouped, 2.6% spread within the arm. It reproduces.

MatrixMult is the array-heaviest workload in the suite, so a destructuring change landing hardest there is at least consistent — but a fast path that costs 44% on the workload it should most help is worth understanding before merge.

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 +43.8% is real, and it is not this change.

No benchmark fixture in ./tests contains a single OpArrayDestruct
instruction, so the fast path this PR adds never runs in any of them. This PR
also emits byte-identical bytecode for factorial.ts and matrix_mult.ts,
the two fixtures behind the Fib +17.3% and MatrixMult +43.8% readings — the
VM executes the same instructions on both sides. (Verified over 12 dumps per
binary; a single dump can flip, per #50.)

What is left is code layout — this change re-encodes 44.1% of (*VM).run's
65,397 instructions, the largest of the three. Details and the missing control in
#52.

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