From 62d591276b04ea032f97116cbaa2241583c1f4ed Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 13 Oct 2026 07:51:37 +0000 Subject: [PATCH] fix(attribution): GRAPH_GROWTH also requires what outlived each navigation to climb (#4007) GRAPH_GROWTH compared the whole graph's size at each settle of a route pattern, so a page whose correctly owned content is bigger on each visit (/rows/1, /rows/3, /rows/5) read as a leak although leaving disposed it. While graphGrowth is on, a non-initial navigation now walks the graph as it opens and adds every node to a WeakSet; its settle's walk also counts the nodes from that set still alive (an edge when both ends are) and drops the set. A series reports only when it climbs in the totals, as before, and in that survived count. A navigation that stays on the route pattern the app is on continues the visit and is not compared. The finding's data gains `survived` beside `history`; the message shows both. Engine-only: prod artifacts byte-identical. The observe + attribution size scenario grows +230 B brotli (+594 B minified); cap raised with an audit note pending the maintainer's acceptance. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01NcUAu8BQ6Xsf8XRBhr9V1w --- .../graph-growth-outlived-navigation.md | 5 + documentation/solid-2.0/08-dev-diagnostics.md | 10 +- packages/signals/src/core/attribution.ts | 137 ++++++++--- .../tests/attribution-graph-growth.test.ts | 223 +++++++++++++++++- .../skills/reactivity-diagnostics/SKILL.md | 14 +- scripts/size/scenarios.js | 14 +- 6 files changed, 363 insertions(+), 40 deletions(-) create mode 100644 .changeset/graph-growth-outlived-navigation.md diff --git a/.changeset/graph-growth-outlived-navigation.md b/.changeset/graph-growth-outlived-navigation.md new file mode 100644 index 000000000..c13915a67 --- /dev/null +++ b/.changeset/graph-growth-outlived-navigation.md @@ -0,0 +1,5 @@ +--- +"@solidjs/signals": patch +--- + +`GRAPH_GROWTH` no longer reports a route whose correctly owned content is bigger on each visit (`/rows/1`, `/rows/3`, `/rows/5`) when leaving it disposes everything. While the check is on, each non-initial navigation also counts what outlived it — the nodes alive when it opened that are still alive at its settle — and a series reports only when it climbs both in the totals, as before, and in that count. A navigation that stays on the route pattern (only params or query change) continues the visit and is not compared. The finding's `data` gains `survived` beside `history`, and its message shows both (`owners 3 → 5 → 7 (of 6 → 10 → 14)`). Fixes #4007. diff --git a/documentation/solid-2.0/08-dev-diagnostics.md b/documentation/solid-2.0/08-dev-diagnostics.md index 9d18ef4c5..90d235dbc 100644 --- a/documentation/solid-2.0/08-dev-diagnostics.md +++ b/documentation/solid-2.0/08-dev-diagnostics.md @@ -428,13 +428,13 @@ Related: `WIDE_SCOPE_DEPS` (below) fires at a lower threshold, but only while th #### `GRAPH_GROWTH` -**Message:** "the live graph grew on 3 consecutive visits to `/orders`: computations 900 → 903 → 906; edges 900 → 903 → 906 (2 roots), across visits to `/orders`, `/`. Each visit left something behind that the next did not reclaim — the shape says an effect or memo created with no owner (a module-level or callback `createEffect`) that only its sources keep alive. Dispose what a visit creates (`onCleanup`, or return the disposer from `onSettled`) and own it under the route's component so leaving the route tears it down." +**Message:** "the live graph grew on 3 consecutive visits to `/orders`, and so did the part that outlived each navigation there: computations 880 → 883 → 886 (of 900 → 903 → 906); edges 880 → 883 → 886 (of 900 → 903 → 906) (2 roots), across visits to `/orders`, `/`. Each visit left something behind that the next did not reclaim — the shape says an effect or memo created with no owner (a module-level or callback `createEffect`) that only its sources keep alive. Dispose what a visit creates (`onCleanup`, or return the disposer from `onSettled`) and own it under the route's component so leaving the route tears it down." -Attribution-engine only; the leak class a heap snapshot finds, as a finding. At every navigation's settle (`withOrigin({ kind: "navigation" })`) the engine measures the live graph with a **walk**, never a per-node counter: the owner tree from the registered top-level roots (`owners`: roots, component owners, owned computations), then everything reachable from it through the reactive graph — each computation's dependency links (`edges`) and the `signals` and computations they reach, and each reached node's subscriber list, which is how a computation **no owner holds** (a `createEffect` with no owner, kept alive only by the sources it reads) is found and counted in `computations`. It emits a `graph` record (`GraphEvent`: the `GraphSize` fields plus `at`, `route`, `navigation`) for an `OBSERVE.records.subscribe("graph", …)` listener, and keeps the size at each settle of the same route. When any series — `owners`, `computations`, `signals`, `edges` — has climbed on `graphGrowth.visits` consecutive visits (default 3) to `ratio` or more of the first (default 1.25), the route reports, and _which_ series climbed names the leak: `owners` is an undisposed root or a Portal per visit; `computations` with `owners` flat is an ownerless effect; `edges` alone is a subscription per visit to something long-lived. The count is the whole graph's, so a leak shows at every route's settle: the first route to complete its climb reports and `data.routes` names the others seen; the verdict then resets. `data`: `route`, `grew` (the series that climbed), `history` (the `GraphSize` at each settle, oldest first), `roots`, `routes`, `interaction`. No subject. +Attribution-engine only; the leak class a heap snapshot finds, as a finding. At every navigation's settle (`withOrigin({ kind: "navigation" })`) the engine measures the live graph with a **walk**, never a per-node counter: the owner tree from the registered top-level roots (`owners`: roots, component owners, owned computations), then everything reachable from it through the reactive graph — each computation's dependency links (`edges`) and the `signals` and computations they reach, and each reached node's subscriber list, which is how a computation **no owner holds** (a `createEffect` with no owner, kept alive only by the sources it reads) is found and counted in `computations`. It emits a `graph` record (`GraphEvent`: the `GraphSize` fields plus `at`, `route`, `navigation`) for an `OBSERVE.records.subscribe("graph", …)` listener, and keeps the size at each settle of the same route. A count taken at an arrival also holds everything that visit just created, so a page whose correctly owned content is bigger on each visit (`/rows/1`, `/rows/3`, `/rows/5`) climbs too, although leaving it disposes everything. So the engine also counts what **outlived** each navigation: while the check is on, a non-initial navigation walks the graph once more as it opens, adding every node it meets to a `WeakSet`, and its settle's walk counts the nodes from that set still alive (an edge counts when both its ends are), then drops the set. A visit's own content is not among them; a root, an ownerless effect or a retained signal an earlier visit left behind is, whatever the content does. When any series — `owners`, `computations`, `signals`, `edges` — has climbed on `graphGrowth.visits` consecutive visits (default 3) to `ratio` or more of the first (default 1.25), both in the totals and in what outlived each navigation, the route reports, and _which_ series climbed names the leak: `owners` is an undisposed root or a Portal per visit; `computations` with `owners` flat is an ownerless effect; `edges` alone is a subscription per visit to something long-lived. The count is the whole graph's, so a leak shows at every route's settle: the first route to complete its climb reports and `data.routes` names the others seen; the verdict then resets. `data`: `route`, `grew` (the series that climbed), `history` (the `GraphSize` at each settle, oldest first), `survived` (the `GraphSize` of what outlived each of those navigations — alive when it opened and still alive at its settle — beside `history`), `roots`, `routes`, `interaction`. No subject. -Navigation declarations with `initial: true` still emit a `graph` record but do not count toward `GRAPH_GROWTH` history. The initial declaration runs before route content mounts, so its graph size cannot be compared with completed visits. Later visits to that route count as usual. +Navigation declarations with `initial: true` still emit a `graph` record but do not count toward `GRAPH_GROWTH` history. The initial declaration runs before route content mounts, so its graph size cannot be compared with completed visits. Later visits to that route count as usual. A navigation that stays on the route pattern the app is on — only its params or query change (`/users/1` → `/users/2` under `/users/:id`) — continues that visit and is not compared either: the page it kept outlived the move by design, so counting it would read the page itself as growth. Only arrivals that enter a route are compared; a leak made during a visit is still alive when the next visit starts, so it still shows there. A navigation that was superseded never showed, so it does not change the route the app is on. -The observe core's part is the root registry: `createOwner` with no parent registers the root (weakly — a `WeakRef`, reaped by a `FinalizationRegistry` — so an undisposed root nothing references still collects; one a subscription keeps alive is exactly what the walk counts), and its disposal unregisters it. One Set write per top-level root, nothing per node; the walk runs at navigation cadence, only when the check is on or something listens for `graph` records. Measured on the observe artifacts (components of 1 signal, 3 memos, 6 effects): 10k owners walk in 0.6ms, 50k in 3.6ms — a `Set` for the reached nodes is most of it. A reactive node the app's own graph never reaches (an ownerless effect over a signal nothing owned reads) is not counted. `graphSize()` is exported from `solid-js/attribution` for a consumer that wants the measure on its own schedule. `false` disables. +The observe core's part is the root registry: `createOwner` with no parent registers the root (weakly — a `WeakRef`, reaped by a `FinalizationRegistry` — so an undisposed root nothing references still collects; one a subscription keeps alive is exactly what the walk counts), and its disposal unregisters it. One Set write per top-level root, nothing per node; the walk runs at navigation cadence, only when the check is on or something listens for `graph` records — twice per navigation while the check is on (as it opens and at its settle). Measured on the observe artifacts (components of 1 signal, 3 memos, 6 effects): 10k owners walk in 0.6ms, 50k in 3.6ms — a `Set` for the reached nodes is most of it. A reactive node the app's own graph never reaches (an ownerless effect over a signal nothing owned reads) is not counted. `graphSize()` is exported from `solid-js/attribution` for a consumer that wants the measure on its own schedule. `false` disables. #### `HOT_SCOPE_RERUNS`, `HOT_SCOPE_TIME`, `WIDE_SCOPE_DEPS` @@ -952,7 +952,7 @@ The runtime derives a request's trace itself in every tier — the W3C `tracepar | `FLUSH_IN_EFFECT_CALLBACK` | warn | lifecycle | `flush()` from an effect callback (no-op; the drain is already running) | | `HUGE_FAN_OUT` | warn | graph | One change reached 2000 live subscribers (always on), or 250 — `fanOut` — on a stamped root write (attribution enabled) | | `HUGE_FAN_IN` | warn | graph | One recompute tracked 2000 sources (always on) | -| `GRAPH_GROWTH` | warn | perf | The live owner count at the same route's settle climbed on 3 consecutive visits to 1.25× — something each visit leaves behind (attribution enabled) | +| `GRAPH_GROWTH` | warn | perf | The live graph and what outlived each navigation climbed at a route's settle on 3 visits to 1.25× — something each visit leaves behind (attribution enabled) | | `HOT_SCOPE_RERUNS` | warn | perf | 120+ re-runs of one scope in 1s (attribution enabled) | | `HOT_SCOPE_FANOUT` | warn | perf | 5+/50+/500+ scopes hot from one root cause (attribution enabled) | | `HOT_SCOPE_TIME` | warn | perf | 8ms+ self-time in one scope in 1s (attribution enabled) | diff --git a/packages/signals/src/core/attribution.ts b/packages/signals/src/core/attribution.ts index 8c3177913..b3019510f 100644 --- a/packages/signals/src/core/attribution.ts +++ b/packages/signals/src/core/attribution.ts @@ -537,9 +537,13 @@ export interface AttributionOptions { /** * Graph-growth warning: emit GRAPH_GROWTH when the live owner count at the * settle of the same route has climbed on `visits` consecutive visits - * (default 3) to `ratio` or more of the first (default 1.25) — a root or a + * (default 3) to `ratio` or more of the first (default 1.25), and so has + * the part of it that outlived each of those navigations — a root or a * subscription each visit leaves behind, the leak class a heap snapshot - * finds. The count is a walk at settle, never per node. `false` disables. + * finds, not content a visit owns and leaving disposes. The count is a walk + * when a navigation opens and at its settle, never per node. Navigations + * that stay on a route pattern continue its visit and are not compared. + * `false` disables. */ graphGrowth?: { visits: number; ratio: number } | false; /** @@ -3109,8 +3113,17 @@ function checkOptimisticRevert( // --- Graph growth ----------------------------------------------------------------- -/** Per route: the graph's size at its last `visits` settles, oldest first. */ -const routeCounts = new Map(); +/** + * Per route: at its last `visits` compared settles, oldest first, the + * graph's size and the part of it that was already alive when that + * navigation opened. + */ +const routeCounts = new Map(); +/** + * The route the app last arrived on: a navigation that stays on it (only + * its params or query change) continues the visit and is not compared. + */ +let visitRoute: string | undefined; /** * The live graph's size — a walk, not a counter: nothing is charged at node @@ -3126,26 +3139,43 @@ const routeCounts = new Map(); * counted unless a subscription still reaches them. */ export function graphSize(): GraphSize { + return walkGraph(); +} + +/** + * The walk behind `graphSize()`. Given `before` alone, it adds every node it + * meets to that set: what is alive as a navigation opens. Given `before` and + * `survived`, it also counts into `survived` the nodes it meets that are in + * the set, and an edge when both its ends are: what outlived the navigation. + * Nothing is retained — the set is weak. + */ +function walkGraph(before?: WeakSet, survived?: GraphSize): GraphSize { const roots = liveRootOwners(); - const size: GraphSize = { roots: roots.length, owners: 0, computations: 0, signals: 0, edges: 0 }; + const size: GraphSize = { roots: 0, owners: 0, computations: 0, signals: 0, edges: 0 }; const seen = new Set(); // A worklist, not recursion: a subscriber chain can be thousands deep. const work: (Signal | Computed)[] = []; const stack: Owner[] = []; + const count = (node: object, key: keyof GraphSize): void => { + size[key]++; + if (survived !== undefined) { + if (before!.has(node)) survived[key]++; + } else if (before !== undefined) before.add(node); + }; const meet = (node: Signal | Computed): void => { if (seen.has(node)) return; seen.add(node); - if (typeof (node as Computed)._fn === "function") size.computations++; - else size.signals++; + count(node, typeof (node as Computed)._fn === "function" ? "computations" : "signals"); work.push(node); }; for (const root of roots) { - size.owners++; + count(root, "roots"); + count(root, "owners"); // Iterative: a deep tree must not grow the call stack. let child = root._firstChild; for (;;) { while (child !== null) { - size.owners++; + count(child, "owners"); if (typeof (child as Computed)._fn === "function") meet(child as Computed); if (child._firstChild !== null) stack.push(child); child = child._nextSibling; @@ -3164,11 +3194,22 @@ export function graphSize(): GraphSize { for (let link = (node as Computed)._deps; link !== null; link = link._nextDep) { size.edges++; meet(link._dep); + if (survived !== undefined && before!.has(node) && before!.has(link._dep)) survived.edges++; } } return size; } +/** + * openNavigation, while the growth check is on: what is alive as a + * navigation opens, for its settle to tell apart from what it created. + */ +function snapshotGraph(): WeakSet { + const before = new WeakSet(); + walkGraph(before); + return before; +} + /** * settleNavigation: the app has finished moving to a route — count the live * graph. One record per settle for a listener; the growth check compares @@ -3177,47 +3218,78 @@ export function graphSize(): GraphSize { * `createRoot` in an effect with no dispose, a subscription a component * registered outside its owner. Nothing here runs per node; the walk is * the cost, at navigation cadence. + * + * A count taken at an arrival also holds everything the visit just created, + * so a page whose correctly owned content is bigger each visit climbs too. + * What tells the two apart is what outlived the navigation: the nodes alive + * when it opened (`NavState.before`) that are still alive at its settle. A + * visit's own content is not among them; what an earlier visit left behind + * is, whatever the content does. A series reports only when both climb. */ const GRAPH_SERIES: (keyof GraphSize)[] = ["owners", "computations", "signals", "edges"]; -function trackGraph(navigation: NavigationEvent): void { +function trackGraph(state: NavState): void { + const navigation = state.event; + const before = state.before; + state.before = undefined; const cfg = options.graphGrowth; if (cfg === false && !records.observed("graph")) return; - const size = graphSize(); + const survived: GraphSize | undefined = before && { + roots: 0, + owners: 0, + computations: 0, + signals: 0, + edges: 0 + }; + const size = walkGraph(before, survived); const route = navigation.name ?? navigation.to; const event: GraphEvent = { at: now(), ...size, navigation }; if (route !== undefined) event.route = route; records.emit("graph", event, undefined); + if (cfg === false || route === undefined) return; + // A navigation that stays on the route pattern continues the visit: the + // page it kept outlived the move by design, so comparing it would read the + // page itself as growth. Only arrivals that enter the route are compared — + // a leak made during a visit is still alive when the next one starts. A + // superseded navigation never showed, so the app is still where it was. + const left = visitRoute; + if (navigation.outcome !== "superseded") visitRoute = route; // The initial declaration is not a visit (it settles before route content // mounts — the same gate the feedback folds apply as `event.initial !== true`); // `initial` is `true | undefined`, set only by openNavigation, so truthiness is exact. - if (cfg === false || route === undefined || navigation.initial) return; + // A navigation opened before the check was on has no snapshot to compare. + if (navigation.initial || route === left || survived === undefined) return; let history = routeCounts.get(route); if (history === undefined) routeCounts.set(route, (history = [])); - history.push(size); + history.push({ size, survived }); if (history.length > cfg.visits) history.shift(); if (history.length < cfg.visits) return; - // Any series climbing on every visit to the ratio is growth; which ones - // did says what leaked — computations with owners flat is an ownerless - // effect per visit, edges alone is a subscription per visit to something - // long-lived, owners is an undisposed root. - const grew = GRAPH_SERIES.filter(key => { - for (let i = 1; i < history!.length; i++) - if (history![i][key] <= history![i - 1][key]) return false; - return history![history!.length - 1][key] >= history![0][key] * cfg.ratio; - }); + const totals = history.map(h => h.size); + const kept = history.map(h => h.survived); + const climbs = (series: GraphSize[], key: keyof GraphSize): boolean => { + for (let i = 1; i < series.length; i++) if (series[i][key] <= series[i - 1][key]) return false; + return series[series.length - 1][key] >= series[0][key] * cfg.ratio; + }; + // Any series climbing on every visit to the ratio — in the totals and in + // what outlived each navigation — is growth; which ones did says what + // leaked — computations with owners flat is an ownerless effect per visit, + // edges alone is a subscription per visit to something long-lived, owners + // is an undisposed root. + const grew = GRAPH_SERIES.filter(key => climbs(totals, key) && climbs(kept, key)); if (grew.length === 0) return; // The count is the whole graph's, so a leak shows at every route's settle; // the first route to complete its climb reports, naming the others seen. const routes = [...routeCounts.keys()]; - const series = grew.map(key => `${key} ${history!.map(h => h[key]).join(" → ")}`).join("; "); + const join = (series: GraphSize[], key: keyof GraphSize) => series.map(h => h[key]).join(" → "); + const series = grew.map(key => `${key} ${join(kept, key)} (of ${join(totals, key)})`).join("; "); const shape = grew.includes("owners") ? "a createRoot() in an effect or handler with no dispose, or a Portal or panel mounted per visit" : grew.includes("computations") ? "an effect or memo created with no owner (a module-level or callback createEffect) that only its sources keep alive" : "a subscription per visit to something long-lived — a global store or signal read by a computation that outlives the visit"; const message = - `[GRAPH_GROWTH] the live graph grew on ${cfg.visits} consecutive visits to ${route}: ${series} ` + + `[GRAPH_GROWTH] the live graph grew on ${cfg.visits} consecutive visits to ${route}, and so did ` + + `the part that outlived each navigation there: ${series} ` + `(${size.roots} roots)${routes.length > 1 ? `, across visits to ${routes.join(", ")}` : ""}. Each ` + `visit left something behind that the next did not reclaim — the shape says ${shape}. Dispose ` + `what a visit creates (onCleanup, or return the disposer from onSettled) and own it under the ` + @@ -3225,7 +3297,8 @@ function trackGraph(navigation: NavigationEvent): void { const data: Record = { route, grew, - history: history.map(h => ({ ...h })), + history: totals, + survived: kept, roots: size.roots, routes }; @@ -3473,6 +3546,11 @@ interface NavState { held: boolean; /** `drainSeq` at its last write — a later drain committed it. */ writeDrain: number; + /** + * While `graphGrowth` is on: every node alive when it opened, weakly, for + * its settle's count of what outlived it (see `trackGraph`). Dropped there. + */ + before?: WeakSet; } const navStates = new WeakMap(); /** Opened, not yet settled. */ @@ -3487,7 +3565,7 @@ let drainSeq = 0; * (#3989). `land()` runs only inside `flush()`'s drain, so `flushEnd` counts * them, after those commits — the instant a one-drain navigation is counted. */ -const landedGraphs: NavigationEvent[] = []; +const landedGraphs: NavState[] = []; /** * Copy what the router currently says onto the frame (what writes stamped — @@ -3524,6 +3602,8 @@ function openNavigation(frame: ChangeOrigin, ref: NavigationRef): void { held: false, writeDrain: drainSeq }; + // The initial declaration is never compared (see `trackGraph`). + if (options.graphGrowth !== false && ref.initial !== true) state.before = snapshotGraph(); syncNavigation(state); navStates.set(frame, state); openNavs.add(state); @@ -3809,8 +3889,8 @@ function settleNavigation( // route's responsiveness in the feedback tables. Consumers get the record. if (event.initial !== true) for (const f of folds) f.navigation?.(event); records.emit("navigation", event, undefined); - if (landed) landedGraphs.push(event); - else trackGraph(event); + if (landed) landedGraphs.push(state); + else trackGraph(state); // The interaction that performed it may have been waiting only on this. const under = openInteractionOf(event.interaction); if (under !== undefined) maybeSettleInteraction(under); @@ -4443,6 +4523,7 @@ function resetTracking(): void { landedGraphs.length = 0; openInteractions.clear(); routeCounts.clear(); + visitRoute = undefined; drainSeq = 0; originFrames.length = 0; effectStack.length = 0; diff --git a/packages/signals/tests/attribution-graph-growth.test.ts b/packages/signals/tests/attribution-graph-growth.test.ts index 38997f95f..b766d4c9e 100644 --- a/packages/signals/tests/attribution-graph-growth.test.ts +++ b/packages/signals/tests/attribution-graph-growth.test.ts @@ -10,7 +10,7 @@ */ import { afterEach, describe, expect, it, vi } from "vitest"; import { attribution, graphSize } from "../src/attribution.js"; -import type { GraphEvent, NavigationEvent } from "../src/attribution.js"; +import type { GraphEvent, GraphSize, NavigationEvent } from "../src/attribution.js"; import { createEffect, createMemo, @@ -18,10 +18,12 @@ import { createRoot, createSignal, flush, + getOwner, OBSERVE, runWithOwner, untrack } from "../src/index.js"; +import type { Accessor, Owner } from "../src/index.js"; import type { DiagnosticEvent, RecordListener, RecordType } from "../src/core/dev.js"; // The engine's records arrive on the channel, whose subscriptions are the @@ -412,4 +414,223 @@ describe("GRAPH_GROWTH", () => { for (const d of leaked) d(); dispose(); }); + + /** + * The routes of #4007: a memo owns the current page's rows and a render + * effect reads them. A, C and D only change how much correctly owned + * content each visit has; B, E and F also leave a detached root behind on + * every `/rows/:count` visit. + */ + type Variant = "A" | "B" | "C" | "D" | "E" | "F"; + function visits(variant: Variant) { + const leak = variant === "B" || variant === "E" || variant === "F"; + const leaked: (() => void)[] = []; + const [location, setLocation] = createSignal({ rows: false, count: 0 }); + const [shared] = createSignal(1); + const dispose = createRoot(dispose => { + const page = createMemo(() => { + const current = location(); + const n = current.count; + if (n === 0) return shared; + if (current.rows && leak) + runWithOwner(null, () => + createRoot(dispose => { + leaked.push(dispose); + createRenderEffect(shared, () => {}); + }) + ); + const rows = Array.from({ length: n }, () => createMemo(shared)); + return () => rows.reduce((sum, row) => sum + row(), 0); + }); + createRenderEffect( + () => page()(), + () => {} + ); + return dispose; + }); + flush(); + const counts = variant === "F" ? [5, 3, 1] : [1, 3, 5]; + counts.forEach((n, index) => { + OBSERVE!.attribution.withOrigin( + { kind: "navigation", name: "/rows/:count", to: `/rows/${n}`, params: { count: `${n}` } }, + () => setLocation({ rows: true, count: n }) + ); + flush(); + if (variant === "C") return; + const other = variant === "D" ? n : variant === "E" ? [101, 51, 1][index] : 0; + OBSERVE!.attribution.withOrigin( + { + kind: "navigation", + name: other ? "/other/:count" : "/rest", + to: other ? `/other/${other}` : "/rest" + }, + () => setLocation({ rows: false, count: other }) + ); + flush(); + }); + return () => { + dispose(); + for (const d of leaked) d(); + flush(); + }; + } + + it.each([ + ["A", "growing content, a fixed route between visits"], + ["C", "growing content, consecutive parameter changes"], + ["D", "both routes' content grows"] + ] as const)( + "content that differs per visit but is disposed on leaving does not report (%s: %s)", + (variant, _) => { + const before = graphSize(); + const { findings, graphs } = arm(); + const cleanup = visits(variant); + try { + // The totals at the arrivals really climb — the page has more rows each time… + const rows = graphs.filter(g => g.route === "/rows/:count").map(g => g.owners); + expect(rows).toHaveLength(3); + expect(rows[1]).toBeGreaterThan(rows[0]); + expect(rows[2]).toBeGreaterThan(rows[1]); + // …but nothing a visit created outlived the navigation away from it. + expect(findings).toHaveLength(0); + } finally { + cleanup(); + } + expect(graphSize()).toEqual(before); + } + ); + + it.each([ + ["B", "a root per visit", "/rows/:count"], + ["E", "a root per visit while the intervening content shrinks", "/rows/:count"], + ["F", "a root per visit while its own content shrinks", "/rest"] + ] as const)("a root each visit leaves behind still reports (%s: %s)", (variant, _, route) => { + const before = graphSize(); + const { findings } = arm(); + const cleanup = visits(variant); + try { + expect(findings).toHaveLength(1); + const data = findings[0].data as { + route: string; + grew: string[]; + history: GraphSize[]; + survived: GraphSize[]; + }; + expect(data.route).toBe(route); + expect(data.grew).toContain("owners"); + expect(data.survived).toHaveLength(3); + // What outlived each navigation climbs by the leaked root (its owner and + // its render effect) per visit; the totals include the page too. + expect(data.survived.map(s => s.owners - data.survived[0].owners)).toEqual([0, 2, 4]); + for (let i = 0; i < 3; i++) + expect(data.survived[i].owners).toBeLessThanOrEqual(data.history[i].owners); + const series = (of: GraphSize[]) => of.map(s => s.owners).join(" → "); + expect(findings[0].message).toContain( + `owners ${series(data.survived)} (of ${series(data.history)})` + ); + } finally { + cleanup(); + } + expect(graphSize()).toEqual(before); + }); + + it("a signal kept per visit and read by a long-lived computation reports signals and edges", () => { + const before = graphSize(); + const { findings } = arm(); + const [location, setLocation] = createSignal("/"); + // The mistake: a module-level list each visit appends a signal to, read by + // a computation that lives as long as the app. + const kept: Accessor[] = []; + const dispose = createRoot(dispose => { + const page = createMemo(() => { + const route = location(); + if (route === "/orders") { + const [selection] = createSignal(0); + kept.push(selection); + } + return route; + }); + createRenderEffect( + () => (page(), kept.reduce((sum, s) => sum + s(), 0)), + () => {} + ); + return dispose; + }); + flush(); + try { + for (let i = 0; i < 3; i++) { + navigate(setLocation, "/orders"); + navigate(setLocation, "/home"); + } + expect(findings).toHaveLength(1); + const data = findings[0].data as { route: string; grew: string[]; survived: GraphSize[] }; + expect(data.route).toBe("/orders"); + expect(data.grew).toEqual(["signals", "edges"]); + expect(data.survived.map(s => s.signals - data.survived[0].signals)).toEqual([0, 1, 2]); + expect(data.survived.map(s => s.edges - data.survived[0].edges)).toEqual([0, 1, 2]); + } finally { + dispose(); + flush(); + } + expect(graphSize()).toEqual(before); + }); + + it("a page kept across navigations within its route, growing a row per query, does not report", () => { + const before = graphSize(); + const { findings, graphs } = arm(); + const [location, setLocation] = createSignal({ route: "/", q: "" }); + const [shared] = createSignal(1); + const dispose = createRoot(dispose => { + const route = createMemo(() => location().route); + const query = createMemo(() => location().q); + // Keyed on the route pattern: a query change keeps the page. + const page = createMemo(() => { + if (route() !== "/search") return () => 0; + const owner = getOwner() as Owner; + const rows: Accessor[] = []; + // Each query asked adds a row the page owns until it leaves. + createRenderEffect( + () => { + query(); + untrack(() => runWithOwner(owner, () => rows.push(createMemo(shared)))); + }, + () => {} + ); + return () => rows.reduce((sum, row) => sum + row(), 0); + }); + createRenderEffect( + () => page()(), + () => {} + ); + return dispose; + }); + flush(); + const go = (route: string, q = "") => { + OBSERVE!.attribution.withOrigin( + { kind: "navigation", name: route, to: q ? `${route}?q=${q}` : route }, + () => setLocation({ route, q }) + ); + flush(); + }; + try { + go("/home"); + for (const q of ["a", "b", "c", "d"]) go("/search", q); + const totals = graphs.filter(g => g.route === "/search").map(g => g.owners); + // The page grows on every query: the totals alone read that as a leak. + expect(totals).toHaveLength(4); + for (let i = 1; i < totals.length; i++) expect(totals[i]).toBeGreaterThan(totals[i - 1]); + // Navigations within the route continue the visit: the page they kept + // outlived each of them by design, so they are not compared. + expect(findings).toHaveLength(0); + // Leaving disposes the page with every row it kept; coming back starts over. + go("/home"); + go("/search", "e"); + expect(graphs[graphs.length - 1].owners).toBe(totals[0]); + expect(findings).toHaveLength(0); + } finally { + dispose(); + flush(); + } + expect(graphSize()).toEqual(before); + }); }); diff --git a/packages/solid/skills/reactivity-diagnostics/SKILL.md b/packages/solid/skills/reactivity-diagnostics/SKILL.md index a7491fae4..6295d6ada 100644 --- a/packages/solid/skills/reactivity-diagnostics/SKILL.md +++ b/packages/solid/skills/reactivity-diagnostics/SKILL.md @@ -293,10 +293,16 @@ the scope as described under [HUGE_FAN_IN](#huge_fan_in). ### GRAPH_GROWTH The live graph, counted when each navigation settles, got bigger on every -one of the last `data.history.length` visits to `data.route` — something -each visit creates is never disposed. This is the leak a heap -snapshot finds; the engine finds it by measuring the reactive graph at each -navigation's settle. `data.grew` says which measure climbed, and that names +one of the last `data.history.length` visits to `data.route`, and so did the +part of it that outlived each of those navigations (`data.survived`, beside +`data.history`: what was alive when the navigation began and still is) — +something each visit creates is never disposed. A page that is merely bigger +on one visit than the last does not report: its content is new at each +arrival, so it is not in `data.survived`. Navigations that only change a +route's params or query continue the visit and are not compared. This is the +leak a heap snapshot finds; the engine finds it by measuring the reactive +graph as each navigation opens and at its settle. `data.grew` says which +measure climbed, and that names the leak: `owners` is an undisposed root or a Portal mounted per visit; `computations` with `owners` flat is an effect or memo created with no owner, kept alive only by the sources it reads; `edges` alone is a diff --git a/scripts/size/scenarios.js b/scripts/size/scenarios.js index bbb3afa54..2df39ef53 100644 --- a/scripts/size/scenarios.js +++ b/scripts/size/scenarios.js @@ -3354,8 +3354,18 @@ module.exports = [ // the recorded minified (86,937 -> 87,216). Both merged with the // maintainer's go-ahead 2026-10-12. Cap set at measured + 10 B rounded up // to 0.01 KB. - limit: "28.96 KB", - capMinified: 87216, + // Size-Exception (GRAPH_GROWTH also requires what outlived each + // navigation to climb, #4007, 2026-10-13): 28.96 KB -> 29.17 KB, measured + // locally at 29,160 B against `next` @ d8ba741d's 28,930 (+230 B; +594 B + // minified, 87,216 -> 87,810; recorded 87,216 -> 87,810). Engine-only: + // the navigation's snapshot walk (a WeakSet of what was alive as it + // opened), the settle walk's count of what survived it, the same-pattern + // visit rule, and the finding's `survived` series and message. Every + // prod artifact byte-identical; the observe tier scenario above did not + // move. Cap set at measured + 10 B rounded up to 0.01 KB. AWAITING the + // maintainer's acceptance — re-measure on CI before accepting. + limit: "29.17 KB", + capMinified: 87810, alias: observeAlias }, // Compiled-template scenarios (2026-10-05): the four `app:` fixtures above