From 0ce6ffe426b141bec27a305fbd8fdf030ca9da87 Mon Sep 17 00:00:00 2001 From: drakeo338 Date: Wed, 30 Sep 2026 02:58:32 +0000 Subject: [PATCH 1/2] fix(signals): a run that committed undefined is not WASTED_RECOMPUTE A projection that mutates its draft, or a memo that does its work by writing a signal, commits undefined on every run, so the equality gate always closed and the re-runs were counted as wasted. The rule that undefined outputs are exempt only applied to effects. recomputeEnd now passes a separate noValue flag to checkWastedRecompute; changed, costs and records are unchanged. Fixes #3715 --- .../wasted-recompute-undefined-output.md | 5 ++ packages/signals/src/core/attribution.ts | 11 +++- .../attribution-wasted-recompute.test.ts | 62 +++++++++++++++++++ 3 files changed, 75 insertions(+), 3 deletions(-) create mode 100644 .changeset/wasted-recompute-undefined-output.md diff --git a/.changeset/wasted-recompute-undefined-output.md b/.changeset/wasted-recompute-undefined-output.md new file mode 100644 index 000000000..69097dbab --- /dev/null +++ b/.changeset/wasted-recompute-undefined-output.md @@ -0,0 +1,5 @@ +--- +"@solidjs/signals": patch +--- + +`WASTED_RECOMPUTE` no longer counts a re-run that committed `undefined` as waste. A projection that mutates its draft, or a memo that does its work by writing a signal, has no output to compare, so its re-runs were reported as pure cost. diff --git a/packages/signals/src/core/attribution.ts b/packages/signals/src/core/attribution.ts index ea75cc811..0242bdab3 100644 --- a/packages/signals/src/core/attribution.ts +++ b/packages/signals/src/core/attribution.ts @@ -1394,13 +1394,16 @@ function checkHotTime(el: Computed, selfMs: number, causes: ChangeRecord[]) * names it while it happens, with the input that keeps triggering it. The * fix is upstream: an equality boundary on the part of the input the scope * depends on, or a narrower read. Plain runs only — a held or overlay run - * may be replayed and is never blamed as waste. + * may be replayed and is never blamed as waste. A run that committed + * `undefined` (`noValue`) is never waste either: its work is a side effect + * (a projection's draft writes, a write from inside a memo), not its output. */ function checkWastedRecompute( el: Computed, at: number, phase: RerunEvent["phase"], changed: boolean, + noValue: boolean, selfMs: number, causes: ChangeRecord[] ): void { @@ -1417,7 +1420,7 @@ function checkWastedRecompute( node._devWasteWarned = false; } node._devWasteRuns = node._devWasteRuns! + 1; - if (!changed) { + if (!changed && !noValue) { node._devWasted = node._devWasted! + 1; node._devWastedMs = node._devWastedMs! + selfMs; } @@ -1464,6 +1467,7 @@ function recordRerun( frame: RunFrame, timing: { selfMs: number; totalMs: number }, changed: boolean, + noValue: boolean, phase: "plain" | "held" | "optimistic", held: boolean ): void { @@ -1498,7 +1502,7 @@ function recordRerun( checkRelayTear(el, causes, prevCauses); checkHotRuns(el, causes); checkHotTime(el, timing.selfMs, causes); - checkWastedRecompute(el, frame.start, phase, changed, timing.selfMs, causes); + checkWastedRecompute(el, frame.start, phase, changed, noValue, timing.selfMs, causes); checkDepWidth(el); // The record: built only when something wanted it at run start (see // `wantsRerun`) — a listener, a fold, the log. @@ -4171,6 +4175,7 @@ const engineHooks: AttributionHooks = { frame, { selfMs, totalMs }, changed, + (el._pendingValue !== NOT_PENDING ? el._pendingValue : el._value) === undefined, optimistic ? "optimistic" : transition ? "held" : "plain", held ); diff --git a/packages/signals/tests/attribution-wasted-recompute.test.ts b/packages/signals/tests/attribution-wasted-recompute.test.ts index 6d36f56f9..3135e9512 100644 --- a/packages/signals/tests/attribution-wasted-recompute.test.ts +++ b/packages/signals/tests/attribution-wasted-recompute.test.ts @@ -13,6 +13,7 @@ import type { AttributionOptions } from "../src/attribution.js"; import { createEffect, createMemo, + createProjection, createRoot, createSignal, flush, @@ -158,4 +159,65 @@ describe("WASTED_RECOMPUTE", () => { } expect(disabled.events).toHaveLength(0); }); + + describe("computeds whose value is undefined", () => { + function run(make: (x: () => number) => () => unknown) { + const [x, setX] = createSignal(0, { name: "x" }); + let read!: () => unknown; + createRoot(() => { + read = make(x); + }); + flush(); + const { events } = capture(); + for (let i = 1; i <= 10; i++) { + setX(i); + flush(); + read(); + } + return events; + } + + it("a projection that mutates its draft is not waste", () => { + const events = run(x => { + const s = createProjection<{ v: number }>( + d => { + d.v = x(); + }, + { v: 0 }, + { name: "draft projection" } + ); + return () => s.v; + }); + expect(events).toHaveLength(0); + }); + + it("a projection that returns a value to reconcile is not waste", () => { + const events = run(x => { + const s = createProjection(() => ({ v: x() }), { v: 0 }, { name: "returning projection" }); + return () => s.v; + }); + expect(events).toHaveLength(0); + }); + + it("a memo that returns undefined and does its work by writing is not waste", () => { + const [y, setY] = createSignal(0, { name: "y", ownedWrite: true }); + const events = run(x => { + createMemo( + () => { + setY(x() * 2); + }, + { name: "sweep memo" } + ); + return y; + }); + expect(y()).toBe(20); + expect(events).toHaveLength(0); + }); + + it("a memo that keeps returning the same defined value is still waste", () => { + const events = run(x => createMemo(() => (x(), 7), { name: "constant memo" })); + expect(events).toHaveLength(1); + expect(events[0]).toMatchObject({ nodeName: "constant memo" }); + }); + }); }); From 7d723f865b93fe2e89498dbdb8b1dde064a4af9c Mon Sep 17 00:00:00 2001 From: Ryan Carniato Date: Wed, 30 Sep 2026 09:21:03 -0700 Subject: [PATCH 2/2] fix(signals): report undefined-output runs as changed so every waste consumer agrees The exemption for runs that commit `undefined` lived only inside checkWastedRecompute, so the RerunEvent still said `changed: false` and costs().wastedMs, expectNoWaste, the performance tracks and the why-run log kept counting projection and writing-memo runs as waste. Apply it to `changed` in recomputeEnd instead, mirroring the effect-output rule, and drop the noValue parameter. This is the documented behavior: an `undefined`-output compute reports `changed: true` and is exempt. Co-authored-by: Claude via Cursor Co-authored-by: Cursor --- .../wasted-recompute-undefined-output.md | 2 +- packages/signals/src/core/attribution.ts | 23 +++++++++++-------- .../attribution-wasted-recompute.test.ts | 9 +++++++- 3 files changed, 23 insertions(+), 11 deletions(-) diff --git a/.changeset/wasted-recompute-undefined-output.md b/.changeset/wasted-recompute-undefined-output.md index 69097dbab..123972443 100644 --- a/.changeset/wasted-recompute-undefined-output.md +++ b/.changeset/wasted-recompute-undefined-output.md @@ -2,4 +2,4 @@ "@solidjs/signals": patch --- -`WASTED_RECOMPUTE` no longer counts a re-run that committed `undefined` as waste. A projection that mutates its draft, or a memo that does its work by writing a signal, has no output to compare, so its re-runs were reported as pure cost. +A re-run that commits `undefined` is no longer counted as waste. A projection that mutates its draft or reconciles a returned value, or a memo that does its work by writing a signal, has no output to compare, so its re-runs were reported as pure cost. `RerunEvent.changed` now reports `true` for these runs (as it already did for side-effect-only effects), so `WASTED_RECOMPUTE` no longer fires for them and `costs().wastedMs`, `expectNoWaste` and the performance tracks stop counting them as wasted. diff --git a/packages/signals/src/core/attribution.ts b/packages/signals/src/core/attribution.ts index 0242bdab3..d5733cc55 100644 --- a/packages/signals/src/core/attribution.ts +++ b/packages/signals/src/core/attribution.ts @@ -169,8 +169,9 @@ export interface RerunEvent { * effect phase re-fires on every recompute), so the engine derives this * fact itself: an effect run whose compute output is identical to the * previous run's reports `changed: false` — the phase re-fired with the - * same input, pure waste. Side-effect-only computes (`undefined` output) - * are exempt: identity of `undefined` proves nothing about their work. + * same input, pure waste. Side-effect-only computes (`undefined` output — + * effects, projections, memos that work by writing) report `true`: + * identity of `undefined` proves nothing about their work. * Summed as `wastedMs` in costs() (plain, non-held runs only — see * `phase`). */ @@ -1395,15 +1396,15 @@ function checkHotTime(el: Computed, selfMs: number, causes: ChangeRecord[]) * fix is upstream: an equality boundary on the part of the input the scope * depends on, or a narrower read. Plain runs only — a held or overlay run * may be replayed and is never blamed as waste. A run that committed - * `undefined` (`noValue`) is never waste either: its work is a side effect - * (a projection's draft writes, a write from inside a memo), not its output. + * `undefined` arrives here as changed (see `recomputeEnd`): its work is a + * side effect (a projection's draft writes, a write from inside a memo), not + * its output. */ function checkWastedRecompute( el: Computed, at: number, phase: RerunEvent["phase"], changed: boolean, - noValue: boolean, selfMs: number, causes: ChangeRecord[] ): void { @@ -1420,7 +1421,7 @@ function checkWastedRecompute( node._devWasteWarned = false; } node._devWasteRuns = node._devWasteRuns! + 1; - if (!changed && !noValue) { + if (!changed) { node._devWasted = node._devWasted! + 1; node._devWastedMs = node._devWastedMs! + selfMs; } @@ -1467,7 +1468,6 @@ function recordRerun( frame: RunFrame, timing: { selfMs: number; totalMs: number }, changed: boolean, - noValue: boolean, phase: "plain" | "held" | "optimistic", held: boolean ): void { @@ -1502,7 +1502,7 @@ function recordRerun( checkRelayTear(el, causes, prevCauses); checkHotRuns(el, causes); checkHotTime(el, timing.selfMs, causes); - checkWastedRecompute(el, frame.start, phase, changed, noValue, timing.selfMs, causes); + checkWastedRecompute(el, frame.start, phase, changed, timing.selfMs, causes); checkDepWidth(el); // The record: built only when something wanted it at run start (see // `wantsRerun`) — a listener, a fold, the log. @@ -4169,13 +4169,18 @@ const engineHooks: AttributionHooks = { frame.prevValue, el._pendingValue !== NOT_PENDING ? el._pendingValue : el._value ); + // The same exemption for memos: a run that committed `undefined` did its + // work as a side effect (a projection's draft writes or reconcile, a + // write from inside a memo), so core's `undefined === undefined` gate + // proves nothing. Reported as changed, so no consumer counts it as waste. + if (!changed && (el._pendingValue !== NOT_PENDING ? el._pendingValue : el._value) === undefined) + changed = true; if (frame.causes !== null) recordRerun( el, frame, { selfMs, totalMs }, changed, - (el._pendingValue !== NOT_PENDING ? el._pendingValue : el._value) === undefined, optimistic ? "optimistic" : transition ? "held" : "plain", held ); diff --git a/packages/signals/tests/attribution-wasted-recompute.test.ts b/packages/signals/tests/attribution-wasted-recompute.test.ts index 3135e9512..1179250c5 100644 --- a/packages/signals/tests/attribution-wasted-recompute.test.ts +++ b/packages/signals/tests/attribution-wasted-recompute.test.ts @@ -8,7 +8,7 @@ * whose waste is cheap are not reported. `checks: false` folds it off. */ import { afterEach, describe, expect, it, vi } from "vitest"; -import { attribution } from "../src/attribution.js"; +import { attribution, costs } from "../src/attribution.js"; import type { AttributionOptions } from "../src/attribution.js"; import { createEffect, @@ -189,6 +189,13 @@ describe("WASTED_RECOMPUTE", () => { return () => s.v; }); expect(events).toHaveLength(0); + // Every consumer reads the same fact: the records and costs() agree. + const reruns = attribution.history("rerun").filter(e => e.nodeName === "draft projection"); + expect(reruns).toHaveLength(10); + expect(reruns.every(e => e.changed)).toBe(true); + const scope = costs().scopes.find(s => s.name === "draft projection"); + expect(scope?.runs).toBe(10); + expect(scope?.wastedMs).toBe(0); }); it("a projection that returns a value to reconcile is not waste", () => {