perf(relay): sort latency samples once per percentile pair (#20427)

cloud/apps/relay/src/relay-observability.test.ts

@@ -5,6 +5,7 @@ import { observeRelayDatabase } from './observed-relay-database.js'
 import {
   CONTROL_RTT_RESERVOIR_LIMIT,
   observedRelayRequests,
+  percentile,
   RelayObservability,
   type RelayProcessCounts
 } from './relay-observability.js'
@@ -412,3 +413,181 @@ describe('relay observability', () => {
     expect(recordSql.mock.calls.map((call) => call[1])).toEqual([true, false, true, false])
   })
 })
+
+// The pre-change implementation, kept verbatim as the differential oracle. Both
+// ranks sorted their own copy and the maximum was a zero-seeded fold.
+function legacyPercentile(values: number[], percentileRank: number): number {
+  if (values.length === 0) return 0
+  const sorted = [...values].sort((left, right) => left - right)
+  return sorted[Math.ceil(percentileRank * sorted.length) - 1] ?? 0
+}
+
+function legacyLatencySummary(samples: number[]): { p50: number; p95: number; max: number } {
+  const round = (value: number): number => Number(value.toFixed(3))
+  return {
+    p50: round(legacyPercentile(samples, 0.5)),
+    p95: round(legacyPercentile(samples, 0.95)),
+    max: round(samples.reduce((highest, sample) => Math.max(highest, sample), 0))
+  }
+}
+
+// `-0` and `NaN` both survive a string round trip, unlike a bare equality check.
+function describeNumber(value: number): string {
+  return Object.is(value, -0) ? '-0' : String(value)
+}
+
+function expectSameNumber(actual: number, expected: number, label: string): void {
+  expect(`${label} = ${describeNumber(actual)}`).toBe(`${label} = ${describeNumber(expected)}`)
+}
+
+function sparseWindow(size: number, filled: Record<number, number>): number[] {
+  const values: number[] = new Array<number>(size)
+  for (const [index, value] of Object.entries(filled)) values[Number(index)] = value
+  return values
+}
+
+// Lehmer generator: stays inside the safe-integer range so the window is
+// byte-identical on every engine the relay runs on.
+function deterministicWindow(size: number): number[] {
+  let seed = 20_260_912
+  return Array.from({ length: size }, () => {
+    seed = (seed * 48_271) % 2_147_483_647
+    return (seed % 4_000_000) / 1_000
+  })
+}
+
+const DENSE_WINDOWS: Array<{ name: string; values: number[] }> = [
+  { name: 'empty', values: [] },
+  { name: 'single', values: [7.5] },
+  { name: 'single negative', values: [-7.5] },
+  { name: 'ascending', values: [1, 2, 3, 4, 5, 6, 7, 8, 9, 10] },
+  { name: 'descending', values: [10, 9, 8, 7, 6, 5, 4, 3, 2, 1] },
+  { name: 'duplicates', values: [4, 4, 4, 4, 4] },
+  // The trap: a sorted last element reads -1 here, the zero-seeded fold reads 0.
+  { name: 'all negative', values: [-5, -1, -9, -3, -2] },
+  { name: 'mixed signs', values: [-2, 3, -7, 0, 11, -0.5] },
+  { name: 'signed zero', values: [0, -0, -0, 0] },
+  { name: 'negative then signed zero', values: [-3, -0, -1] },
+  { name: 'nan leading', values: [NaN, 5, 1, 9] },
+  { name: 'nan trailing', values: [5, 1, 9, NaN] },
+  { name: 'nan interleaved', values: [5, NaN, 1, NaN, 9] },
+  { name: 'all nan', values: [NaN, NaN, NaN] },
+  { name: 'positive infinity', values: [Infinity, 3, 1] },
+  { name: 'negative infinity', values: [-Infinity, -3, -1] },
+  { name: 'both infinities', values: [Infinity, -Infinity, 3, -Infinity] },
+  { name: 'infinities and nan', values: [Infinity, NaN, -Infinity, 0] },
+  { name: 'sub-millisecond rounding', values: [0.00049, 0.0005, 0.00051, 0.9995] },
+  { name: 'reservoir sized', values: deterministicWindow(CONTROL_RTT_RESERVOIR_LIMIT) }
+]
+
+// Holes cannot reach the recorders, so they are exercised through `percentile`
+// alone — the surface `host-session-registry` also calls.
+const SPARSE_WINDOWS: Array<{ name: string; values: number[] }> = [
+  { name: 'all holes', values: sparseWindow(4, {}) },
+  { name: 'leading hole', values: sparseWindow(5, { 3: 8, 4: 2 }) },
+  { name: 'trailing hole', values: sparseWindow(5, { 0: 8, 1: 2 }) },
+  { name: 'interleaved holes', values: sparseWindow(6, { 0: 3, 2: -4, 5: 1 }) },
+  { name: 'holes with nan', values: sparseWindow(5, { 1: NaN, 3: 6 }) }
+]
+
+const PERCENTILE_RANKS = [0, 0.05, 0.5, 0.9, 0.95, 0.99, 1]
+
+type SortWork = { sorts: number; comparisons: number; copiedElements: number }
+
+// Every sorted array here is a fresh spread copy, so its length is the number of
+// elements copied to produce it.
+function countSortWork(run: () => void): SortWork {
+  const work: SortWork = { sorts: 0, comparisons: 0, copiedElements: 0 }
+  const original = Array.prototype.sort
+  const patched = Array.prototype as { sort: unknown }
+  patched.sort = function <T>(this: T[], compare?: (left: T, right: T) => number): T[] {
+    work.sorts++
+    work.copiedElements += this.length
+    return original.call(this, (left: T, right: T) => {
+      work.comparisons++
+      return compare ? compare(left, right) : String(left) < String(right) ? -1 : 1
+    })
+  }
+  try {
+    run()
+  } finally {
+    patched.sort = original
+  }
+  return work
+}
+
+function summaryThroughFlush(samples: number[]): { p50: number; p95: number; max: number } {
+  const entries: Array<Record<string, unknown>> = []
+  const observability = new RelayObservability(
+    { role: 'cell', cellId: 'staging-c1', region: 'us-central1' },
+    (entry) => entries.push(entry)
+  )
+  for (const sample of samples) observability.recordControlRenewal(sample, 'renewed')
+  observability.flush(counts)
+  const entry = entries[0]!
+  return {
+    p50: entry.controlRenewalLatencyMsP50 as number,
+    p95: entry.controlRenewalLatencyMsP95 as number,
+    max: entry.controlRenewalLatencyMsMax as number
+  }
+}
+
+describe('latency window summarisation', () => {
+  it('matches the pre-change percentile on every edge-case window', () => {
+    let compared = 0
+    for (const { name, values } of [...DENSE_WINDOWS, ...SPARSE_WINDOWS]) {
+      for (const rank of PERCENTILE_RANKS) {
+        expectSameNumber(
+          percentile(values, rank),
+          legacyPercentile(values, rank),
+          `${name} @ p${rank}`
+        )
+        compared++
+      }
+    }
+    expect(compared).toBe((DENSE_WINDOWS.length + SPARSE_WINDOWS.length) * PERCENTILE_RANKS.length)
+  })
+
+  it('matches the pre-change p50, p95 and maximum through a flush', () => {
+    let compared = 0
+    for (const { name, values } of DENSE_WINDOWS) {
+      const actual = summaryThroughFlush(values)
+      const expected = legacyLatencySummary(values)
+      expectSameNumber(actual.p50, expected.p50, `${name} p50`)
+      expectSameNumber(actual.p95, expected.p95, `${name} p95`)
+      // The zero-seeded fold, not the sorted last element: all-negative and NaN
+      // windows disagree between the two.
+      expectSameNumber(actual.max, expected.max, `${name} max`)
+      compared += 3
+    }
+    expect(compared).toBe(DENSE_WINDOWS.length * 3)
+    // The trap, spelled out: the sorted window ends at -1 but the fold reports 0.
+    expect(summaryThroughFlush([-5, -1, -9, -3, -2]).max).toBe(0)
+    expect(Number.isNaN(summaryThroughFlush([5, NaN, 1]).max)).toBe(true)
+  })
+
+  it('sorts each latency window once instead of once per rank', () => {
+    const samples = deterministicWindow(CONTROL_RTT_RESERVOIR_LIMIT)
+    const before = countSortWork(() => legacyLatencySummary(samples))
+    const after = countSortWork(() => summaryThroughFlush(samples))
+
+    expect(before.sorts).toBe(2)
+    expect(after.sorts).toBe(1)
+    expect(before.copiedElements).toBe(2 * CONTROL_RTT_RESERVOIR_LIMIT)
+    expect(after.copiedElements).toBe(CONTROL_RTT_RESERVOIR_LIMIT)
+    // Identical input and comparator, so the dropped sort is exactly half the
+    // comparator calls rather than an engine-specific constant.
+    expect(before.comparisons).toBeGreaterThan(CONTROL_RTT_RESERVOIR_LIMIT)
+    expect(after.comparisons).toBe(before.comparisons / 2)
+  })
+
+  it('never sorts an empty window and leaves the caller window untouched', () => {
+    const samples = [5, -1, NaN, 3, -0]
+    const before = samples.map(describeNumber)
+    expect(countSortWork(() => summaryThroughFlush([])).sorts).toBe(0)
+    expect(countSortWork(() => percentile([], 0.95)).sorts).toBe(0)
+    countSortWork(() => summaryThroughFlush(samples))
+    percentile(samples, 0.5)
+    expect(samples.map(describeNumber)).toEqual(before)
+  })
+})

