review: re-anchor time-above-mark and stop refreshes evicting the peak census

src/main/crash-reporting/crash-breadcrumb-store.ts

@@ -72,7 +72,9 @@ export function recordCrashBreadcrumb(
   }
   const retainedKey = retainedBreadcrumbKey(breadcrumb)
   if (retainedKey) {
-    retainedBreadcrumbs.delete(retainedKey)
+    // Why no delete-then-set: re-inserting moves the key to the back, so a slot that refreshes
+    // outranks one that froze at its peak — and the frozen peak census is exactly the evidence
+    // worth keeping. Plain set preserves insertion order, keeping eviction FIFO by first crossing.
     retainedBreadcrumbs.set(retainedKey, breadcrumb)
     while (retainedBreadcrumbs.size > MAX_RETAINED_BREADCRUMBS) {
       const oldestKey = retainedBreadcrumbs.keys().next()

src/renderer/src/lib/renderer-memory-sampling.test.ts

@@ -190,6 +190,28 @@ describe('renderer memory highwater census re-arming', () => {
     expect(censuses().at(-1)).toMatchObject({ thresholdPrivateMB: 600, aboveMarkMinutes: 120 })
   })
 
+  // Sawtooth (build, GC, build) is the ordinary shape of renderer memory. Two spikes hours apart
+  // must not read as sustained pressure, or triage starts a leak hunt that has no leak.
+  it('re-anchors minutes-above-mark after a long spell below the band', async () => {
+    stubFootprint(658)
+    await tick()
+    await tick()
+    expect(censuses().at(-1)).toMatchObject({ aboveMarkMinutes: 0 })
+
+    // 10 hours far below the band, then back up.
+    stubFootprint(100)
+    for (let minute = 0; minute < 600; minute += 1) {
+      await tick()
+    }
+    stubFootprint(700)
+    await tick()
+    await tick()
+
+    // Not 602: the renderer spent those 600 minutes at 100MB.
+    expect(censuses().at(-1)).toMatchObject({ thresholdPrivateMB: 600 })
+    expect(censuses().at(-1)?.aboveMarkMinutes).toBeLessThanOrEqual(2)
+  })
+
   it('emits at most one census per mark while a renderer oscillates around it', async () => {
     stubFootprint(601)
     await tick()

src/renderer/src/lib/renderer-memory-sampling.ts

@@ -53,7 +53,11 @@ type HeapMetrics = BrowserPerformanceMemory & {
 
 /** Mark -> monotonic time it last emitted a census. */
 /** First crossing is kept separately: a refresh replaces the crumb's own `createdAt`. */
-type HighwaterMarkState = { firstCrossedAtMs: number; lastEmittedAtMs: number }
+type HighwaterMarkState = {
+  firstCrossedAtMs: number
+  lastEmittedAtMs: number
+  lastWithinBandAtMs: number
+}
 const emittedHighwaterRatios = new Map<number, HighwaterMarkState>()
 const emittedPrivateHighwaterMarks = new Map<number, HighwaterMarkState>()
 let lastProcessFootprint: RendererProcessMemory | null = null
@@ -173,6 +177,16 @@ function recordRendererMemoryHighwater(
   const privateMB =
     footprint === null ? null : (toMegabytes(footprint.privateKB * BYTES_PER_KILOBYTE) ?? null)
   const nowMs = performance.now()
+  if (ratio !== null) {
+    for (const threshold of RENDERER_MEMORY_HIGHWATER_RATIOS) {
+      noteHighwaterBandResidency(emittedHighwaterRatios, threshold, nowMs, ratio)
+    }
+  }
+  if (privateMB !== null) {
+    for (const mark of RENDERER_PRIVATE_HIGHWATER_MB) {
+      noteHighwaterBandResidency(emittedPrivateHighwaterMarks, mark, nowMs, privateMB)
+    }
+  }
   let crossedThreshold = false
   if (ratio !== null) {
     for (const threshold of RENDERER_MEMORY_HIGHWATER_RATIOS) {
@@ -246,6 +260,29 @@ function recordRendererMemoryHighwater(
  * would lose the census taken at its peak. Why monotonic: a wall-clock correction must not
  * stretch or collapse the window.
  */
+/**
+ * Tracks whether the mark is still occupied. Why re-anchor after a gap: the census reports how
+ * long the renderer has been heavy, and two spikes hours apart are not sustained pressure —
+ * sawtooth (build, GC, build) is the ordinary shape of renderer memory.
+ */
+function noteHighwaterBandResidency(
+  emitted: Map<number, HighwaterMarkState>,
+  mark: number,
+  nowMs: number,
+  value: number
+): void {
+  const state = emitted.get(mark)
+  if (state === undefined || value < mark * RENDERER_HIGHWATER_RECENSUS_BAND) {
+    return
+  }
+  const lapsed = nowMs - state.lastWithinBandAtMs > RENDERER_HIGHWATER_RECENSUS_MS
+  emitted.set(mark, {
+    firstCrossedAtMs: lapsed ? nowMs : state.firstCrossedAtMs,
+    lastEmittedAtMs: state.lastEmittedAtMs,
+    lastWithinBandAtMs: nowMs
+  })
+}
+
 /**
  * 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
@@ -257,7 +294,7 @@ function stampHighwaterMark(
   nowMs: number
 ): number {
   const firstCrossedAtMs = emitted.get(mark)?.firstCrossedAtMs ?? nowMs
-  emitted.set(mark, { firstCrossedAtMs, lastEmittedAtMs: nowMs })
+  emitted.set(mark, { firstCrossedAtMs, lastEmittedAtMs: nowMs, lastWithinBandAtMs: nowMs })
   return Math.round((nowMs - firstCrossedAtMs) / 60_000)
 }