From f0c223530cf9680e1be0ff19419b4db7433b40aa Mon Sep 17 00:00:00 2001 From: m4air Date: Thu, 10 Sep 2026 17:14:41 -0700 Subject: [PATCH] review: carry aboveMarkMinutes so a refresh does not erase time-above-mark --- .../src/lib/renderer-memory-sampling.test.ts | 18 ++++++++++ .../src/lib/renderer-memory-sampling.ts | 35 ++++++++++++++----- 2 files changed, 45 insertions(+), 8 deletions(-) diff --git a/src/renderer/src/lib/renderer-memory-sampling.test.ts b/src/renderer/src/lib/renderer-memory-sampling.test.ts index d74fd4d034d..cad8fb3b8f5 100644 --- a/src/renderer/src/lib/renderer-memory-sampling.test.ts +++ b/src/renderer/src/lib/renderer-memory-sampling.test.ts @@ -172,6 +172,24 @@ describe('renderer memory highwater census re-arming', () => { expect(censuses().map((c) => c.privateMB)).toEqual([1200, 1200]) }) + // A refresh replaces the retained crumb's own createdAt, so the census must carry how long the + // renderer has been over the mark — that is the axis that identified the 21h-stale census. + it('reports minutes above the mark across refreshes', async () => { + stubFootprint(658) + await tick() + await tick() + expect(censuses().at(-1)).toMatchObject({ thresholdPrivateMB: 600, aboveMarkMinutes: 0 }) + + stubFootprint(577) + for (let minute = 0; minute < 120; minute += 1) { + await tick() + } + + // Still over the band, so it refreshed — and it says how long it has been up there. + expect(censuses().length).toBeGreaterThan(1) + expect(censuses().at(-1)).toMatchObject({ thresholdPrivateMB: 600, aboveMarkMinutes: 120 }) + }) + it('emits at most one census per mark while a renderer oscillates around it', async () => { stubFootprint(601) await tick() diff --git a/src/renderer/src/lib/renderer-memory-sampling.ts b/src/renderer/src/lib/renderer-memory-sampling.ts index 529921f2505..39272a080f1 100644 --- a/src/renderer/src/lib/renderer-memory-sampling.ts +++ b/src/renderer/src/lib/renderer-memory-sampling.ts @@ -52,8 +52,10 @@ type HeapMetrics = BrowserPerformanceMemory & { } /** Mark -> monotonic time it last emitted a census. */ -const emittedHighwaterRatios = new Map() -const emittedPrivateHighwaterMarks = new Map() +/** First crossing is kept separately: a refresh replaces the crumb's own `createdAt`. */ +type HighwaterMarkState = { firstCrossedAtMs: number; lastEmittedAtMs: number } +const emittedHighwaterRatios = new Map() +const emittedPrivateHighwaterMarks = new Map() let lastProcessFootprint: RendererProcessMemory | null = null let processFootprintReadGeneration = 0 let processFootprintReadInFlight = false @@ -212,10 +214,11 @@ function recordRendererMemoryHighwater( if (!isHighwaterCensusDue(emittedHighwaterRatios, threshold, nowMs, ratio)) { continue } - emittedHighwaterRatios.set(threshold, nowMs) + const aboveMarkMinutes = stampHighwaterMark(emittedHighwaterRatios, threshold, nowMs) recordRendererCrashBreadcrumb('renderer_memory_highwater', { ...profile, - thresholdPct: Math.round(threshold * 100) + thresholdPct: Math.round(threshold * 100), + aboveMarkMinutes }) } } @@ -224,10 +227,11 @@ function recordRendererMemoryHighwater( if (!isHighwaterCensusDue(emittedPrivateHighwaterMarks, mark, nowMs, privateMB)) { continue } - emittedPrivateHighwaterMarks.set(mark, nowMs) + const aboveMarkMinutes = stampHighwaterMark(emittedPrivateHighwaterMarks, mark, nowMs) recordRendererCrashBreadcrumb('renderer_memory_highwater', { ...profile, - thresholdPrivateMB: mark + thresholdPrivateMB: mark, + aboveMarkMinutes }) } } @@ -242,13 +246,28 @@ function recordRendererMemoryHighwater( * would lose the census taken at its peak. Why monotonic: a wall-clock correction must not * stretch or collapse the window. */ +/** + * Records this emission and returns minutes since the mark was first crossed. Why carry it in the + * payload: a refresh replaces the retained crumb's `createdAt`, so without this "how long has the + * renderer been over the mark" — the axis that identified the stale census — becomes unrecoverable. + */ +function stampHighwaterMark( + emitted: Map, + mark: number, + nowMs: number +): number { + const firstCrossedAtMs = emitted.get(mark)?.firstCrossedAtMs ?? nowMs + emitted.set(mark, { firstCrossedAtMs, lastEmittedAtMs: nowMs }) + return Math.round((nowMs - firstCrossedAtMs) / 60_000) +} + function isHighwaterCensusDue( - emitted: Map, + emitted: Map, mark: number, nowMs: number, value: number ): boolean { - const lastEmittedAtMs = emitted.get(mark) + const lastEmittedAtMs = emitted.get(mark)?.lastEmittedAtMs if (lastEmittedAtMs === undefined) { return value >= mark }