* perf: dedupe relay process-table scans behind a short-TTL cache (#6288) Agent foreground-process inspection runs `ps -axo pid=,ppid=,stat=,command=` (a full system process-table scan) on a 750ms/2000ms per-pane cadence. On a shared SSH relay every tracked agent terminal drives it, so concurrent panes each forked their own `ps` — sustaining up to the per-second inspection cap of full-table scans for as long as agents are open, pinning idle relay CPU and amplified by AV process scanning. This is the CPU half of #6288 (PR #6564 covers the memory-leak half). Memoize the scan behind a single in-flight promise + 500ms TTL shared by the relay and local main-process call sites. 500ms sits below the active poll's minimum inter-poll gap (~675ms after jitter), so a single pane never reuses a snapshot older than it would have scanned itself — same data and freshness, just deduplicated within the cadence window (worst case ~8 scans/sec -> ~2). Failures are never cached (in-flight cleared on settle) so a transient `ps` error retries and the existing best-effort fall-through is preserved. Windows branches are untouched; no git-provider implications. Co-authored-by: Orca <help@stably.ai> * test: regression guard for #6288 ps-scan volume (repro + measurement) Drives the real local foreground-inspection call site under the documented 750ms agent-completion cadence across 6 concurrently-inspecting agent panes over a 30s window, counting actual `ps -axo pid=,ppid=,stat=,command=` full-table scans. Reproduces the waste on `main` (240 inspections -> 240 scans, 1.0/inspection; the test fails there) and proves the fix (240 inspections -> 40 scans, 0.167/inspection — bounded by poll ticks, not pane count) while every pane still resolves its foreground agent. Guards against regressing the cache back to a per-call scan. Co-authored-by: Orca <help@stably.ai> * docs: clarify 500ms TTL covers cadence floor + tolerated event-driven staleness (#6288) Co-authored-by: Orca <help@stably.ai> --------- Co-authored-by: Orca <help@stably.ai>
This commit is contained in:
parent
7c6f88ba6e
commit
06392a4523
|
|
@ -0,0 +1,93 @@
|
|||
// Regression guard for issue #6288 (CPU half): bound the volume of full
|
||||
// process-table `ps` scans driven by agent foreground-process inspection.
|
||||
//
|
||||
// Drives the REAL local call site (`resolveAgentForegroundProcess`) under the
|
||||
// documented agent-completion cadence (ACTIVE_POLL_INTERVAL_MS = 750ms in
|
||||
// agent-completion-coordinator.ts) across several concurrently-inspecting agent
|
||||
// panes, and counts how many `ps -axo pid=,ppid=,stat=,command=` scans actually
|
||||
// spawn. Pre-fix the call site forked one `ps` per pane per tick; with the
|
||||
// shared snapshot cache the scans collapse to ~one per tick regardless of pane
|
||||
// count, while each pane still resolves the same foreground identity.
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
|
||||
const { execFileMock, psScanCount } = vi.hoisted(() => ({
|
||||
execFileMock: vi.fn(),
|
||||
psScanCount: { value: 0 }
|
||||
}))
|
||||
|
||||
vi.mock('child_process', () => ({ execFile: execFileMock }))
|
||||
|
||||
import { resetProcessTableSnapshotForTests } from '../../shared/process-table-snapshot'
|
||||
import { resolveAgentForegroundProcess } from './agent-foreground-process'
|
||||
|
||||
const ACTIVE_POLL_INTERVAL_MS = 750 // mirrors agent-completion-coordinator.ts
|
||||
const PANE_COUNT = 6 // reporter saw it with "only three projects" -> several agent panes
|
||||
const WINDOW_SECONDS = 30
|
||||
const TICKS = Math.floor((WINDOW_SECONDS * 1000) / ACTIVE_POLL_INTERVAL_MS)
|
||||
|
||||
const shellPid = (pane: number): number => 100 + pane * 1000
|
||||
|
||||
// A real `ps` returns the whole system, so one shared snapshot must contain
|
||||
// every pane's shell + foreground codex child. Each pane resolves its own
|
||||
// agent from the single scan.
|
||||
const PS_OUTPUT = Array.from({ length: PANE_COUNT }, (_, pane) => {
|
||||
const shell = shellPid(pane)
|
||||
return [
|
||||
`${shell} 99 Ss bash -i`,
|
||||
`${shell + 1} ${shell} S+ node /Users/dev/.nvm/versions/node/bin/codex`
|
||||
].join('\n')
|
||||
}).join('\n')
|
||||
|
||||
function installCountingPsMock(): void {
|
||||
execFileMock.mockImplementation((cmd: string, args: string[], _opts: unknown, cb: unknown) => {
|
||||
const callback = cb as (err: unknown, result: { stdout: string; stderr: string }) => void
|
||||
if (cmd === 'ps' && Array.isArray(args) && args.includes('-axo')) {
|
||||
psScanCount.value += 1
|
||||
}
|
||||
callback(null, { stdout: PS_OUTPUT, stderr: '' })
|
||||
})
|
||||
}
|
||||
|
||||
describe('#6288 agent foreground inspection ps-scan volume', () => {
|
||||
let platform: PropertyDescriptor | undefined
|
||||
|
||||
beforeEach(() => {
|
||||
execFileMock.mockReset()
|
||||
resetProcessTableSnapshotForTests()
|
||||
psScanCount.value = 0
|
||||
platform = Object.getOwnPropertyDescriptor(process, 'platform')
|
||||
Object.defineProperty(process, 'platform', { configurable: true, value: 'darwin' })
|
||||
vi.useFakeTimers({ toFake: ['Date'] })
|
||||
vi.setSystemTime(0)
|
||||
})
|
||||
|
||||
afterEach(() => {
|
||||
vi.useRealTimers()
|
||||
if (platform) {
|
||||
Object.defineProperty(process, 'platform', platform)
|
||||
}
|
||||
})
|
||||
|
||||
it('bounds ps scans by poll ticks, not by pane count, while resolving every pane', async () => {
|
||||
installCountingPsMock()
|
||||
|
||||
for (let tick = 0; tick < TICKS; tick++) {
|
||||
vi.setSystemTime(tick * ACTIVE_POLL_INTERVAL_MS)
|
||||
// All panes inspect concurrently within the tick (worst case for a busy relay).
|
||||
const resolved = await Promise.all(
|
||||
Array.from({ length: PANE_COUNT }, (_, pane) =>
|
||||
resolveAgentForegroundProcess(shellPid(pane), 'node')
|
||||
)
|
||||
)
|
||||
// Caching must not change the answer: every pane still resolves the agent.
|
||||
expect(resolved.every((name) => name === 'codex')).toBe(true)
|
||||
}
|
||||
|
||||
const totalInspections = PANE_COUNT * TICKS
|
||||
// Pre-fix this equals totalInspections (one scan per inspection). With the
|
||||
// shared cache, concurrent panes within a tick share one scan and the 500ms
|
||||
// TTL forces a fresh scan each new 750ms tick -> ~one scan per tick.
|
||||
expect(psScanCount.value).toBeLessThanOrEqual(TICKS + 1)
|
||||
expect(psScanCount.value).toBeLessThan(totalInspections / 2)
|
||||
})
|
||||
})
|
||||
|
|
@ -8,6 +8,7 @@ vi.mock('child_process', () => ({
|
|||
execFile: execFileMock
|
||||
}))
|
||||
|
||||
import { resetProcessTableSnapshotForTests } from '../../shared/process-table-snapshot'
|
||||
import { resolveAgentForegroundProcess } from './agent-foreground-process'
|
||||
|
||||
// Why: the module wraps execFile with promisify, so the mock must honor the
|
||||
|
|
@ -71,6 +72,7 @@ describe('resolveAgentForegroundProcess', () => {
|
|||
|
||||
beforeEach(() => {
|
||||
execFileMock.mockReset()
|
||||
resetProcessTableSnapshotForTests()
|
||||
platform = Object.getOwnPropertyDescriptor(process, 'platform')
|
||||
Object.defineProperty(process, 'platform', { value: 'darwin' })
|
||||
})
|
||||
|
|
|
|||
|
|
@ -1,6 +1,5 @@
|
|||
import { execFile } from 'child_process'
|
||||
import { promisify } from 'util'
|
||||
import { recognizeAgentProcessFromCommandLine } from '../../shared/agent-process-recognition'
|
||||
import { getProcessTableSnapshot } from '../../shared/process-table-snapshot'
|
||||
import {
|
||||
resolveWindowsAgentForegroundProcess,
|
||||
shouldInspectWindowsAgentForeground,
|
||||
|
|
@ -9,8 +8,6 @@ import {
|
|||
|
||||
export type { AgentForegroundResolutionOptions } from './windows-agent-foreground-process'
|
||||
|
||||
const execFileAsync = promisify(execFile)
|
||||
|
||||
type ProcessRow = {
|
||||
pid: number
|
||||
ppid: number
|
||||
|
|
@ -85,10 +82,7 @@ export async function resolveAgentForegroundProcess(
|
|||
}
|
||||
|
||||
try {
|
||||
const { stdout } = await execFileAsync('ps', ['-axo', 'pid=,ppid=,stat=,command='], {
|
||||
encoding: 'utf8',
|
||||
timeout: 3000
|
||||
})
|
||||
const stdout = await getProcessTableSnapshot()
|
||||
return resolveAgentForegroundProcessFromPs(stdout, shellPid) ?? fallbackProcess
|
||||
} catch {
|
||||
// Fall through to node-pty's process name. Foreground process inspection is
|
||||
|
|
|
|||
|
|
@ -8,6 +8,7 @@ vi.mock('child_process', () => ({
|
|||
execFile: execFileMock
|
||||
}))
|
||||
|
||||
import { resetProcessTableSnapshotForTests } from '../shared/process-table-snapshot'
|
||||
import {
|
||||
getForegroundProcessName,
|
||||
resolveDefaultCwd,
|
||||
|
|
@ -47,6 +48,7 @@ async function withProcessPlatform<T>(
|
|||
|
||||
beforeEach(() => {
|
||||
execFileMock.mockReset()
|
||||
resetProcessTableSnapshotForTests()
|
||||
})
|
||||
|
||||
describe('resolveWindowsDefaultShell', () => {
|
||||
|
|
|
|||
|
|
@ -10,6 +10,7 @@ import {
|
|||
recognizeAgentProcessFromCommandLine
|
||||
} from '../shared/agent-process-recognition'
|
||||
import { getFirstCommandToken } from '../shared/command-token-scanner'
|
||||
import { getProcessTableSnapshot } from '../shared/process-table-snapshot'
|
||||
import { isShellProcess } from '../shared/shell-process-detection'
|
||||
import {
|
||||
resolveWindowsAgentForegroundProcess,
|
||||
|
|
@ -210,10 +211,7 @@ async function getRecognizedForegroundDescendant(
|
|||
fallbackProcess?: string | null
|
||||
): Promise<string | null> {
|
||||
try {
|
||||
const { stdout } = await execFile('ps', ['-axo', 'pid=,ppid=,stat=,command='], {
|
||||
encoding: 'utf-8',
|
||||
timeout: 3000
|
||||
})
|
||||
const stdout = await getProcessTableSnapshot()
|
||||
const rows = parsePsRows(stdout)
|
||||
const root = rows.find((row) => row.pid === pid)
|
||||
const candidates = collectDescendants(rows, pid).sort(
|
||||
|
|
|
|||
|
|
@ -0,0 +1,123 @@
|
|||
import { describe, expect, it } from 'vitest'
|
||||
import { createProcessTableSnapshotReader } from './process-table-snapshot'
|
||||
|
||||
function deferred<T>(): {
|
||||
promise: Promise<T>
|
||||
resolve: (v: T) => void
|
||||
reject: (e: unknown) => void
|
||||
} {
|
||||
let resolve!: (v: T) => void
|
||||
let reject!: (e: unknown) => void
|
||||
const promise = new Promise<T>((res, rej) => {
|
||||
resolve = res
|
||||
reject = rej
|
||||
})
|
||||
return { promise, resolve, reject }
|
||||
}
|
||||
|
||||
describe('process-table-snapshot reader', () => {
|
||||
it('collapses concurrent calls into a single ps scan', async () => {
|
||||
let scans = 0
|
||||
const gate = deferred<string>()
|
||||
const reader = createProcessTableSnapshotReader({
|
||||
runPs: () => {
|
||||
scans += 1
|
||||
return gate.promise
|
||||
},
|
||||
now: () => 0
|
||||
})
|
||||
|
||||
const a = reader.getSnapshot()
|
||||
const b = reader.getSnapshot()
|
||||
const c = reader.getSnapshot()
|
||||
gate.resolve('ps-output')
|
||||
|
||||
expect(await a).toBe('ps-output')
|
||||
expect(await b).toBe('ps-output')
|
||||
expect(await c).toBe('ps-output')
|
||||
// Why: the in-flight promise is shared, so a burst of panes inspecting at
|
||||
// once forks `ps` exactly once.
|
||||
expect(scans).toBe(1)
|
||||
})
|
||||
|
||||
it('reuses the cached snapshot within the TTL window', async () => {
|
||||
let scans = 0
|
||||
let clock = 0
|
||||
const reader = createProcessTableSnapshotReader({
|
||||
runPs: () => {
|
||||
scans += 1
|
||||
return Promise.resolve(`scan-${scans}`)
|
||||
},
|
||||
now: () => clock,
|
||||
ttlMs: 500
|
||||
})
|
||||
|
||||
expect(await reader.getSnapshot()).toBe('scan-1')
|
||||
clock = 499
|
||||
expect(await reader.getSnapshot()).toBe('scan-1')
|
||||
expect(scans).toBe(1)
|
||||
})
|
||||
|
||||
it('rescans once the TTL expires', async () => {
|
||||
let scans = 0
|
||||
let clock = 0
|
||||
const reader = createProcessTableSnapshotReader({
|
||||
runPs: () => {
|
||||
scans += 1
|
||||
return Promise.resolve(`scan-${scans}`)
|
||||
},
|
||||
now: () => clock,
|
||||
ttlMs: 500
|
||||
})
|
||||
|
||||
expect(await reader.getSnapshot()).toBe('scan-1')
|
||||
clock = 500
|
||||
expect(await reader.getSnapshot()).toBe('scan-2')
|
||||
expect(scans).toBe(2)
|
||||
})
|
||||
|
||||
it('stamps capture time after the scan resolves so a slow ps cannot serve a stale snapshot', async () => {
|
||||
let scans = 0
|
||||
let clock = 0
|
||||
const gate = deferred<string>()
|
||||
const reader = createProcessTableSnapshotReader({
|
||||
runPs: () => {
|
||||
scans += 1
|
||||
return scans === 1 ? gate.promise : Promise.resolve(`scan-${scans}`)
|
||||
},
|
||||
now: () => clock,
|
||||
ttlMs: 500
|
||||
})
|
||||
|
||||
const first = reader.getSnapshot()
|
||||
// The scan takes 600ms of wall clock to return — longer than the TTL.
|
||||
clock = 600
|
||||
gate.resolve('scan-1')
|
||||
expect(await first).toBe('scan-1')
|
||||
|
||||
// capturedAt is stamped at now()=600, so a call at 900 is still within TTL.
|
||||
clock = 900
|
||||
expect(await reader.getSnapshot()).toBe('scan-1')
|
||||
expect(scans).toBe(1)
|
||||
})
|
||||
|
||||
it('does not cache failures and retries on the next call', async () => {
|
||||
let scans = 0
|
||||
const reader = createProcessTableSnapshotReader({
|
||||
runPs: () => {
|
||||
scans += 1
|
||||
if (scans === 1) {
|
||||
return Promise.reject(new Error('ps timed out'))
|
||||
}
|
||||
return Promise.resolve('recovered')
|
||||
},
|
||||
now: () => 0
|
||||
})
|
||||
|
||||
await expect(reader.getSnapshot()).rejects.toThrow('ps timed out')
|
||||
// Why: a transient ps failure must not poison the cache — the next
|
||||
// inspection re-scans rather than returning a cached error.
|
||||
expect(await reader.getSnapshot()).toBe('recovered')
|
||||
expect(scans).toBe(2)
|
||||
})
|
||||
})
|
||||
|
|
@ -0,0 +1,108 @@
|
|||
import { execFile as execFileCb } from 'child_process'
|
||||
import { promisify } from 'util'
|
||||
|
||||
const execFile = promisify(execFileCb)
|
||||
|
||||
// Why: agent foreground-process inspection runs this full process-table scan on
|
||||
// a 750ms/2000ms per-pane cadence. On a shared SSH relay every tracked agent
|
||||
// terminal drives it, so concurrent panes used to each fork their own `ps`,
|
||||
// pinning idle CPU (issue #6288). Memoizing collapses overlapping scans to one.
|
||||
const PS_ARGS = ['-axo', 'pid=,ppid=,stat=,command='] as const
|
||||
const PS_TIMEOUT_MS = 3000
|
||||
|
||||
// Why: 500ms is below the active cadence poll's minimum inter-poll gap (~675ms
|
||||
// = 750ms less jitter), so a cadence-driven pane never reuses a snapshot older
|
||||
// than it would have scanned itself; a burst of panes polling in the same
|
||||
// window collapses from up to 8 scans/sec down to ~2/sec. The faster
|
||||
// event-driven follow-up inspections (e.g. the pending-title confirmation,
|
||||
// which can re-fire <500ms apart) intentionally accept a <=500ms-stale table:
|
||||
// they only confirm the same agent still owns the pane, and process-exit is
|
||||
// debounced across repeated samples, so a near-instant cached scan answers
|
||||
// identically to a fresh fork.
|
||||
const DEFAULT_SNAPSHOT_TTL_MS = 500
|
||||
|
||||
type Snapshot = { stdout: string; capturedAtMs: number }
|
||||
|
||||
type ProcessTableSnapshotReaderDeps = {
|
||||
runPs: () => Promise<string>
|
||||
now: () => number
|
||||
ttlMs?: number
|
||||
}
|
||||
|
||||
/**
|
||||
* Build a process-table snapshot reader that deduplicates concurrent and
|
||||
* near-simultaneous `ps` scans behind a single in-flight promise + short TTL.
|
||||
* Exposed as a factory so tests can inject the scan and clock; production code
|
||||
* uses the shared `getProcessTableSnapshot` instance below.
|
||||
*/
|
||||
export function createProcessTableSnapshotReader(deps: ProcessTableSnapshotReaderDeps): {
|
||||
getSnapshot: () => Promise<string>
|
||||
reset: () => void
|
||||
} {
|
||||
const ttlMs = deps.ttlMs ?? DEFAULT_SNAPSHOT_TTL_MS
|
||||
let cached: Snapshot | null = null
|
||||
let inFlight: Promise<string> | null = null
|
||||
|
||||
async function getSnapshot(): Promise<string> {
|
||||
if (cached && deps.now() - cached.capturedAtMs < ttlMs) {
|
||||
return cached.stdout
|
||||
}
|
||||
if (inFlight) {
|
||||
return inFlight
|
||||
}
|
||||
const promise = deps.runPs()
|
||||
inFlight = promise
|
||||
try {
|
||||
const stdout = await promise
|
||||
// Why: stamp capture time AFTER the scan returns so a slow `ps` can't
|
||||
// hand back a snapshot that is already older than its TTL.
|
||||
cached = { stdout, capturedAtMs: deps.now() }
|
||||
return stdout
|
||||
} finally {
|
||||
// Clear in-flight on success and failure so a transient `ps` error
|
||||
// (timeout, nonzero exit) retries on the next call instead of being
|
||||
// cached; callers keep their existing best-effort fall-through.
|
||||
if (inFlight === promise) {
|
||||
inFlight = null
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
return {
|
||||
getSnapshot,
|
||||
// Why: lets tests that mock `ps` per case clear the cross-call cache so one
|
||||
// case's snapshot can't satisfy the next within the TTL window.
|
||||
reset: () => {
|
||||
cached = null
|
||||
inFlight = null
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
const defaultReader = createProcessTableSnapshotReader({
|
||||
runPs: async () => {
|
||||
const { stdout } = await execFile('ps', [...PS_ARGS], {
|
||||
encoding: 'utf-8',
|
||||
timeout: PS_TIMEOUT_MS
|
||||
})
|
||||
return stdout
|
||||
},
|
||||
now: () => Date.now()
|
||||
})
|
||||
|
||||
/**
|
||||
* Run (or reuse a recent) `ps -axo pid=,ppid=,stat=,command=` scan and return
|
||||
* its raw stdout. Per-process singleton: the relay and local main processes
|
||||
* each dedupe their own scans.
|
||||
*/
|
||||
export function getProcessTableSnapshot(): Promise<string> {
|
||||
return defaultReader.getSnapshot()
|
||||
}
|
||||
|
||||
/**
|
||||
* Test-only: clear the shared snapshot cache so suites that mock `ps` between
|
||||
* cases don't have one case's snapshot served to the next within the TTL.
|
||||
*/
|
||||
export function resetProcessTableSnapshotForTests(): void {
|
||||
defaultReader.reset()
|
||||
}
|
||||
Loading…
Reference in New Issue