Improve hidden terminal restore under PTY backpressure (#4833)
* Defer inactive hidden terminal restores * Reserve active PTY delivery headroom * fix: guard deferred hidden terminal restores
This commit is contained in:
parent
57c967eed3
commit
f75dfc1e14
|
|
@ -4502,6 +4502,53 @@ describe('registerPtyHandlers', () => {
|
|||
}
|
||||
})
|
||||
|
||||
it('lets active PTY output exceed its old background in-flight cap', async () => {
|
||||
vi.useFakeTimers()
|
||||
const activeProc = createMockProc()
|
||||
spawnMock.mockReturnValue(activeProc.proc)
|
||||
|
||||
try {
|
||||
registerPtyHandlers(mainWindow as never)
|
||||
const activeSpawn = (await handlers.get('pty:spawn')!(null, {
|
||||
cols: 80,
|
||||
rows: 24,
|
||||
cwd: '/tmp'
|
||||
})) as { id: string }
|
||||
const setActiveRendererPty = getPtySetActiveRendererPtyListener()
|
||||
mainWindow.webContents.send.mockClear()
|
||||
|
||||
activeProc.emitData('x'.repeat(768 * 1024))
|
||||
vi.advanceTimersByTime(8)
|
||||
for (let index = 0; index < 31; index++) {
|
||||
vi.advanceTimersByTime(1)
|
||||
}
|
||||
|
||||
expect(mainWindow.webContents.send).toHaveBeenCalledTimes(32)
|
||||
expect(getPtyRendererDeliveryDebugSnapshot()).toMatchObject({
|
||||
pendingPtyCount: 1,
|
||||
pendingChars: 256 * 1024,
|
||||
rendererInFlightChars: 512 * 1024,
|
||||
maxRendererInFlightCharsByPty: 512 * 1024
|
||||
})
|
||||
|
||||
setActiveRendererPty(null, { id: activeSpawn.id, active: true })
|
||||
vi.advanceTimersByTime(1)
|
||||
|
||||
expect(mainWindow.webContents.send).toHaveBeenCalledTimes(33)
|
||||
expect(mainWindow.webContents.send).toHaveBeenNthCalledWith(33, 'pty:data', {
|
||||
id: activeSpawn.id,
|
||||
data: 'x'.repeat(16 * 1024)
|
||||
})
|
||||
expect(getPtyRendererDeliveryDebugSnapshot()).toMatchObject({
|
||||
pendingChars: 240 * 1024,
|
||||
rendererInFlightChars: 528 * 1024,
|
||||
maxRendererInFlightCharsByPty: 528 * 1024
|
||||
})
|
||||
} finally {
|
||||
vi.useRealTimers()
|
||||
}
|
||||
})
|
||||
|
||||
it('batches stale PTY output after the interactive window expires', async () => {
|
||||
vi.useFakeTimers()
|
||||
const mockProc = createMockProc()
|
||||
|
|
|
|||
|
|
@ -1073,6 +1073,9 @@ export function registerPtyHandlers(
|
|||
const PTY_RENDERER_IN_FLIGHT_HIGH_WATER_CHARS = 512 * 1024
|
||||
const PTY_RENDERER_TOTAL_IN_FLIGHT_HIGH_WATER_CHARS = 8 * 1024 * 1024
|
||||
const PTY_RENDERER_INTERACTIVE_RESERVE_CHARS = 256 * 1024
|
||||
// Why: active panes need a bounded lane through old hidden bulk output so a
|
||||
// keystroke redraw can reach the renderer before every background ACK lands.
|
||||
const PTY_RENDERER_ACTIVE_PTY_IN_FLIGHT_RESERVE_CHARS = 512 * 1024
|
||||
// Why: keep the immediate path bounded to keystroke-sized TUI redraws;
|
||||
// large output and non-interactive output must still use the batcher.
|
||||
const INTERACTIVE_OUTPUT_WINDOW_MS = 100
|
||||
|
|
@ -1193,8 +1196,13 @@ export function registerPtyHandlers(
|
|||
const totalLimit =
|
||||
PTY_RENDERER_TOTAL_IN_FLIGHT_HIGH_WATER_CHARS +
|
||||
(options.interactive === true ? PTY_RENDERER_INTERACTIVE_RESERVE_CHARS : 0)
|
||||
// Why: the reserve is per active PTY, not global; one active pane should
|
||||
// stay responsive without letting every background pane burst past the cap.
|
||||
const ptyLimit =
|
||||
PTY_RENDERER_IN_FLIGHT_HIGH_WATER_CHARS +
|
||||
(options.interactive === true ? PTY_RENDERER_ACTIVE_PTY_IN_FLIGHT_RESERVE_CHARS : 0)
|
||||
return (
|
||||
(rendererInFlightCharsByPty.get(id) ?? 0) < PTY_RENDERER_IN_FLIGHT_HIGH_WATER_CHARS &&
|
||||
(rendererInFlightCharsByPty.get(id) ?? 0) < ptyLimit &&
|
||||
rendererInFlightTotalChars < totalLimit
|
||||
)
|
||||
}
|
||||
|
|
|
|||
|
|
@ -0,0 +1,70 @@
|
|||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
|
||||
import {
|
||||
cancelScheduledHiddenOutputRestore,
|
||||
resetHiddenOutputRestoreSchedulerForTests,
|
||||
scheduleHiddenOutputRestore
|
||||
} from './hidden-output-restore-scheduler'
|
||||
|
||||
describe('hidden output restore scheduler', () => {
|
||||
beforeEach(() => {
|
||||
vi.useFakeTimers()
|
||||
resetHiddenOutputRestoreSchedulerForTests()
|
||||
})
|
||||
|
||||
afterEach(() => {
|
||||
resetHiddenOutputRestoreSchedulerForTests()
|
||||
vi.useRealTimers()
|
||||
})
|
||||
|
||||
it('runs active restores immediately', () => {
|
||||
const target = {}
|
||||
const requestRestore = vi.fn()
|
||||
|
||||
scheduleHiddenOutputRestore(target, requestRestore, 'active')
|
||||
|
||||
expect(requestRestore).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
|
||||
it('spreads inactive restores across timer ticks', () => {
|
||||
const firstRestore = vi.fn()
|
||||
const secondRestore = vi.fn()
|
||||
|
||||
scheduleHiddenOutputRestore({}, firstRestore, 'inactive')
|
||||
scheduleHiddenOutputRestore({}, secondRestore, 'inactive')
|
||||
|
||||
expect(firstRestore).not.toHaveBeenCalled()
|
||||
expect(secondRestore).not.toHaveBeenCalled()
|
||||
|
||||
vi.advanceTimersByTime(16)
|
||||
expect(firstRestore).toHaveBeenCalledTimes(1)
|
||||
expect(secondRestore).not.toHaveBeenCalled()
|
||||
|
||||
vi.advanceTimersByTime(16)
|
||||
expect(secondRestore).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
|
||||
it('cancels pending inactive restore when a target is promoted', () => {
|
||||
const target = {}
|
||||
const inactiveRestore = vi.fn()
|
||||
const activeRestore = vi.fn()
|
||||
|
||||
scheduleHiddenOutputRestore(target, inactiveRestore, 'inactive')
|
||||
scheduleHiddenOutputRestore(target, activeRestore, 'active')
|
||||
vi.runOnlyPendingTimers()
|
||||
|
||||
expect(inactiveRestore).not.toHaveBeenCalled()
|
||||
expect(activeRestore).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
|
||||
it('can cancel pending inactive restores', () => {
|
||||
const target = {}
|
||||
const requestRestore = vi.fn()
|
||||
|
||||
scheduleHiddenOutputRestore(target, requestRestore, 'inactive')
|
||||
cancelScheduledHiddenOutputRestore(target)
|
||||
vi.runOnlyPendingTimers()
|
||||
|
||||
expect(requestRestore).not.toHaveBeenCalled()
|
||||
})
|
||||
})
|
||||
|
|
@ -0,0 +1,67 @@
|
|||
type HiddenOutputRestorePriority = 'active' | 'inactive'
|
||||
|
||||
type HiddenOutputRestoreRequest = () => void
|
||||
|
||||
type HiddenOutputRestoreEntry = {
|
||||
requestRestore: HiddenOutputRestoreRequest
|
||||
}
|
||||
|
||||
// Why: one inactive xterm scrollback replay per frame keeps tab return focused
|
||||
// on the active pane while still catching watched split panes up quickly.
|
||||
const INACTIVE_RESTORE_INTERVAL_MS = 16
|
||||
|
||||
const inactiveRestoreQueue = new Map<object, HiddenOutputRestoreEntry>()
|
||||
let inactiveRestoreTimer: ReturnType<typeof setTimeout> | null = null
|
||||
|
||||
function clearInactiveRestoreTimer(): void {
|
||||
if (inactiveRestoreTimer === null) {
|
||||
return
|
||||
}
|
||||
clearTimeout(inactiveRestoreTimer)
|
||||
inactiveRestoreTimer = null
|
||||
}
|
||||
|
||||
function scheduleInactiveRestoreDrain(): void {
|
||||
if (inactiveRestoreTimer !== null || inactiveRestoreQueue.size === 0) {
|
||||
return
|
||||
}
|
||||
inactiveRestoreTimer = setTimeout(drainInactiveRestoreQueue, INACTIVE_RESTORE_INTERVAL_MS)
|
||||
}
|
||||
|
||||
function drainInactiveRestoreQueue(): void {
|
||||
inactiveRestoreTimer = null
|
||||
const next = inactiveRestoreQueue.entries().next()
|
||||
if (next.done) {
|
||||
return
|
||||
}
|
||||
const [target, entry] = next.value
|
||||
inactiveRestoreQueue.delete(target)
|
||||
entry.requestRestore()
|
||||
scheduleInactiveRestoreDrain()
|
||||
}
|
||||
|
||||
export function scheduleHiddenOutputRestore(
|
||||
target: object,
|
||||
requestRestore: HiddenOutputRestoreRequest,
|
||||
priority: HiddenOutputRestorePriority
|
||||
): void {
|
||||
if (priority === 'active') {
|
||||
cancelScheduledHiddenOutputRestore(target)
|
||||
requestRestore()
|
||||
return
|
||||
}
|
||||
inactiveRestoreQueue.set(target, { requestRestore })
|
||||
scheduleInactiveRestoreDrain()
|
||||
}
|
||||
|
||||
export function cancelScheduledHiddenOutputRestore(target: object): void {
|
||||
inactiveRestoreQueue.delete(target)
|
||||
if (inactiveRestoreQueue.size === 0) {
|
||||
clearInactiveRestoreTimer()
|
||||
}
|
||||
}
|
||||
|
||||
export function resetHiddenOutputRestoreSchedulerForTests(): void {
|
||||
inactiveRestoreQueue.clear()
|
||||
clearInactiveRestoreTimer()
|
||||
}
|
||||
|
|
@ -3230,6 +3230,127 @@ describe('connectPanePty', () => {
|
|||
disposable.dispose()
|
||||
})
|
||||
|
||||
it('defers inactive split-pane hidden restores', async () => {
|
||||
const { resetHiddenOutputRestoreSchedulerForTests } =
|
||||
await import('./hidden-output-restore-scheduler')
|
||||
let disposable: { dispose: () => void } | null = null
|
||||
try {
|
||||
const { connectPanePty } = await import('./pty-connection')
|
||||
const transport = createMockTransport('pty-id')
|
||||
const capturedDataCallback: {
|
||||
current: ((data: string, meta?: { seq?: number; rawLength?: number }) => void) | null
|
||||
} = { current: null }
|
||||
transport.connect.mockImplementation(
|
||||
async ({ callbacks }: { callbacks: ConnectCallbacks }) => {
|
||||
capturedDataCallback.current = callbacks.onData ?? null
|
||||
return 'pty-id'
|
||||
}
|
||||
)
|
||||
transportFactoryQueue.push(transport)
|
||||
const getMainBufferSnapshot = window.api.pty.getMainBufferSnapshot as unknown as ReturnType<
|
||||
typeof vi.fn
|
||||
>
|
||||
getMainBufferSnapshot.mockResolvedValue({
|
||||
data: 'inactive snapshot\r\n',
|
||||
cols: 100,
|
||||
rows: 30,
|
||||
seq: 64
|
||||
})
|
||||
|
||||
const pane = createPane(1)
|
||||
const manager = createManager(2)
|
||||
manager.getActivePane.mockReturnValue({ id: 2 })
|
||||
const deps = createDeps({ isVisibleRef: { current: false } })
|
||||
disposable = connectPanePty(pane as never, manager as never, deps as never)
|
||||
await flushAsyncTicks(6)
|
||||
|
||||
const hidden = 'hidden inactive output\r\n'
|
||||
const live = 'visible inactive output\r\n'
|
||||
expect(capturedDataCallback.current).not.toBeNull()
|
||||
capturedDataCallback.current?.(hidden, { seq: hidden.length, rawLength: hidden.length })
|
||||
;(deps.isVisibleRef as { current: boolean }).current = true
|
||||
capturedDataCallback.current?.(live, {
|
||||
seq: hidden.length + live.length,
|
||||
rawLength: live.length
|
||||
})
|
||||
await flushAsyncTicks(4)
|
||||
|
||||
expect(getMainBufferSnapshot).not.toHaveBeenCalled()
|
||||
|
||||
// Why: inactive split restore is frame-spaced, so this waits past one
|
||||
// scheduler tick without depending on fake timers for xterm callbacks.
|
||||
await new Promise((resolve) => setTimeout(resolve, 30))
|
||||
await flushAsyncTicks(20)
|
||||
|
||||
expect(getMainBufferSnapshot).toHaveBeenCalledWith('pty-id', { scrollbackRows: 5000 })
|
||||
expect(pane.terminal.write).toHaveBeenCalledWith(
|
||||
'inactive snapshot\r\n',
|
||||
expect.any(Function)
|
||||
)
|
||||
} finally {
|
||||
disposable?.dispose()
|
||||
resetHiddenOutputRestoreSchedulerForTests()
|
||||
}
|
||||
})
|
||||
|
||||
it('drops a deferred inactive hidden restore when the pane is hidden again', async () => {
|
||||
const { resetHiddenOutputRestoreSchedulerForTests } =
|
||||
await import('./hidden-output-restore-scheduler')
|
||||
let disposable: { dispose: () => void } | null = null
|
||||
try {
|
||||
const { connectPanePty } = await import('./pty-connection')
|
||||
const transport = createMockTransport('pty-id')
|
||||
const capturedDataCallback: {
|
||||
current: ((data: string, meta?: { seq?: number; rawLength?: number }) => void) | null
|
||||
} = { current: null }
|
||||
transport.connect.mockImplementation(
|
||||
async ({ callbacks }: { callbacks: ConnectCallbacks }) => {
|
||||
capturedDataCallback.current = callbacks.onData ?? null
|
||||
return 'pty-id'
|
||||
}
|
||||
)
|
||||
transportFactoryQueue.push(transport)
|
||||
const getMainBufferSnapshot = window.api.pty.getMainBufferSnapshot as unknown as ReturnType<
|
||||
typeof vi.fn
|
||||
>
|
||||
getMainBufferSnapshot.mockResolvedValue({
|
||||
data: 'inactive snapshot\r\n',
|
||||
cols: 100,
|
||||
rows: 30,
|
||||
seq: 64
|
||||
})
|
||||
|
||||
const pane = createPane(1)
|
||||
const manager = createManager(2)
|
||||
manager.getActivePane.mockReturnValue({ id: 2 })
|
||||
const deps = createDeps({ isVisibleRef: { current: false } })
|
||||
disposable = connectPanePty(pane as never, manager as never, deps as never)
|
||||
await flushAsyncTicks(6)
|
||||
|
||||
const hidden = 'hidden inactive output\r\n'
|
||||
const live = 'visible inactive output\r\n'
|
||||
expect(capturedDataCallback.current).not.toBeNull()
|
||||
capturedDataCallback.current?.(hidden, { seq: hidden.length, rawLength: hidden.length })
|
||||
;(deps.isVisibleRef as { current: boolean }).current = true
|
||||
capturedDataCallback.current?.(live, {
|
||||
seq: hidden.length + live.length,
|
||||
rawLength: live.length
|
||||
})
|
||||
;(deps.isVisibleRef as { current: boolean }).current = false
|
||||
await new Promise((resolve) => setTimeout(resolve, 30))
|
||||
await flushAsyncTicks(20)
|
||||
|
||||
expect(getMainBufferSnapshot).not.toHaveBeenCalled()
|
||||
expect(pane.terminal.write).not.toHaveBeenCalledWith(
|
||||
'inactive snapshot\r\n',
|
||||
expect.any(Function)
|
||||
)
|
||||
} finally {
|
||||
disposable?.dispose()
|
||||
resetHiddenOutputRestoreSchedulerForTests()
|
||||
}
|
||||
})
|
||||
|
||||
it('retries hidden remote runtime restore after a null transport snapshot', async () => {
|
||||
const { connectPanePty } = await import('./pty-connection')
|
||||
const transport = createMockTransport('remote:env-1@@terminal-1')
|
||||
|
|
|
|||
|
|
@ -77,6 +77,10 @@ import { createCommandCodeOutputStatusDetector } from './command-code-output-sta
|
|||
import type { PtyDataMeta } from './pty-dispatcher'
|
||||
import { createTerminalGitHubPRLinkDetector } from '@/lib/terminal-github-pr-link-detector'
|
||||
import { installConptyDeviceAttributesHandler } from './terminal-conpty-device-attributes'
|
||||
import {
|
||||
cancelScheduledHiddenOutputRestore,
|
||||
scheduleHiddenOutputRestore
|
||||
} from './hidden-output-restore-scheduler'
|
||||
|
||||
const pendingSpawnByPaneKey = new Map<string, Promise<string | null>>()
|
||||
const SSH_SESSION_EXPIRED_ERROR = 'SSH_SESSION_EXPIRED'
|
||||
|
|
@ -1850,6 +1854,7 @@ export function connectPanePty(
|
|||
let hiddenOutputRestorePendingOverflow = false
|
||||
let hiddenOutputRestoreFreshSnapshotNeeded = false
|
||||
let hiddenOutputRestoreRetryDeferred = false
|
||||
let hiddenOutputRestoreScheduled = false
|
||||
let hiddenOutputRestoreDeferredRetryTimer: ReturnType<typeof setTimeout> | null = null
|
||||
let hiddenOutputRestoreDeferredRetryAttempts = 0
|
||||
// Why: hidden recovery state belongs to one PTY stream. Reattach/restart
|
||||
|
|
@ -2204,6 +2209,8 @@ export function connectPanePty(
|
|||
hiddenOutputRestorePendingOverflow = false
|
||||
hiddenOutputRestoreFreshSnapshotNeeded = false
|
||||
hiddenOutputRestoreRetryDeferred = false
|
||||
hiddenOutputRestoreScheduled = false
|
||||
cancelScheduledHiddenOutputRestore(pane.terminal)
|
||||
clearHiddenOutputRestoreDeferredRetryTimer()
|
||||
hiddenOutputRestoreDeferredRetryAttempts = 0
|
||||
}
|
||||
|
|
@ -2334,7 +2341,7 @@ export function connectPanePty(
|
|||
restoreScrollStateAfterSnapshotReplay(scrollState)
|
||||
}
|
||||
|
||||
function requestHiddenOutputRestoreIfNeeded(): boolean {
|
||||
function requestHiddenOutputRestoreIfNeeded(opts?: { bypassScheduler?: boolean }): boolean {
|
||||
resetHiddenOutputRestoreIfPtyChanged()
|
||||
const ptyId = hiddenOutputRestorePtyId ?? transport.getPtyId()
|
||||
if (!hiddenOutputRestoreNeeded && hiddenOutputRestorePendingChunks.length === 0) {
|
||||
|
|
@ -2347,6 +2354,41 @@ export function connectPanePty(
|
|||
if (hiddenOutputRestoreInFlight) {
|
||||
return true
|
||||
}
|
||||
if (!opts?.bypassScheduler) {
|
||||
const priority = isActiveSplitPane() ? 'active' : 'inactive'
|
||||
if (priority === 'inactive') {
|
||||
if (!hiddenOutputRestoreScheduled) {
|
||||
hiddenOutputRestoreScheduled = true
|
||||
const scheduledPtyId = ptyId
|
||||
const scheduledGeneration = hiddenOutputRestoreGeneration
|
||||
// Why: tab/worktree resume can make many split panes visible at once.
|
||||
// Restore the focused pane immediately and spread inactive replays
|
||||
// across frames so xterm scrollback replay does not block return.
|
||||
scheduleHiddenOutputRestore(
|
||||
pane.terminal,
|
||||
() => {
|
||||
hiddenOutputRestoreScheduled = false
|
||||
if (
|
||||
disposed ||
|
||||
hiddenOutputRestoreGeneration !== scheduledGeneration ||
|
||||
hiddenOutputRestorePtyId !== scheduledPtyId ||
|
||||
transport.getPtyId() !== scheduledPtyId ||
|
||||
!canUseHiddenOutputSnapshot(scheduledPtyId) ||
|
||||
(!hiddenOutputRestoreNeeded && hiddenOutputRestorePendingChunks.length === 0) ||
|
||||
!shouldWritePtyOutputForeground(deps.isVisibleRef.current)
|
||||
) {
|
||||
return
|
||||
}
|
||||
requestHiddenOutputRestoreIfNeeded({ bypassScheduler: true })
|
||||
},
|
||||
priority
|
||||
)
|
||||
}
|
||||
return true
|
||||
}
|
||||
cancelScheduledHiddenOutputRestore(pane.terminal)
|
||||
hiddenOutputRestoreScheduled = false
|
||||
}
|
||||
clearHiddenOutputRestoreDeferredRetryTimer()
|
||||
hiddenOutputRestoreRetryDeferred = false
|
||||
|
||||
|
|
|
|||
Loading…
Reference in New Issue