review: band the re-census so a released-memory renderer keeps its peak

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

@@ -121,6 +121,57 @@ describe('renderer memory highwater census re-arming', () => {
     expect(censuses().length).toBeLessThanOrEqual(90)
   })
 
+  // The literal fb476b1c ending: crossed 600MB, then spent 21h BELOW the mark and died at 577MB.
+  // A re-census gated on the current value never fires here, which was the whole defect.
+  it('re-censuses a renderer that crossed a mark and then settled back below it', async () => {
+    stubFootprint(658)
+    await tick()
+    await tick()
+    expect(censuses()).toHaveLength(1)
+    const firstCensus = censuses()[0]
+
+    // New workload, permanently below the mark, for 21 hours.
+    stubFootprint(577)
+    heapStats.blinkAllocatedKB = 21 * KB
+    webviewProfile = { browserWebviewCount: 1, registeredBrowserGuestCount: 1 }
+    for (let minute = 0; minute < 1260; minute += 1) {
+      await tick()
+    }
+
+    expect(censuses().length).toBeGreaterThan(1)
+    expect(censuses().at(-1)).not.toBe(firstCensus)
+    expect(censuses().at(-1)).toMatchObject({
+      thresholdPrivateMB: 600,
+      privateMB: 577,
+      blinkAllocatedMB: 21,
+      browserWebviews: 1
+    })
+    expect(censuses().length).toBeLessThanOrEqual(90)
+  })
+
+  // The retained crumb is one slot per mark, so a refresh overwrites the peak census. A renderer
+  // that released its memory must keep the peak evidence rather than ship privateMB:50 under a
+  // thresholdPrivateMB:600 label.
+  it('keeps the peak census when a renderer releases its memory', async () => {
+    stubFootprint(1200)
+    heapStats.blinkAllocatedKB = 900 * KB
+    await tick()
+    await tick()
+    // 1200MB crosses both private marks, so both slots are censused at the peak.
+    expect(censuses()).toHaveLength(2)
+
+    // Leak released: 6 hours far below the mark.
+    stubFootprint(50)
+    heapStats.blinkAllocatedKB = 1 * KB
+    for (let minute = 0; minute < 360; minute += 1) {
+      await tick()
+    }
+
+    // No refresh fired, so the retained censuses still describe the peak.
+    expect(censuses()).toHaveLength(2)
+    expect(censuses().map((c) => c.privateMB)).toEqual([1200, 1200])
+  })
+
   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

@@ -31,6 +31,10 @@ const RENDERER_PRIVATE_HIGHWATER_MB = [600, 1000] as const
 // stale census. 15min = 1 census per 15 samples, and the store keys retained
 // highwater crumbs by mark, so a refresh replaces the stale one.
 const RENDERER_HIGHWATER_RECENSUS_MS = 15 * 60_000
+// Why 0.9 and not 0: the retained crumb is one slot per mark, so a refresh overwrites the census
+// taken at the peak. Refreshing only while the renderer is still near the mark keeps the peak
+// evidence for a renderer that released its memory, and still re-censuses one that stays large.
+const RENDERER_HIGHWATER_RECENSUS_BAND = 0.9
 
 export type RendererSurface = 'main' | 'dashboard-popout'
 
@@ -170,7 +174,7 @@ function recordRendererMemoryHighwater(
   let crossedThreshold = false
   if (ratio !== null) {
     for (const threshold of RENDERER_MEMORY_HIGHWATER_RATIOS) {
-      if (ratio >= threshold && isHighwaterMarkArmed(emittedHighwaterRatios, threshold, nowMs)) {
+      if (isHighwaterCensusDue(emittedHighwaterRatios, threshold, nowMs, ratio)) {
         crossedThreshold = true
         break
       }
@@ -178,7 +182,7 @@ function recordRendererMemoryHighwater(
   }
   if (privateMB !== null) {
     for (const mark of RENDERER_PRIVATE_HIGHWATER_MB) {
-      if (privateMB >= mark && isHighwaterMarkArmed(emittedPrivateHighwaterMarks, mark, nowMs)) {
+      if (isHighwaterCensusDue(emittedPrivateHighwaterMarks, mark, nowMs, privateMB)) {
         crossedThreshold = true
         break
       }
@@ -205,7 +209,7 @@ function recordRendererMemoryHighwater(
   })
   if (ratio !== null) {
     for (const threshold of RENDERER_MEMORY_HIGHWATER_RATIOS) {
-      if (ratio < threshold || !isHighwaterMarkArmed(emittedHighwaterRatios, threshold, nowMs)) {
+      if (!isHighwaterCensusDue(emittedHighwaterRatios, threshold, nowMs, ratio)) {
         continue
       }
       emittedHighwaterRatios.set(threshold, nowMs)
@@ -217,7 +221,7 @@ function recordRendererMemoryHighwater(
   }
   if (privateMB !== null) {
     for (const mark of RENDERER_PRIVATE_HIGHWATER_MB) {
-      if (privateMB < mark || !isHighwaterMarkArmed(emittedPrivateHighwaterMarks, mark, nowMs)) {
+      if (!isHighwaterCensusDue(emittedPrivateHighwaterMarks, mark, nowMs, privateMB)) {
         continue
       }
       emittedPrivateHighwaterMarks.set(mark, nowMs)
@@ -229,10 +233,29 @@ function recordRendererMemoryHighwater(
   }
 }
 
-/** Why monotonic: a wall-clock correction must not stretch or collapse the window. */
-function isHighwaterMarkArmed(emitted: Map<number, number>, mark: number, nowMs: number): boolean {
+/**
+ * A mark is due on its first crossing, and thereafter every `RENDERER_HIGHWATER_RECENSUS_MS`
+ * while the renderer stays within `RENDERER_HIGHWATER_RECENSUS_BAND` of it. Why not a strict
+ * `value >= mark`: report fb476b1c crossed 600MB then died 21h later at 577MB, and a re-census
+ * gated on the mark never fires again — that is the stale-census bug. Why not value-independent
+ * either: the refresh overwrites the one retained slot, so a renderer that released its memory
+ * would lose the census taken at its peak. Why monotonic: a wall-clock correction must not
+ * stretch or collapse the window.
+ */
+function isHighwaterCensusDue(
+  emitted: Map<number, number>,
+  mark: number,
+  nowMs: number,
+  value: number
+): boolean {
   const lastEmittedAtMs = emitted.get(mark)
-  return lastEmittedAtMs === undefined || nowMs - lastEmittedAtMs >= RENDERER_HIGHWATER_RECENSUS_MS
+  if (lastEmittedAtMs === undefined) {
+    return value >= mark
+  }
+  return (
+    nowMs - lastEmittedAtMs >= RENDERER_HIGHWATER_RECENSUS_MS &&
+    value >= mark * RENDERER_HIGHWATER_RECENSUS_BAND
+  )
 }
 
 function isFiniteHeapBytes(value: number | undefined): value is number {