From 9e8bb22432e2cbe3730c160bf89281b2a8ce8aa0 Mon Sep 17 00:00:00 2001 From: Jinwoo-H Date: Thu, 2 Jul 2026 22:13:45 -0400 Subject: [PATCH] Parse-clock high-priority terminal drains instead of fixed-nap dripping MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Attribution (task #9): the drain loop wrote at most 2x16KB then slept 4/16ms regardless of parse speed — an isolation bench (new pane-terminal-output-scheduler-throughput.bench.test.ts) measures that drip at 1.9 MB/s background / 27 MB/s foreground against xterm's ~103 MB/s parse rate, matching the baseline-jul02 end-to-end numbers (agent-tui 2.0 MB/s in prod 1.4.91). Fix: high-priority (visible-pane) drains now re-arm on xterm's parse-completion callback and carry 8 writes per tick; the isolation ceiling rises 27 -> 117.6 MB/s (parse-limited). Background cadence is deliberately unchanged (2 MB/s drip protects the focused pane; hidden delivery is term-speed-2's job). DRAIN_TIME_BUDGET_MS still bounds per-tick work, preserving #7139's cooperative-drain intent. Validation: 621 scheduler/guard/pty tests green, typecheck clean. Co-authored-by: Orca --- ...-output-scheduler-throughput.bench.test.ts | 96 +++++++++++++++++++ .../pane-terminal-output-scheduler.test.ts | 7 +- .../pane-terminal-output-scheduler.ts | 52 +++++++++- tools/benchmarks/terminal-pipeline-bench.mjs | 9 +- 4 files changed, 157 insertions(+), 7 deletions(-) create mode 100644 src/renderer/src/lib/pane-manager/pane-terminal-output-scheduler-throughput.bench.test.ts diff --git a/src/renderer/src/lib/pane-manager/pane-terminal-output-scheduler-throughput.bench.test.ts b/src/renderer/src/lib/pane-manager/pane-terminal-output-scheduler-throughput.bench.test.ts new file mode 100644 index 00000000000..7bddc53d9d0 --- /dev/null +++ b/src/renderer/src/lib/pane-manager/pane-terminal-output-scheduler-throughput.bench.test.ts @@ -0,0 +1,96 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' + +// Benchmark harness for the terminal performance initiative: measures the +// scheduler-imposed drain ceiling in isolation. A mock terminal parses +// instantly, so the measured rate is pure scheduler drip (writes-per-tick x +// chunk size / reschedule interval). Baseline-jul02 measured agent-tui at +// 2.0 MB/s end-to-end while bare xterm parses the same bytes at ~103 MB/s; +// this pins how much of that ceiling the drain loop itself imposes. +// Run with: +// ORCA_TERMINAL_PERF_BENCH=1 pnpm vitest run \ +// src/renderer/src/lib/pane-manager/pane-terminal-output-scheduler-throughput.bench.test.ts \ +// --config config/vitest.config.ts +const benchEnabled = process.env.ORCA_TERMINAL_PERF_BENCH === '1' + +vi.mock('@/lib/e2e-config', () => ({ + e2eConfig: { exposeStore: false } +})) + +vi.mock('@/lib/crash-breadcrumb-recorder', () => ({ + recordRendererCrashBreadcrumb: vi.fn() +})) + +const TOTAL_CHARS = 4 * 1024 * 1024 +const FEED_CHUNK_CHARS = 8 * 1024 +const MAX_SIMULATED_MS = 60_000 + +function createInstantParseTerminal() { + let written = 0 + return { + get written() { + return written + }, + buffer: { active: { cursorY: 0, baseY: 0, viewportY: 0 } }, + rows: 24, + refresh: vi.fn(), + _core: { refresh: vi.fn() }, + write: vi.fn((data: string, callback?: () => void) => { + written += data.length + callback?.() + }) + } +} + +async function loadScheduler() { + vi.resetModules() + return import('./pane-terminal-output-scheduler') +} + +async function measure(options: { foreground: boolean }): Promise { + vi.useFakeTimers() + const scheduler = await loadScheduler() + const terminal = createInstantParseTerminal() + const payload = 'x'.repeat(FEED_CHUNK_CHARS) + // Why paced feeding: dumping the whole payload trips the backlog cap + // (replaceBacklogWithWarning). Real sources are paced by main's 512KB + // delivery watermark; keep in-flight below a 256KB window like a live PTY. + const IN_FLIGHT_WINDOW_CHARS = 256 * 1024 + let fed = 0 + let elapsed = 0 + while (terminal.written < TOTAL_CHARS && elapsed < MAX_SIMULATED_MS) { + while (fed < TOTAL_CHARS && fed - terminal.written < IN_FLIGHT_WINDOW_CHARS) { + scheduler.writeTerminalOutput(terminal as never, payload, { + foreground: options.foreground, + // Why false: floods are classified latency-insensitive by + // pty-connection's isLatencySensitiveForegroundOutput once the + // immediate budget is spent — this is the sustained-throughput path. + latencySensitive: false + }) + fed += FEED_CHUNK_CHARS + } + vi.advanceTimersByTime(1) + elapsed += 1 + } + expect(terminal.written).toBe(TOTAL_CHARS) + return TOTAL_CHARS / 1024 / 1024 / (elapsed / 1000) +} + +describe.skipIf(!benchEnabled)('scheduler drain ceiling', () => { + beforeEach(() => { + vi.stubGlobal('window', globalThis) + }) + + afterEach(() => { + vi.useRealTimers() + vi.unstubAllGlobals() + }) + + it('measures foreground (visible pane flood) and background ceilings', async () => { + const foreground = await measure({ foreground: true }) + const background = await measure({ foreground: false }) + // eslint-disable-next-line no-console -- bench harness output + console.log( + `\n[scheduler-ceiling] foreground flood: ${foreground.toFixed(1)} MB/s, background: ${background.toFixed(1)} MB/s (simulated time, instant parse)` + ) + }) +}) diff --git a/src/renderer/src/lib/pane-manager/pane-terminal-output-scheduler.test.ts b/src/renderer/src/lib/pane-manager/pane-terminal-output-scheduler.test.ts index 932bf0b37af..aa7d2433ab9 100644 --- a/src/renderer/src/lib/pane-manager/pane-terminal-output-scheduler.test.ts +++ b/src/renderer/src/lib/pane-manager/pane-terminal-output-scheduler.test.ts @@ -906,11 +906,14 @@ describe('pane terminal output scheduler', () => { expect(terminal.write).not.toHaveBeenCalled() + // Why 8: promoted backlogs use the parse-clocked high-priority budget + // (HIGH_PRIORITY_MAX_WRITES_PER_DRAIN) so a visible flood drains at the + // parser's pace instead of a fixed 2-write drip. vi.advanceTimersByTime(0) - expect(terminal.write).toHaveBeenCalledTimes(2) + expect(terminal.write).toHaveBeenCalledTimes(8) vi.advanceTimersByTime(4) - expect(terminal.write).toHaveBeenCalledTimes(4) + expect(terminal.write).toHaveBeenCalledTimes(16) }) it('yields high-priority backlog drains when writes spend the frame budget', async () => { diff --git a/src/renderer/src/lib/pane-manager/pane-terminal-output-scheduler.ts b/src/renderer/src/lib/pane-manager/pane-terminal-output-scheduler.ts index 7beed10f054..9e1ecceb9ed 100644 --- a/src/renderer/src/lib/pane-manager/pane-terminal-output-scheduler.ts +++ b/src/renderer/src/lib/pane-manager/pane-terminal-output-scheduler.ts @@ -77,7 +77,13 @@ const BACKGROUND_DRAIN_INTERVAL_MS = 16 const HIGH_PRIORITY_DRAIN_INTERVAL_MS = 4 const BACKGROUND_CHUNK_CHARS = 16 * 1024 const MAX_WRITES_PER_DRAIN = 2 -const HIGH_PRIORITY_MAX_WRITES_PER_DRAIN = 2 +// Why 8: with the parse-clock pacer, high-priority ticks fire only after +// xterm confirms the previous batch parsed, and Chromium clamps chained +// timers to ~4ms — so per-tick volume (8 x 16KB = 128KB ≈ 1.3ms of parse) +// sets the sustained ceiling (~30MB/s) while staying far inside +// DRAIN_TIME_BUDGET_MS. At 2 the ceiling was 8MB/s against a ~100MB/s +// parser (see pane-terminal-output-scheduler-throughput.bench.test.ts). +const HIGH_PRIORITY_MAX_WRITES_PER_DRAIN = 8 const DRAIN_TIME_BUDGET_MS = 8 const LARGE_BACKLOG_CHARS = 512 * 1024 const SYNC_FOREGROUND_FLUSH_CHARS = 256 * 1024 @@ -739,11 +745,47 @@ function takeNextDrainableEntry(): QueueEntry | null { return null } +// Why: the parse-completion pacer re-arms a zero-delay drain as soon as xterm +// reports the previous high-priority batch parsed. Without it, cadence is a +// fixed 4/16ms nap per <=32KB batch — a ~2-8 MB/s drip against xterm's +// ~100 MB/s parse rate (measured: scheduler-throughput bench + baseline-jul02). +// Only high-priority (visible-pane) backlogs are pacer-clocked; background +// panes keep the fixed cadence that protects the focused terminal. +function makeParseClockPacer(): () => void { + return () => { + try { + if (queuedByTerminal.size > 0 && hasHighPriorityBacklog()) { + scheduleDrain(0) + } + } catch { + // Why: runs inside xterm's write-callback chain; a throw here would + // wedge the terminal (see xterm-write-callback-guard.ts). + } + } +} + +function composeParsedCallback( + onParsed: TerminalOutputParsedCallback | undefined, + pacer: (() => void) | undefined +): TerminalOutputParsedCallback | undefined { + if (!pacer) { + return onParsed + } + if (!onParsed) { + return pacer + } + return () => { + onParsed() + pacer() + } +} + function writeQueuedChunk(entry: QueueEntry): 'foreground' | 'background' | null { const queuedWrite = takeQueuedChunk(entry, BACKGROUND_CHUNK_CHARS) if (!queuedWrite) { return null } + const pacer = entry.highPriority ? makeParseClockPacer() : undefined try { entry.beforeWrite?.(queuedWrite.data) if (queuedWrite.foreground) { @@ -755,11 +797,15 @@ function writeQueuedChunk(entry: QueueEntry): 'foreground' | 'background' | null { forceViewportRefresh: queuedWrite.forceForegroundRefresh, followupViewportRefresh: queuedWrite.followupForegroundRefresh, - onParsed: queuedWrite.onParsed + onParsed: composeParsedCallback(queuedWrite.onParsed, pacer) } ) } else { - writeBackgroundTerminalChunk(entry.terminal, queuedWrite.data, queuedWrite.onParsed) + writeBackgroundTerminalChunk( + entry.terminal, + queuedWrite.data, + composeParsedCallback(queuedWrite.onParsed, pacer) + ) } } catch { // Why: pane.terminal.dispose() can race with a queued late-arriving PTY ping; diff --git a/tools/benchmarks/terminal-pipeline-bench.mjs b/tools/benchmarks/terminal-pipeline-bench.mjs index 6274befd9ae..9d013dcc3ca 100644 --- a/tools/benchmarks/terminal-pipeline-bench.mjs +++ b/tools/benchmarks/terminal-pipeline-bench.mjs @@ -47,7 +47,9 @@ const DSR_QUERY = `${ESC}[6n` // oxlint-disable-next-line no-control-regex -- the ESC byte is the payload: this parses the terminal's cursor-position reply const DSR_REPLY_RE = /\x1b\[(\d+);(\d+)R/ const CHUNK_BYTES = 64 * 1024 -const DSR_TIMEOUT_MS = 15_000 +// Overridable: a dev-mode Electron terminal can hold >15s of parse backlog at +// a fence, which is a measurement (slow), not a hang — don't die on it. +let dsrTimeoutMs = 15_000 function parseArgs(argv) { const args = { @@ -81,6 +83,9 @@ function parseArgs(argv) { case '--fixtures': args.fixtures = next().split(',') break + case '--dsr-timeout-ms': + dsrTimeoutMs = Number(next()) + break case '--skip-load': args.skipLoad = true break @@ -263,7 +268,7 @@ function dsrRoundTrip() { dsrWaiters.splice(idx, 1) } reject(new Error('DSR reply timeout')) - }, DSR_TIMEOUT_MS) + }, dsrTimeoutMs) const waiter = { resolve: (endTs) => { clearTimeout(timer)