diff --git a/.changeset/wasted-recompute-undefined-output.md b/.changeset/wasted-recompute-undefined-output.md new file mode 100644 index 000000000..123972443 --- /dev/null +++ b/.changeset/wasted-recompute-undefined-output.md @@ -0,0 +1,5 @@ +--- +"@solidjs/signals": patch +--- + +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 ea75cc811..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`). */ @@ -1394,7 +1395,10 @@ 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` 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, @@ -4165,6 +4169,12 @@ 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, diff --git a/packages/signals/tests/attribution-wasted-recompute.test.ts b/packages/signals/tests/attribution-wasted-recompute.test.ts index 6d36f56f9..1179250c5 100644 --- a/packages/signals/tests/attribution-wasted-recompute.test.ts +++ b/packages/signals/tests/attribution-wasted-recompute.test.ts @@ -8,11 +8,12 @@ * 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, createMemo, + createProjection, createRoot, createSignal, flush, @@ -158,4 +159,72 @@ 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); + // 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", () => { + 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" }); + }); + }); });