From 501aefeff28513c8bbd7c39c58eb0caa0f429f50 Mon Sep 17 00:00:00 2001 From: Jinjing <6427696+AmethystLiang@users.noreply.github.com> Date: Wed, 15 Jul 2026 19:00:01 -0700 Subject: [PATCH] Keep parse-clocked drain going for backlog that the frame gate already r Extract the frame-gate logic into pane-terminal-output-frame-gate.ts (with tests) and stop re-arming an animation-frame wait on every passive chunk once the initial backlog has been released. The gate should only cover the first drain of an idle queue; re-gating each arrival was throttling sustained visible floods to one 128KB write per frame (~4MB/s) instead of riding the parse-clocked continuation (~30MB/s). --- .../pane-terminal-output-frame-gate.test.ts | 205 ++++++++++++++++++ .../pane-terminal-output-scheduler.test.ts | 41 +++- .../pane-terminal-output-scheduler.ts | 10 +- 3 files changed, 253 insertions(+), 3 deletions(-) create mode 100644 src/renderer/src/lib/pane-manager/pane-terminal-output-frame-gate.test.ts diff --git a/src/renderer/src/lib/pane-manager/pane-terminal-output-frame-gate.test.ts b/src/renderer/src/lib/pane-manager/pane-terminal-output-frame-gate.test.ts new file mode 100644 index 00000000000..5f8ab0fb755 --- /dev/null +++ b/src/renderer/src/lib/pane-manager/pane-terminal-output-frame-gate.test.ts @@ -0,0 +1,205 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { + createPaneTerminalOutputFrameGate, + TERMINAL_OUTPUT_FRAME_FALLBACK_MS +} from './pane-terminal-output-frame-gate' + +type ManualFrames = { + pendingCount: () => number + runNext: (timestamp?: number) => void +} + +function installManualAnimationFrames(): ManualFrames { + let nextId = 1 + const callbacks = new Map() + vi.stubGlobal( + 'requestAnimationFrame', + vi.fn((callback: FrameRequestCallback) => { + const id = nextId++ + callbacks.set(id, callback) + return id + }) + ) + vi.stubGlobal( + 'cancelAnimationFrame', + vi.fn((id: number) => { + callbacks.delete(id) + }) + ) + return { + pendingCount: () => callbacks.size, + runNext: (timestamp = 16) => { + const next = callbacks.entries().next().value as [number, FrameRequestCallback] | undefined + if (!next) { + throw new Error('No animation frame is pending') + } + callbacks.delete(next[0]) + next[1](timestamp) + } + } +} + +describe('pane terminal output frame gate', () => { + beforeEach(() => { + vi.useFakeTimers() + }) + + afterEach(() => { + vi.useRealTimers() + vi.unstubAllGlobals() + }) + + it('fires once on the animation frame and disarms the fallback', () => { + const frames = installManualAnimationFrames() + const onFrame = vi.fn() + const gate = createPaneTerminalOutputFrameGate(onFrame) + + gate.schedule() + expect(gate.isPending()).toBe(true) + frames.runNext() + + expect(onFrame).toHaveBeenCalledTimes(1) + expect(gate.isPending()).toBe(false) + vi.advanceTimersByTime(TERMINAL_OUTPUT_FRAME_FALLBACK_MS) + expect(onFrame).toHaveBeenCalledTimes(1) + }) + + it('fires the one-shot fallback when no frame runs and ignores the stale frame', () => { + installManualAnimationFrames() + const onFrame = vi.fn() + const gate = createPaneTerminalOutputFrameGate(onFrame) + + gate.schedule() + vi.advanceTimersByTime(TERMINAL_OUTPUT_FRAME_FALLBACK_MS - 1) + expect(onFrame).not.toHaveBeenCalled() + vi.advanceTimersByTime(1) + expect(onFrame).toHaveBeenCalledTimes(1) + expect(gate.isPending()).toBe(false) + }) + + it('ignores a stale frame callback that fires after the fallback released', () => { + // Why: without cancelAnimationFrame (or when Chromium delivers a frame the + // gate already cancelled), the stale callback must not double-drain. + const captured: { frameCallback: FrameRequestCallback | null } = { frameCallback: null } + vi.stubGlobal( + 'requestAnimationFrame', + vi.fn((callback: FrameRequestCallback) => { + captured.frameCallback = callback + return 7 + }) + ) + vi.stubGlobal('cancelAnimationFrame', undefined) + const onFrame = vi.fn() + const gate = createPaneTerminalOutputFrameGate(onFrame) + + gate.schedule() + vi.advanceTimersByTime(TERMINAL_OUTPUT_FRAME_FALLBACK_MS) + expect(onFrame).toHaveBeenCalledTimes(1) + + captured.frameCallback?.(16) + expect(onFrame).toHaveBeenCalledTimes(1) + }) + + it('ignores a previous generation frame after cancel and reschedule', () => { + const callbacks: FrameRequestCallback[] = [] + vi.stubGlobal( + 'requestAnimationFrame', + vi.fn((callback: FrameRequestCallback) => { + callbacks.push(callback) + return callbacks.length + }) + ) + vi.stubGlobal('cancelAnimationFrame', undefined) + const onFrame = vi.fn() + const gate = createPaneTerminalOutputFrameGate(onFrame) + + gate.schedule() + gate.cancel() + gate.schedule() + callbacks[0]?.(16) + expect(onFrame).not.toHaveBeenCalled() + + callbacks[1]?.(16) + expect(onFrame).toHaveBeenCalledTimes(1) + }) + + it('cancel disarms both the frame and the fallback', () => { + const frames = installManualAnimationFrames() + const onFrame = vi.fn() + const gate = createPaneTerminalOutputFrameGate(onFrame) + + gate.schedule() + gate.cancel() + + expect(gate.isPending()).toBe(false) + expect(frames.pendingCount()).toBe(0) + vi.advanceTimersByTime(TERMINAL_OUTPUT_FRAME_FALLBACK_MS) + expect(onFrame).not.toHaveBeenCalled() + }) + + it('schedule while pending keeps the single armed frame', () => { + const frames = installManualAnimationFrames() + const requestFrame = vi.mocked(globalThis.requestAnimationFrame) + const onFrame = vi.fn() + const gate = createPaneTerminalOutputFrameGate(onFrame) + + gate.schedule() + gate.schedule() + gate.schedule() + + expect(requestFrame).toHaveBeenCalledTimes(1) + frames.runNext() + expect(onFrame).toHaveBeenCalledTimes(1) + }) + + it('does not resurrect a synchronously invoked frame with its returned id', () => { + // Why: test and embedded runtimes can invoke the rAF callback inside the + // request call; the returned id must not re-arm an already-released gate. + vi.stubGlobal( + 'requestAnimationFrame', + vi.fn((callback: FrameRequestCallback) => { + callback(16) + return 99 + }) + ) + vi.stubGlobal('cancelAnimationFrame', vi.fn()) + const onFrame = vi.fn() + const gate = createPaneTerminalOutputFrameGate(onFrame) + + gate.schedule() + + expect(onFrame).toHaveBeenCalledTimes(1) + expect(gate.isPending()).toBe(false) + vi.advanceTimersByTime(TERMINAL_OUTPUT_FRAME_FALLBACK_MS) + expect(onFrame).toHaveBeenCalledTimes(1) + }) + + it('runs on the fallback alone when requestAnimationFrame is unavailable', () => { + vi.stubGlobal('requestAnimationFrame', undefined) + vi.stubGlobal('cancelAnimationFrame', undefined) + const onFrame = vi.fn() + const gate = createPaneTerminalOutputFrameGate(onFrame) + + gate.schedule() + expect(gate.isPending()).toBe(true) + vi.advanceTimersByTime(TERMINAL_OUTPUT_FRAME_FALLBACK_MS) + + expect(onFrame).toHaveBeenCalledTimes(1) + expect(gate.isPending()).toBe(false) + }) + + it('can be rescheduled and fired again after a release', () => { + const frames = installManualAnimationFrames() + const onFrame = vi.fn() + const gate = createPaneTerminalOutputFrameGate(onFrame) + + gate.schedule() + frames.runNext() + gate.schedule() + expect(gate.isPending()).toBe(true) + frames.runNext() + + expect(onFrame).toHaveBeenCalledTimes(2) + expect(gate.isPending()).toBe(false) + }) +}) 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 972e3b2466b..6e9bff07839 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 @@ -631,6 +631,41 @@ describe('pane terminal output scheduler', () => { ) }) + it('keeps the parse-clocked continuation when passive chunks arrive behind a released backlog', async () => { + // Why: the frame gate must cover only the initial drain. Re-gating every + // passive arrival collapsed sustained visible floods to one 128KB drain + // per frame (~4MB/s vs the ~30MB/s parse-clocked ceiling). + vi.useFakeTimers() + const frames = installManualAnimationFrames() + const { writeTerminalOutput } = await loadScheduler() + const terminal = createTerminal() + + writeTerminalOutput(terminal, 'x'.repeat(512 * 1024), { + foreground: true, + latencySensitive: false + }) + expect(frames.pendingCount()).toBe(1) + frames.runNext() + const writesAfterFrame = terminal.write.mock.calls.length + expect(writesAfterFrame).toBeGreaterThan(0) + + writeTerminalOutput(terminal, 'y'.repeat(64 * 1024), { + foreground: true, + latencySensitive: false + }) + + expect(frames.pendingCount()).toBe(0) + vi.advanceTimersByTime(8) + expect(terminal.write.mock.calls.length).toBeGreaterThan(writesAfterFrame) + + for (let tick = 0; tick < 200; tick++) { + vi.advanceTimersByTime(4) + } + const written = terminal.write.mock.calls.map(([data]) => data as string).join('') + expect(written).toBe('x'.repeat(512 * 1024) + 'y'.repeat(64 * 1024)) + expect(frames.pendingCount()).toBe(0) + }) + it('coalesces passive tiny chunks and their ACK credits into one frame write', async () => { vi.useFakeTimers() const frames = installManualAnimationFrames() @@ -1515,12 +1550,14 @@ describe('pane terminal output scheduler', () => { expect(output).not.toContain('Orca skipped') expect(output).toContain('x'.repeat(1024)) - // But the scaled cap still bounds a runaway flood. + // But the scaled cap still bounds a runaway flood. The backlog was already + // frame-released, so these arrivals ride the parse-clocked continuation. terminal.write.mockClear() for (let i = 0; i < 13; i++) { writeTerminalOutput(terminal, chunk, { foreground: true, latencySensitive: false }) } - frames.runNext() + expect(frames.pendingCount()).toBe(0) + vi.advanceTimersByTime(0) output = terminal.write.mock.calls.map(([data]) => data).join('') expect(output).toContain('Orca skipped a burst of terminal output') }) 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 7f6d005cd93..fc57a78f929 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 @@ -1222,7 +1222,15 @@ export function writeTerminalOutput( clearForegroundCoalesce(queued) clearForegroundHoldSafety(queued) if (options.latencySensitive === false) { - scheduleForegroundFrameDrain(queued) + if (queued.foregroundFrameWait) { + scheduleForegroundFrameDrain(queued) + return + } + // Why: the frame gate covers only the initial drain of an idle queue. + // A backlog the frame already released keeps the parse-clocked + // continuation; re-gating every arrival would collapse sustained + // floods to one drain per animation frame. + scheduleDrain(0) return } releaseForegroundFrameWait(queued)