From 888189a205864569cf625fe040aaf48bd1cee42a Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Wed, 29 Jul 2026 04:22:39 -0700 Subject: [PATCH] perf(terminal): prioritize active hidden restores --- .../hidden-output-restore-scheduler.test.ts | 78 +++++++++++++++++++ .../hidden-output-restore-scheduler.ts | 47 ++++++++++- .../terminal-pane/pty-connection.ts | 10 +++ 3 files changed, 132 insertions(+), 3 deletions(-) diff --git a/src/renderer/src/components/terminal-pane/hidden-output-restore-scheduler.test.ts b/src/renderer/src/components/terminal-pane/hidden-output-restore-scheduler.test.ts index 354fff67009..d6c70c005c5 100644 --- a/src/renderer/src/components/terminal-pane/hidden-output-restore-scheduler.test.ts +++ b/src/renderer/src/components/terminal-pane/hidden-output-restore-scheduler.test.ts @@ -57,6 +57,38 @@ describe('hidden output restore scheduler', () => { expect(activeRestore).toHaveBeenCalledTimes(1) }) + it('waits for every active restore before draining inactive panes', async () => { + let settleFirst: (() => void) | undefined + let settleSecond: (() => void) | undefined + const inactiveRestore = vi.fn() + const firstActiveRestore = vi.fn( + () => + new Promise((resolve) => { + settleFirst = resolve + }) + ) + const secondActiveRestore = vi.fn( + () => + new Promise((resolve) => { + settleSecond = resolve + }) + ) + + scheduleHiddenOutputRestore({}, inactiveRestore, 'inactive') + scheduleHiddenOutputRestore({}, firstActiveRestore, 'active') + scheduleHiddenOutputRestore({}, secondActiveRestore, 'active') + await vi.advanceTimersByTimeAsync(160) + expect(inactiveRestore).not.toHaveBeenCalled() + + settleFirst?.() + await vi.advanceTimersByTimeAsync(160) + expect(inactiveRestore).not.toHaveBeenCalled() + + settleSecond?.() + await vi.advanceTimersByTimeAsync(16) + expect(inactiveRestore).toHaveBeenCalledTimes(1) + }) + it('can cancel pending inactive restores', () => { const target = {} const requestRestore = vi.fn() @@ -67,4 +99,50 @@ describe('hidden output restore scheduler', () => { expect(requestRestore).not.toHaveBeenCalled() }) + + it('releases active priority when its target is canceled', async () => { + const activeTarget = {} + const activeRestore = vi.fn(() => new Promise(() => undefined)) + const inactiveRestore = vi.fn() + + scheduleHiddenOutputRestore(activeTarget, activeRestore, 'active') + scheduleHiddenOutputRestore({}, inactiveRestore, 'inactive') + cancelScheduledHiddenOutputRestore(activeTarget) + await vi.advanceTimersByTimeAsync(16) + + expect(inactiveRestore).toHaveBeenCalledTimes(1) + }) + + it('ignores completion from a superseded active restore', async () => { + const activeTarget = {} + let settleFirst: (() => void) | undefined + let settleSecond: (() => void) | undefined + const inactiveRestore = vi.fn() + + scheduleHiddenOutputRestore( + activeTarget, + () => + new Promise((resolve) => { + settleFirst = resolve + }), + 'active' + ) + scheduleHiddenOutputRestore( + activeTarget, + () => + new Promise((resolve) => { + settleSecond = resolve + }), + 'active' + ) + scheduleHiddenOutputRestore({}, inactiveRestore, 'inactive') + + settleFirst?.() + await vi.advanceTimersByTimeAsync(160) + expect(inactiveRestore).not.toHaveBeenCalled() + + settleSecond?.() + await vi.advanceTimersByTimeAsync(16) + expect(inactiveRestore).toHaveBeenCalledTimes(1) + }) }) diff --git a/src/renderer/src/components/terminal-pane/hidden-output-restore-scheduler.ts b/src/renderer/src/components/terminal-pane/hidden-output-restore-scheduler.ts index 909dfe29d59..28199fd6ac5 100644 --- a/src/renderer/src/components/terminal-pane/hidden-output-restore-scheduler.ts +++ b/src/renderer/src/components/terminal-pane/hidden-output-restore-scheduler.ts @@ -1,6 +1,6 @@ type HiddenOutputRestorePriority = 'active' | 'inactive' -type HiddenOutputRestoreRequest = () => void +type HiddenOutputRestoreRequest = () => void | Promise type HiddenOutputRestoreEntry = { requestRestore: HiddenOutputRestoreRequest @@ -11,6 +11,8 @@ type HiddenOutputRestoreEntry = { const INACTIVE_RESTORE_INTERVAL_MS = 16 const inactiveRestoreQueue = new Map() +// Why: overlapping deep replays can starve the active pane the user is returning to. +const activeRestoreTokensByTarget = new Map() let inactiveRestoreTimer: ReturnType | null = null function clearInactiveRestoreTimer(): void { @@ -22,14 +24,48 @@ function clearInactiveRestoreTimer(): void { } function scheduleInactiveRestoreDrain(): void { - if (inactiveRestoreTimer !== null || inactiveRestoreQueue.size === 0) { + if ( + inactiveRestoreTimer !== null || + activeRestoreTokensByTarget.size > 0 || + inactiveRestoreQueue.size === 0 + ) { return } inactiveRestoreTimer = setTimeout(drainInactiveRestoreQueue, INACTIVE_RESTORE_INTERVAL_MS) } +function finishActiveRestore(target: object, token: object): void { + if (activeRestoreTokensByTarget.get(target) !== token) { + return + } + activeRestoreTokensByTarget.delete(target) + scheduleInactiveRestoreDrain() +} + +function runActiveRestore(target: object, requestRestore: HiddenOutputRestoreRequest): void { + const token = {} + activeRestoreTokensByTarget.set(target, token) + try { + const completion = requestRestore() + if (completion) { + void completion.then( + () => finishActiveRestore(target, token), + () => finishActiveRestore(target, token) + ) + return + } + } catch (error) { + finishActiveRestore(target, token) + throw error + } + finishActiveRestore(target, token) +} + function drainInactiveRestoreQueue(): void { inactiveRestoreTimer = null + if (activeRestoreTokensByTarget.size > 0) { + return + } const next = inactiveRestoreQueue.entries().next() if (next.done) { return @@ -47,7 +83,7 @@ export function scheduleHiddenOutputRestore( ): void { if (priority === 'active') { cancelScheduledHiddenOutputRestore(target) - requestRestore() + runActiveRestore(target, requestRestore) return } inactiveRestoreQueue.set(target, { requestRestore }) @@ -56,12 +92,17 @@ export function scheduleHiddenOutputRestore( export function cancelScheduledHiddenOutputRestore(target: object): void { inactiveRestoreQueue.delete(target) + const activeToken = activeRestoreTokensByTarget.get(target) + if (activeToken) { + finishActiveRestore(target, activeToken) + } if (inactiveRestoreQueue.size === 0) { clearInactiveRestoreTimer() } } export function resetHiddenOutputRestoreSchedulerForTests(): void { + activeRestoreTokensByTarget.clear() inactiveRestoreQueue.clear() clearInactiveRestoreTimer() } diff --git a/src/renderer/src/components/terminal-pane/pty-connection.ts b/src/renderer/src/components/terminal-pane/pty-connection.ts index a17d5027789..4cc273b3903 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection.ts @@ -7048,6 +7048,16 @@ export function connectPanePty( } cancelScheduledHiddenOutputRestore(pane.terminal) hiddenOutputRestoreScheduled = false + let restoreRequested = false + scheduleHiddenOutputRestore( + pane.terminal, + () => { + restoreRequested = requestHiddenOutputRestoreIfNeeded({ bypassScheduler: true }) + return hiddenOutputRestoreInFlight ?? undefined + }, + priority + ) + return restoreRequested } clearHiddenOutputRestoreDeferredRetryTimer() hiddenOutputRestoreRetryDeferred = false