From 7661d94ec4b94b23e5b66f59d8f51f62d4f76dbf Mon Sep 17 00:00:00 2001 From: Ryan Carniato Date: Thu, 10 Sep 2026 15:36:23 -0700 Subject: [PATCH 1/2] perf(signals): mark heap insertions in place instead of invalidating the markHeap memo MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Mounting N rows in one flush was O(N²) when each row created a user effect whose source was written during row creation (the ref-effect pattern). An unmarked node entering an already-marked pure heap set heap._marked = false, so every later mid-tick memo pull re-walked the whole heap: N full walks, N²/2 nodes visited. The insertion now runs markNode(n) in place — the same flags the deferred walk would have produced (#2922 semantics unchanged) — and the mount is linear (8000 rows, one ref effect each, prod: 231 → 12 ms). Adds a mount-scaling bench and a tripwire + mid-flush freshness test. Fixes #3350 Co-authored-by: Claude via Cursor --- .changeset/heap-mark-incremental.md | 5 + packages/signals/src/core/heap.ts | 17 ++- .../tests/heap-mark-incremental.test.ts | 113 ++++++++++++++++++ packages/signals/tests/mount-rows.bench.ts | 63 ++++++++++ scripts/size/.size-limit.js | 14 ++- 5 files changed, 204 insertions(+), 8 deletions(-) create mode 100644 .changeset/heap-mark-incremental.md create mode 100644 packages/signals/tests/heap-mark-incremental.test.ts create mode 100644 packages/signals/tests/mount-rows.bench.ts diff --git a/.changeset/heap-mark-incremental.md b/.changeset/heap-mark-incremental.md new file mode 100644 index 000000000..cbbb94623 --- /dev/null +++ b/.changeset/heap-mark-incremental.md @@ -0,0 +1,5 @@ +--- +"@solidjs/signals": patch +--- + +Mounting N rows in one flush was O(N²) when each row created a user effect whose source was written during row creation (the `ref` effect pattern). An unmarked node entering an already-marked pure heap invalidated the `markHeap` memo, so every later mid-tick memo pull re-walked the whole heap. The insertion now marks the incoming node in place instead; the mount is linear (8000 rows with a ref effect each: 231 ms → 12 ms). diff --git a/packages/signals/src/core/heap.ts b/packages/signals/src/core/heap.ts index 3064f5521..582c4771d 100644 --- a/packages/signals/src/core/heap.ts +++ b/packages/signals/src/core/heap.ts @@ -70,12 +70,17 @@ export function insertIntoHeap(n: Computed, heap: Heap) { n._flags = (flags & ~(REACTIVE_CHECK | REACTIVE_DIRTY)) | REACTIVE_DIRTY | REACTIVE_IN_HEAP; } else { n._flags = flags | REACTIVE_IN_HEAP; - // An unmarked node entering a marked heap invalidates the markHeap memo: - // `_marked` is only reset by runHeap, so a write between two mid-tick - // pulls (read-time markHeap + updateIfNecessary) would otherwise leave - // this node unmarked and every downstream pull stale until the next - // flush (#2922: the second `latest()` returned the first write's value). - if (heap._marked && !(flags & REACTIVE_DIRTY)) heap._marked = false; + // An unmarked node entering an already-marked heap is marked on the + // spot, keeping the markHeap memo valid. `_marked` is only reset by + // runHeap, so a write between two mid-tick pulls (read-time markHeap + + // updateIfNecessary) would otherwise leave this node unmarked and every + // downstream pull stale until the next flush (#2922: the second + // `latest()` returned the first write's value). Invalidating the memo + // instead re-walked the WHOLE heap on the next pull — with N effects + // parked in the heap for a synchronous mount (each row writing a ref + // signal its effect subscribes to), mounting N rows was O(N²) (#3350). + // markNode's own guard skips an already-DIRTY node. + if (heap._marked) markNode(n); } if (!(flags & REACTIVE_IN_HEAP_HEIGHT)) actualInsertIntoHeap(n, heap); } diff --git a/packages/signals/tests/heap-mark-incremental.test.ts b/packages/signals/tests/heap-mark-incremental.test.ts new file mode 100644 index 000000000..9e6979611 --- /dev/null +++ b/packages/signals/tests/heap-mark-incremental.test.ts @@ -0,0 +1,113 @@ +/** + * #3350: mounting N rows in one flush was O(N²). + * + * Writes schedule subscribers by heap insertion alone; DIRTY/CHECK marks are + * propagated lazily by markHeap, memoized on `heap._marked` until runHeap + * resets it. An unmarked node entering an already-marked heap used to + * invalidate that memo, so the next mid-tick pull re-walked the entire heap. + * A synchronous mount creating a user effect per row (each parked in the pure + * heap until the effect phase) hit that on every row's first memo read. + * Insertions now mark the incoming node in place instead. + */ +import { + createEffect, + createMemo, + createRenderEffect, + createRoot, + createSignal, + flush, + mapArray +} from "../src/index.js"; + +function mount(N: number) { + let ms = 0; + createRoot(dispose => { + const [zoom] = createSignal(1); + const viewport = createMemo(() => 600 / zoom()); + const [items, setItems] = createSignal([], { ownedWrite: true }); + const rows = createMemo( + mapArray(items, i => { + const visible = createMemo(() => i * 10 < viewport()); + createRenderEffect( + () => visible(), + () => {}, + { sync: true } as any + ); + const [el, setEl] = createSignal(null, { ownedWrite: true }); + createEffect( + () => el(), + () => {} + ); + setEl({ i }); + return visible; + }) + ); + createRenderEffect( + () => rows().length, + () => {} + ); + flush(); + const start = performance.now(); + setItems(Array.from({ length: N }, (_, i) => i)); + flush(); + ms = performance.now() - start; + dispose(); + }); + return ms; +} + +describe("heap marking stays incremental across mid-tick pulls", () => { + it("mounting rows with a per-row user effect is linear in N (#3350)", () => { + mount(200); // warm + // The quadratic regime measured ~760ms at 8000 rows here (4× per 2× N); + // the linear one ~35ms. Leave room for shared CI runners while keeping + // the O(N²) regime far above the tripwire. + expect(mount(8000)).toBeLessThan(250); + }); + + it("a write landing between two mid-tick pulls is visible through a memo chain in the same pass", () => { + // The first sync render effect's read marks the heap; the row's write + // then inserts an UNMARKED subscriber (doubled) into the marked heap. + // Its downstream memo (label) must be pulled fresh by the next sync + // reader in the same flush — the eager mark has to propagate CHECK past + // the inserted node, not just flag the node itself. + const seen: string[] = []; + createRoot(() => { + const [base] = createSignal(1); + const gate = createMemo(() => base() + 1); + const [n, setN] = createSignal(0, { ownedWrite: true }); + const doubled = createMemo(() => n() * 2); + const label = createMemo(() => `n=${doubled()}`); + const [items, setItems] = createSignal([], { ownedWrite: true }); + const rows = createMemo( + mapArray(items, i => { + createRenderEffect( + () => gate(), + () => {}, + { sync: true } as any + ); + setN(i + 10); + createRenderEffect( + () => label(), + v => { + seen.push(v); + }, + { sync: true } as any + ); + return i; + }) + ); + createRenderEffect( + () => rows().length, + () => {} + ); + flush(); + setItems([1, 2, 3]); + flush(); + }); + // Each row's first run sees its own write; earlier rows' effects then + // settle on the final value when the label memo lands in the heap pass. + expect(seen.slice(0, 3)).toEqual(["n=22", "n=24", "n=26"]); + expect(seen.slice(3)).toEqual(["n=26", "n=26"]); + }); +}); diff --git a/packages/signals/tests/mount-rows.bench.ts b/packages/signals/tests/mount-rows.bench.ts new file mode 100644 index 000000000..bea7b75e9 --- /dev/null +++ b/packages/signals/tests/mount-rows.bench.ts @@ -0,0 +1,63 @@ +// Synchronous mount scaling (#3350 repro shape): a mapArray of N rows, each +// creating a memo read by a sync render effect plus a user effect over a +// per-row ref signal. Every user effect created during the mount parks in +// the pure heap until the effect phase; each row's first mid-tick memo pull +// then ran markHeap. Before the fix an unmarked insertion invalidated the +// markHeap memo, so every pull re-walked the whole heap — O(N²) (4× per 2× +// N). Now the insertion marks the node in place and the mount is linear. +import { bench, describe } from "vitest"; +import { + createEffect, + createMemo, + createRenderEffect, + createRoot, + createSignal, + flush, + mapArray +} from "../src/index.js"; + +function mount(N: number, withEffect: boolean) { + createRoot(dispose => { + const [zoom] = createSignal(1); + const viewport = createMemo(() => 600 / zoom()); + const [items, setItems] = createSignal([], { ownedWrite: true }); + const rows = createMemo( + mapArray(items, i => { + const visible = createMemo(() => i * 10 < viewport()); + createRenderEffect( + () => visible(), + () => {}, + { sync: true } as any + ); + if (withEffect) { + const [el, setEl] = createSignal(null, { ownedWrite: true }); + createEffect( + () => el(), + () => {} + ); + setEl({ i }); + } + return visible; + }) + ); + createRenderEffect( + () => rows().length, + () => {} + ); + flush(); + setItems(Array.from({ length: N }, (_, i) => i)); + flush(); + dispose(); + }); +} + +for (const N of [1000, 4000]) { + describe(`mount ${N} rows`, () => { + bench("memo + sync render effect + user effect over a ref signal (#3350)", () => { + mount(N, true); + }); + bench("memo + sync render effect only (reference)", () => { + mount(N, false); + }); + }); +} diff --git a/scripts/size/.size-limit.js b/scripts/size/.size-limit.js index 0bff6a1c5..e32042d1a 100644 --- a/scripts/size/.size-limit.js +++ b/scripts/size/.size-limit.js @@ -502,7 +502,13 @@ module.exports = [ // measured at 10.82. Core scheduler cost; see the core-floor note. // Effect ownership on finalize re-entry (#3319, 2026-09-09): 10.85 KB -> 10.92 KB, // measured at 10.883. Core scheduler cost; see the core-floor note. - limit: "10.92 KB", + // Incremental heap marking (#3350, 2026-09-10): 10.92 -> 10.96 KB, + // measured at 10.924 against next's 10.895. A one-call swap in + // insertIntoHeap (`heap._marked = false` -> `markNode(n)`, dropping the + // DIRTY test markNode already performs); createStore and isPending + // scenarios both shrank on the same build, so the +29 B here is brotli + // layout drift, not retained code. + limit: "10.96 KB", modifyEsbuildConfig }, { @@ -866,7 +872,11 @@ module.exports = [ // in favour of one rule for every router — wrap the write whose landing // is the destination showing, pass `at` — with the loader wait itself // being router work (see 08-dev-diagnostics.md, Navigations). - limit: "26.65 KB", + // Incremental heap marking (#3350, 2026-09-10): 26.65 -> 26.68 KB, + // measured at 26.652 against next's 26.598. The insertIntoHeap change is + // a one-call swap that dropped a flag test; the CSR observe scenario on + // the same artifacts did not move, so this is brotli layout drift. + limit: "26.68 KB", modifyEsbuildConfig: observeEsbuildConfig }, { From 20f3ac048f2ea6c6566adf9b29082d394a86f5d8 Mon Sep 17 00:00:00 2001 From: Ryan Carniato Date: Thu, 10 Sep 2026 15:42:33 -0700 Subject: [PATCH 2/2] test(signals): make the #3350 mount tripwire relative MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The absolute bound (250 ms at 8000 rows) measured 403 ms on the coverage-instrumented CI job (12× slower than local). Compare 1000 vs 8000 rows within one process instead: ~8-10× fixed, ~50× on next; threshold 24. Co-authored-by: Claude via Cursor Co-authored-by: Cursor --- .../tests/heap-mark-incremental.test.ts | 20 ++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/packages/signals/tests/heap-mark-incremental.test.ts b/packages/signals/tests/heap-mark-incremental.test.ts index 9e6979611..a3279578f 100644 --- a/packages/signals/tests/heap-mark-incremental.test.ts +++ b/packages/signals/tests/heap-mark-incremental.test.ts @@ -58,11 +58,21 @@ function mount(N: number) { describe("heap marking stays incremental across mid-tick pulls", () => { it("mounting rows with a per-row user effect is linear in N (#3350)", () => { - mount(200); // warm - // The quadratic regime measured ~760ms at 8000 rows here (4× per 2× N); - // the linear one ~35ms. Leave room for shared CI runners while keeping - // the O(N²) regime far above the tripwire. - expect(mount(8000)).toBeLessThan(250); + // Relative tripwire: absolute wall-clock bounds do not survive the + // coverage-instrumented CI job (12× slower than a local run). Compare 8× + // the rows within one process instead — linear scaling lands near 8×, + // the quadratic regime near 64×. Best-of-k tames JIT/GC noise at the + // small end. Measured locally: ~10× fixed (3 → 30 ms), ~50× on next + // (17 → 850 ms). + const best = (N: number, k: number) => { + let ms = Infinity; + for (let i = 0; i < k; i++) ms = Math.min(ms, mount(N)); + return ms; + }; + best(1000, 2); // warm + const small = best(1000, 3); + const large = best(8000, 2); + expect(large / small).toBeLessThan(24); }); it("a write landing between two mid-tick pulls is visible through a memo chain in the same pass", () => {