review: measure observed band residency and survive a stalled sampler

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

@@ -174,11 +174,11 @@ describe('renderer memory highwater census re-arming', () => {
 
   // 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 () => {
+  it('reports minutes near the mark across refreshes', async () => {
     stubFootprint(658)
     await tick()
     await tick()
-    expect(censuses().at(-1)).toMatchObject({ thresholdPrivateMB: 600, aboveMarkMinutes: 0 })
+    expect(censuses().at(-1)).toMatchObject({ thresholdPrivateMB: 600, nearMarkMinutes: 0 })
 
     stubFootprint(577)
     for (let minute = 0; minute < 120; minute += 1) {
@@ -187,16 +187,16 @@ describe('renderer memory highwater census re-arming', () => {
 
     // 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 })
+    expect(censuses().at(-1)).toMatchObject({ thresholdPrivateMB: 600, nearMarkMinutes: 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 () => {
+  it('re-anchors minutes-near-mark after a long observed spell below the band', async () => {
     stubFootprint(658)
     await tick()
     await tick()
-    expect(censuses().at(-1)).toMatchObject({ aboveMarkMinutes: 0 })
+    expect(censuses().at(-1)).toMatchObject({ nearMarkMinutes: 0 })
 
     // 10 hours far below the band, then back up.
     stubFootprint(100)
@@ -209,7 +209,27 @@ describe('renderer memory highwater census re-arming', () => {
 
     // Not 602: the renderer spent those 600 minutes at 100MB.
     expect(censuses().at(-1)).toMatchObject({ thresholdPrivateMB: 600 })
-    expect(censuses().at(-1)?.aboveMarkMinutes).toBeLessThanOrEqual(2)
+    expect(censuses().at(-1)?.nearMarkMinutes).toBeLessThanOrEqual(2)
+  })
+
+  // A wedged main thread or a suspend stops the 60s sampler. That is not the renderer leaving the
+  // band — and it is exactly when the renderer is sickest, so the clock must not reset.
+  it('does not reset the clock when sampling itself stalls', async () => {
+    stubFootprint(658)
+    await tick()
+    await tick()
+    for (let minute = 0; minute < 30; minute += 1) {
+      await tick()
+    }
+    const beforeGap = censuses().at(-1)?.nearMarkMinutes as number
+    expect(beforeGap).toBeGreaterThanOrEqual(30)
+
+    // Sampler stalls for 8 hours, then resumes with the renderer still heavy.
+    nowMs += 8 * 60 * 60_000
+    await tick()
+    await tick()
+
+    expect(censuses().at(-1)?.nearMarkMinutes as number).toBeGreaterThan(beforeGap + 400)
   })
 
   it('emits at most one census per mark while a renderer oscillates around it', async () => {

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

@@ -57,6 +57,8 @@ type HighwaterMarkState = {
   firstCrossedAtMs: number
   lastEmittedAtMs: number
   lastWithinBandAtMs: number
+  /** When the renderer was first observed below the band; null while it is in band. */
+  belowBandSinceMs: number | null
 }
 const emittedHighwaterRatios = new Map<number, HighwaterMarkState>()
 const emittedPrivateHighwaterMarks = new Map<number, HighwaterMarkState>()
@@ -228,11 +230,11 @@ function recordRendererMemoryHighwater(
       if (!isHighwaterCensusDue(emittedHighwaterRatios, threshold, nowMs, ratio)) {
         continue
       }
-      const aboveMarkMinutes = stampHighwaterMark(emittedHighwaterRatios, threshold, nowMs)
+      const nearMarkMinutes = stampHighwaterMark(emittedHighwaterRatios, threshold, nowMs)
       recordRendererCrashBreadcrumb('renderer_memory_highwater', {
         ...profile,
         thresholdPct: Math.round(threshold * 100),
-        aboveMarkMinutes
+        nearMarkMinutes
       })
     }
   }
@@ -241,11 +243,11 @@ function recordRendererMemoryHighwater(
       if (!isHighwaterCensusDue(emittedPrivateHighwaterMarks, mark, nowMs, privateMB)) {
         continue
       }
-      const aboveMarkMinutes = stampHighwaterMark(emittedPrivateHighwaterMarks, mark, nowMs)
+      const nearMarkMinutes = stampHighwaterMark(emittedPrivateHighwaterMarks, mark, nowMs)
       recordRendererCrashBreadcrumb('renderer_memory_highwater', {
         ...profile,
         thresholdPrivateMB: mark,
-        aboveMarkMinutes
+        nearMarkMinutes
       })
     }
   }
@@ -272,21 +274,33 @@ function noteHighwaterBandResidency(
   value: number
 ): void {
   const state = emitted.get(mark)
-  if (state === undefined || value < mark * RENDERER_HIGHWATER_RECENSUS_BAND) {
+  if (state === undefined) {
     return
   }
-  const lapsed = nowMs - state.lastWithinBandAtMs > RENDERER_HIGHWATER_RECENSUS_MS
+  if (value < mark * RENDERER_HIGHWATER_RECENSUS_BAND) {
+    // Why stamp rather than re-anchor here: the spell has to be measured from observed samples.
+    emitted.set(mark, { ...state, belowBandSinceMs: state.belowBandSinceMs ?? nowMs })
+    return
+  }
+  // Why an observed spell and not elapsed time: a renderer that is simply not being sampled — a
+  // main thread wedged past the sample interval, or a suspend — has not left the band, and must
+  // not have its clock reset. Only samples we actually saw below the band count.
+  const lapsed =
+    state.belowBandSinceMs !== null &&
+    nowMs - state.belowBandSinceMs > RENDERER_HIGHWATER_RECENSUS_MS
   emitted.set(mark, {
     firstCrossedAtMs: lapsed ? nowMs : state.firstCrossedAtMs,
     lastEmittedAtMs: state.lastEmittedAtMs,
-    lastWithinBandAtMs: nowMs
+    lastWithinBandAtMs: nowMs,
+    belowBandSinceMs: null
   })
 }
 
 /**
- * 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.
+ * Records this emission and returns minutes the renderer has been *within the refresh band* of the
+ * mark (>=90% of it), not strictly above it — a renderer sitting at 577MB under a 600MB mark is
+ * the sustained-pressure signal worth reporting. Why carry it in the payload: a refresh replaces
+ * the retained crumb's `createdAt`, so without this the duration axis becomes unrecoverable.
  */
 function stampHighwaterMark(
   emitted: Map<number, HighwaterMarkState>,
@@ -294,7 +308,12 @@ function stampHighwaterMark(
   nowMs: number
 ): number {
   const firstCrossedAtMs = emitted.get(mark)?.firstCrossedAtMs ?? nowMs
-  emitted.set(mark, { firstCrossedAtMs, lastEmittedAtMs: nowMs, lastWithinBandAtMs: nowMs })
+  emitted.set(mark, {
+    firstCrossedAtMs,
+    lastEmittedAtMs: nowMs,
+    lastWithinBandAtMs: nowMs,
+    belowBandSinceMs: null
+  })
   return Math.round((nowMs - firstCrossedAtMs) / 60_000)
 }