diff --git a/.do/inbox/2026-10-25-array-mutation-loop-requirement-wording.md b/.do/inbox/2026-10-25-array-mutation-loop-requirement-wording.md new file mode 100644 index 0000000..2bd5dfe --- /dev/null +++ b/.do/inbox/2026-10-25-array-mutation-loop-requirement-wording.md @@ -0,0 +1,5 @@ +# The array-mutation loop-detection requirement contradicts its own fill scenario + +**Status:** needs-triage + +In the `array-mutation` spec as the `wrap-fill-and-copywithin` delta writes it, the requirement "Mutating methods are subject to loop detection" says in its second sentence that when a synchronous top-level effect calls a mutating method and the call writes a property, the system raises the infinite-loop error. The third sentence and the scenario "Fill throws only when the effect read an index it writes" say that `fill` alone writes properties and raises nothing. The scenario "Same loop outcome on both kinds of state" under the first requirement says both kinds raise the same error for a mutating method call, which is false for `fill` alone, where neither raises. The scout of the apply run found both on 2026-10-25. Code is unaffected; the fix is one sentence in the requirement and one qualifier in the scenario, through `/opsx:update` while the change is open or in the main spec after archive. The `tests/AGENTS.md` table also lists the test count of `state-hooks.test.ts` as 19 and goes stale when ticket 03 adds hook assertions there. diff --git a/.do/inbox/2026-10-25-length-only-readers-wake-on-fill-after-batching.md b/.do/inbox/2026-10-25-length-only-readers-wake-on-fill-after-batching.md new file mode 100644 index 0000000..33dd640 --- /dev/null +++ b/.do/inbox/2026-10-25-length-only-readers-wake-on-fill-after-batching.md @@ -0,0 +1,5 @@ +# Length-only readers wake on fill and copyWithin once they are batched + +**Status:** needs-triage + +An effect that reads only `length` of a reactive array never re-runs on `fill` today, because `fill` writes no `length` and the unwrapped call notifies per written index. Once `fill` and `copyWithin` run inside the batch wrapper, as the `wrap-fill-and-copywithin` change makes them, the batch marks the whole array dirty and wakes every subscriber of the array once per call, so a length-only reader re-runs once per `fill` call. Probed on `a7e06bc` with Node v24.21.0: two `fill` calls give a length-only effect 0 re-runs unbatched and 2 re-runs batched. This follows the rule the `array-mutation` spec already states for every wrapped method, "regardless of which properties the subscriber read", and `push(3)` wakes an effect that reads only index 1 today for the same reason. Whether a method call should wake only subscribers of the properties it changed is a question about that rule, not about this change. The inside-effect variant is in `.do/inbox/2026-10-23-per-property-wake-inside-effects.md`. diff --git a/.do/inbox/2026-10-25-same-value-writes-to-frozen-targets-do-not-throw.md b/.do/inbox/2026-10-25-same-value-writes-to-frozen-targets-do-not-throw.md new file mode 100644 index 0000000..e5f45e2 --- /dev/null +++ b/.do/inbox/2026-10-25-same-value-writes-to-frozen-targets-do-not-throw.md @@ -0,0 +1,5 @@ +# Same-value writes to frozen targets do not throw + +**Status:** needs-triage + +A native array rejects every write to a frozen target with a TypeError, whether or not the value changes: `Object.freeze([9]).fill(9)`, `Object.freeze([1, 2]).sort()` and a strict-mode `frozen[0] = 9` all throw. Reactive state over the same frozen arrays throws only when the value differs, because the set trap returns early on an unchanged write before it reaches the frozen target. Probed on `a7e06bc` with Node v24.21.0 during the `wrap-fill-and-copywithin` run: `state(Object.freeze([9])).fill(9)`, `state(Object.freeze([9]))[0] = 9` and `state(Object.freeze([1, 2])).sort()` return without throwing, while `state(Object.freeze([1, 2, 3])).fill(9)` and `state(Object.freeze([9]))[0] = 5` throw the native TypeError. The root cause is the unchanged-write check shared by every write, not an array method, so the fix touches the write path the benchmark gate measures. The user ruled on Q5 of that run that it is fixed as its own change; ticket 02 of `wrap-fill-and-copywithin` tests the frozen case with `[1, 2, 3]` only. diff --git a/.do/inbox/2026-10-25-shared-load-sites-in-subscriber-scheduling.md b/.do/inbox/2026-10-25-shared-load-sites-in-subscriber-scheduling.md new file mode 100644 index 0000000..1bc2ae0 --- /dev/null +++ b/.do/inbox/2026-10-25-shared-load-sites-in-subscriber-scheduling.md @@ -0,0 +1,5 @@ +# Shared load sites in subscriber scheduling deoptimize on mixed array and object writes + +**Status:** needs-triage + +The ticket 04 investigation of change `wrap-fill-and-copywithin` (second implementer, 2026-10-25, report section 15) found that `scheduleSubscribersForTarget` and `findSubscribers` in `src/index.ts` serve array and plain-object targets from the same load sites, and that a benchmark sequence mixing array and object writes (`state write 1 sub`, push/pop, `fill`, `many dependencies`) makes V8 bail out of them 14 and 3 to 6 times per run on every commit measured, including the pre-change baseline `87c8823`. It is the same class of pollution that commit `b595300` removed from `handleBatchFastPath` by giving arrays their own set handler and batch write path, where it cost `many dependencies` about 10% on the median after `fill` moved onto the batch path. This one is not a regression from the change and was left alone. Candidate for the library-wide performance tournament (`2026-10-23-library-wide-performance-tournament.md`): measure a split of those two functions by target kind with the same interleaved protocol, or confirm the bailouts cost nothing measurable. diff --git a/.do/ledger/wrap-fill-and-copywithin.md b/.do/ledger/wrap-fill-and-copywithin.md index bec3a67..6bc2eb0 100644 --- a/.do/ledger/wrap-fill-and-copywithin.md +++ b/.do/ledger/wrap-fill-and-copywithin.md @@ -1,2 +1,23 @@ Ruling: Change branch is `fix/wrap-fill-and-copywithin`, created at trunk `18f5a71` on the user's answer to Q1 (a) - no earlier run of this change exists, the historian found `/` in every recent human PR branch, and the sibling change PR 186 used the `fix` type - if wrong, the branch is renamed before the pull request and nothing else moves. Ruling: The uncommitted OpenSpec and .do/ files are committed on the change branch as `213bbcf` before the first dispatch, on the user's answer to Q2 (a) - worktrees check out tracked files only, and ticket 03 edits the main spec `openspec/specs/array-mutation/spec.md`, which was untracked - if wrong, one docs commit carries planning text onto the fix branch, which is harmless. +Ruling: Ticket 01's benchmark effect reads every index of the array, not only `length` - user chose it (Q3 a) after a probe on `a7e06bc` showed a length-only reader gets 0 wakes per `fill` today and 1 after the change, while an every-index reader gets 3 today and 1 after, and the every-index effect is the developer use `fill` affects - if wrong, the new case measures a path nobody writes and ticket 04's "expected to be faster" is judged on it. +Ruling: Ticket 03 criterion 3 "the arrays end with the native contents" means the contents a plain array holds after the same sequence of calls, so the microtask `copyWithin(0, 1)` effect ends at `[3, 3, 3]` after two runs, the child effect at `[2, 3, 3]`, both `fill` effects at `[9, 9, 9]` - user chose it (Q4 a); the microtask effect re-runs once because it wrote index 1 after reading it, and a plain array after two calls reads `[3, 3, 3]` - if wrong, the test pins a convergence outcome the author did not intend. +Ruling: Same-value writes to a frozen array, which the native array rejects with a TypeError and Beacon swallows through the set trap's unchanged-write check, are fixed as their own change and not in ticket 02; ticket 02's frozen test uses `[1, 2, 3]`, which throws today - user chose it (Q5 a); the gap predates the change and the fix is on the shared write path the ticket 04 gate measures - if wrong, the divergence ships one release longer; it is recorded in `.do/inbox/2026-10-25-same-value-writes-to-frozen-targets-do-not-throw.md`. +Ruling: Tickets 02 and 03 govern where the main spec's purpose line and the agent docs change, which is ticket 03, over design.md's sentence that puts them in the same commit as the set - design.md's migration plan and both tickets agree on the ticket order - if wrong, `src/AGENTS.md` says "five" for the one commit between 02 and 03. +Ruling: Ticket 02 criterion 5 governs the property-based test, which stays hookless as the file is today; the "both kinds of state" sentence of the ticket applies to the array-method test file - the criterion names only the generation of `fill` and `copyWithin` calls beside the existing methods - if wrong, the property test misses a hooked-state divergence the example tests also cover. +Ruling: Biome's reflow of the method-name set over several lines, forced by the 120-character line width when two names join the one-line set, counts as "changed only in the set of method names" for ticket 02 criterion 7 - the reflow is formatting of the same declaration - if wrong, the reviewer reads a formatting hunk as a library change. +Verdict: 01 - approve - Nothing wrong found. Criteria 1 and 2 are met, and the standards checks pass. Landed by fast-forward at `87c8823`. +Ruling: The ticket 01 worktree `.claude/worktrees/agent-a98f18db12d80bac2` and its branch `worktree-agent-a98f18db12d80bac2` stay on disk - the merger's `git worktree remove` exited 255 with "Operation not permitted" from the sandbox, and `git branch -d` refuses while the worktree is registered - if wrong, nothing is lost, the branch is at `87c8823`, which the change branch contains, and the closing report carries the commands that remove it. +Ruling: The ticket 02 implementer runs on after six `env` markers between 11:55:20Z and 11:56:10Z, all PreToolUse hook refusals of a scripted-edit detour, and no further question is asked about environment markers in this attempt - user chose it (Q6 a) after the implementer moved on to passing test runs at 11:55:59Z, 11:56:24Z and 11:56:56Z; a stall or a contract violation still stops for the user - if wrong, hook refusals that signal real trouble pass without a question. +Ruling: The ticket 02 review of `87c8823..4f85493` returned one finding, low standards, on the commit message of `4f85493` (body unwrapped, past tense), so the attempt does not fail and the finding goes to the same implementer as a fix-up, which is not its retry - section 4 of the apply skill - if wrong, a fix-up round is spent on a message the merger could have rewritten. +Verdict: 02 - approve - No findings. Criteria 1-7 met by `5b42960`; the round 1 commit message finding was fixed by amending the message only. Landed by fast-forward at `5b42960`. +Ruling: The ticket 02 worktree `.claude/worktrees/agent-afca106c6cf68d909` and its branch `worktree-agent-afca106c6cf68d909` stay on disk - the merger's `git worktree remove` exited 255 with "Operation not permitted" from the sandbox, and `git branch -d` exited 1 because the worktree is registered - if wrong, nothing is lost, the branch is at `5b42960`, which the change branch contains, and the closing report carries the commands that remove it. +Verdict: 03 - approve - No findings. Criteria 1-8 met by `8464f3f` in two commits, `17a14c5` tests and `8464f3f` docs; no library source changed. Landed by fast-forward at `8464f3f`. +Ruling: The ticket 03 worktree `.claude/worktrees/agent-a8b1f67e276534874` and its branch `worktree-agent-a8b1f67e276534874` stay on disk - the merger's `git worktree remove` exited 128 with "cannot remove a locked working tree, lock reason: claude agent agent-a8b1f67e276534874 (pid 63381 ...)" because the implementer agent still waits on a background shell it already killed, and `git branch -d` exited 1 because the worktree is registered - if wrong, nothing is lost, the branch is at `8464f3f`, which the change branch contains, and the closing report carries the unlock and removal commands. +Ruling: Ticket 04's 10-cycle alternating re-measurement for a case over the 1% line runs as four full-suite runs with `node --expose-gc scripts/benchmark.ts -R 10` in the order baseline `87c8823`, final, final, baseline `87c8823`, because the script has no single-case filter and a full run at 10 cycles costs about 75 seconds, so the four cost about 5 minutes - if wrong, the implementer spends 5 minutes the user would rather have skipped, and the result is still the measurement the ticket asks for. +Ruling: Q7 - ticket 04's two cases over the line after the four-run re-measurement, `state write 1 sub` +2.43% and `many dependencies` +7.57% on means of totals, +2.3% to +4.1% and +8.9% to +11.4% on medians against the nearest baseline, are investigated and not re-rolled or waved through: the maintainer directed senior methodology on 2026-10-25, which the record defines as an A/A test to learn each case's noise, interleaved A/B runs with medians per case and a verdict of faster, slower or indistinguishable within the A/A spread (`.do/inbox/2026-10-23-library-wide-performance-tournament.md`), targeted micro-benchmarks and CPU and deopt profiles of the two cases on both commits (Q66 d: "profiling is a good tool"), a library fix if a mechanism shows, and no existing benchmark case altered; ticket 04's Area gains the benchmark script for additions only, as Q66 d did for ticket 07 of the previous change - if wrong, ticket 04 spends about half an hour of machine time on two cases that noise alone explains, and the evidence for that is then on file. +Ruling: The ticket 04 investigation runs in a second implementer's fresh worktree as build B at `8464f3f`, with the finished ticket 01 worktree `.claude/worktrees/agent-a98f18db12d80bac2` at `87c8823` used read-only as build A, and no checkout switching - the first implementer's worktree was removed by Claude Code when it finished without a commit, its resume landed in the main checkout and it stopped correctly rather than detach HEAD there, and this session's `git worktree add` failed with "Could not reset index file to revision 'HEAD'" under the sandbox; the first attempt counts as attempt 04a, stopped without approval on the gate, and its report stands in the ticket file - if wrong, build A's worktree carries another agent's name in the report, which the report states. +Ruling: The ticket 04 attempt 04b review of `8464f3f..b595300` returned three findings, all low standards (a `src/AGENTS.md` sentence overstating which closures are split by target kind, a comment's "7 to 10%" against measured +10.6% medians and +6.8% totals, and a six-line dirty-check-and-lookup tail that `recordDirectWrite` does in one line), so the attempt does not fail and the findings go to the same implementer as a fix-up, which is not its retry; the duplicated-tail swap is adopted only if an interleaved re-measure shows `array fill` and `many dependencies` indistinguishable or faster, per the maintainer's methodology directive - if wrong, a fix-up round and about ten minutes of measurement are spent on a comment, a sentence and six lines. +Verdict: 04 - approve - Attempt 04a measured only and failed the gate after the four-run re-measurement; attempt 04b found the mechanism (arrays reaching the object batch path deoptimized its subscriber load, costing `many dependencies` +10.6% on medians interleaved against `87c8823`), fixed it in `b595300`, and round 1 returned three low standards findings fixed or measured in the amend `2dbcee2`; round 2 found nothing. After the fix no existing case is slower than `87c8823` beyond its A/A spread, `array fill (hookless)` is 50% faster, manifests read 2002.0.0 and the handbook is untouched. Landed by fast-forward at `2dbcee2`. +Ruling: The ticket 04 worktree `.claude/worktrees/agent-a0d5861de6e8b46d8` and its branch `worktree-agent-a0d5861de6e8b46d8` stay on disk - the merger's `git worktree remove` exited 255 with "Operation not permitted" from the sandbox and `git branch -d` exited 1 because the worktree is registered - if wrong, nothing is lost, the branch is at `2dbcee2`, which the change branch contains, and the closing report carries the removal commands. +Ruling: Q8 a - the change branch `fix/wrap-fill-and-copywithin` at `2dbcee2` is pushed to `origin` with `git push -u origin fix/wrap-fill-and-copywithin`; `git ls-remote` shows `2dbcee2a0597cdd8e5717c01a0fb1a0afca521a8 refs/heads/fix/wrap-fill-and-copywithin` on the remote, while the sandbox refused the upstream config write ("error: unable to write upstream branch configuration"), so the tracking link is set by the user outside the sandbox - if wrong, a second push without `-u` goes to the same remote branch and nothing diverges. diff --git a/.do/review/01-a7e06bc..87c8823.diff b/.do/review/01-a7e06bc..87c8823.diff new file mode 100644 index 0000000..875b3ed --- /dev/null +++ b/.do/review/01-a7e06bc..87c8823.diff @@ -0,0 +1,105 @@ +# Review package: ticket 01, a7e06bc..87c8823 + +## Commits +### 87c8823 +perf(state): add array fill benchmark case for the baseline + +The wrap-fill-and-copywithin change alters how many wakes one fill call +causes. This case subscribes one effect that reads every index of a +hookless array and times fill calls that change every element, so the +unchanged library records the baseline. Typed perf because it measures +performance; no library behaviour changes. + + +## Files changed + scripts/benchmark.ts | 20 ++++++++++++++++++++ + 1 file changed, 20 insertions(+) + +## Diff +diff --git a/scripts/benchmark.ts b/scripts/benchmark.ts +index 9685a7b..30a3d81 100644 +--- a/scripts/benchmark.ts ++++ b/scripts/benchmark.ts +@@ -5,20 +5,22 @@ import { batch, derive, effect, state } from '../src/index.ts' + import type { StateHooks } from '../src/types.ts' + + const DEFAULT_RUN_COUNT = 10 + const LOOP_LENGTH = 1_000_000 + const ERROR_FREQUENCY = 1000 + const WARMUP_RUNS = 3 + const BENCH_RUNS = 7 + + const EFFECT_TRIGGER_WRITES = 50_000 + const ARRAY_PUSH_POP_CALLS = 50_000 ++const ARRAY_FILL_CALLS = 50_000 ++const ARRAY_FILL_LENGTH = 3 + const MULTI_SOURCE_COUNT = 100 + const MULTI_SOURCE_ITERATIONS = 100 + const DERIVE_CHAIN_DEPTH = 10 + const DERIVE_CHAIN_ITERATIONS = 1_000 + const STATE_CREATION_COUNT = 100_000 + const STATE_READ_COUNT = 1_000_000 + const SINGLE_SUB_WRITES = 100_000 + const MANY_SUB_COUNT = 100 + const MANY_SUB_WRITES = 10_000 + const DISJOINT_PROP_COUNT = 100 +@@ -310,20 +312,34 @@ function arrayPushPop(items: number[]): number { + } + const end = performance.now() + cleanup() + return end - start + } + + const arrayPushPopHookless = (): number => arrayPushPop(state([])) + + const arrayPushPopHooks = (): number => arrayPushPop(state([], noopStateHooks)) + ++// The effect reads every index because fill never changes length: a length-only reader would not wake at all. ++// Alternating the fill value changes every element on every call ++function arrayFillHookless(): number { ++ const items = state(new Array(ARRAY_FILL_LENGTH).fill(0)) ++ const cleanup: Unsubscribe = effect((): void => { ++ for (let i = 0; i < items.length; i++) void items[i] ++ }) ++ const start = performance.now() ++ for (let i = 0; i < ARRAY_FILL_CALLS; i++) items.fill((i % 2) + 1) ++ const end = performance.now() ++ cleanup() ++ return end - start ++} ++ + function manyDependencies(): number { + const sources = Array.from( + { + length: MULTI_SOURCE_COUNT, + }, + ( + _: unknown, + i: number + ): { + v: number +@@ -600,20 +616,24 @@ const suite: SuiteEntry[] = [ + name: 'effect triggers ', + }, + { + fn: arrayPushPopHookless, + name: 'array push/pop (hookless) ', + }, + { + fn: arrayPushPopHooks, + name: 'array push/pop (hooks) ', + }, ++ { ++ fn: arrayFillHookless, ++ name: 'array fill (hookless) ', ++ }, + { + fn: manyDependencies, + name: 'many dependencies ', + }, + { + fn: deriveChainDepth10, + name: 'derive chain depth 10 ', + }, + { + fn: update100StatesIndividual, diff --git a/.do/review/02-87c8823..4f85493.diff b/.do/review/02-87c8823..4f85493.diff new file mode 100644 index 0000000..e3ab809 --- /dev/null +++ b/.do/review/02-87c8823..4f85493.diff @@ -0,0 +1,448 @@ +# Review package: ticket 02, 87c8823..4f85493 + +## Commits +### 4f85493 +fix(state): wrap fill and copyWithin through the Proxy + +A reactive array handed out the native fill and copyWithin, so each element write woke an every-index subscriber on its own. Add both names to the mutating method set so they run in a batch and wake each subscriber once after the call returns. + +Cover wake counts, batches, bounds and frozen arrays on both kinds of state, and add fill and copyWithin to the property-based array test. + + +## Files changed + src/index.ts | 14 ++++- + tests/array-methods.test.ts | 99 ++++++++++++++++++++++++++++++++++ + tests/property-array-mutations.test.ts | 86 ++++++++++++++++++++++++++++- + 3 files changed, 195 insertions(+), 4 deletions(-) + +## Diff +diff --git a/src/index.ts b/src/index.ts +index e103daa..2a3f490 100644 +--- a/src/index.ts ++++ b/src/index.ts +@@ -87,21 +87,31 @@ let hasFallbackSubs = false + const proxyCache: WeakMap = new WeakMap() + const proxyCacheSubs: WeakMap> = new WeakMap>() + // Effects by the property they read, per subscriber Set. It may hold stale entries: scheduling re-checks __reads, + // so it only has to be a superset of the true readers. Kept off the Set: an own property there cost ~8% on + // single-subscriber writes in the benchmark suite + const subscribersByProp: WeakMap, Map>> = new WeakMap< + Set, + Map> + >() + +-const MUTATING_ARRAY_METHODS: Set = new Set(['push', 'pop', 'shift', 'unshift', 'splice', 'sort', 'reverse']) ++const MUTATING_ARRAY_METHODS: Set = new Set([ ++ 'push', ++ 'pop', ++ 'shift', ++ 'unshift', ++ 'splice', ++ 'sort', ++ 'reverse', ++ 'fill', ++ 'copyWithin', ++]) + + // A native mutating array method, or the wrapper the Proxy hands out in its place + type ArrayMethod = (...args: unknown[]) => unknown + // Keyed by the native method: the wrapper acts on its call receiver, so one wrapper serves every array + const arrayMethodCache: WeakMap = new WeakMap() + // Only these two take the direct path (Q52 a); any other function under their names keeps the Proxy receiver + const NATIVE_PUSH: ArrayMethod = Array.prototype.push as ArrayMethod + const NATIVE_POP: ArrayMethod = Array.prototype.pop as ArrayMethod + + type ArrayTarget = ProxyTarget & unknown[] +@@ -204,21 +214,21 @@ function wrapArrayMethod(nativeMethod: ArrayMethod): ArrayMethod { + } + arrayMethodCache.set(nativeMethod, wrapped) + } + return wrapped + } + + // The wrappers the cache hands out for the native push and pop: a Proxy receiver whose [HOOKS] slot holds a + // context is a reactive array and takes the direct path; any other receiver, such as push.call(otherArray), runs + // the native method in a batch. A nullish receiver has no slot to read, so the native method raises its own + // TypeError. The raw target carries the same slot but never enters the traps, so a call on it runs the native +-// method silently, as the other five wrappers and a direct write on the raw object do (Q71 b) ++// method silently, as the other seven wrappers and a direct write on the raw object do (Q71 b) + function directPushWrapper(this: unknown, ...items: unknown[]): unknown { + const context = (this as ProxyObject | null | undefined)?.[HOOKS] + if (!(context instanceof DirectArrayContext) || this === context.target) { + return applyInBatch(NATIVE_PUSH, this, items) + } + return applyInBatch(directPush as ArrayMethod, context, items) + } + + function directPopWrapper(this: unknown, ...args: unknown[]): unknown { + const context = (this as ProxyObject | null | undefined)?.[HOOKS] +diff --git a/tests/array-methods.test.ts b/tests/array-methods.test.ts +index 84fc79d..fd9205e 100644 +--- a/tests/array-methods.test.ts ++++ b/tests/array-methods.test.ts +@@ -100,36 +100,135 @@ describe('Array Mutating Methods', { + it('returns what the native method returns and leaves the same contents', (): void => { + const descending = (a: number, b: number): number => b - a + const cases: [string, (list: number[]) => unknown][] = [ + ['push', (list: number[]): unknown => list.push(4, 5)], + ['pop', (list: number[]): unknown => list.pop()], + ['shift', (list: number[]): unknown => list.shift()], + ['unshift', (list: number[]): unknown => list.unshift(8, 9)], + ['splice', (list: number[]): unknown => list.splice(1, 2, 9)], + ['sort', (list: number[]): unknown => list.sort(descending)], + ['reverse', (list: number[]): unknown => list.reverse()], ++ ['fill', (list: number[]): unknown => list.fill(9, -3, 3)], ++ ['copyWithin', (list: number[]): unknown => list.copyWithin(1, 2)], + ] + for (const [name, call] of cases) { + const plain = [0, 1, 2, 3] + const expected = call(plain) + for (const $list of bothKinds([0, 1, 2, 3])) { + const result = call($list) + assert.deepStrictEqual( + result === $list ? '' : result, + expected === plain ? '' : expected, + name + ) + assert.deepStrictEqual([...$list], plain, name) + } + } + }) + ++ it('wakes an effect reading every index once after fill and once after copyWithin', (): void => { ++ const cases: [string, (list: number[]) => unknown, number[]][] = [ ++ ['fill', (list: number[]): unknown => list.fill(9), [9, 9, 9]], ++ ['copyWithin', (list: number[]): unknown => list.copyWithin(0, 1), [2, 3, 3]], ++ ] ++ for (const [name, call, expected] of cases) { ++ for (const $list of bothKinds([1, 2, 3])) { ++ const seen: number[][] = [] ++ const dispose = effect((): void => { ++ seen.push([0, 1, 2].map((i: number): number => $list[i] as number)) ++ }) ++ ++ call($list) ++ ++ assert.deepStrictEqual(seen, [[1, 2, 3], expected], name) ++ dispose() ++ } ++ } ++ }) ++ ++ it('does not wake an effect when fill or copyWithin changes no element', (): void => { ++ const cases: [string, (list: number[]) => unknown, number[]][] = [ ++ ['fill', (list: number[]): unknown => list.fill(9), [9, 9, 9]], ++ ['copyWithin', (list: number[]): unknown => list.copyWithin(0, 1), [7, 7, 7]], ++ ] ++ for (const [name, call, initial] of cases) { ++ for (const $list of bothKinds(initial)) { ++ let runs = 0 ++ const dispose = effect((): void => { ++ runs++ ++ for (let i = 0; i < 3; i++) void $list[i] ++ }) ++ ++ call($list) ++ ++ assert.strictEqual(runs, 1, name) ++ dispose() ++ } ++ } ++ }) ++ ++ it('defers wakes from fill and copyWithin to the end of a user batch', (): void => { ++ for (const $list of bothKinds([1, 2, 3])) { ++ const seen: number[][] = [] ++ const dispose = effect((): void => { ++ seen.push([0, 1, 2].map((i: number): number => $list[i] as number)) ++ }) ++ ++ batch((): void => { ++ $list.fill(9, 0, 1) ++ $list.copyWithin(2, 0) ++ assert.strictEqual(seen.length, 1) ++ }) ++ ++ assert.deepStrictEqual(seen, [ ++ [1, 2, 3], ++ [9, 2, 9], ++ ]) ++ dispose() ++ } ++ }) ++ ++ it('resolves negative and out-of-range bounds in fill and copyWithin like the native method', (): void => { ++ for (const $list of bothKinds([1, 2, 3])) { ++ assert.strictEqual($list.fill(9, -2), $list) ++ assert.deepStrictEqual([...$list], [1, 9, 9]) ++ assert.strictEqual($list.fill(0, 1, 100), $list) ++ assert.deepStrictEqual([...$list], [1, 0, 0]) ++ assert.strictEqual($list.length, 3) ++ } ++ ++ for (const $list of bothKinds([1, 2, 3, 4])) { ++ assert.strictEqual($list.copyWithin(-2, 0), $list) ++ assert.deepStrictEqual([...$list], [1, 2, 1, 2]) ++ } ++ }) ++ ++ it('propagates the TypeError from fill on a frozen array and stays usable', (): void => { ++ const frozen = (): number[] => Object.freeze([1, 2, 3]) as number[] ++ for (const $list of [state(frozen()), state(frozen(), observingHooks)]) { ++ const $other = state({ n: 0 }) ++ let runs = 0 ++ const dispose = effect((): void => { ++ runs++ ++ void $other.n ++ }) ++ ++ assert.throws((): void => { ++ $list.fill(9) ++ }, TypeError) ++ ++ $other.n = 1 ++ assert.strictEqual(runs, 2) ++ dispose() ++ } ++ }) ++ + it('propagates the TypeError from push on a sealed array and stays usable', (): void => { + for (const $list of [state(Object.seal([1])), state(Object.seal([1]), observingHooks)]) { + let runs = 0 + const dispose = effect((): void => { + runs++ + void $list[0] + }) + + assert.throws((): void => { + $list.push(2) +diff --git a/tests/property-array-mutations.test.ts b/tests/property-array-mutations.test.ts +index af965c6..e2d8c71 100644 +--- a/tests/property-array-mutations.test.ts ++++ b/tests/property-array-mutations.test.ts +@@ -8,20 +8,34 @@ import { batch, effect, state } from '../src/index.ts' + * + * Uses model-based testing where a plain JavaScript Array serves as the + * oracle. Every operation is applied to both a reactive state array and + * a plain array, then their contents are compared. + * + * @see tests/PROPERTY_BASED_TESTING.md — Opportunity #1 + */ + + // --- Operation types (keys sorted per Biome useSortedKeys) --- + ++interface CopyWithinOp { ++ end: number | undefined ++ op: 'copyWithin' ++ start: number ++ target: number ++} ++ ++interface FillOp { ++ end: number | undefined ++ op: 'fill' ++ start: number | undefined ++ value: number ++} ++ + interface PopOp { + op: 'pop' + } + + interface PushOp { + args: number[] + op: 'push' + } + + interface ReverseOp { +@@ -41,26 +55,30 @@ interface SpliceOp { + items: number[] + op: 'splice' + start: number + } + + interface UnshiftOp { + args: number[] + op: 'unshift' + } + +-type ArrayOp = PopOp | PushOp | ReverseOp | ShiftOp | SortOp | SpliceOp | UnshiftOp ++type ArrayOp = CopyWithinOp | FillOp | PopOp | PushOp | ReverseOp | ShiftOp | SortOp | SpliceOp | UnshiftOp + + // --- Operations that return a meaningful value (not the array itself) --- + + type ValueReturningOp = PopOp | PushOp | ShiftOp | SpliceOp | UnshiftOp + ++// --- Operations that return the receiver --- ++ ++type ReceiverReturningOp = CopyWithinOp | FillOp | ReverseOp | SortOp ++ + // --- Arbitraries --- + + const pushArb: fc.Arbitrary = fc.record({ + args: fc.array( + fc.integer({ + max: 1000, + min: -1000, + }), + { + maxLength: 5, +@@ -111,30 +129,57 @@ const spliceArb: fc.Arbitrary = fc.record({ + }) + + const sortArb: fc.Arbitrary = fc.record({ + op: fc.constant('sort' as const), + }) + + const reverseArb: fc.Arbitrary = fc.record({ + op: fc.constant('reverse' as const), + }) + ++// Bounds reach past both ends of arrays of up to 20 elements, so negative and out-of-range cases occur ++const boundArb: fc.Arbitrary = fc.integer({ ++ max: 30, ++ min: -30, ++}) ++ ++const fillArb: fc.Arbitrary = fc.record({ ++ end: fc.option(boundArb, { nil: undefined }), ++ op: fc.constant('fill' as const), ++ start: fc.option(boundArb, { nil: undefined }), ++ value: fc.integer({ ++ max: 1000, ++ min: -1000, ++ }), ++}) ++ ++const copyWithinArb: fc.Arbitrary = fc.record({ ++ end: fc.option(boundArb, { nil: undefined }), ++ op: fc.constant('copyWithin' as const), ++ start: boundArb, ++ target: boundArb, ++}) ++ + const arrayOpArb: fc.Arbitrary = fc.oneof( + pushArb, + popArb, + shiftArb, + unshiftArb, + spliceArb, + sortArb, +- reverseArb ++ reverseArb, ++ fillArb, ++ copyWithinArb + ) + ++const receiverReturningOpArb: fc.Arbitrary = fc.oneof(sortArb, reverseArb, fillArb, copyWithinArb) ++ + const valueReturningOpArb: fc.Arbitrary = fc.oneof(pushArb, popArb, shiftArb, unshiftArb, spliceArb) + + const initialArrayArb: fc.Arbitrary = fc.array( + fc.integer({ + max: 1000, + min: -1000, + }), + { + maxLength: 20, + minLength: 0, +@@ -143,20 +188,26 @@ const initialArrayArb: fc.Arbitrary = fc.array( + + const opsArb: fc.Arbitrary = fc.array(arrayOpArb, { + maxLength: 15, + minLength: 1, + }) + + // --- Helpers --- + + function applyOp(arr: number[], op: ArrayOp): void { + switch (op.op) { ++ case 'copyWithin': ++ arr.copyWithin(op.target, op.start, op.end) ++ break ++ case 'fill': ++ arr.fill(op.value, op.start, op.end) ++ break + case 'pop': + arr.pop() + break + case 'push': + arr.push(...op.args) + break + case 'reverse': + arr.reverse() + break + case 'shift': +@@ -182,20 +233,33 @@ function applyOpWithReturn(arr: number[], op: ValueReturningOp): number | number + return arr.push(...op.args) + case 'shift': + return arr.shift() + case 'splice': + return arr.splice(op.start, op.deleteCount, ...op.items) + case 'unshift': + return arr.unshift(...op.args) + } + } + ++function applyOpWithReceiver(arr: number[], op: ReceiverReturningOp): number[] { ++ switch (op.op) { ++ case 'copyWithin': ++ return arr.copyWithin(op.target, op.start, op.end) ++ case 'fill': ++ return arr.fill(op.value, op.start, op.end) ++ case 'reverse': ++ return arr.reverse() ++ case 'sort': ++ return arr.sort() ++ } ++} ++ + // --- Tests --- + + describe('Property-Based: Array Mutations', { + concurrency: true, + timeout: 30000, + }, (): void => { + it('produces same contents as a plain array after arbitrary mutations', (): void => { + fc.assert( + fc.property(initialArrayArb, opsArb, (initial: number[], ops: ArrayOp[]): void => { + const $reactive = state([...initial]) +@@ -226,20 +290,38 @@ describe('Property-Based: Array Mutations', { + + assert.deepStrictEqual(reactiveResult, plainResult) + assert.deepStrictEqual([...$reactive], plain) + }), + { + numRuns: 300, + } + ) + }) + ++ it('returns the receiver from fill, copyWithin, sort and reverse', (): void => { ++ fc.assert( ++ fc.property(initialArrayArb, receiverReturningOpArb, (initial: number[], op: ReceiverReturningOp): void => { ++ const $reactive = state([...initial]) ++ const plain = [...initial] ++ ++ const reactiveResult = applyOpWithReceiver($reactive, op) ++ applyOpWithReceiver(plain, op) ++ ++ assert.strictEqual(reactiveResult, $reactive) ++ assert.deepStrictEqual([...$reactive], plain) ++ }), ++ { ++ numRuns: 300, ++ } ++ ) ++ }) ++ + it('fires effect at most once per batch of mutations', (): void => { + fc.assert( + fc.property(initialArrayArb, opsArb, (initial: number[], ops: ArrayOp[]): void => { + const $arr = state([...initial]) + const plain = [...initial] + let effectRuns = 0 + + const dispose = effect((): void => { + effectRuns++ + // Subscribe to length and all indices diff --git a/.do/review/02-87c8823..5b42960.diff b/.do/review/02-87c8823..5b42960.diff new file mode 100644 index 0000000..794aa62 --- /dev/null +++ b/.do/review/02-87c8823..5b42960.diff @@ -0,0 +1,452 @@ +# Review package: ticket 02, 87c8823..5b42960 + +## Commits +### 5b42960 +fix(state): wrap fill and copyWithin through the Proxy + +A reactive array hands out the native fill and copyWithin, so each +element write wakes an every-index subscriber on its own. Add both +names to the mutating method set so they run in a batch and wake each +subscriber once after the call returns. + +Cover wake counts, batches, bounds and frozen arrays on both kinds of +state, and add fill and copyWithin to the property-based array test. + + +## Files changed + src/index.ts | 14 ++++- + tests/array-methods.test.ts | 99 ++++++++++++++++++++++++++++++++++ + tests/property-array-mutations.test.ts | 86 ++++++++++++++++++++++++++++- + 3 files changed, 195 insertions(+), 4 deletions(-) + +## Diff +diff --git a/src/index.ts b/src/index.ts +index e103daa..2a3f490 100644 +--- a/src/index.ts ++++ b/src/index.ts +@@ -87,21 +87,31 @@ let hasFallbackSubs = false + const proxyCache: WeakMap = new WeakMap() + const proxyCacheSubs: WeakMap> = new WeakMap>() + // Effects by the property they read, per subscriber Set. It may hold stale entries: scheduling re-checks __reads, + // so it only has to be a superset of the true readers. Kept off the Set: an own property there cost ~8% on + // single-subscriber writes in the benchmark suite + const subscribersByProp: WeakMap, Map>> = new WeakMap< + Set, + Map> + >() + +-const MUTATING_ARRAY_METHODS: Set = new Set(['push', 'pop', 'shift', 'unshift', 'splice', 'sort', 'reverse']) ++const MUTATING_ARRAY_METHODS: Set = new Set([ ++ 'push', ++ 'pop', ++ 'shift', ++ 'unshift', ++ 'splice', ++ 'sort', ++ 'reverse', ++ 'fill', ++ 'copyWithin', ++]) + + // A native mutating array method, or the wrapper the Proxy hands out in its place + type ArrayMethod = (...args: unknown[]) => unknown + // Keyed by the native method: the wrapper acts on its call receiver, so one wrapper serves every array + const arrayMethodCache: WeakMap = new WeakMap() + // Only these two take the direct path (Q52 a); any other function under their names keeps the Proxy receiver + const NATIVE_PUSH: ArrayMethod = Array.prototype.push as ArrayMethod + const NATIVE_POP: ArrayMethod = Array.prototype.pop as ArrayMethod + + type ArrayTarget = ProxyTarget & unknown[] +@@ -204,21 +214,21 @@ function wrapArrayMethod(nativeMethod: ArrayMethod): ArrayMethod { + } + arrayMethodCache.set(nativeMethod, wrapped) + } + return wrapped + } + + // The wrappers the cache hands out for the native push and pop: a Proxy receiver whose [HOOKS] slot holds a + // context is a reactive array and takes the direct path; any other receiver, such as push.call(otherArray), runs + // the native method in a batch. A nullish receiver has no slot to read, so the native method raises its own + // TypeError. The raw target carries the same slot but never enters the traps, so a call on it runs the native +-// method silently, as the other five wrappers and a direct write on the raw object do (Q71 b) ++// method silently, as the other seven wrappers and a direct write on the raw object do (Q71 b) + function directPushWrapper(this: unknown, ...items: unknown[]): unknown { + const context = (this as ProxyObject | null | undefined)?.[HOOKS] + if (!(context instanceof DirectArrayContext) || this === context.target) { + return applyInBatch(NATIVE_PUSH, this, items) + } + return applyInBatch(directPush as ArrayMethod, context, items) + } + + function directPopWrapper(this: unknown, ...args: unknown[]): unknown { + const context = (this as ProxyObject | null | undefined)?.[HOOKS] +diff --git a/tests/array-methods.test.ts b/tests/array-methods.test.ts +index 84fc79d..fd9205e 100644 +--- a/tests/array-methods.test.ts ++++ b/tests/array-methods.test.ts +@@ -100,36 +100,135 @@ describe('Array Mutating Methods', { + it('returns what the native method returns and leaves the same contents', (): void => { + const descending = (a: number, b: number): number => b - a + const cases: [string, (list: number[]) => unknown][] = [ + ['push', (list: number[]): unknown => list.push(4, 5)], + ['pop', (list: number[]): unknown => list.pop()], + ['shift', (list: number[]): unknown => list.shift()], + ['unshift', (list: number[]): unknown => list.unshift(8, 9)], + ['splice', (list: number[]): unknown => list.splice(1, 2, 9)], + ['sort', (list: number[]): unknown => list.sort(descending)], + ['reverse', (list: number[]): unknown => list.reverse()], ++ ['fill', (list: number[]): unknown => list.fill(9, -3, 3)], ++ ['copyWithin', (list: number[]): unknown => list.copyWithin(1, 2)], + ] + for (const [name, call] of cases) { + const plain = [0, 1, 2, 3] + const expected = call(plain) + for (const $list of bothKinds([0, 1, 2, 3])) { + const result = call($list) + assert.deepStrictEqual( + result === $list ? '' : result, + expected === plain ? '' : expected, + name + ) + assert.deepStrictEqual([...$list], plain, name) + } + } + }) + ++ it('wakes an effect reading every index once after fill and once after copyWithin', (): void => { ++ const cases: [string, (list: number[]) => unknown, number[]][] = [ ++ ['fill', (list: number[]): unknown => list.fill(9), [9, 9, 9]], ++ ['copyWithin', (list: number[]): unknown => list.copyWithin(0, 1), [2, 3, 3]], ++ ] ++ for (const [name, call, expected] of cases) { ++ for (const $list of bothKinds([1, 2, 3])) { ++ const seen: number[][] = [] ++ const dispose = effect((): void => { ++ seen.push([0, 1, 2].map((i: number): number => $list[i] as number)) ++ }) ++ ++ call($list) ++ ++ assert.deepStrictEqual(seen, [[1, 2, 3], expected], name) ++ dispose() ++ } ++ } ++ }) ++ ++ it('does not wake an effect when fill or copyWithin changes no element', (): void => { ++ const cases: [string, (list: number[]) => unknown, number[]][] = [ ++ ['fill', (list: number[]): unknown => list.fill(9), [9, 9, 9]], ++ ['copyWithin', (list: number[]): unknown => list.copyWithin(0, 1), [7, 7, 7]], ++ ] ++ for (const [name, call, initial] of cases) { ++ for (const $list of bothKinds(initial)) { ++ let runs = 0 ++ const dispose = effect((): void => { ++ runs++ ++ for (let i = 0; i < 3; i++) void $list[i] ++ }) ++ ++ call($list) ++ ++ assert.strictEqual(runs, 1, name) ++ dispose() ++ } ++ } ++ }) ++ ++ it('defers wakes from fill and copyWithin to the end of a user batch', (): void => { ++ for (const $list of bothKinds([1, 2, 3])) { ++ const seen: number[][] = [] ++ const dispose = effect((): void => { ++ seen.push([0, 1, 2].map((i: number): number => $list[i] as number)) ++ }) ++ ++ batch((): void => { ++ $list.fill(9, 0, 1) ++ $list.copyWithin(2, 0) ++ assert.strictEqual(seen.length, 1) ++ }) ++ ++ assert.deepStrictEqual(seen, [ ++ [1, 2, 3], ++ [9, 2, 9], ++ ]) ++ dispose() ++ } ++ }) ++ ++ it('resolves negative and out-of-range bounds in fill and copyWithin like the native method', (): void => { ++ for (const $list of bothKinds([1, 2, 3])) { ++ assert.strictEqual($list.fill(9, -2), $list) ++ assert.deepStrictEqual([...$list], [1, 9, 9]) ++ assert.strictEqual($list.fill(0, 1, 100), $list) ++ assert.deepStrictEqual([...$list], [1, 0, 0]) ++ assert.strictEqual($list.length, 3) ++ } ++ ++ for (const $list of bothKinds([1, 2, 3, 4])) { ++ assert.strictEqual($list.copyWithin(-2, 0), $list) ++ assert.deepStrictEqual([...$list], [1, 2, 1, 2]) ++ } ++ }) ++ ++ it('propagates the TypeError from fill on a frozen array and stays usable', (): void => { ++ const frozen = (): number[] => Object.freeze([1, 2, 3]) as number[] ++ for (const $list of [state(frozen()), state(frozen(), observingHooks)]) { ++ const $other = state({ n: 0 }) ++ let runs = 0 ++ const dispose = effect((): void => { ++ runs++ ++ void $other.n ++ }) ++ ++ assert.throws((): void => { ++ $list.fill(9) ++ }, TypeError) ++ ++ $other.n = 1 ++ assert.strictEqual(runs, 2) ++ dispose() ++ } ++ }) ++ + it('propagates the TypeError from push on a sealed array and stays usable', (): void => { + for (const $list of [state(Object.seal([1])), state(Object.seal([1]), observingHooks)]) { + let runs = 0 + const dispose = effect((): void => { + runs++ + void $list[0] + }) + + assert.throws((): void => { + $list.push(2) +diff --git a/tests/property-array-mutations.test.ts b/tests/property-array-mutations.test.ts +index af965c6..e2d8c71 100644 +--- a/tests/property-array-mutations.test.ts ++++ b/tests/property-array-mutations.test.ts +@@ -8,20 +8,34 @@ import { batch, effect, state } from '../src/index.ts' + * + * Uses model-based testing where a plain JavaScript Array serves as the + * oracle. Every operation is applied to both a reactive state array and + * a plain array, then their contents are compared. + * + * @see tests/PROPERTY_BASED_TESTING.md — Opportunity #1 + */ + + // --- Operation types (keys sorted per Biome useSortedKeys) --- + ++interface CopyWithinOp { ++ end: number | undefined ++ op: 'copyWithin' ++ start: number ++ target: number ++} ++ ++interface FillOp { ++ end: number | undefined ++ op: 'fill' ++ start: number | undefined ++ value: number ++} ++ + interface PopOp { + op: 'pop' + } + + interface PushOp { + args: number[] + op: 'push' + } + + interface ReverseOp { +@@ -41,26 +55,30 @@ interface SpliceOp { + items: number[] + op: 'splice' + start: number + } + + interface UnshiftOp { + args: number[] + op: 'unshift' + } + +-type ArrayOp = PopOp | PushOp | ReverseOp | ShiftOp | SortOp | SpliceOp | UnshiftOp ++type ArrayOp = CopyWithinOp | FillOp | PopOp | PushOp | ReverseOp | ShiftOp | SortOp | SpliceOp | UnshiftOp + + // --- Operations that return a meaningful value (not the array itself) --- + + type ValueReturningOp = PopOp | PushOp | ShiftOp | SpliceOp | UnshiftOp + ++// --- Operations that return the receiver --- ++ ++type ReceiverReturningOp = CopyWithinOp | FillOp | ReverseOp | SortOp ++ + // --- Arbitraries --- + + const pushArb: fc.Arbitrary = fc.record({ + args: fc.array( + fc.integer({ + max: 1000, + min: -1000, + }), + { + maxLength: 5, +@@ -111,30 +129,57 @@ const spliceArb: fc.Arbitrary = fc.record({ + }) + + const sortArb: fc.Arbitrary = fc.record({ + op: fc.constant('sort' as const), + }) + + const reverseArb: fc.Arbitrary = fc.record({ + op: fc.constant('reverse' as const), + }) + ++// Bounds reach past both ends of arrays of up to 20 elements, so negative and out-of-range cases occur ++const boundArb: fc.Arbitrary = fc.integer({ ++ max: 30, ++ min: -30, ++}) ++ ++const fillArb: fc.Arbitrary = fc.record({ ++ end: fc.option(boundArb, { nil: undefined }), ++ op: fc.constant('fill' as const), ++ start: fc.option(boundArb, { nil: undefined }), ++ value: fc.integer({ ++ max: 1000, ++ min: -1000, ++ }), ++}) ++ ++const copyWithinArb: fc.Arbitrary = fc.record({ ++ end: fc.option(boundArb, { nil: undefined }), ++ op: fc.constant('copyWithin' as const), ++ start: boundArb, ++ target: boundArb, ++}) ++ + const arrayOpArb: fc.Arbitrary = fc.oneof( + pushArb, + popArb, + shiftArb, + unshiftArb, + spliceArb, + sortArb, +- reverseArb ++ reverseArb, ++ fillArb, ++ copyWithinArb + ) + ++const receiverReturningOpArb: fc.Arbitrary = fc.oneof(sortArb, reverseArb, fillArb, copyWithinArb) ++ + const valueReturningOpArb: fc.Arbitrary = fc.oneof(pushArb, popArb, shiftArb, unshiftArb, spliceArb) + + const initialArrayArb: fc.Arbitrary = fc.array( + fc.integer({ + max: 1000, + min: -1000, + }), + { + maxLength: 20, + minLength: 0, +@@ -143,20 +188,26 @@ const initialArrayArb: fc.Arbitrary = fc.array( + + const opsArb: fc.Arbitrary = fc.array(arrayOpArb, { + maxLength: 15, + minLength: 1, + }) + + // --- Helpers --- + + function applyOp(arr: number[], op: ArrayOp): void { + switch (op.op) { ++ case 'copyWithin': ++ arr.copyWithin(op.target, op.start, op.end) ++ break ++ case 'fill': ++ arr.fill(op.value, op.start, op.end) ++ break + case 'pop': + arr.pop() + break + case 'push': + arr.push(...op.args) + break + case 'reverse': + arr.reverse() + break + case 'shift': +@@ -182,20 +233,33 @@ function applyOpWithReturn(arr: number[], op: ValueReturningOp): number | number + return arr.push(...op.args) + case 'shift': + return arr.shift() + case 'splice': + return arr.splice(op.start, op.deleteCount, ...op.items) + case 'unshift': + return arr.unshift(...op.args) + } + } + ++function applyOpWithReceiver(arr: number[], op: ReceiverReturningOp): number[] { ++ switch (op.op) { ++ case 'copyWithin': ++ return arr.copyWithin(op.target, op.start, op.end) ++ case 'fill': ++ return arr.fill(op.value, op.start, op.end) ++ case 'reverse': ++ return arr.reverse() ++ case 'sort': ++ return arr.sort() ++ } ++} ++ + // --- Tests --- + + describe('Property-Based: Array Mutations', { + concurrency: true, + timeout: 30000, + }, (): void => { + it('produces same contents as a plain array after arbitrary mutations', (): void => { + fc.assert( + fc.property(initialArrayArb, opsArb, (initial: number[], ops: ArrayOp[]): void => { + const $reactive = state([...initial]) +@@ -226,20 +290,38 @@ describe('Property-Based: Array Mutations', { + + assert.deepStrictEqual(reactiveResult, plainResult) + assert.deepStrictEqual([...$reactive], plain) + }), + { + numRuns: 300, + } + ) + }) + ++ it('returns the receiver from fill, copyWithin, sort and reverse', (): void => { ++ fc.assert( ++ fc.property(initialArrayArb, receiverReturningOpArb, (initial: number[], op: ReceiverReturningOp): void => { ++ const $reactive = state([...initial]) ++ const plain = [...initial] ++ ++ const reactiveResult = applyOpWithReceiver($reactive, op) ++ applyOpWithReceiver(plain, op) ++ ++ assert.strictEqual(reactiveResult, $reactive) ++ assert.deepStrictEqual([...$reactive], plain) ++ }), ++ { ++ numRuns: 300, ++ } ++ ) ++ }) ++ + it('fires effect at most once per batch of mutations', (): void => { + fc.assert( + fc.property(initialArrayArb, opsArb, (initial: number[], ops: ArrayOp[]): void => { + const $arr = state([...initial]) + const plain = [...initial] + let effectRuns = 0 + + const dispose = effect((): void => { + effectRuns++ + // Subscribe to length and all indices diff --git a/.do/review/03-5b42960..8464f3f.diff b/.do/review/03-5b42960..8464f3f.diff new file mode 100644 index 0000000..20acb97 --- /dev/null +++ b/.do/review/03-5b42960..8464f3f.diff @@ -0,0 +1,320 @@ +# Review package: ticket 03, 5b42960..8464f3f + +## Commits +### 8464f3f +docs(state): name nine mutating array methods in spec and agent docs + +The main array-mutation spec purpose and requirement, the root key +behaviors and the source wrapArrayMethod row now count fill and +copyWithin. + +### 17a14c5 +test(state): pin loop detection and hooks for fill and copyWithin + +A sync effect that only calls fill raises nothing and raises once it +also read a written index. copyWithin raises only when its source and +target ranges overlap. Child and microtask effects raise nothing. + +On hooked state, onRead sees the method name and every property read, +and onWrite sees every property written and no other. + + +## Files changed + AGENTS.md | 2 +- + openspec/specs/array-mutation/spec.md | 4 +- + src/AGENTS.md | 2 +- + tests/infinite-loop.test.ts | 117 ++++++++++++++++++++++++++++++++++ + tests/state-hooks.test.ts | 42 ++++++++++++ + 5 files changed, 163 insertions(+), 4 deletions(-) + +## Diff +diff --git a/AGENTS.md b/AGENTS.md +index cee1f32..38443eb 100644 +--- a/AGENTS.md ++++ b/AGENTS.md +@@ -26,21 +26,21 @@ ## Architecture + - `onIdle(callback: IdleCallback): Unsubscribe` — runs the callback each time the graph goes idle after doing work (no effect queued, no microtask flush armed); a pending async computation does not keep the graph busy + - `until(predicate: () => T, options?: UntilOptions): Promise` — resolves once the predicate is truthy with no derive pending; `timeout` and `signal` reject; throws inside a reactive scope + - Types: `Unsubscribe`, `EffectCallback`, `EffectName`, `DerivedValue`, `EffectSchedule`, `EffectOptions`, `UntilOptions`, `IdleCallback` + - Subpath `@nerdalytics/beacon/hooks`: hook types and `composeHook` + + **Key behaviors:** + - Dependency tracking via WeakMaps + Symbols at the property level + - Nested objects automatically wrapped in Proxies (deep reactivity) + - Infinite loop detection: throws if effect writes to a state property it reads from (child effects and microtask effects are exempt; microtask re-runs stop after 100 chained microtask flushes with a `console.error`) + - Cyclic dependencies between different effects: allowed via queue-based non-recursive `flushEffects` +-- Array mutating methods intercepted on state with and without hooks: `push`, `pop`, `shift`, `unshift`, `splice`, `sort`, `reverse` ++- Array mutating methods intercepted on state with and without hooks: `push`, `pop`, `shift`, `unshift`, `splice`, `sort`, `reverse`, `fill`, `copyWithin` + - Each runs through the Proxy inside a batch, so it wakes each subscriber of the array at most once per call, is subject to the loop check inside an effect, and is seen by the read and write hooks + - Frozen/sealed objects: fallback to WeakMap storage + - `derive()` must be disposed to prevent memory leaks (creates internal state + effect pair) + + ## Versioning + + Epoch Semantic Versioning: `{EPOCH * 1000 + MAJOR}.MINOR.PATCH` + - Current: v2002.0.0 (`package.json` and `jsr.json` must match) + + Commit format: `(): ` — imperative, present tense, no period, under 72 chars. +diff --git a/openspec/specs/array-mutation/spec.md b/openspec/specs/array-mutation/spec.md +index b0bd9b0..8ed4f3b 100644 +--- a/openspec/specs/array-mutation/spec.md ++++ b/openspec/specs/array-mutation/spec.md +@@ -1,20 +1,20 @@ + # array-mutation Specification + + ## Purpose + +-Defines how a mutating array method (`push`, `pop`, `shift`, `unshift`, `splice`, `sort`, `reverse`) called on reactive state notifies subscribers, composes with batches, is checked for self-feeding loops, and is observed by state hooks, identically for state created with and without hooks; and that the presence of a hook never changes which subscribers a write wakes or when. ++Defines how a mutating array method (`push`, `pop`, `shift`, `unshift`, `splice`, `sort`, `reverse`, `fill`, `copyWithin`) called on reactive state notifies subscribers, composes with batches, is checked for self-feeding loops, and is observed by state hooks, identically for state created with and without hooks; and that the presence of a hook never changes which subscribers a write wakes or when. + + ## Requirements + + ### Requirement: Mutating methods behave the same with and without hooks +-Reactive state SHALL expose the same behavior for the seven mutating array methods whether the state was created with hooks or without. Hooks SHALL only observe; they SHALL NOT change which subscribers wake, how often, or whether a loop error is raised. ++Reactive state SHALL expose the same behavior for the nine mutating array methods (`push`, `pop`, `shift`, `unshift`, `splice`, `sort`, `reverse`, `fill`, `copyWithin`) whether the state was created with hooks or without. Hooks SHALL only observe; they SHALL NOT change which subscribers wake, how often, or whether a loop error is raised. + + #### Scenario: Same wake count on both kinds of state + - **WHEN** an effect reads index 0 and `length` of an empty reactive array, and `push(1)` is called from outside any effect, on state created without hooks and on state created with hooks + - **THEN** the effect re-runs exactly once on each + + #### Scenario: Same loop outcome on both kinds of state + - **WHEN** a synchronous top-level effect calls a mutating method on a reactive array, on state created without hooks and on state created with hooks + - **THEN** both raise the same infinite-loop error + + ### Requirement: Hook presence does not change notification +diff --git a/src/AGENTS.md b/src/AGENTS.md +index 7d366f1..ca2b18e 100644 +--- a/src/AGENTS.md ++++ b/src/AGENTS.md +@@ -89,21 +89,21 @@ ### Proxy Handlers + |----------|---------| + | `trackDependency` | First run: adds `currentEffect` to the target's subscribers, then `recordEffectRead`. Re-run (`isTrackingOnly`): `recordEffectRead` in silent mode | + | `recordEffectRead` | Non-silent: records into `__deps`/`__reads` + fires `onDependencyAdd`. Silent: advances the read-list replay while it matches, else replays the prefix into temp collections and falls back to `recordSilentRead` | + | `recordSilentRead` | Records into `rerunTempDeps`/`rerunTempReads` (and `buildingReadList`) without hooks or subscriber registration | + | `readListMatchesRead` | Checks the next `[target, prop]` pair of the replayed read list | + | `replayReadListToTempCollections` | Copies the matched read-list prefix into fresh temp collections when the fast path breaks | + | `wrapNestedObject` | Returns the nested object's `[PROXY]`, else `state()` (which finds a non-extensible object's Proxy in `proxyCache` or wraps it) | + | `isInternalSymbol` | Guards access to `SUBSCRIBERS`, `PROXY`, `HOOKS` symbols | + | `getWrappedArrayMethod` | Returns the wrapper for a mutating array method read from an array target (`Array.isArray` first, so plain-object reads pay one call and one check; then the `arrayMethodCache` hit, before the method-name check). Its body is a read-path hot spot: any change to it, even code a plain-object read never reaches, cost 2 to 5% on the read benchmark, so the direct `push`/`pop` dispatch lives in the cached wrappers | + | `applyInBatch` | Runs a method on a receiver inside the hookless batch bookkeeping (`batchDepth++`, then `abortBatch` or `endBatch`, without the `batch(fn)` closure); shared by the Proxy-receiver wrappers and the direct `push`/`pop` | +-| `wrapArrayMethod` | Wraps a native mutating array method once, cached in `arrayMethodCache`: the native `push` and `pop` get `directPushWrapper`/`directPopWrapper`; the other five get a wrapper that applies them via `applyInBatch` to the call receiver (the Proxy), so every read and write passes the traps (seen by `onRead`/`onWrite`, subject to the loop check inside an effect) and each subscriber wakes at most once per call | ++| `wrapArrayMethod` | Wraps a native mutating array method once, cached in `arrayMethodCache`: the native `push` and `pop` get `directPushWrapper`/`directPopWrapper`; the other seven get a wrapper that applies them via `applyInBatch` to the call receiver (the Proxy), so every read and write passes the traps (seen by `onRead`/`onWrite`, subject to the loop check inside an effect) and each subscriber wakes at most once per call | + | `directPushWrapper` / `directPopWrapper` | Read the receiver's `[HOOKS]` slot (optional chaining, so a nullish receiver reads no slot): a `DirectArrayContext` (checked with `instanceof`, so only a reactive array's slot qualifies) on a receiver that is not the context's own raw target runs `directPush`/`directPop` on it inside `applyInBatch`; any other receiver (such as `push.call(otherArray)`, the raw array target, `null` or `undefined`) gets the native method inside `applyInBatch`: the raw target never enters the traps, so the call is silent (no hook call, no wake, Q71 b), and a nullish receiver raises the native TypeError | + | `hooksSlotFor` | What `state()` stores in the `[HOOKS]` slot: for an array target a new `DirectArrayContext`, otherwise `undefined` (no slot) | + | `directPush` / `directPop` | Run the native method on the raw target inside `applyInBatch` and replay by hand what the traps would have done: `push` reads `length`, then sets each new index and `length`; `pop` reads `length` and the last index, deletes that index (`onDelete`, readers of the index scheduled, no loop or equal check) and sets `length`; `pop` returns the nested wrapper the get trap would | + | `recordDirectRead` | The get trap's bookkeeping for a property the native method read: `trackDependency` inside an effect, then `onRead` | + | `recordDirectWrite` | The set trap's bookkeeping for a write the native method already performed: the loop check (inside an effect only), the equal skip, then `onWrite`, shared by both paths; only the scheduling branches: per property inside an effect (plus `length` for an index write that grew the array), one `ARRAY_DIRTY_PROPS` entry per batch outside. A write that grew the array went to an absent index, so it is never skipped as equal (Q65 a) | + | `isUnchangedWrite` | The equal skip of every set path: `Object.is` against the current read, and for an equal `undefined` a `prop in rawTarget` check, so a write of `undefined` to an absent property counts as a change (Q65 a); a read that is not `undefined` proves the property exists | + | `performUnbatchedWrite` | Outside any effect and batch: skips an unchanged write (`isUnchangedWrite`), skips scheduling when the target has no subscribers, else notifies per property | + | `performWrite` | Checks infinite loop, skips an unchanged write (`isUnchangedWrite`), notifies subscribers | + | `checkInfiniteLoop` | Throws if effect writes to a property it read (exempt: child effects, microtask effects) | + | `handleBatchFastPath` | Defers writes to `dirtyTargets` during batch (skips an unchanged write via `isUnchangedWrite`, calls `onWrite`, skips the loop check); inlines the `findSubscribers` lookup, and only for a target not yet dirty | +diff --git a/tests/infinite-loop.test.ts b/tests/infinite-loop.test.ts +index c7bf841..249d985 100644 +--- a/tests/infinite-loop.test.ts ++++ b/tests/infinite-loop.test.ts +@@ -364,20 +364,137 @@ describe('Infinite Loop Detection', { + unsubscribes.push( + effect((): void => { + $list.reverse() + }) + ) + assert.deepStrictEqual([...$list], initial) + } + } + }) + ++ it('raises nothing when fill alone in a sync effect writes indices it did not read', (): void => { ++ for (const [, $list] of bothKinds([1, 2, 3])) { ++ unsubscribes.push( ++ effect((): void => { ++ $list.fill(9) ++ }) ++ ) ++ assert.deepStrictEqual([...$list], [9, 9, 9]) ++ } ++ }) ++ ++ it('raises the loop error when a sync effect reads an index before fill writes it on both kinds of state', (): void => { ++ for (const [kind, $list] of bothKinds([1, 2, 3])) { ++ assert.throws( ++ (): void => { ++ unsubscribes.push( ++ effect((): void => { ++ void $list[0] ++ $list.fill(9) ++ }, 'filler') ++ ) ++ }, ++ /Infinite loop detected: effect "filler" cannot update property "/, ++ kind ++ ) ++ } ++ }) ++ ++ it('raises nothing when copyWithin in a sync effect has source and target ranges that do not overlap', (): void => { ++ for (const [, $list] of bothKinds([1, 2, 3])) { ++ unsubscribes.push( ++ effect((): void => { ++ $list.copyWithin(2, 0, 1) ++ }) ++ ) ++ assert.deepStrictEqual([...$list], [1, 2, 1]) ++ } ++ }) ++ ++ it('raises the loop error when copyWithin in a sync effect has overlapping ranges on both kinds of state', (): void => { ++ for (const [kind, $list] of bothKinds([1, 2, 3])) { ++ assert.throws( ++ (): void => { ++ unsubscribes.push( ++ effect((): void => { ++ $list.copyWithin(0, 1) ++ }, 'copier') ++ ) ++ }, ++ /Infinite loop detected: effect "copier" cannot update property "/, ++ kind ++ ) ++ } ++ }) ++ ++ it('lets a child effect call fill and copyWithin on indices it read without raising', (): void => { ++ for (const [, $list] of bothKinds([1, 2, 3])) { ++ unsubscribes.push( ++ effect((): void => { ++ effect((): void => { ++ void $list[0] ++ $list.fill(9) ++ }) ++ }) ++ ) ++ assert.deepStrictEqual([...$list], [9, 9, 9]) ++ } ++ for (const [, $list] of bothKinds([1, 2, 3])) { ++ unsubscribes.push( ++ effect((): void => { ++ effect((): void => { ++ $list.copyWithin(0, 1) ++ }) ++ }) ++ ) ++ assert.deepStrictEqual([...$list], [2, 3, 3]) ++ } ++ }) ++ ++ it('lets a microtask effect call fill and copyWithin on indices it read without raising', async (): Promise => { ++ const settle = async (): Promise => { ++ // A few turns cover the one re-run each call causes, plus a spare to prove it stops ++ for (let turn = 0; turn < 4; turn++) { ++ await new Promise((resolve): void => queueMicrotask(resolve)) ++ } ++ } ++ for (const [, $list] of bothKinds([1, 2, 3])) { ++ unsubscribes.push( ++ effect( ++ (): void => { ++ void $list[0] ++ $list.fill(9) ++ }, ++ undefined, ++ undefined, ++ { schedule: 'microtask' } ++ ) ++ ) ++ await settle() ++ assert.deepStrictEqual([...$list], [9, 9, 9]) ++ } ++ for (const [, $list] of bothKinds([1, 2, 3])) { ++ unsubscribes.push( ++ effect( ++ (): void => { ++ $list.copyWithin(0, 1) ++ }, ++ undefined, ++ undefined, ++ { schedule: 'microtask' } ++ ) ++ ) ++ await settle() ++ assert.deepStrictEqual([...$list], [3, 3, 3]) ++ } ++ }) ++ + it('lets a child effect push onto a reactive array without raising', (): void => { + for (const [, $list] of bothKinds([])) { + unsubscribes.push( + effect((): void => { + effect((): void => { + $list.push(1) + }) + }) + ) + assert.deepStrictEqual([...$list], [1]) +diff --git a/tests/state-hooks.test.ts b/tests/state-hooks.test.ts +index e2153d4..6e7efa0 100644 +--- a/tests/state-hooks.test.ts ++++ b/tests/state-hooks.test.ts +@@ -272,20 +272,62 @@ describe('State Hooks', { + reads.push(prop) + }, + }) + + $list.push(1) + + assert.ok(reads.includes('push')) + assert.ok(reads.includes('length')) + }) + ++ it('fires onRead with the method name and length and onWrite only for the indices fill changes', () => { ++ const reads: PropertyKey[] = [] ++ const writes: unknown[][] = [] ++ const $list = state([1, 2, 3], { ++ onRead: (prop: PropertyKey) => { ++ reads.push(prop) ++ }, ++ onWrite: (prop: PropertyKey, oldValue: unknown, newValue: unknown) => { ++ writes.push([prop, oldValue, newValue]) ++ }, ++ }) ++ ++ $list.fill(9, 1) ++ ++ assert.ok(reads.includes('fill')) ++ assert.ok(reads.includes('length')) ++ assert.deepStrictEqual(writes, [ ++ ['1', 2, 9], ++ ['2', 3, 9], ++ ]) ++ }) ++ ++ it('fires onRead with the method name, length and source index and onWrite only for the target index of copyWithin', () => { ++ const reads: PropertyKey[] = [] ++ const writes: unknown[][] = [] ++ const $list = state([1, 2, 3], { ++ onRead: (prop: PropertyKey) => { ++ reads.push(prop) ++ }, ++ onWrite: (prop: PropertyKey, oldValue: unknown, newValue: unknown) => { ++ writes.push([prop, oldValue, newValue]) ++ }, ++ }) ++ ++ $list.copyWithin(0, 2) ++ ++ assert.ok(reads.includes('copyWithin')) ++ assert.ok(reads.includes('length')) ++ assert.ok(reads.includes('2')) ++ assert.deepStrictEqual(writes, [['0', 1, 3]]) ++ }) ++ + it('fires onWrite for direct array index assignment', () => { + const writes: string[] = [] + const $s = state( + { + items: [1, 2, 3], + }, + { + onWrite: (prop: PropertyKey) => { + writes.push(String(prop)) + }, diff --git a/.do/review/04-8464f3f..2dbcee2.diff b/.do/review/04-8464f3f..2dbcee2.diff new file mode 100644 index 0000000..adea81d --- /dev/null +++ b/.do/review/04-8464f3f..2dbcee2.diff @@ -0,0 +1,298 @@ +# Review package: ticket 04, 8464f3f..2dbcee2 + +## Commits +### 2dbcee2 +perf(state): keep array writes off the object batch path + +Once fill and copyWithin run through the Proxy inside a batch, array +targets reach handleBatchFastPath, which until then sees only plain +objects. Its [SUBSCRIBERS] load then deoptimizes 36 times per 10-cycle +run (wrong map), and the case 'many dependencies', which runs after +'array fill (hookless)', gets slower: +10.6% on medians and +6.8% on +totals over three interleaved A/B runs of 10 cycles against 87c8823 +(A/A spread 4.3% and 2.4%). + +Control: on 87c8823, with fill wrapped by hand in batch(), the same +slowdown and the same deopt pattern appear (2.2 ms against 1.8 ms on a +fill-then-many-dependencies sequence). The cause is arrays on the +shared batch write path, not the two names added to the method set, +its Set lookup or code layout. + +handleBatchArrayWrite now serves arrays through a set trap of its own, +picked by one Array.isArray per state() call, shared with hooksSlotFor. +Interleaved A/B after the change, against 87c8823: many dependencies +-0.5% on medians and -1.4% on totals, array fill -50.2%, no case +slower beyond its A/A spread. The subscriber lookup stays inlined: +calling findSubscribers instead costs 2.2% on medians and 2.4% on +totals of array fill (three interleaved 10-cycle pairs). + + +## Files changed + src/AGENTS.md | 8 ++++--- + src/index.ts | 67 ++++++++++++++++++++++++++++++++++++++++++++++++++--------- + 2 files changed, 62 insertions(+), 13 deletions(-) + +## Diff +diff --git a/src/AGENTS.md b/src/AGENTS.md +index ca2b18e..0a445c9 100644 +--- a/src/AGENTS.md ++++ b/src/AGENTS.md +@@ -66,26 +66,27 @@ ## Symbols + |--------|---------| + | `OWN_KEYS_SYMBOL` | Tracks key enumeration reads (ownKeys trap) | + | `SUBSCRIBERS` | Stores the subscriber Set on target objects; absent until the first subscription and on targets that cannot take it (those keep the set in `proxyCacheSubs`) | + | `PROXY` | Stores Proxy reference on target objects | + | `HOOKS` | Only on an extensible array target: its `DirectArrayContext` (raw target, raw hooks, composed `onDelete`/`onRead`/`onWrite`), which the direct `push`/`pop` read through the Proxy receiver (`proxy[HOOKS]` returns the raw slot; the raw target as receiver runs the native method silently, Q71 b). A non-array target gets no slot: its hooks reach the traps by closure, and nested propagation is by closure too. A WeakMap keyed by the Proxy cost 2.7x on array state creation | + + ## Internal Function Architecture + + ### Proxy Handlers + +-Five handler factories, each returns a ProxyHandler method. `state()` without hooks shares one prebuilt `HOOKLESS_HANDLER`; the get factory returns a leaner variant when no hooks are passed, the set factory returns one trap for every state object. ++Five handler factories, each returns a ProxyHandler method, and a sixth for the array set trap. `state()` without hooks shares one prebuilt `HOOKLESS_HANDLER`, or `HOOKLESS_ARRAY_HANDLER` for an array target; the get factory returns a leaner variant when no hooks are passed, the set factory returns one trap for every state object that is not an array. + + | Handler | Traps | Key behavior | + |---------|-------|-------------| + | `createGetHandler()` | `get` | One keyed load of the raw target, then the internal-symbol early return (a separate load site for the symbols saw only array maps, lost its feedback when those arrays were collected, and deoptimized the trap on the next direct push/pop); delegates to `trackDependency` for reads and `wrapNestedObject` for nested wrapping; both variants route mutating array methods through `getWrappedArrayMethod` before the non-object early return | +-| `createSetHandler()` | `set` | One trap for hooked and hookless state: `handleBatchFastPath` during batch from outside any effect, `performUnbatchedWrite` outside any effect, `performWrite` inside an effect; `onWrite` only observes after the raw write on every path | ++| `createSetHandler()` | `set` | One trap for hooked and hookless state of a non-array target: `handleBatchFastPath` during batch from outside any effect, `performUnbatchedWrite` outside any effect, `performWrite` inside an effect; `onWrite` only observes after the raw write on every path | ++| `createArraySetHandler()` | `set` | The same trap for an array target, `state()` picks it by `Array.isArray` once per call; its batch branch is `handleBatchArrayWrite`, so no set trap closure and no batch write function sees both arrays and objects | + | `createDeleteHandler()` | `deleteProperty` | Deletion with subscriber notification | + | `createHasHandler()` | `has` | `in` operator tracking via `trackDependency` | + | `createOwnKeysHandler()` | `ownKeys` | Key enumeration tracking via `OWN_KEYS_SYMBOL` | + + Handler helpers: + + | Function | Purpose | + |----------|---------| + | `trackDependency` | First run: adds `currentEffect` to the target's subscribers, then `recordEffectRead`. Re-run (`isTrackingOnly`): `recordEffectRead` in silent mode | + | `recordEffectRead` | Non-silent: records into `__deps`/`__reads` + fires `onDependencyAdd`. Silent: advances the read-list replay while it matches, else replays the prefix into temp collections and falls back to `recordSilentRead` | +@@ -99,21 +100,22 @@ ### Proxy Handlers + | `wrapArrayMethod` | Wraps a native mutating array method once, cached in `arrayMethodCache`: the native `push` and `pop` get `directPushWrapper`/`directPopWrapper`; the other seven get a wrapper that applies them via `applyInBatch` to the call receiver (the Proxy), so every read and write passes the traps (seen by `onRead`/`onWrite`, subject to the loop check inside an effect) and each subscriber wakes at most once per call | + | `directPushWrapper` / `directPopWrapper` | Read the receiver's `[HOOKS]` slot (optional chaining, so a nullish receiver reads no slot): a `DirectArrayContext` (checked with `instanceof`, so only a reactive array's slot qualifies) on a receiver that is not the context's own raw target runs `directPush`/`directPop` on it inside `applyInBatch`; any other receiver (such as `push.call(otherArray)`, the raw array target, `null` or `undefined`) gets the native method inside `applyInBatch`: the raw target never enters the traps, so the call is silent (no hook call, no wake, Q71 b), and a nullish receiver raises the native TypeError | + | `hooksSlotFor` | What `state()` stores in the `[HOOKS]` slot: for an array target a new `DirectArrayContext`, otherwise `undefined` (no slot) | + | `directPush` / `directPop` | Run the native method on the raw target inside `applyInBatch` and replay by hand what the traps would have done: `push` reads `length`, then sets each new index and `length`; `pop` reads `length` and the last index, deletes that index (`onDelete`, readers of the index scheduled, no loop or equal check) and sets `length`; `pop` returns the nested wrapper the get trap would | + | `recordDirectRead` | The get trap's bookkeeping for a property the native method read: `trackDependency` inside an effect, then `onRead` | + | `recordDirectWrite` | The set trap's bookkeeping for a write the native method already performed: the loop check (inside an effect only), the equal skip, then `onWrite`, shared by both paths; only the scheduling branches: per property inside an effect (plus `length` for an index write that grew the array), one `ARRAY_DIRTY_PROPS` entry per batch outside. A write that grew the array went to an absent index, so it is never skipped as equal (Q65 a) | + | `isUnchangedWrite` | The equal skip of every set path: `Object.is` against the current read, and for an equal `undefined` a `prop in rawTarget` check, so a write of `undefined` to an absent property counts as a change (Q65 a); a read that is not `undefined` proves the property exists | + | `performUnbatchedWrite` | Outside any effect and batch: skips an unchanged write (`isUnchangedWrite`), skips scheduling when the target has no subscribers, else notifies per property | + | `performWrite` | Checks infinite loop, skips an unchanged write (`isUnchangedWrite`), notifies subscribers | + | `checkInfiniteLoop` | Throws if effect writes to a property it read (exempt: child effects, microtask effects) | +-| `handleBatchFastPath` | Defers writes to `dirtyTargets` during batch (skips an unchanged write via `isUnchangedWrite`, calls `onWrite`, skips the loop check); inlines the `findSubscribers` lookup, and only for a target not yet dirty | ++| `handleBatchFastPath` | Defers writes to `dirtyTargets` during batch (skips an unchanged write via `isUnchangedWrite`, calls `onWrite`, skips the loop check); inlines the `findSubscribers` lookup, and only for a target not yet dirty; object targets only | ++| `handleBatchArrayWrite` | The array twin of `handleBatchFastPath`: the same write, equal skip and `onWrite`, then one `ARRAY_DIRTY_PROPS` entry for the array. Kept apart because arrays written through the shared path (once `fill` and `copyWithin` went through it) made its `[SUBSCRIBERS]` load deoptimize on every benchmark cycle and slowed `many dependencies` by 10.6% on medians and 6.8% on totals (interleaved 10-cycle A/B against 87c8823) | + | `getArrayLengthBeforeMutation` | Captures array length before index assignment | + | `notifyLengthChangeIfNeeded` | Notifies `length` subscribers if array grew/shrank | + + ### Subscriber Scheduling Pipeline + + ``` + scheduleSubscribersForTarget(target, prop?) + → findSubscribers(target) + → for each subscriber: + prop undefined → addPendingEffect(subscriber) +diff --git a/src/index.ts b/src/index.ts +index 2a3f490..4d82aa6 100644 +--- a/src/index.ts ++++ b/src/index.ts +@@ -749,26 +749,27 @@ function createGetHandler( + if (wrapped) return wrapped + + if (value === null || typeof value !== 'object') return value + return wrapNestedObject(value as object, hooks as StateHooks | undefined) + } + } + + // Only an array target gets a [HOOKS] slot: the context its direct push and pop find through the Proxy receiver + function hooksSlotFor( + target: ProxyTarget, ++ isArray: boolean, + hooks: StateHooks | undefined, + onDelete: HookFunction<[PropertyKey, boolean, T]> | undefined, + onRead: HookFunction<[PropertyKey, unknown, T]> | undefined, + onWrite: HookFunction<[PropertyKey, unknown, unknown, T]> | undefined + ): HooksSlot { +- if (!Array.isArray(target)) return undefined ++ if (!isArray) return undefined + return new DirectArrayContext( + target as ArrayTarget, + hooks as StateHooks | undefined, + onDelete as DirectArrayContext['onDelete'], + onRead as DirectArrayContext['onRead'], + onWrite as DirectArrayContext['onWrite'] + ) + } + + function createHasHandler( +@@ -813,38 +814,60 @@ function handleBatchFastPath( + onWrite: HookFunction<[PropertyKey, unknown, unknown, T]> | undefined + ): boolean { + const oldValue = rawTarget[prop] + if (isUnchangedWrite(rawTarget, prop, oldValue, value)) return true + + rawTarget[prop] = value + if (onWrite) callHookSafe(onWrite, prop, oldValue, value, rawTarget as T) + + // A target already in dirtyTargets had subscribers at its first write, so only the first write looks them up + const props = dirtyTargets.get(rawTarget) +- if (props === ARRAY_DIRTY_PROPS) return true + if (props) { + props.add(prop) + return true + } + + // inlined, not findSubscribers: the call cost ~5% on the batch + derive benchmarks + let subs = (rawTarget as SubscribersObject)[SUBSCRIBERS] + if (subs === undefined && (hasFallbackSubs || (rawTarget as ProxyObject)[PROXY] === undefined)) { + subs = proxyCacheSubs.get(rawTarget) + } + if (!subs?.size) return true + +- if (Array.isArray(rawTarget)) { +- dirtyTargets.set(rawTarget, ARRAY_DIRTY_PROPS) +- } else { +- dirtyTargets.set(rawTarget, new Set([prop])) ++ dirtyTargets.set(rawTarget, new Set([prop])) ++ return true ++} ++ ++// The array twin of handleBatchFastPath, a separate function so that the object path never sees an array: once ++// fill and copyWithin wrote arrays through the shared path, its [SUBSCRIBERS] load deoptimized on every benchmark ++// cycle and `many dependencies` ran 10.6% slower on medians and 6.8% on totals (interleaved 10-cycle A/B against ++// 87c8823). An array goes dirty once and wakes all its subscribers ++function handleBatchArrayWrite( ++ rawTarget: ProxyTarget, ++ prop: PropertyKey, ++ value: unknown, ++ onWrite: HookFunction<[PropertyKey, unknown, unknown, T]> | undefined ++): boolean { ++ const oldValue = rawTarget[prop] ++ if (isUnchangedWrite(rawTarget, prop, oldValue, value)) return true ++ ++ rawTarget[prop] = value ++ if (onWrite) callHookSafe(onWrite, prop, oldValue, value, rawTarget as T) ++ ++ if (dirtyTargets.get(rawTarget) === ARRAY_DIRTY_PROPS) return true ++ ++ // inlined, not findSubscribers: the call cost 2.2% on the fill benchmark ++ let subs = (rawTarget as SubscribersObject)[SUBSCRIBERS] ++ if (subs === undefined && (hasFallbackSubs || (rawTarget as ProxyObject)[PROXY] === undefined)) { ++ subs = proxyCacheSubs.get(rawTarget) + } ++ if (subs?.size) dirtyTargets.set(rawTarget, ARRAY_DIRTY_PROPS) + return true + } + + // Equal means equal to the value of a property that exists: a write of undefined to an absent property is a + // change, as it is on a plain object (Q65 a). A read that is not undefined proves the property exists, so only + // an equal undefined write pays the existence check; `in` costs 4% there where Object.hasOwn cost 35% + function isUnchangedWrite(rawTarget: ProxyTarget, prop: PropertyKey, oldValue: unknown, value: unknown): boolean { + return Object.is(oldValue, value) && (value !== undefined || prop in rawTarget) + } + +@@ -869,20 +892,36 @@ function createSetHandler( + if (batchDepth > 0 && !currentEffect) { + return handleBatchFastPath(rawTarget, prop, value, onWrite) + } + if (!currentEffect) { + return performUnbatchedWrite(rawTarget, prop, value, onWrite) + } + return performWrite(rawTarget, prop, value, onWrite) + } + } + ++// The set trap of an array target: createSetHandler with the batch branch on handleBatchArrayWrite, a literal of ++// its own so that no trap closure calls both batch paths ++function createArraySetHandler( ++ onWrite: HookFunction<[PropertyKey, unknown, unknown, T]> | undefined ++): NonNullable['set']> { ++ return (rawTarget: ProxyTarget, prop: PropertyKey, value: unknown): boolean => { ++ if (batchDepth > 0 && !currentEffect) { ++ return handleBatchArrayWrite(rawTarget, prop, value, onWrite) ++ } ++ if (!currentEffect) { ++ return performUnbatchedWrite(rawTarget, prop, value, onWrite) ++ } ++ return performWrite(rawTarget, prop, value, onWrite) ++ } ++} ++ + // Outside any effect and batch: no loop check, and a target without subscribers skips scheduling + function performUnbatchedWrite( + rawTarget: ProxyTarget, + prop: PropertyKey, + value: unknown, + onWrite: HookFunction<[PropertyKey, unknown, unknown, T]> | undefined + ): boolean { + const oldValue = rawTarget[prop] + if (isUnchangedWrite(rawTarget, prop, oldValue, value)) return true + const subs = findSubscribers(rawTarget) +@@ -952,56 +991,64 @@ function defineProxyProperties(target: ProxyTarget, proxy: unknown, hooks: Hooks + } + + const HOOKLESS_HANDLER: ProxyHandler = { + deleteProperty: createDeleteHandler(undefined), + get: createGetHandler(undefined, undefined), + has: createHasHandler(undefined), + ownKeys: createOwnKeysHandler(undefined), + set: createSetHandler(undefined), + } + ++const HOOKLESS_ARRAY_HANDLER: ProxyHandler = { ++ ...HOOKLESS_HANDLER, ++ set: createArraySetHandler(undefined), ++} ++ + /** Wrap an object in a reactive Proxy. Property reads are tracked, writes notify subscribers. */ + export function state(initial: T, hooks?: StateHooks): T { + if (initial === null || initial === undefined || typeof initial !== 'object') return initial + + const initialWithProxy = initial as ProxyObject + const existingProxy = initialWithProxy?.[PROXY] + if (existingProxy) return existingProxy as T + + const target = initial as ProxyTarget + const cached = proxyCache.get(target) + if (cached) return cached as T + ++ const isArray = Array.isArray(target) + if (!hooks) { +- const proxy = new Proxy(target, HOOKLESS_HANDLER) as T +- if (!defineProxyProperties(target, proxy, hooksSlotFor(target, undefined, undefined, undefined, undefined))) { ++ const proxy = new Proxy(target, isArray ? HOOKLESS_ARRAY_HANDLER : HOOKLESS_HANDLER) as T ++ if ( ++ !defineProxyProperties(target, proxy, hooksSlotFor(target, isArray, undefined, undefined, undefined, undefined)) ++ ) { + proxyCache.set(target, proxy) + } + return proxy + } + + const onDelete = composeHook(hooks.onDelete) + const onHas = composeHook(hooks.onHas) + const onOwnKeys = composeHook(hooks.onOwnKeys) + const onRead = composeHook(hooks.onRead) + const onWrite = composeHook(hooks.onWrite) + + const handler: ProxyHandler = { + deleteProperty: createDeleteHandler(onDelete), + get: createGetHandler(onRead, hooks), + has: createHasHandler(onHas), + ownKeys: createOwnKeysHandler(onOwnKeys), +- set: createSetHandler(onWrite), ++ set: isArray ? createArraySetHandler(onWrite) : createSetHandler(onWrite), + } as ProxyHandler + + const proxy = new Proxy(target, handler) as T +- if (!defineProxyProperties(target, proxy, hooksSlotFor(target, hooks, onDelete, onRead, onWrite))) { ++ if (!defineProxyProperties(target, proxy, hooksSlotFor(target, isArray, hooks, onDelete, onRead, onWrite))) { + proxyCache.set(target, proxy) + } + return proxy + } + + function disposeChildEffects(eff: EffectFunction): void { + const existing = eff.__children + if (!existing?.size) return + eff.__children = undefined + for (const c of existing) { diff --git a/.do/review/04-8464f3f..b595300.diff b/.do/review/04-8464f3f..b595300.diff new file mode 100644 index 0000000..8408db3 --- /dev/null +++ b/.do/review/04-8464f3f..b595300.diff @@ -0,0 +1,294 @@ +# Review package: ticket 04, 8464f3f..b595300 + +## Commits +### b595300 +perf(state): keep array writes off the object batch path + +Once fill and copyWithin ran through the Proxy inside a batch, array +targets reached handleBatchFastPath, which had seen only plain objects. +Its [SUBSCRIBERS] load then deoptimized 36 times per 10-cycle run +(wrong map), and the case 'many dependencies', which runs after +'array fill (hookless)', got slower: +10.6% on medians and +6.8% on +totals over three interleaved A/B runs of 10 cycles against 87c8823 +(A/A spread 4.3% and 2.4%). + +Control: on 87c8823, with fill wrapped by hand in batch(), the same +slowdown and the same deopt pattern appear (2.2 ms against 1.8 ms on a +fill-then-many-dependencies sequence). The cause is arrays on the +shared batch write path, not the two names added to the method set, +its Set lookup or code layout. + +handleBatchArrayWrite now serves arrays through a set trap of its own, +picked by one Array.isArray per state() call, shared with hooksSlotFor. +Interleaved A/B after the change: many dependencies -0.5% on medians +and -1.4% on totals, array fill -50.2%, no case slower beyond its A/A +spread. + + +## Files changed + src/AGENTS.md | 8 +++++--- + src/index.ts | 65 ++++++++++++++++++++++++++++++++++++++++++++++++++--------- + 2 files changed, 60 insertions(+), 13 deletions(-) + +## Diff +diff --git a/src/AGENTS.md b/src/AGENTS.md +index ca2b18e..76dd6aa 100644 +--- a/src/AGENTS.md ++++ b/src/AGENTS.md +@@ -66,26 +66,27 @@ ## Symbols + |--------|---------| + | `OWN_KEYS_SYMBOL` | Tracks key enumeration reads (ownKeys trap) | + | `SUBSCRIBERS` | Stores the subscriber Set on target objects; absent until the first subscription and on targets that cannot take it (those keep the set in `proxyCacheSubs`) | + | `PROXY` | Stores Proxy reference on target objects | + | `HOOKS` | Only on an extensible array target: its `DirectArrayContext` (raw target, raw hooks, composed `onDelete`/`onRead`/`onWrite`), which the direct `push`/`pop` read through the Proxy receiver (`proxy[HOOKS]` returns the raw slot; the raw target as receiver runs the native method silently, Q71 b). A non-array target gets no slot: its hooks reach the traps by closure, and nested propagation is by closure too. A WeakMap keyed by the Proxy cost 2.7x on array state creation | + + ## Internal Function Architecture + + ### Proxy Handlers + +-Five handler factories, each returns a ProxyHandler method. `state()` without hooks shares one prebuilt `HOOKLESS_HANDLER`; the get factory returns a leaner variant when no hooks are passed, the set factory returns one trap for every state object. ++Five handler factories, each returns a ProxyHandler method, and a sixth for the array set trap. `state()` without hooks shares one prebuilt `HOOKLESS_HANDLER`, or `HOOKLESS_ARRAY_HANDLER` for an array target; the get factory returns a leaner variant when no hooks are passed, the set factory returns one trap for every state object that is not an array. + + | Handler | Traps | Key behavior | + |---------|-------|-------------| + | `createGetHandler()` | `get` | One keyed load of the raw target, then the internal-symbol early return (a separate load site for the symbols saw only array maps, lost its feedback when those arrays were collected, and deoptimized the trap on the next direct push/pop); delegates to `trackDependency` for reads and `wrapNestedObject` for nested wrapping; both variants route mutating array methods through `getWrappedArrayMethod` before the non-object early return | +-| `createSetHandler()` | `set` | One trap for hooked and hookless state: `handleBatchFastPath` during batch from outside any effect, `performUnbatchedWrite` outside any effect, `performWrite` inside an effect; `onWrite` only observes after the raw write on every path | ++| `createSetHandler()` | `set` | One trap for hooked and hookless state of a non-array target: `handleBatchFastPath` during batch from outside any effect, `performUnbatchedWrite` outside any effect, `performWrite` inside an effect; `onWrite` only observes after the raw write on every path | ++| `createArraySetHandler()` | `set` | The same trap for an array target, `state()` picks it by `Array.isArray` once per call; its batch branch is `handleBatchArrayWrite`, so no trap closure and no batch write function sees both arrays and objects | + | `createDeleteHandler()` | `deleteProperty` | Deletion with subscriber notification | + | `createHasHandler()` | `has` | `in` operator tracking via `trackDependency` | + | `createOwnKeysHandler()` | `ownKeys` | Key enumeration tracking via `OWN_KEYS_SYMBOL` | + + Handler helpers: + + | Function | Purpose | + |----------|---------| + | `trackDependency` | First run: adds `currentEffect` to the target's subscribers, then `recordEffectRead`. Re-run (`isTrackingOnly`): `recordEffectRead` in silent mode | + | `recordEffectRead` | Non-silent: records into `__deps`/`__reads` + fires `onDependencyAdd`. Silent: advances the read-list replay while it matches, else replays the prefix into temp collections and falls back to `recordSilentRead` | +@@ -99,21 +100,22 @@ ### Proxy Handlers + | `wrapArrayMethod` | Wraps a native mutating array method once, cached in `arrayMethodCache`: the native `push` and `pop` get `directPushWrapper`/`directPopWrapper`; the other seven get a wrapper that applies them via `applyInBatch` to the call receiver (the Proxy), so every read and write passes the traps (seen by `onRead`/`onWrite`, subject to the loop check inside an effect) and each subscriber wakes at most once per call | + | `directPushWrapper` / `directPopWrapper` | Read the receiver's `[HOOKS]` slot (optional chaining, so a nullish receiver reads no slot): a `DirectArrayContext` (checked with `instanceof`, so only a reactive array's slot qualifies) on a receiver that is not the context's own raw target runs `directPush`/`directPop` on it inside `applyInBatch`; any other receiver (such as `push.call(otherArray)`, the raw array target, `null` or `undefined`) gets the native method inside `applyInBatch`: the raw target never enters the traps, so the call is silent (no hook call, no wake, Q71 b), and a nullish receiver raises the native TypeError | + | `hooksSlotFor` | What `state()` stores in the `[HOOKS]` slot: for an array target a new `DirectArrayContext`, otherwise `undefined` (no slot) | + | `directPush` / `directPop` | Run the native method on the raw target inside `applyInBatch` and replay by hand what the traps would have done: `push` reads `length`, then sets each new index and `length`; `pop` reads `length` and the last index, deletes that index (`onDelete`, readers of the index scheduled, no loop or equal check) and sets `length`; `pop` returns the nested wrapper the get trap would | + | `recordDirectRead` | The get trap's bookkeeping for a property the native method read: `trackDependency` inside an effect, then `onRead` | + | `recordDirectWrite` | The set trap's bookkeeping for a write the native method already performed: the loop check (inside an effect only), the equal skip, then `onWrite`, shared by both paths; only the scheduling branches: per property inside an effect (plus `length` for an index write that grew the array), one `ARRAY_DIRTY_PROPS` entry per batch outside. A write that grew the array went to an absent index, so it is never skipped as equal (Q65 a) | + | `isUnchangedWrite` | The equal skip of every set path: `Object.is` against the current read, and for an equal `undefined` a `prop in rawTarget` check, so a write of `undefined` to an absent property counts as a change (Q65 a); a read that is not `undefined` proves the property exists | + | `performUnbatchedWrite` | Outside any effect and batch: skips an unchanged write (`isUnchangedWrite`), skips scheduling when the target has no subscribers, else notifies per property | + | `performWrite` | Checks infinite loop, skips an unchanged write (`isUnchangedWrite`), notifies subscribers | + | `checkInfiniteLoop` | Throws if effect writes to a property it read (exempt: child effects, microtask effects) | +-| `handleBatchFastPath` | Defers writes to `dirtyTargets` during batch (skips an unchanged write via `isUnchangedWrite`, calls `onWrite`, skips the loop check); inlines the `findSubscribers` lookup, and only for a target not yet dirty | ++| `handleBatchFastPath` | Defers writes to `dirtyTargets` during batch (skips an unchanged write via `isUnchangedWrite`, calls `onWrite`, skips the loop check); inlines the `findSubscribers` lookup, and only for a target not yet dirty; object targets only | ++| `handleBatchArrayWrite` | The array twin of `handleBatchFastPath`: the same write, equal skip and `onWrite`, then one `ARRAY_DIRTY_PROPS` entry for the array. Kept apart because arrays written through the shared path (once `fill` and `copyWithin` went through it) made its `[SUBSCRIBERS]` load deoptimize on every benchmark cycle and slowed `many dependencies` by 7 to 10% | + | `getArrayLengthBeforeMutation` | Captures array length before index assignment | + | `notifyLengthChangeIfNeeded` | Notifies `length` subscribers if array grew/shrank | + + ### Subscriber Scheduling Pipeline + + ``` + scheduleSubscribersForTarget(target, prop?) + → findSubscribers(target) + → for each subscriber: + prop undefined → addPendingEffect(subscriber) +diff --git a/src/index.ts b/src/index.ts +index 2a3f490..1139c2b 100644 +--- a/src/index.ts ++++ b/src/index.ts +@@ -749,26 +749,27 @@ function createGetHandler( + if (wrapped) return wrapped + + if (value === null || typeof value !== 'object') return value + return wrapNestedObject(value as object, hooks as StateHooks | undefined) + } + } + + // Only an array target gets a [HOOKS] slot: the context its direct push and pop find through the Proxy receiver + function hooksSlotFor( + target: ProxyTarget, ++ isArray: boolean, + hooks: StateHooks | undefined, + onDelete: HookFunction<[PropertyKey, boolean, T]> | undefined, + onRead: HookFunction<[PropertyKey, unknown, T]> | undefined, + onWrite: HookFunction<[PropertyKey, unknown, unknown, T]> | undefined + ): HooksSlot { +- if (!Array.isArray(target)) return undefined ++ if (!isArray) return undefined + return new DirectArrayContext( + target as ArrayTarget, + hooks as StateHooks | undefined, + onDelete as DirectArrayContext['onDelete'], + onRead as DirectArrayContext['onRead'], + onWrite as DirectArrayContext['onWrite'] + ) + } + + function createHasHandler( +@@ -813,38 +814,58 @@ function handleBatchFastPath( + onWrite: HookFunction<[PropertyKey, unknown, unknown, T]> | undefined + ): boolean { + const oldValue = rawTarget[prop] + if (isUnchangedWrite(rawTarget, prop, oldValue, value)) return true + + rawTarget[prop] = value + if (onWrite) callHookSafe(onWrite, prop, oldValue, value, rawTarget as T) + + // A target already in dirtyTargets had subscribers at its first write, so only the first write looks them up + const props = dirtyTargets.get(rawTarget) +- if (props === ARRAY_DIRTY_PROPS) return true + if (props) { + props.add(prop) + return true + } + + // inlined, not findSubscribers: the call cost ~5% on the batch + derive benchmarks + let subs = (rawTarget as SubscribersObject)[SUBSCRIBERS] + if (subs === undefined && (hasFallbackSubs || (rawTarget as ProxyObject)[PROXY] === undefined)) { + subs = proxyCacheSubs.get(rawTarget) + } + if (!subs?.size) return true + +- if (Array.isArray(rawTarget)) { +- dirtyTargets.set(rawTarget, ARRAY_DIRTY_PROPS) +- } else { +- dirtyTargets.set(rawTarget, new Set([prop])) ++ dirtyTargets.set(rawTarget, new Set([prop])) ++ return true ++} ++ ++// The array twin of handleBatchFastPath, a separate function so that the object path never sees an array: once ++// fill and copyWithin wrote arrays through the shared path, its [SUBSCRIBERS] load deoptimized on every benchmark ++// cycle and `many dependencies` ran 7 to 10% slower. An array goes dirty once and wakes all its subscribers ++function handleBatchArrayWrite( ++ rawTarget: ProxyTarget, ++ prop: PropertyKey, ++ value: unknown, ++ onWrite: HookFunction<[PropertyKey, unknown, unknown, T]> | undefined ++): boolean { ++ const oldValue = rawTarget[prop] ++ if (isUnchangedWrite(rawTarget, prop, oldValue, value)) return true ++ ++ rawTarget[prop] = value ++ if (onWrite) callHookSafe(onWrite, prop, oldValue, value, rawTarget as T) ++ ++ if (dirtyTargets.get(rawTarget) === ARRAY_DIRTY_PROPS) return true ++ ++ let subs = (rawTarget as SubscribersObject)[SUBSCRIBERS] ++ if (subs === undefined && (hasFallbackSubs || (rawTarget as ProxyObject)[PROXY] === undefined)) { ++ subs = proxyCacheSubs.get(rawTarget) + } ++ if (subs?.size) dirtyTargets.set(rawTarget, ARRAY_DIRTY_PROPS) + return true + } + + // Equal means equal to the value of a property that exists: a write of undefined to an absent property is a + // change, as it is on a plain object (Q65 a). A read that is not undefined proves the property exists, so only + // an equal undefined write pays the existence check; `in` costs 4% there where Object.hasOwn cost 35% + function isUnchangedWrite(rawTarget: ProxyTarget, prop: PropertyKey, oldValue: unknown, value: unknown): boolean { + return Object.is(oldValue, value) && (value !== undefined || prop in rawTarget) + } + +@@ -869,20 +890,36 @@ function createSetHandler( + if (batchDepth > 0 && !currentEffect) { + return handleBatchFastPath(rawTarget, prop, value, onWrite) + } + if (!currentEffect) { + return performUnbatchedWrite(rawTarget, prop, value, onWrite) + } + return performWrite(rawTarget, prop, value, onWrite) + } + } + ++// The set trap of an array target: createSetHandler with the batch branch on handleBatchArrayWrite, a literal of ++// its own so that no trap closure calls both batch paths ++function createArraySetHandler( ++ onWrite: HookFunction<[PropertyKey, unknown, unknown, T]> | undefined ++): NonNullable['set']> { ++ return (rawTarget: ProxyTarget, prop: PropertyKey, value: unknown): boolean => { ++ if (batchDepth > 0 && !currentEffect) { ++ return handleBatchArrayWrite(rawTarget, prop, value, onWrite) ++ } ++ if (!currentEffect) { ++ return performUnbatchedWrite(rawTarget, prop, value, onWrite) ++ } ++ return performWrite(rawTarget, prop, value, onWrite) ++ } ++} ++ + // Outside any effect and batch: no loop check, and a target without subscribers skips scheduling + function performUnbatchedWrite( + rawTarget: ProxyTarget, + prop: PropertyKey, + value: unknown, + onWrite: HookFunction<[PropertyKey, unknown, unknown, T]> | undefined + ): boolean { + const oldValue = rawTarget[prop] + if (isUnchangedWrite(rawTarget, prop, oldValue, value)) return true + const subs = findSubscribers(rawTarget) +@@ -952,56 +989,64 @@ function defineProxyProperties(target: ProxyTarget, proxy: unknown, hooks: Hooks + } + + const HOOKLESS_HANDLER: ProxyHandler = { + deleteProperty: createDeleteHandler(undefined), + get: createGetHandler(undefined, undefined), + has: createHasHandler(undefined), + ownKeys: createOwnKeysHandler(undefined), + set: createSetHandler(undefined), + } + ++const HOOKLESS_ARRAY_HANDLER: ProxyHandler = { ++ ...HOOKLESS_HANDLER, ++ set: createArraySetHandler(undefined), ++} ++ + /** Wrap an object in a reactive Proxy. Property reads are tracked, writes notify subscribers. */ + export function state(initial: T, hooks?: StateHooks): T { + if (initial === null || initial === undefined || typeof initial !== 'object') return initial + + const initialWithProxy = initial as ProxyObject + const existingProxy = initialWithProxy?.[PROXY] + if (existingProxy) return existingProxy as T + + const target = initial as ProxyTarget + const cached = proxyCache.get(target) + if (cached) return cached as T + ++ const isArray = Array.isArray(target) + if (!hooks) { +- const proxy = new Proxy(target, HOOKLESS_HANDLER) as T +- if (!defineProxyProperties(target, proxy, hooksSlotFor(target, undefined, undefined, undefined, undefined))) { ++ const proxy = new Proxy(target, isArray ? HOOKLESS_ARRAY_HANDLER : HOOKLESS_HANDLER) as T ++ if ( ++ !defineProxyProperties(target, proxy, hooksSlotFor(target, isArray, undefined, undefined, undefined, undefined)) ++ ) { + proxyCache.set(target, proxy) + } + return proxy + } + + const onDelete = composeHook(hooks.onDelete) + const onHas = composeHook(hooks.onHas) + const onOwnKeys = composeHook(hooks.onOwnKeys) + const onRead = composeHook(hooks.onRead) + const onWrite = composeHook(hooks.onWrite) + + const handler: ProxyHandler = { + deleteProperty: createDeleteHandler(onDelete), + get: createGetHandler(onRead, hooks), + has: createHasHandler(onHas), + ownKeys: createOwnKeysHandler(onOwnKeys), +- set: createSetHandler(onWrite), ++ set: isArray ? createArraySetHandler(onWrite) : createSetHandler(onWrite), + } as ProxyHandler + + const proxy = new Proxy(target, handler) as T +- if (!defineProxyProperties(target, proxy, hooksSlotFor(target, hooks, onDelete, onRead, onWrite))) { ++ if (!defineProxyProperties(target, proxy, hooksSlotFor(target, isArray, hooks, onDelete, onRead, onWrite))) { + proxyCache.set(target, proxy) + } + return proxy + } + + function disposeChildEffects(eff: EffectFunction): void { + const existing = eff.__children + if (!existing?.size) return + eff.__children = undefined + for (const c of existing) { diff --git a/openspec/changes/wrap-fill-and-copywithin/.openspec.yaml b/openspec/changes/archive/2026-10-25-wrap-fill-and-copywithin/.openspec.yaml similarity index 100% rename from openspec/changes/wrap-fill-and-copywithin/.openspec.yaml rename to openspec/changes/archive/2026-10-25-wrap-fill-and-copywithin/.openspec.yaml diff --git a/openspec/changes/wrap-fill-and-copywithin/design.md b/openspec/changes/archive/2026-10-25-wrap-fill-and-copywithin/design.md similarity index 100% rename from openspec/changes/wrap-fill-and-copywithin/design.md rename to openspec/changes/archive/2026-10-25-wrap-fill-and-copywithin/design.md diff --git a/openspec/changes/wrap-fill-and-copywithin/proposal.md b/openspec/changes/archive/2026-10-25-wrap-fill-and-copywithin/proposal.md similarity index 100% rename from openspec/changes/wrap-fill-and-copywithin/proposal.md rename to openspec/changes/archive/2026-10-25-wrap-fill-and-copywithin/proposal.md diff --git a/openspec/changes/wrap-fill-and-copywithin/specs/array-mutation/spec.md b/openspec/changes/archive/2026-10-25-wrap-fill-and-copywithin/specs/array-mutation/spec.md similarity index 100% rename from openspec/changes/wrap-fill-and-copywithin/specs/array-mutation/spec.md rename to openspec/changes/archive/2026-10-25-wrap-fill-and-copywithin/specs/array-mutation/spec.md diff --git a/openspec/changes/archive/2026-10-25-wrap-fill-and-copywithin/tickets/01-benchmark-case-and-baseline.md b/openspec/changes/archive/2026-10-25-wrap-fill-and-copywithin/tickets/01-benchmark-case-and-baseline.md new file mode 100644 index 0000000..7b4cdaf --- /dev/null +++ b/openspec/changes/archive/2026-10-25-wrap-fill-and-copywithin/tickets/01-benchmark-case-and-baseline.md @@ -0,0 +1,61 @@ +# 01: Benchmark case for fill and the trunk baseline + +**Area:** state + +**Blocked by:** None (can start immediately) + +**What to build:** The benchmark suite gains one case that measures the pattern this change alters: one effect subscribed to a reactive array created without hooks, then a timed loop that calls `fill` on it with a value that changes every element each time (for example alternating two values), shaped like the existing array push/pop case and reporting through the same result printer. Nothing else changes: this ticket touches the benchmark script only, so the numbers it records are the baseline of the unchanged library. The full suite is then run with the package's benchmark script at its default run count (`npm run benchmark`, 3 cycles, as the previous ledgers ruled), and the complete output, including every case line and the grand total line, is pasted into the ticket report. Ticket 04 compares against this output, so the report must hold the exact figures, not a summary. + +- [x] The benchmark suite lists one new entry that subscribes one effect to a hookless reactive array and times a loop of `fill` calls that change every element on each call; it reports through the same result printer as the existing cases +- [x] The library source, tests, docs, specs and manifests are unchanged in this ticket (`git diff --stat` against the branch base lists only the benchmark script) +- [x] The ticket report contains the full output of the package's benchmark script, including every case line and the grand total line, from a run on this ticket's commit, with the Node version and machine named + +## Report + +Run on commit `87c8823` (branch `worktree-agent-a98f18db12d80bac2`, base `a7e06bc`) with `npm run benchmark`, which is `node --expose-gc scripts/benchmark.ts -R 3`. Node v24.21.0, arm64, Apple M4 Pro, 14 cores, 25769803776 bytes of memory as node's `os.totalmem()` reports it. `sysctl -n hw.memsize` was refused by the sandbox, so the memory figure is not verified against it. The new case is `array fill (hookless)`: a hookless reactive array of 3 zeros, one effect reading every index, 50,000 `fill` calls alternating the values 1 and 2. + +``` +> @nerdalytics/beacon@2002.0.0 benchmark +> node --expose-gc scripts/benchmark.ts -R 3 + +=== Beacon Benchmark (1,000,000 iterations, 3 cycles × 7 samples = 21 total) === + +classic loop : med=1.77ms mean=1.78ms min=1.50ms max=2.32ms sd=0.18ms heap=11kb total=37.41ms avg/cycle=12.47ms total mem=240kb avg mem/cycle=80kb +state no subs : med=40.97ms mean=36.32ms min=25.59ms max=42.37ms sd=7.20ms heap=10kb total=762.77ms avg/cycle=254.26ms total mem=269kb avg mem/cycle=90kb +state + derive : med=173.06ms mean=164.93ms min=141.58ms max=181.45ms sd=14.42ms heap=21191kb total=3463.54ms avg/cycle=1154.51ms total mem=714722kb avg mem/cycle=238241kb +state + derive + 2 effects: med=323.77ms mean=311.96ms min=273.84ms max=336.89ms sd=24.45ms heap=39803kb total=6551.06ms avg/cycle=2183.69ms total mem=850173kb avg mem/cycle=283391kb +batch + derive : med=42.89ms mean=40.14ms min=32.52ms max=44.47ms sd=4.67ms heap=19kb total=843.01ms avg/cycle=281.00ms total mem=531kb avg mem/cycle=177kb +batch + derive + 2 effects: med=43.18ms mean=40.13ms min=31.37ms max=45.75ms sd=5.50ms heap=24kb total=842.78ms avg/cycle=280.93ms total mem=612kb avg mem/cycle=204kb + +--- Targeted benchmarks --- + +state creation : med=9.99ms mean=9.95ms min=9.39ms max=10.44ms sd=0.29ms heap=15682kb total=208.91ms avg/cycle=69.64ms total mem=329336kb avg mem/cycle=109779kb +state read (no effect) : med=16.52ms mean=16.55ms min=15.86ms max=17.84ms sd=0.54ms heap=7kb total=347.59ms avg/cycle=115.86ms total mem=146kb avg mem/cycle=49kb +state write 1 sub : med=13.21ms mean=13.20ms min=11.63ms max=15.65ms sd=0.89ms heap=16892kb total=277.22ms avg/cycle=92.41ms total mem=327910kb avg mem/cycle=109303kb +state write 100 subs : med=85.27ms mean=85.36ms min=81.10ms max=90.52ms sd=2.21ms heap=80109kb total=1792.52ms avg/cycle=597.51ms total mem=1363243kb avg mem/cycle=454414kb +effect triggers : med=7.47ms mean=7.58ms min=6.42ms max=9.64ms sd=0.87ms heap=11033kb total=159.25ms avg/cycle=53.08ms total mem=212319kb avg mem/cycle=70773kb +array push/pop (hookless) : med=23.56ms mean=23.72ms min=22.66ms max=26.39ms sd=0.78ms heap=49353kb total=498.13ms avg/cycle=166.04ms total mem=914921kb avg mem/cycle=304974kb +array push/pop (hooks) : med=25.58ms mean=25.81ms min=24.74ms max=27.96ms sd=0.76ms heap=48784kb total=541.92ms avg/cycle=180.64ms total mem=945871kb avg mem/cycle=315290kb +array fill (hookless) : med=79.53ms mean=79.70ms min=78.26ms max=81.35ms sd=0.73ms heap=20291kb total=1673.64ms avg/cycle=557.88ms total mem=428928kb avg mem/cycle=142976kb +many dependencies : med=1.92ms mean=1.96ms min=1.45ms max=3.10ms sd=0.37ms heap=5947kb total=41.23ms avg/cycle=13.74ms total mem=132271kb avg mem/cycle=44090kb +derive chain depth 10 : med=1.94ms mean=2.48ms min=1.64ms max=4.22ms sd=0.86ms heap=4257kb total=52.01ms avg/cycle=17.34ms total mem=84846kb avg mem/cycle=28282kb +100 states individual : med=25.59ms mean=25.92ms min=25.08ms max=27.36ms sd=0.64ms heap=6846kb total=544.40ms avg/cycle=181.47ms total mem=139204kb avg mem/cycle=46401kb +100 states batched : med=1.79ms mean=1.88ms min=1.32ms max=2.84ms sd=0.37ms heap=5906kb total=39.43ms avg/cycle=13.14ms total mem=136078kb avg mem/cycle=45359kb +100 subs disjoint props : med=2.07ms mean=2.05ms min=1.38ms max=3.58ms sd=0.69ms heap=2557kb total=43.08ms avg/cycle=14.36ms total mem=46107kb avg mem/cycle=15369kb + +--- Non-extensible targets (subscriber set lives in the WeakMap fallback) --- + +sealed write 1 sub : med=14.90ms mean=14.88ms min=13.54ms max=17.53ms sd=0.99ms heap=16509kb total=312.38ms avg/cycle=104.13ms total mem=329429kb avg mem/cycle=109810kb +state no subs after seal : med=41.61ms mean=41.38ms min=39.05ms max=43.05ms sd=1.08ms heap=22kb total=868.97ms avg/cycle=289.66ms total mem=367kb avg mem/cycle=122kb + +--- Blind spots (creation shapes, nested read, equal and undefined writes) --- + +state creation (array) : med=19.26ms mean=19.32ms min=18.67ms max=19.95ms sd=0.35ms heap=3239kb total=405.62ms avg/cycle=135.21ms total mem=297261kb avg mem/cycle=99087kb +state creation (sealed) : med=19.29ms mean=19.54ms min=17.04ms max=22.52ms sd=1.47ms heap=6302kb total=410.36ms avg/cycle=136.79ms total mem=132370kb avg mem/cycle=44123kb +state creation (hooks) : med=13.51ms mean=14.00ms min=13.06ms max=24.63ms sd=2.39ms heap=4074kb total=293.91ms avg/cycle=97.97ms total mem=85560kb avg mem/cycle=28520kb +state read nested : med=36.05ms mean=35.88ms min=34.56ms max=36.85ms sd=0.64ms heap=8kb total=753.54ms avg/cycle=251.18ms total mem=173kb avg mem/cycle=58kb +state write equal value : med=18.62ms mean=18.53ms min=17.53ms max=19.84ms sd=0.49ms heap=7kb total=389.03ms avg/cycle=129.68ms total mem=145kb avg mem/cycle=48kb +array push/pop (undefined): med=23.60ms mean=23.85ms min=22.82ms max=26.23ms sd=0.84ms heap=49428kb total=500.81ms avg/cycle=166.94ms total mem=1042720kb avg mem/cycle=347573kb + +Total: 22.7s Avg/cycle: 7.6s Total mem: 8515753kb Avg mem/cycle: 2838584kb +``` diff --git a/openspec/changes/wrap-fill-and-copywithin/tickets/02-wrap-fill-and-copywithin-through-the-proxy.md b/openspec/changes/archive/2026-10-25-wrap-fill-and-copywithin/tickets/02-wrap-fill-and-copywithin-through-the-proxy.md similarity index 81% rename from openspec/changes/wrap-fill-and-copywithin/tickets/02-wrap-fill-and-copywithin-through-the-proxy.md rename to openspec/changes/archive/2026-10-25-wrap-fill-and-copywithin/tickets/02-wrap-fill-and-copywithin-through-the-proxy.md index 3ae6774..d18cc27 100644 --- a/openspec/changes/wrap-fill-and-copywithin/tickets/02-wrap-fill-and-copywithin-through-the-proxy.md +++ b/openspec/changes/archive/2026-10-25-wrap-fill-and-copywithin/tickets/02-wrap-fill-and-copywithin-through-the-proxy.md @@ -6,10 +6,10 @@ **What to build:** A reactive array hands out `fill` and `copyWithin` the way it hands out `shift`, `unshift`, `splice`, `sort` and `reverse`: the cached wrapper that applies the native method to the Proxy receiver inside a batch. The only library change is the two names joining the set of mutating method names the get trap consults, plus any comment that counts the Proxy-receiver wrappers as five. No direct path, no change to the lookup body, the cache or the batch bookkeeping. From outside a batch a call wakes each subscriber of the array at most once and after the method returns; a call that changes no element wakes nobody; inside a user batch the wakes wait for the outermost boundary; the return value and contents equal the native method, including negative and out-of-range bounds; a frozen array raises the native `TypeError` and the system stays usable. Every assertion runs on state created without hooks and on state created with hooks, in the existing array-method test file and its property-based counterpart. -- [ ] An effect reading every index of a reactive `[1, 2, 3]` re-runs exactly once after `fill(9)` and reads `[9, 9, 9]`, and exactly once after `copyWithin(0, 1)` and reads `[2, 3, 3]`, on both kinds of state -- [ ] `fill(9)` on `[9, 9, 9]` and `copyWithin(0, 1)` on `[7, 7, 7]` re-run such an effect zero times, on both kinds of state -- [ ] Inside a user batch that calls `fill(9, 0, 1)` then `copyWithin(2, 0)` on `[1, 2, 3]`, the effect does not re-run before the batch ends and re-runs exactly once after it, reading `[9, 2, 9]` -- [ ] `fill(9, -2)` then `fill(0, 1, 100)` on `[1, 2, 3]` each return the same array and leave `[1, 9, 9]` then `[1, 0, 0]` with `length` 3; `copyWithin(-2, 0)` on `[1, 2, 3, 4]` returns the same array and leaves `[1, 2, 1, 2]` -- [ ] The property-based array test generates `fill` and `copyWithin` calls with arbitrary bounds beside the existing methods and finds the reactive result equal to the plain-array result -- [ ] `fill(9)` on reactive state over a frozen array with at least one element raises a `TypeError` and a later write on other state still notifies -- [ ] `npm test`, `npm run test:coverage` and `npm run check` pass, and `git diff --stat` against the ticket's base shows the library source changed only in the set of method names and a comment +- [x] An effect reading every index of a reactive `[1, 2, 3]` re-runs exactly once after `fill(9)` and reads `[9, 9, 9]`, and exactly once after `copyWithin(0, 1)` and reads `[2, 3, 3]`, on both kinds of state +- [x] `fill(9)` on `[9, 9, 9]` and `copyWithin(0, 1)` on `[7, 7, 7]` re-run such an effect zero times, on both kinds of state +- [x] Inside a user batch that calls `fill(9, 0, 1)` then `copyWithin(2, 0)` on `[1, 2, 3]`, the effect does not re-run before the batch ends and re-runs exactly once after it, reading `[9, 2, 9]` +- [x] `fill(9, -2)` then `fill(0, 1, 100)` on `[1, 2, 3]` each return the same array and leave `[1, 9, 9]` then `[1, 0, 0]` with `length` 3; `copyWithin(-2, 0)` on `[1, 2, 3, 4]` returns the same array and leaves `[1, 2, 1, 2]` +- [x] The property-based array test generates `fill` and `copyWithin` calls with arbitrary bounds beside the existing methods and finds the reactive result equal to the plain-array result +- [x] `fill(9)` on reactive state over a frozen array with at least one element raises a `TypeError` and a later write on other state still notifies +- [x] `npm test`, `npm run test:coverage` and `npm run check` pass, and `git diff --stat` against the ticket's base shows the library source changed only in the set of method names and a comment diff --git a/openspec/changes/wrap-fill-and-copywithin/tickets/03-loop-detection-hooks-and-docs.md b/openspec/changes/archive/2026-10-25-wrap-fill-and-copywithin/tickets/03-loop-detection-hooks-and-docs.md similarity index 80% rename from openspec/changes/wrap-fill-and-copywithin/tickets/03-loop-detection-hooks-and-docs.md rename to openspec/changes/archive/2026-10-25-wrap-fill-and-copywithin/tickets/03-loop-detection-hooks-and-docs.md index e1cb4c8..16e3159 100644 --- a/openspec/changes/wrap-fill-and-copywithin/tickets/03-loop-detection-hooks-and-docs.md +++ b/openspec/changes/archive/2026-10-25-wrap-fill-and-copywithin/tickets/03-loop-detection-hooks-and-docs.md @@ -6,11 +6,11 @@ **What to build:** Loop detection keeps its rule, a write to a property the effect read raises the infinite-loop error, and the tests pin what that rule means for the two methods whose writes are not all to properties they read: `fill` alone in a synchronous top-level effect raises nothing, and raises when the effect also read an index the call writes; `copyWithin` raises only when its source and target ranges overlap; child effects and microtask effects raise nothing for either. On hooked state the read hook sees the method name and every property the method reads, and the write hook sees every property it writes and no other, in the existing state-hooks test file. The main `array-mutation` spec's purpose line, which archive does not rewrite from the delta, names nine methods, as does the key-behaviors sentence of the root agent docs; the source folder agent docs stop counting "the other five" wrappers and say seven. The handbook is not touched. -- [ ] A synchronous top-level effect that calls `fill(9)` on a reactive `[1, 2, 3]` it did not otherwise read raises nothing and the array reads `[9, 9, 9]`; one that reads index 0 first and then calls `fill(9)` raises the infinite-loop error on its first run, on both kinds of state -- [ ] A synchronous top-level effect that calls `copyWithin(2, 0, 1)` on a reactive `[1, 2, 3]` raises nothing and the array reads `[1, 2, 1]`; one that calls `copyWithin(0, 1)` raises the infinite-loop error on its first run, on both kinds of state -- [ ] A child effect and a microtask effect that each read index 0 and call `fill(9)`, and a pair that each call `copyWithin(0, 1)`, raise nothing, and the arrays end with the native contents -- [ ] On state with `onRead` and `onWrite` hooks holding `[1, 2, 3]`, `fill(9, 1)` calls `onRead` with `fill` and with `length`, and calls `onWrite` for index `1` (old `2`, new `9`) and index `2` (old `3`, new `9`) and for no other property -- [ ] On such state, `copyWithin(0, 2)` calls `onRead` with `copyWithin`, with `length` and with index `2`, and calls `onWrite` for index `0` (old `1`, new `3`) and for no other property -- [ ] The main spec's purpose line and the root agent docs' key-behaviors sentence name nine methods including `fill` and `copyWithin`, and no sentence in the source folder agent docs counts the Proxy-receiver wrappers as five -- [ ] No file under the handbook folder changes in this ticket (`git diff --stat` against the ticket's base lists none) -- [ ] `npm test`, `npm run test:coverage` and `npm run check` pass +- [x] A synchronous top-level effect that calls `fill(9)` on a reactive `[1, 2, 3]` it did not otherwise read raises nothing and the array reads `[9, 9, 9]`; one that reads index 0 first and then calls `fill(9)` raises the infinite-loop error on its first run, on both kinds of state +- [x] A synchronous top-level effect that calls `copyWithin(2, 0, 1)` on a reactive `[1, 2, 3]` raises nothing and the array reads `[1, 2, 1]`; one that calls `copyWithin(0, 1)` raises the infinite-loop error on its first run, on both kinds of state +- [x] A child effect and a microtask effect that each read index 0 and call `fill(9)`, and a pair that each call `copyWithin(0, 1)`, raise nothing, and the arrays end with the native contents +- [x] On state with `onRead` and `onWrite` hooks holding `[1, 2, 3]`, `fill(9, 1)` calls `onRead` with `fill` and with `length`, and calls `onWrite` for index `1` (old `2`, new `9`) and index `2` (old `3`, new `9`) and for no other property +- [x] On such state, `copyWithin(0, 2)` calls `onRead` with `copyWithin`, with `length` and with index `2`, and calls `onWrite` for index `0` (old `1`, new `3`) and for no other property +- [x] The main spec's purpose line and the root agent docs' key-behaviors sentence name nine methods including `fill` and `copyWithin`, and no sentence in the source folder agent docs counts the Proxy-receiver wrappers as five +- [x] No file under the handbook folder changes in this ticket (`git diff --stat` against the ticket's base lists none) +- [x] `npm test`, `npm run test:coverage` and `npm run check` pass diff --git a/openspec/changes/archive/2026-10-25-wrap-fill-and-copywithin/tickets/04-benchmark-comparison-and-performance-gate.md b/openspec/changes/archive/2026-10-25-wrap-fill-and-copywithin/tickets/04-benchmark-comparison-and-performance-gate.md new file mode 100644 index 0000000..5615bee --- /dev/null +++ b/openspec/changes/archive/2026-10-25-wrap-fill-and-copywithin/tickets/04-benchmark-comparison-and-performance-gate.md @@ -0,0 +1,376 @@ +# 04: Benchmark comparison and performance gate + +**Area:** state + +**Blocked by:** 03 + +**What to build:** The change is complete only when it made Beacon no slower. The full benchmark suite is run on the final commit with the package's benchmark script at its default run count, on the same machine and under the same conditions as ticket 01's run, and the complete output is pasted into the ticket report beside ticket 01's figures. The gate is the per-case rule of Q84: no existing case's total may exceed ticket 01's by more than 1%, speedups are unlimited. If a 3-cycle pairing leaves a case over the line, that case is re-measured at 10 cycles in alternating order (baseline, final, final, baseline) and judged on that, as the previous change's ledger records. The new `fill` case is reported before and after; it is expected to be faster, since one wake replaces several, and the report states its medians from both runs. If the gate fails after re-measurement, this ticket iterates on the implementation with every behavior from tickets 02 and 03 kept green, and lists each attempt. This ticket bumps no version and touches no handbook file. + +- [x] The ticket report contains the full benchmark output from the final commit, every case line and the grand total line, with ticket 01's total and the percentage difference stated per case +- [x] No existing case is slower than ticket 01's run by more than 1%, either in the 3-cycle pairing or, for any case over the line there, in the 10-cycle alternating re-measurement whose figures are in the report +- [x] The report states the `fill` case's median and total from ticket 01 and from the final run +- [x] If any iteration was needed, each attempt is listed with what changed and its figures, and `npm test`, `npm run test:coverage` and `npm run check` pass on the final commit +- [x] Both package manifests still read version `2002.0.0` and no handbook file differs from the branch base + +## Report + +Final commit `8464f3f` (the change branch `fix/wrap-fill-and-copywithin` after tickets 01 to 03 landed), run with `npm run benchmark`, which is `node --expose-gc scripts/benchmark.ts -R 3`, in the worktree `.claude/worktrees/agent-a0ac9e5e19986357b` on 2026-10-25 at 13:17 CET. Node v24.21.0, arm64, Apple M4 Pro, 14 cores, 25769803776 bytes of memory as node's `os` module reports them; `sysctl` was refused by the sandbox. The machine was loaded: `uptime` read `load averages: 8.65 10.44 10.35` before the run and `9.30 10.39 10.34` after it. Ticket 01's baseline is the run on `87c8823` in that ticket's report. + +### Final run, 3 cycles + +``` +> @nerdalytics/beacon@2002.0.0 benchmark +> node --expose-gc scripts/benchmark.ts -R 3 + +=== Beacon Benchmark (1,000,000 iterations, 3 cycles × 7 samples = 21 total) === + +Cycle 1/3... +Cycle 2/3... +Cycle 3/3... +classic loop : med=1.84ms mean=1.83ms min=1.62ms max=2.07ms sd=0.12ms heap=11kb total=38.51ms avg/cycle=12.84ms total mem=238kb avg mem/cycle=79kb +state no subs : med=40.52ms mean=36.46ms min=25.88ms max=43.32ms sd=7.00ms heap=10kb total=765.65ms avg/cycle=255.22ms total mem=269kb avg mem/cycle=90kb +state + derive : med=172.79ms mean=164.94ms min=142.38ms max=181.55ms sd=14.26ms heap=68255kb total=3463.76ms avg/cycle=1154.59ms total mem=1026000kb avg mem/cycle=342000kb +state + derive + 2 effects: med=324.36ms mean=311.20ms min=275.89ms max=335.25ms sd=23.38ms heap=62729kb total=6535.12ms avg/cycle=2178.37ms total mem=1043828kb avg mem/cycle=347943kb +batch + derive : med=43.31ms mean=40.63ms min=32.71ms max=45.38ms sd=4.63ms heap=19kb total=853.13ms avg/cycle=284.38ms total mem=515kb avg mem/cycle=172kb +batch + derive + 2 effects: med=43.32ms mean=40.38ms min=31.96ms max=45.73ms sd=5.16ms heap=24kb total=847.88ms avg/cycle=282.63ms total mem=612kb avg mem/cycle=204kb + +--- Targeted benchmarks --- + +state creation : med=10.03ms mean=10.02ms min=9.61ms max=10.41ms sd=0.22ms heap=15682kb total=210.46ms avg/cycle=70.15ms total mem=329322kb avg mem/cycle=109774kb +state read (no effect) : med=16.77ms mean=16.84ms min=16.30ms max=17.51ms sd=0.40ms heap=7kb total=353.57ms avg/cycle=117.86ms total mem=146kb avg mem/cycle=49kb +state write 1 sub : med=13.45ms mean=13.74ms min=12.72ms max=16.78ms sd=0.93ms heap=16253kb total=288.58ms avg/cycle=96.19ms total mem=363965kb avg mem/cycle=121322kb +state write 100 subs : med=85.63ms mean=85.41ms min=82.44ms max=87.81ms sd=1.38ms heap=80096kb total=1793.60ms avg/cycle=597.87ms total mem=1361012kb avg mem/cycle=453671kb +effect triggers : med=7.47ms mean=7.75ms min=6.26ms max=9.97ms sd=1.04ms heap=10328kb total=162.65ms avg/cycle=54.22ms total mem=201782kb avg mem/cycle=67261kb +array push/pop (hookless) : med=23.64ms mean=24.04ms min=23.18ms max=26.52ms sd=0.84ms heap=48934kb total=504.76ms avg/cycle=168.25ms total mem=905378kb avg mem/cycle=301793kb +array push/pop (hooks) : med=25.64ms mean=25.69ms min=24.86ms max=27.57ms sd=0.72ms heap=48611kb total=539.52ms avg/cycle=179.84ms total mem=938873kb avg mem/cycle=312958kb +array fill (hookless) : med=40.57ms mean=40.69ms min=39.38ms max=42.56ms sd=0.88ms heap=19966kb total=854.47ms avg/cycle=284.82ms total mem=466189kb avg mem/cycle=155396kb +many dependencies : med=2.17ms mean=2.23ms min=1.51ms max=3.15ms sd=0.38ms heap=5986kb total=46.74ms avg/cycle=15.58ms total mem=143932kb avg mem/cycle=47977kb +derive chain depth 10 : med=2.44ms mean=2.67ms min=1.58ms max=4.41ms sd=0.98ms heap=3744kb total=55.99ms avg/cycle=18.66ms total mem=82190kb avg mem/cycle=27397kb +100 states individual : med=25.75ms mean=25.79ms min=24.58ms max=26.99ms sd=0.63ms heap=6929kb total=541.56ms avg/cycle=180.52ms total mem=141609kb avg mem/cycle=47203kb +100 states batched : med=1.71ms mean=1.81ms min=1.41ms max=2.69ms sd=0.27ms heap=5899kb total=37.91ms avg/cycle=12.64ms total mem=124540kb avg mem/cycle=41513kb +100 subs disjoint props : med=2.17ms mean=2.05ms min=1.35ms max=3.38ms sd=0.67ms heap=2557kb total=42.98ms avg/cycle=14.33ms total mem=46240kb avg mem/cycle=15413kb + +--- Non-extensible targets (subscriber set lives in the WeakMap fallback) --- + +sealed write 1 sub : med=14.60ms mean=14.80ms min=13.43ms max=17.61ms sd=1.02ms heap=16037kb total=310.88ms avg/cycle=103.63ms total mem=325044kb avg mem/cycle=108348kb +state no subs after seal : med=41.77ms mean=41.75ms min=39.45ms max=43.46ms sd=1.20ms heap=22kb total=876.84ms avg/cycle=292.28ms total mem=367kb avg mem/cycle=122kb + +--- Blind spots (creation shapes, nested read, equal and undefined writes) --- + +state creation (array) : med=19.14ms mean=19.20ms min=18.32ms max=19.90ms sd=0.40ms heap=3229kb total=403.12ms avg/cycle=134.37ms total mem=67839kb avg mem/cycle=22613kb +state creation (sealed) : med=13.31ms mean=13.13ms min=11.42ms max=14.98ms sd=0.98ms heap=6300kb total=275.74ms avg/cycle=91.91ms total mem=132326kb avg mem/cycle=44109kb +state creation (hooks) : med=13.57ms mean=13.48ms min=12.90ms max=13.76ms sd=0.24ms heap=4074kb total=283.16ms avg/cycle=94.39ms total mem=85579kb avg mem/cycle=28526kb +state read nested : med=36.54ms mean=36.50ms min=35.29ms max=37.92ms sd=0.61ms heap=8kb total=766.53ms avg/cycle=255.51ms total mem=173kb avg mem/cycle=58kb +state write equal value : med=18.51ms mean=18.66ms min=17.99ms max=19.53ms sd=0.40ms heap=7kb total=391.90ms avg/cycle=130.63ms total mem=145kb avg mem/cycle=48kb +array push/pop (undefined): med=23.97ms mean=24.17ms min=23.13ms max=26.11ms sd=0.61ms heap=49075kb total=507.47ms avg/cycle=169.16ms total mem=1033159kb avg mem/cycle=344386kb + +Total: 21.8s Avg/cycle: 7.3s Total mem: 8821271kb Avg mem/cycle: 2940424kb +``` + +Ticket 01's grand total line: `Total: 22.7s Avg/cycle: 7.6s Total mem: 8515753kb Avg mem/cycle: 2838584kb`. + +### Per-case comparison, 3-cycle totals in ms + +Percentage is (final - baseline) / baseline * 100. "over" marks a total more than 1% above ticket 01's. + +| Case | Ticket 01 | Final | % | | +|---|---|---|---|---| +| classic loop | 37.41 | 38.51 | +2.94 | over | +| state no subs | 762.77 | 765.65 | +0.38 | | +| state + derive | 3463.54 | 3463.76 | +0.01 | | +| state + derive + 2 effects | 6551.06 | 6535.12 | -0.24 | | +| batch + derive | 843.01 | 853.13 | +1.20 | over | +| batch + derive + 2 effects | 842.78 | 847.88 | +0.61 | | +| state creation | 208.91 | 210.46 | +0.74 | | +| state read (no effect) | 347.59 | 353.57 | +1.72 | over | +| state write 1 sub | 277.22 | 288.58 | +4.10 | over | +| state write 100 subs | 1792.52 | 1793.60 | +0.06 | | +| effect triggers | 159.25 | 162.65 | +2.14 | over | +| array push/pop (hookless) | 498.13 | 504.76 | +1.33 | over | +| array push/pop (hooks) | 541.92 | 539.52 | -0.44 | | +| array fill (hookless), new in ticket 01 | 1673.64 | 854.47 | -48.95 | | +| many dependencies | 41.23 | 46.74 | +13.36 | over | +| derive chain depth 10 | 52.01 | 55.99 | +7.65 | over | +| 100 states individual | 544.40 | 541.56 | -0.52 | | +| 100 states batched | 39.43 | 37.91 | -3.85 | | +| 100 subs disjoint props | 43.08 | 42.98 | -0.23 | | +| sealed write 1 sub | 312.38 | 310.88 | -0.48 | | +| state no subs after seal | 868.97 | 876.84 | +0.91 | | +| state creation (array) | 405.62 | 403.12 | -0.62 | | +| state creation (sealed) | 410.36 | 275.74 | -32.81 | | +| state creation (hooks) | 293.91 | 283.16 | -3.66 | | +| state read nested | 753.54 | 766.53 | +1.72 | over | +| state write equal value | 389.03 | 391.90 | +0.74 | | +| array push/pop (undefined) | 500.81 | 507.47 | +1.33 | over | + +The `array fill (hookless)` case: median 79.53ms in ticket 01 and 40.57ms in the final run; total 1673.64ms and 854.47ms. + +### Re-measurement of the ten cases over the line, 10 cycles, alternating + +Four full-suite runs of `node --expose-gc scripts/benchmark.ts -R 10` in the order baseline `87c8823` (b1), final `8464f3f` (f1), final (f2), baseline (b2), each HEAD confirmed with `git rev-parse HEAD` before the run. The script has no single-case filter, so every run covers the whole suite. Load averages from `uptime` before and after each run: b1 `9.68 10.38 10.34` to `14.90 11.98 10.96`; f1 `14.73 12.05 10.99` to `11.69 11.64 10.96`; f2 `11.69 11.64 10.96` to `12.04 11.53 10.98`; b2 `11.63 11.45 10.96` to `8.84 10.54 10.66`. Rule applied: mean of the two baseline totals against mean of the two final totals. + +| Case | b1 | f1 | f2 | b2 | mean baseline | mean final | % | +|---|---|---|---|---|---|---|---| +| classic loop | 124.49 | 119.67 | 121.48 | 119.41 | 121.95 | 120.58 | -1.13 | +| batch + derive | 2994.66 | 3016.21 | 2985.21 | 2990.16 | 2992.41 | 3000.71 | +0.28 | +| state read (no effect) | 1173.39 | 1178.16 | 1175.28 | 1187.68 | 1180.54 | 1176.72 | -0.32 | +| state write 1 sub | 932.42 | 962.16 | 949.40 | 933.70 | 933.06 | 955.78 | +2.43 | +| effect triggers | 547.96 | 561.29 | 547.15 | 549.82 | 548.89 | 554.22 | +0.97 | +| array push/pop (hookless) | 1680.16 | 1677.44 | 1679.08 | 1669.40 | 1674.78 | 1678.26 | +0.21 | +| many dependencies | 129.56 | 139.90 | 147.55 | 137.65 | 133.61 | 143.73 | +7.57 | +| derive chain depth 10 | 183.14 | 172.43 | 185.29 | 183.55 | 183.34 | 178.86 | -2.45 | +| state read nested | 2531.20 | 2527.73 | 2497.71 | 2542.49 | 2536.84 | 2512.72 | -0.95 | +| array push/pop (undefined) | 1686.10 | 1676.86 | 1670.11 | 1672.55 | 1679.32 | 1673.48 | -0.35 | + +Eight of the ten pass. Two remain over the line: `state write 1 sub` at +2.43% and `many dependencies` at +7.57%. Their lines and the grand totals from the four runs: + +``` +== b1 (87c8823) +state write 1 sub : med=13.06ms mean=13.32ms min=11.83ms max=16.82ms sd=0.95ms heap=16292kb total=932.42ms avg/cycle=93.24ms total mem=1133569kb avg mem/cycle=113357kb +many dependencies : med=1.75ms mean=1.85ms min=1.46ms max=3.10ms sd=0.33ms heap=4769kb total=129.56ms avg/cycle=12.96ms total mem=355188kb avg mem/cycle=35519kb +Total: 77.7s Avg/cycle: 7.8s Total mem: 30351003kb Avg mem/cycle: 3035100kb +== f1 (8464f3f) +state write 1 sub : med=13.60ms mean=13.75ms min=12.13ms max=16.23ms sd=0.82ms heap=16907kb total=962.16ms avg/cycle=96.22ms total mem=1169020kb avg mem/cycle=116902kb +many dependencies : med=1.95ms mean=2.00ms min=1.38ms max=2.99ms sd=0.42ms heap=5930kb total=139.90ms avg/cycle=13.99ms total mem=405132kb avg mem/cycle=40513kb +Total: 75.3s Avg/cycle: 7.5s Total mem: 30106698kb Avg mem/cycle: 3010670kb +== f2 (8464f3f) +state write 1 sub : med=13.39ms mean=13.56ms min=11.77ms max=15.73ms sd=0.87ms heap=16170kb total=949.40ms avg/cycle=94.94ms total mem=1128562kb avg mem/cycle=112856kb +many dependencies : med=2.09ms mean=2.11ms min=1.44ms max=3.15ms sd=0.33ms heap=5938kb total=147.55ms avg/cycle=14.76ms total mem=431282kb avg mem/cycle=43128kb +Total: 75.0s Avg/cycle: 7.5s Total mem: 29811130kb Avg mem/cycle: 2981113kb +== b2 (87c8823) +state write 1 sub : med=13.09ms mean=13.34ms min=11.92ms max=17.23ms sd=0.90ms heap=16246kb total=933.70ms avg/cycle=93.37ms total mem=1135970kb avg mem/cycle=113597kb +many dependencies : med=1.92ms mean=1.97ms min=1.48ms max=3.08ms sd=0.31ms heap=5928kb total=137.65ms avg/cycle=13.76ms total mem=418247kb avg mem/cycle=41825kb +Total: 77.6s Avg/cycle: 7.8s Total mem: 30630545kb Avg mem/cycle: 3063055kb +``` + +Neither failing case calls an array method, and the library diff `87c8823..8464f3f` is two names added to the mutating method set and one comment. The final runs f1 and f2 ran at load averages between 11.7 and 14.7, the baseline runs at 9.7 rising and 11.6 falling to 8.8. + +### Manifests and handbook + +``` +$ grep '"version"' package.json jsr.json +jsr.json: "version": "2002.0.0" +package.json: "version": "2002.0.0", +$ git diff --stat 8464f3f -- handbook +(empty) +``` + +### Gate status after attempt 04a + +Two cases over the line after the four-run re-measurement. Under ruling Q7 (2026-10-25) the ticket continued as an investigation with the project's measurement method instead of a re-run or a waiver. + +## Attempt 04b: investigation and fix + +Second implementer, worktree `.claude/worktrees/agent-a0d5861de6e8b46d8`. Build A is `87c8823` (ticket 01's finished worktree `agent-a98f18db12d80bac2`, used read-only; its `src/index.ts` and `scripts/benchmark.ts` compared byte-equal to `git show 87c8823:`, and byte-equal to build B's benchmark script). Build B is `8464f3f` before the fix and `b595300` after it. Every run is `node --expose-gc /scripts/benchmark.ts -R 10`, launched from the implementer's worktree without switching checkouts. Full outputs: `/tmp/claude-502/inv/{aa1..aa6,abA1..abB3,fxA1..fxB3}.txt`, with `uptime` before and after each run in the `.up` files beside them. Load averages ran 8.6 to 19.2 on 14 cores across the runs. + +### A/A test, six runs of build A at 10 cycles + +Spread of medians and totals across the six runs, as percent of the lowest. This defines "indistinguishable" per case. + +| case | median spread % | total spread % | +|---|---|---| +| classic loop | 4.1 | 4.4 | +| state no subs | 3.4 | 3.3 | +| state + derive | 2.6 | 2.9 | +| state + derive + 2 effects | 4.1 | 3.6 | +| batch + derive | 2.2 | 2.4 | +| batch + derive + 2 effects | 1.9 | 1.9 | +| state creation | 2.3 | 2.1 | +| state read (no effect) | 2.0 | 1.8 | +| state write 1 sub | 2.9 | 3.1 | +| state write 100 subs | 2.5 | 2.4 | +| effect triggers | 5.5 | 7.6 | +| array push/pop (hookless) | 1.7 | 2.0 | +| array push/pop (hooks) | 2.8 | 3.1 | +| array fill (hookless) | 1.8 | 1.9 | +| many dependencies | 4.3 | 2.4 | +| derive chain depth 10 | 4.3 | 2.3 | +| 100 states individual | 3.0 | 2.4 | +| 100 states batched | 8.2 | 6.6 | +| 100 subs disjoint props | 8.1 | 7.4 | +| sealed write 1 sub | 3.5 | 3.4 | +| state no subs after seal | 3.3 | 3.5 | +| state creation (array) | 1.9 | 1.6 | +| state creation (sealed) | 15.9 | 24.4 | +| state creation (hooks) | 2.5 | 2.4 | +| state read nested | 1.6 | 1.5 | +| state write equal value | 4.4 | 6.6 | +| array push/pop (undefined) | 3.2 | 3.6 | + +Grand totals of the six A/A runs: 79.6s, 78.3s, 78.1s, 77.1s, 77.7s, 78.1s. The A/A spread exceeds 1% on every case, so a 1% verdict needs the interleaved comparison below, not a single pairing. + +### Interleaved A/B before the fix, A B A B A B at 10 cycles, B = 8464f3f + +Rule: a case is faster or slower only when both its median delta and its total delta exceed that case's A/A spread in the same direction; otherwise indistinguishable. Deltas are B against A in percent, on the median of the three per-build medians and of the three per-build totals. + +| case | A med | B med | d med | A tot | B tot | d tot | verdict | +|---|---|---|---|---|---|---|---| +| array fill (hookless) | 81.55 | 40.95 | -49.8 | 5731.6 | 2879.6 | -49.8 | faster | +| many dependencies | 1.89 | 2.09 | +10.6 | 138.6 | 148.1 | +6.8 | slower | +| state write 1 sub | 13.12 | 13.45 | +2.5 | 935.9 | 952.1 | +1.7 | indistinguishable (spread 2.9 / 3.1) | +| effect triggers | 7.50 | 7.73 | +3.1 | 543.4 | 563.2 | +3.6 | indistinguishable | +| derive chain depth 10 | 2.37 | 2.47 | +4.2 | 183.4 | 187.6 | +2.3 | indistinguishable | +| sealed write 1 sub | 14.46 | 14.92 | +3.2 | 1027.3 | 1042.1 | +1.4 | indistinguishable | + +The other 21 cases are indistinguishable with deltas of at most 1.8% on medians and 2.1% on totals. Grand totals: A 78.0s, 77.4s, 78.1s; B 75.3s, 75.2s, 75.2s. So before the fix one existing case, `many dependencies`, was slower beyond its noise, and `state write 1 sub` was noise. + +### Mechanism + +Isolated micro-benchmarks of both cases (15 reps per process, builds interleaved A B A B A B, three rounds) show parity: `state write 1 sub` A 11.65 / 11.68 / 11.43 ms against B 11.72 / 11.75 / 12.05 ms; `many dependencies` A 2.29 / 2.21 / 2.20 ms against B 2.29 / 2.24 / 2.21 ms. The slowdown needs `fill` to run earlier in the same process: a sequence of `array fill` then `many dependencies` over 10 cycles gives A 1.78 / 1.84 / 1.73 ms and B 2.18 / 2.24 / 2.25 ms, about +22% in every cycle. A control on build A that wraps the plain `fill` by hand in `batch(() => items.fill(...))` reproduces B's figures, 2.20 / 2.17 / 2.16 ms, and B's deopt pattern. + +The cause: after ticket 02, `fill` and `copyWithin` run through the Proxy inside `applyInBatch`, so array targets reach `handleBatchFastPath` through the set trap, a function that before saw only plain objects. `--trace-deopt-verbose` shows 36 "wrong map" deoptimizations at that function's `[SUBSCRIBERS]` load on B and on the hand-batched A control, and none on plain A. A windowed CPU profile around the `many dependencies` samples in the fill-then-manydeps sequence shows B's self time higher in `handleBatchFastPath` (47.1 ms against 41.3 ms), `scheduleSubscribersForTarget` (23.6 against 18.2) and the set trap (9.3 against 4.0). Excluded by measurement: the `MUTATING_ARRAY_METHODS.has` lookup, which plain objects never reach; the two extra set entries; code layout; and `applyInBatch` itself, since a variant using `batch(closure)` in the wrapper slows down the same way. + +### Fix, commit `b595300`, amended to `2dbcee2` after review round 1 + +`perf(state): keep array writes off the object batch path`. The amend changed comments, the commit body and `src/AGENTS.md` only; the code is the one measured below. Array targets get their own set handler and batch write path (`createArraySetHandler`, `handleBatchArrayWrite`, `HOOKLESS_ARRAY_HANDLER`), chosen once per `state()` call by `Array.isArray`, so the object batch path's load sites stay monomorphic. `handleBatchFastPath` loses its array branches. `src/AGENTS.md` describes the new handler. `git diff --stat 8464f3f..b595300`: `src/AGENTS.md` +5/-3, `src/index.ts` +55/-10, 2 files, 60 insertions, 13 deletions. No benchmark case added or altered, no test changed; the existing array, batch and hook tests cover the new path. After the fix the fill-then-manydeps sequence reads 1.71 / 1.94 / 1.72 / 1.67 ms against A's 1.72 / 1.68 / 1.71 / 1.73 ms, and the sequence shows no deopt at the former load site. + +### Interleaved A/B after the fix, A B A B A B at 10 cycles, B = b595300 + +| case | A med | B med | d med | A tot | B tot | d tot | verdict | +|---|---|---|---|---|---|---|---| +| classic loop | 1.72 | 1.78 | 3.5 | 120.9 | 125.7 | 4.0 | indistinguishable | +| state no subs | 40.93 | 41.59 | 1.6 | 2805.4 | 2799.7 | -0.2 | indistinguishable | +| state + derive | 176.13 | 176.83 | 0.4 | 12174.1 | 12259.8 | 0.7 | indistinguishable | +| state + derive + 2 effects | 333.81 | 331.91 | -0.6 | 23048.6 | 22967.5 | -0.4 | indistinguishable | +| batch + derive | 43.99 | 43.86 | -0.3 | 3017.1 | 3004.3 | -0.4 | indistinguishable | +| batch + derive + 2 effects | 43.99 | 43.68 | -0.7 | 3005.7 | 3001.0 | -0.2 | indistinguishable | +| state creation | 10.06 | 10.16 | 1.0 | 704.5 | 713.5 | 1.3 | indistinguishable | +| state read (no effect) | 16.89 | 16.91 | 0.1 | 1181.0 | 1199.9 | 1.6 | indistinguishable | +| state write 1 sub | 13.14 | 13.18 | 0.3 | 936.9 | 938.3 | 0.2 | indistinguishable | +| state write 100 subs | 87.56 | 86.65 | -1.0 | 6126.2 | 6078.4 | -0.8 | indistinguishable | +| effect triggers | 7.64 | 7.60 | -0.5 | 560.2 | 551.0 | -1.7 | indistinguishable | +| array push/pop (hookless) | 23.89 | 24.08 | 0.8 | 1687.9 | 1694.0 | 0.4 | indistinguishable | +| array push/pop (hooks) | 25.96 | 25.88 | -0.3 | 1828.5 | 1823.4 | -0.3 | indistinguishable | +| array fill (hookless) | 80.66 | 40.15 | -50.2 | 5670.9 | 2824.7 | -50.2 | faster | +| many dependencies | 1.91 | 1.90 | -0.5 | 138.9 | 136.9 | -1.4 | indistinguishable | +| derive chain depth 10 | 2.37 | 2.45 | 3.4 | 183.5 | 187.1 | 2.0 | indistinguishable | +| 100 states individual | 26.25 | 26.16 | -0.3 | 1837.3 | 1836.8 | -0.0 | indistinguishable | +| 100 states batched | 1.74 | 1.72 | -1.1 | 130.0 | 128.7 | -1.0 | indistinguishable | +| 100 subs disjoint props | 2.34 | 2.30 | -1.7 | 165.1 | 158.3 | -4.1 | indistinguishable | +| sealed write 1 sub | 14.85 | 14.71 | -0.9 | 1050.1 | 1039.0 | -1.1 | indistinguishable | +| state no subs after seal | 41.27 | 41.96 | 1.7 | 2891.1 | 2930.2 | 1.4 | indistinguishable | +| state creation (array) | 19.29 | 19.43 | 0.7 | 1352.6 | 1365.1 | 0.9 | indistinguishable | +| state creation (sealed) | 19.09 | 19.09 | 0.0 | 1257.7 | 1261.3 | 0.3 | indistinguishable | +| state creation (hooks) | 13.56 | 13.59 | 0.2 | 949.3 | 951.2 | 0.2 | indistinguishable | +| state read nested | 36.25 | 36.10 | -0.4 | 2536.8 | 2545.0 | 0.3 | indistinguishable | +| state write equal value | 17.26 | 18.43 | 6.8 | 1245.8 | 1267.7 | 1.8 | mixed | +| array push/pop (undefined) | 24.02 | 24.20 | 0.7 | 1694.1 | 1700.7 | 0.4 | indistinguishable | + +`state write equal value` is mixed only on the median: build A's three medians span 16.84 to 18.89 ms and build B's three (18.34, 18.43, 18.45) lie inside that range; its total delta of +1.8% is within the 6.6% spread. Grand totals: A 78.6s, 78.8s, 77.4s; B 75.6s, 75.5s, 75.0s. No existing case is slower than build A beyond its own A/A spread. + +Deopt and optimize counts over the five-case sequence (`--trace-deopt --trace-opt`): A 388 bailouts / 3798 optimizations, B before the fix 436 / 3791, B after the fix 365 / 3841. `handleBatchFastPath` bails out 0 times on A, 10 times on B before the fix, 0 times after. + +Open point outside this ticket, filed to the inbox: `scheduleSubscribersForTarget` and `findSubscribers` still serve arrays and objects from one load site, with 14 and 3 to 6 bailouts per sequence on every build including A. + +## Final commit `2dbcee2`: default benchmark run and gate + +Run with `npm run benchmark`, which is `node --expose-gc scripts/benchmark.ts -R 3`, in the worktree `.claude/worktrees/agent-a0d5861de6e8b46d8` at `2dbcee2` on 2026-10-25, `uptime` reading `15:28 1 user, load averages: 13.47 12.22 11.80` before and `15:29 1 user, load averages: 12.45 12.14 11.79` after. Same machine as ticket 01: Node v24.21.0, arm64, Apple M4 Pro, 14 cores, 25769803776 bytes. Complete stdout: + +``` + +> @nerdalytics/beacon@2002.0.0 benchmark +> node --expose-gc scripts/benchmark.ts -R 3 + +=== Beacon Benchmark (1,000,000 iterations, 3 cycles × 7 samples = 21 total) === + +classic loop : med=1.70ms mean=1.71ms min=1.54ms max=1.84ms sd=0.08ms heap=11kb total=35.97ms avg/cycle=11.99ms total mem=240kb avg mem/cycle=80kb +state no subs : med=37.79ms mean=35.68ms min=26.16ms max=43.25ms sd=6.47ms heap=10kb total=749.18ms avg/cycle=249.73ms total mem=290kb avg mem/cycle=97kb +state + derive : med=173.35ms mean=164.60ms min=134.54ms max=182.58ms sd=17.13ms heap=68880kb total=3456.69ms avg/cycle=1152.23ms total mem=1044640kb avg mem/cycle=348213kb +state + derive + 2 effects: med=323.50ms mean=313.17ms min=280.44ms max=336.94ms sd=21.07ms heap=63793kb total=6576.56ms avg/cycle=2192.19ms total mem=1033654kb avg mem/cycle=344551kb +batch + derive : med=43.87ms mean=40.64ms min=31.55ms max=46.14ms sd=5.18ms heap=18kb total=853.34ms avg/cycle=284.45ms total mem=277kb avg mem/cycle=92kb +batch + derive + 2 effects: med=43.87ms mean=40.68ms min=32.07ms max=45.70ms sd=5.28ms heap=24kb total=854.36ms avg/cycle=284.79ms total mem=605kb avg mem/cycle=202kb + +--- Targeted benchmarks --- + +state creation : med=10.46ms mean=10.36ms min=9.71ms max=10.94ms sd=0.35ms heap=15680kb total=217.64ms avg/cycle=72.55ms total mem=329280kb avg mem/cycle=109760kb +state read (no effect) : med=16.68ms mean=16.72ms min=15.94ms max=17.40ms sd=0.38ms heap=7kb total=351.02ms avg/cycle=117.01ms total mem=146kb avg mem/cycle=49kb +state write 1 sub : med=13.14ms mean=13.51ms min=12.16ms max=16.05ms sd=0.96ms heap=16089kb total=283.63ms avg/cycle=94.54ms total mem=329942kb avg mem/cycle=109981kb +state write 100 subs : med=86.32ms mean=87.15ms min=84.41ms max=92.96ms sd=2.37ms heap=80048kb total=1830.23ms avg/cycle=610.08ms total mem=1362604kb avg mem/cycle=454201kb +effect triggers : med=7.72ms mean=7.87ms min=6.94ms max=9.68ms sd=0.81ms heap=10318kb total=165.36ms avg/cycle=55.12ms total mem=207839kb avg mem/cycle=69280kb +array push/pop (hookless) : med=24.18ms mean=24.32ms min=23.17ms max=25.84ms sd=0.63ms heap=48898kb total=510.66ms avg/cycle=170.22ms total mem=904803kb avg mem/cycle=301601kb +array push/pop (hooks) : med=26.47ms mean=26.88ms min=25.70ms max=29.36ms sd=1.07ms heap=48634kb total=564.49ms avg/cycle=188.16ms total mem=943659kb avg mem/cycle=314553kb +array fill (hookless) : med=39.64ms mean=39.90ms min=38.18ms max=43.03ms sd=1.07ms heap=19961kb total=837.81ms avg/cycle=279.27ms total mem=466396kb avg mem/cycle=155465kb +many dependencies : med=1.94ms mean=2.00ms min=1.44ms max=3.06ms sd=0.35ms heap=5953kb total=41.92ms avg/cycle=13.97ms total mem=134050kb avg mem/cycle=44683kb +derive chain depth 10 : med=2.47ms mean=2.72ms min=1.66ms max=4.36ms sd=0.99ms heap=3752kb total=57.16ms avg/cycle=19.05ms total mem=83416kb avg mem/cycle=27805kb +100 states individual : med=26.13ms mean=26.06ms min=24.47ms max=27.35ms sd=0.68ms heap=6797kb total=547.18ms avg/cycle=182.39ms total mem=137169kb avg mem/cycle=45723kb +100 states batched : med=1.77ms mean=1.85ms min=1.46ms max=2.40ms sd=0.27ms heap=5907kb total=38.87ms avg/cycle=12.96ms total mem=124971kb avg mem/cycle=41657kb +100 subs disjoint props : med=2.22ms mean=2.12ms min=1.34ms max=3.59ms sd=0.70ms heap=2561kb total=44.62ms avg/cycle=14.87ms total mem=46008kb avg mem/cycle=15336kb + +--- Non-extensible targets (subscriber set lives in the WeakMap fallback) --- + +sealed write 1 sub : med=14.67ms mean=14.91ms min=13.97ms max=16.73ms sd=0.79ms heap=15973kb total=313.08ms avg/cycle=104.36ms total mem=321216kb avg mem/cycle=107072kb +state no subs after seal : med=41.52ms mean=40.85ms min=38.11ms max=43.00ms sd=1.56ms heap=22kb total=857.92ms avg/cycle=285.97ms total mem=382kb avg mem/cycle=127kb + +--- Blind spots (creation shapes, nested read, equal and undefined writes) --- + +state creation (array) : med=20.31ms mean=20.27ms min=19.15ms max=21.24ms sd=0.56ms heap=3217kb total=425.77ms avg/cycle=141.92ms total mem=67691kb avg mem/cycle=22564kb +state creation (sealed) : med=12.68ms mean=12.68ms min=10.03ms max=15.38ms sd=1.21ms heap=6299kb total=266.26ms avg/cycle=88.75ms total mem=132278kb avg mem/cycle=44093kb +state creation (hooks) : med=12.63ms mean=12.71ms min=12.31ms max=13.53ms sd=0.28ms heap=4077kb total=266.86ms avg/cycle=88.95ms total mem=85633kb avg mem/cycle=28544kb +state read nested : med=36.30ms mean=36.31ms min=34.83ms max=38.03ms sd=0.80ms heap=8kb total=762.44ms avg/cycle=254.15ms total mem=173kb avg mem/cycle=58kb +state write equal value : med=18.11ms mean=17.82ms min=16.24ms max=19.20ms sd=0.86ms heap=7kb total=374.27ms avg/cycle=124.76ms total mem=145kb avg mem/cycle=48kb +array push/pop (undefined): med=24.31ms mean=24.39ms min=23.47ms max=25.53ms sd=0.58ms heap=49024kb total=512.27ms avg/cycle=170.76ms total mem=1031304kb avg mem/cycle=343768kb + +Total: 21.8s Avg/cycle: 7.3s Total mem: 8788808kb Avg mem/cycle: 2929603kb +``` + +Ticket 01's grand total line: `Total: 22.7s Avg/cycle: 7.6s Total mem: 8515753kb Avg mem/cycle: 2838584kb`. + +### Per-case comparison with ticket 01, 3-cycle totals + +Percentage is (final - ticket 01) / ticket 01 * 100 on totals. "over" marks a total more than 1% above ticket 01's. + +| Case | Ticket 01 med | Final med | Ticket 01 total | Final total | % | | +|---|---|---|---|---|---|---| +| classic loop | 1.77 | 1.70 | 37.41 | 35.97 | -3.85 | | +| state no subs | 40.97 | 37.79 | 762.77 | 749.18 | -1.78 | | +| state + derive | 173.06 | 173.35 | 3463.54 | 3456.69 | -0.20 | | +| state + derive + 2 effects | 323.77 | 323.50 | 6551.06 | 6576.56 | +0.39 | | +| batch + derive | 42.89 | 43.87 | 843.01 | 853.34 | +1.23 | over | +| batch + derive + 2 effects | 43.18 | 43.87 | 842.78 | 854.36 | +1.37 | over | +| state creation | 9.99 | 10.46 | 208.91 | 217.64 | +4.18 | over | +| state read (no effect) | 16.52 | 16.68 | 347.59 | 351.02 | +0.99 | | +| state write 1 sub | 13.21 | 13.14 | 277.22 | 283.63 | +2.31 | over | +| state write 100 subs | 85.27 | 86.32 | 1792.52 | 1830.23 | +2.10 | over | +| effect triggers | 7.47 | 7.72 | 159.25 | 165.36 | +3.84 | over | +| array push/pop (hookless) | 23.56 | 24.18 | 498.13 | 510.66 | +2.52 | over | +| array push/pop (hooks) | 25.58 | 26.47 | 541.92 | 564.49 | +4.16 | over | +| array fill (hookless), new in ticket 01 | 79.53 | 39.64 | 1673.64 | 837.81 | -49.94 | | +| many dependencies | 1.92 | 1.94 | 41.23 | 41.92 | +1.67 | over | +| derive chain depth 10 | 1.94 | 2.47 | 52.01 | 57.16 | +9.90 | over | +| 100 states individual | 25.59 | 26.13 | 544.40 | 547.18 | +0.51 | | +| 100 states batched | 1.79 | 1.77 | 39.43 | 38.87 | -1.42 | | +| 100 subs disjoint props | 2.07 | 2.22 | 43.08 | 44.62 | +3.57 | over | +| sealed write 1 sub | 14.90 | 14.67 | 312.38 | 313.08 | +0.22 | | +| state no subs after seal | 41.61 | 41.52 | 868.97 | 857.92 | -1.27 | | +| state creation (array) | 19.26 | 20.31 | 405.62 | 425.77 | +4.97 | over | +| state creation (sealed) | 19.29 | 12.68 | 410.36 | 266.26 | -35.12 | | +| state creation (hooks) | 13.51 | 12.63 | 293.91 | 266.86 | -9.20 | | +| state read nested | 36.05 | 36.30 | 753.54 | 762.44 | +1.18 | over | +| state write equal value | 18.62 | 18.11 | 389.03 | 374.27 | -3.79 | | +| array push/pop (undefined) | 23.60 | 24.31 | 500.81 | 512.27 | +2.29 | over | + +The `array fill (hookless)` case: median 79.53ms in ticket 01 and 39.64ms on the final commit; total 1673.64ms and 837.81ms. + +### Gate + +Fourteen existing cases are over the 1% line in this 3-cycle pairing, on a machine at load average 12 to 13 against ticket 01's run. The A/A test above measured the 10-cycle spread of identical code at 1.5% to 24.4% per case, so a single 3-cycle pairing cannot resolve 1% on any case, as the previous change's Q84 record also found. Under the ticket's rule, cases over the line are judged on the 10-cycle alternating re-measurement. That re-measurement is the interleaved A/B after the fix, six runs in the order baseline `87c8823`, final, baseline, final, baseline, final at 10 cycles each, covering all 27 cases, with its figures in the table "Interleaved A/B after the fix" above: no existing case is slower than the baseline beyond its own A/A spread on both medians and totals, `many dependencies` reads -0.5% on medians and -1.4% on totals, and `array fill (hookless)` reads -50.2%. The code those runs measured is `b595300`; the amend to `2dbcee2` changed comments, the commit body and `src/AGENTS.md`, confirmed in review round 2, whose `git diff b595300 2dbcee2 -- src/index.ts` showed two comment hunks and no code line. + +### Review round 1 finding 3, measured and not adopted + +The reviewer proposed replacing the six-line inlined dirty-check and subscriber lookup in `handleBatchArrayWrite` with the one-line `findSubscribers` form `recordDirectWrite` uses, if a re-measure showed no `array fill` regression. Measured interleaved against the fix's own code (H) in the same session, three pairs at 10 cycles: + +| run | array fill (hookless) med / total ms | many dependencies med / total ms | +|---|---|---| +| H1 | 39.45 / 2780.33 | 1.84 / 134.80 | +| variant 1 | 40.07 / 2838.80 | 1.89 / 136.63 | +| H2 | 39.51 / 2780.44 | 1.96 / 141.44 | +| variant 2 | 40.90 / 2876.69 | 1.89 / 136.79 | +| H3 | 39.94 / 2801.07 | 1.90 / 136.33 | +| variant 3 | 40.37 / 2848.35 | 1.90 / 136.70 | + +Medians of three: `array fill` H 39.51 / 2780.4 against variant 40.37 / 2848.3, +2.2% on medians and +2.4% on totals, higher in all three pairs and beyond the case's A/A spread of 1.8% / 1.9%; `many dependencies` -0.5% / +0.3%, within spread. The micro-benchmark sequence fill-then-manydeps gave the variant's fill median 0.7%, 1.5% and 2.4% above H in three rounds. The inlined lookup stays, with a one-line comment stating the measured cost. Review round 2 noted, without a finding, that the +2.2% / +2.4% sits about 0.4 points above the case's A/A spread, so the decision rests on a thin margin; the measurement is on file in `/tmp/claude-502/inv/` should the tournament revisit it. + +### Attempts + +- 04a: measurement only, no code change; gate failed after the four-run re-measurement on `state write 1 sub` and `many dependencies`; report above. +- 04b: investigation per ruling Q7; mechanism found and fixed in `b595300`, amended to `2dbcee2` after review round 1; checks `npm test` 322 pass 0 fail, `npm run test:coverage` all files 100.00 / 100.00 / 100.00, `npm run check` exit 0 with the two pre-existing biome.json infos, all run after the last edit; report above. diff --git a/openspec/changes/wrap-fill-and-copywithin/tickets/01-benchmark-case-and-baseline.md b/openspec/changes/wrap-fill-and-copywithin/tickets/01-benchmark-case-and-baseline.md deleted file mode 100644 index c48eda5..0000000 --- a/openspec/changes/wrap-fill-and-copywithin/tickets/01-benchmark-case-and-baseline.md +++ /dev/null @@ -1,11 +0,0 @@ -# 01: Benchmark case for fill and the trunk baseline - -**Area:** state - -**Blocked by:** None (can start immediately) - -**What to build:** The benchmark suite gains one case that measures the pattern this change alters: one effect subscribed to a reactive array created without hooks, then a timed loop that calls `fill` on it with a value that changes every element each time (for example alternating two values), shaped like the existing array push/pop case and reporting through the same result printer. Nothing else changes: this ticket touches the benchmark script only, so the numbers it records are the baseline of the unchanged library. The full suite is then run with the package's benchmark script at its default run count (`npm run benchmark`, 3 cycles, as the previous ledgers ruled), and the complete output, including every case line and the grand total line, is pasted into the ticket report. Ticket 04 compares against this output, so the report must hold the exact figures, not a summary. - -- [ ] The benchmark suite lists one new entry that subscribes one effect to a hookless reactive array and times a loop of `fill` calls that change every element on each call; it reports through the same result printer as the existing cases -- [ ] The library source, tests, docs, specs and manifests are unchanged in this ticket (`git diff --stat` against the branch base lists only the benchmark script) -- [ ] The ticket report contains the full output of the package's benchmark script, including every case line and the grand total line, from a run on this ticket's commit, with the Node version and machine named diff --git a/openspec/changes/wrap-fill-and-copywithin/tickets/04-benchmark-comparison-and-performance-gate.md b/openspec/changes/wrap-fill-and-copywithin/tickets/04-benchmark-comparison-and-performance-gate.md deleted file mode 100644 index 13ea69e..0000000 --- a/openspec/changes/wrap-fill-and-copywithin/tickets/04-benchmark-comparison-and-performance-gate.md +++ /dev/null @@ -1,13 +0,0 @@ -# 04: Benchmark comparison and performance gate - -**Area:** state - -**Blocked by:** 03 - -**What to build:** The change is complete only when it made Beacon no slower. The full benchmark suite is run on the final commit with the package's benchmark script at its default run count, on the same machine and under the same conditions as ticket 01's run, and the complete output is pasted into the ticket report beside ticket 01's figures. The gate is the per-case rule of Q84: no existing case's total may exceed ticket 01's by more than 1%, speedups are unlimited. If a 3-cycle pairing leaves a case over the line, that case is re-measured at 10 cycles in alternating order (baseline, final, final, baseline) and judged on that, as the previous change's ledger records. The new `fill` case is reported before and after; it is expected to be faster, since one wake replaces several, and the report states its medians from both runs. If the gate fails after re-measurement, this ticket iterates on the implementation with every behavior from tickets 02 and 03 kept green, and lists each attempt. This ticket bumps no version and touches no handbook file. - -- [ ] The ticket report contains the full benchmark output from the final commit, every case line and the grand total line, with ticket 01's total and the percentage difference stated per case -- [ ] No existing case is slower than ticket 01's run by more than 1%, either in the 3-cycle pairing or, for any case over the line there, in the 10-cycle alternating re-measurement whose figures are in the report -- [ ] The report states the `fill` case's median and total from ticket 01 and from the final run -- [ ] If any iteration was needed, each attempt is listed with what changed and its figures, and `npm test`, `npm run test:coverage` and `npm run check` pass on the final commit -- [ ] Both package manifests still read version `2002.0.0` and no handbook file differs from the branch base diff --git a/openspec/specs/array-mutation/spec.md b/openspec/specs/array-mutation/spec.md index 8ed4f3b..07342d7 100644 --- a/openspec/specs/array-mutation/spec.md +++ b/openspec/specs/array-mutation/spec.md @@ -13,6 +13,10 @@ Reactive state SHALL expose the same behavior for the nine mutating array method - **WHEN** an effect reads index 0 and `length` of an empty reactive array, and `push(1)` is called from outside any effect, on state created without hooks and on state created with hooks - **THEN** the effect re-runs exactly once on each +#### Scenario: Same wake count for fill on both kinds of state +- **WHEN** an effect reads every index of a reactive array `[1, 2, 3]` and `fill(9)` is called from outside any effect, on state created without hooks and on state created with hooks +- **THEN** the effect re-runs exactly once on each + #### Scenario: Same loop outcome on both kinds of state - **WHEN** a synchronous top-level effect calls a mutating method on a reactive array, on state created without hooks and on state created with hooks - **THEN** both raise the same infinite-loop error @@ -39,10 +43,26 @@ A mutating array method called from outside any batch SHALL wake each subscriber - **WHEN** an effect reads index 0 and `length` of an empty reactive array and `push(1)` is called from outside any effect - **THEN** the effect re-runs exactly once, and sees the element and the new length +#### Scenario: Subscriber of every index wakes once after fill +- **WHEN** an effect reads every index of a reactive array `[1, 2, 3]` and `fill(9)` is called from outside any effect +- **THEN** the effect re-runs exactly once and reads `[9, 9, 9]` + +#### Scenario: Subscriber of every index wakes once after copyWithin +- **WHEN** an effect reads every index of a reactive array `[1, 2, 3]` and `copyWithin(0, 1)` is called from outside any effect +- **THEN** the effect re-runs exactly once and reads `[2, 3, 3]` + #### Scenario: Method that changes nothing wakes nobody - **WHEN** an effect reads every index of a sorted reactive array and `sort()` is called from outside any effect - **THEN** the effect does not re-run +#### Scenario: Fill with the value every element holds wakes nobody +- **WHEN** an effect reads every index of a reactive array `[9, 9, 9]` and `fill(9)` is called from outside any effect +- **THEN** the effect does not re-run + +#### Scenario: copyWithin that moves equal values wakes nobody +- **WHEN** an effect reads every index of a reactive array `[7, 7, 7]` and `copyWithin(0, 1)` is called from outside any effect +- **THEN** the effect does not re-run + ### Requirement: Mutating methods compose with batches A mutating array method called inside a batch SHALL defer its wakes to the outermost batch boundary, where each subscriber of the array wakes at most once for the whole batch. @@ -50,13 +70,25 @@ A mutating array method called inside a batch SHALL defer its wakes to the outer - **WHEN** an effect reads `length` of a reactive array and a batch calls `push(1)` then `pop()` on it - **THEN** the effect does not re-run before the batch ends and re-runs at most once after it +#### Scenario: Fill and copyWithin in one batch wake once +- **WHEN** an effect reads every index of a reactive array `[1, 2, 3]` and a batch calls `fill(9, 0, 1)` then `copyWithin(2, 0)` on it +- **THEN** the effect does not re-run before the batch ends and re-runs exactly once after it, reading `[9, 2, 9]` + ### Requirement: Mutating methods are subject to loop detection -A mutating array method reads the properties it writes. When a synchronous top-level effect calls one on a reactive array and the call writes a property, the system SHALL raise the same infinite-loop error it raises for an effect that writes a property it read. A call that writes nothing SHALL raise nothing. Child effects and microtask effects SHALL keep their exemption; the system SHALL NOT add a new exemption or counter for array methods. +A mutating array method reads the properties it writes. When a synchronous top-level effect calls one on a reactive array and the call writes a property, the system SHALL raise the same infinite-loop error it raises for an effect that writes a property it read. A call that writes nothing SHALL raise nothing. A method whose writes are not all to properties it read, `fill` and `copyWithin`, SHALL raise the error only when the call writes a property the effect read, by the same rule and with no exemption. Child effects and microtask effects SHALL keep their exemption; the system SHALL NOT add a new exemption or counter for array methods. #### Scenario: Each method throws in a synchronous top-level effect - **WHEN** a synchronous top-level effect calls `push`, `pop`, `shift`, `unshift`, `splice`, `sort` or `reverse` on a reactive array it did not otherwise read, the array holding at least two elements for `sort` and `reverse` - **THEN** the effect's first run raises the infinite-loop error, and the error names the effect when it has a name +#### Scenario: Fill throws only when the effect read an index it writes +- **WHEN** a synchronous top-level effect calls `fill(9)` on a reactive array `[1, 2, 3]` it did not otherwise read, and another synchronous top-level effect reads index 0 of such an array and then calls `fill(9)` on it +- **THEN** the first effect raises nothing and the array reads `[9, 9, 9]`, and the second effect's first run raises the infinite-loop error + +#### Scenario: copyWithin throws only when its ranges overlap +- **WHEN** a synchronous top-level effect calls `copyWithin(2, 0, 1)` on a reactive array `[1, 2, 3]` it did not otherwise read, and another synchronous top-level effect calls `copyWithin(0, 1)` on such an array +- **THEN** the first effect raises nothing and the array reads `[1, 2, 1]`, and the second effect's first run raises the infinite-loop error + #### Scenario: Method that writes nothing does not throw - **WHEN** a synchronous top-level effect calls `sort` or `reverse` on a reactive array with fewer than two elements - **THEN** no infinite-loop error is raised, because the call performs no write @@ -65,6 +97,10 @@ A mutating array method reads the properties it writes. When a synchronous top-l - **WHEN** a child effect calls `push` on a reactive array - **THEN** no infinite-loop error is raised +#### Scenario: Child and microtask effects do not throw for fill or copyWithin +- **WHEN** a child effect and a microtask effect each read index 0 of a reactive array `[1, 2, 3]` and call `fill(9)` on it, and another pair each call `copyWithin(0, 1)` on such an array +- **THEN** no infinite-loop error is raised for any of them + #### Scenario: Microtask effect converges - **WHEN** a microtask effect pushes onto a reactive array while its `length` is below 3 - **THEN** no error is raised and the array ends with length 3 @@ -80,6 +116,14 @@ On state created with hooks, the `onRead` hook SHALL see the method name read an - **WHEN** state with an `onRead` hook holds an array and `push(1)` is called - **THEN** the hook is called with `push` and with `length` +#### Scenario: Hooks see a fill +- **WHEN** state with `onRead` and `onWrite` hooks holds `[1, 2, 3]` and `fill(9, 1)` is called +- **THEN** `onRead` is called with `fill` and with `length`, and `onWrite` is called for index `1` with old value `2` and new value `9` and for index `2` with old value `3` and new value `9`, and for no other property + +#### Scenario: Hooks see a copyWithin +- **WHEN** state with `onRead` and `onWrite` hooks holds `[1, 2, 3]` and `copyWithin(0, 2)` is called +- **THEN** `onRead` is called with `copyWithin`, with `length` and with index `2`, and `onWrite` is called for index `0` with old value `1` and new value `3`, and for no other property + ### Requirement: Results equal the native method The return value of a wrapped method and the contents of the array afterwards SHALL equal those of the same native method on a plain array with the same contents and arguments, for every argument shape the native method accepts. On a non-extensible array the native method's error SHALL propagate unchanged. @@ -91,9 +135,21 @@ The return value of a wrapped method and the contents of the array afterwards SH - **WHEN** `sort((a, b) => b - a)` is called on a reactive array `[1, 3, 2]` - **THEN** it returns the same array and the array reads `[3, 2, 1]` +#### Scenario: Fill with negative and out-of-range bounds +- **WHEN** `fill(9, -2)` and then `fill(0, 1, 100)` are called on a reactive array `[1, 2, 3]` +- **THEN** each returns the same array, the array reads `[1, 9, 9]` after the first call and `[1, 0, 0]` after the second, and `length` stays `3` + +#### Scenario: copyWithin with negative and overlapping bounds +- **WHEN** `copyWithin(-2, 0)` is called on a reactive array `[1, 2, 3, 4]` +- **THEN** it returns the same array and the array reads `[1, 2, 1, 2]` + #### Scenario: Push of undefined creates the element - WHEN `push(undefined)` is called on a reactive array `[0, 1, 2]`, on state created without hooks and on state created with hooks - THEN `length` reads `4`, index `3` is an own property as it is on a plain array, and on hooked state `onWrite` is called for index `3` with old value `undefined` and new value `undefined` #### Scenario: Push on a sealed array - **WHEN** `push(1)` is called on reactive state over a sealed array - **THEN** a `TypeError` is raised, as the native method raises, and the system stays usable + +#### Scenario: Fill on a frozen array +- **WHEN** `fill(9)` is called on reactive state over a frozen array holding at least one element +- **THEN** a `TypeError` is raised, as the native method raises, and the system stays usable