cloud/apps/relay/src/relay-observability.ts

@@ -168,10 +168,18 @@ const emptyDeltas = (): RelayMetricDeltas => ({
   controlActivityRecoveryFailures: 0
 })
 
+function ascending(values: number[]): number[] {
+  return [...values].sort((left, right) => left - right)
+}
+
+// Holes and NaN land past the requested rank, so the fallback still applies.
+function nearestRank(sorted: number[], percentileRank: number): number {
+  return sorted[Math.ceil(percentileRank * sorted.length) - 1] ?? 0
+}
+
 export function percentile(values: number[], percentileRank: number): number {
   if (values.length === 0) return 0
-  const sorted = [...values].sort((left, right) => left - right)
-  return sorted[Math.ceil(percentileRank * sorted.length) - 1] ?? 0
+  return nearestRank(ascending(values), percentileRank)
 }
 
 function roundMs(value: number): number {
@@ -179,11 +187,15 @@ function roundMs(value: number): number {
 }
 
 // Spreading a window into Math.max blows the stack once a busy cell samples
-// enough of it, so the maximum is folded instead.
+// enough of it, so the maximum is folded instead. The fold is also not
+// interchangeable with the sorted last element: it is seeded with zero, so an
+// all-negative or NaN window reads differently.
 function latencySummary(samples: number[]): { p50: number; p95: number; max: number } {
+  // One sorted copy serves both ranks.
+  const sorted = samples.length === 0 ? samples : ascending(samples)
   return {
-    p50: roundMs(percentile(samples, 0.5)),
-    p95: roundMs(percentile(samples, 0.95)),
+    p50: roundMs(nearestRank(sorted, 0.5)),
+    p95: roundMs(nearestRank(sorted, 0.95)),
     max: roundMs(samples.reduce((highest, sample) => Math.max(highest, sample), 0))
   }
 }