From 552b99298bc5a6be75824074488a1c032d9af2fa Mon Sep 17 00:00:00 2001 From: Brennan Benson Date: Thu, 9 Jul 2026 20:11:17 -0700 Subject: [PATCH] Prevent continuous git status scanning in large repositories On repos where `git status --untracked-files=all` takes tens of seconds, the background status poll restarted a fresh scan 3s after the previous one finished, keeping a git process at high CPU almost continuously while the workspace sat idle (#7983). The coalesced poll runner now paces reruns by the previous run's duration, split by trigger class: - Evidence-free timer ticks wait 5x the last refresh duration (capped at 5 minutes), bounding idle polling to ~1/6 duty cycle. - Change signals (file-watch events, repo metadata pushes, finished terminal commands, window reveal after hidden) wait only 1x, so real changes in a slow repo still surface promptly; a change signal can pull an already-scheduled tick run earlier, and the strongest pending trigger wins for trailing reruns. - Backoff-deferred scans are skipped while the window is hidden; the becoming-visible run catches up on the short lane. Fast repos keep the exact 3s cadence (the multiplier never drops the gap below the existing floor), and user-triggered refreshes are unaffected (they bypass the poll runner). The stale-conflict poll gets the same pacing, which also spaces slow remote SSH probe chains. Fixes #7983 --- .../coalesced-poll-runner.test.ts | 185 +++++++++++++++++- .../right-sidebar/coalesced-poll-runner.ts | 97 ++++++--- .../right-sidebar/useGitStatusPolling.ts | 47 ++++- .../lib/window-visibility-interval.test.ts | 45 +++++ .../src/lib/window-visibility-interval.ts | 6 +- 5 files changed, 347 insertions(+), 33 deletions(-) diff --git a/src/renderer/src/components/right-sidebar/coalesced-poll-runner.test.ts b/src/renderer/src/components/right-sidebar/coalesced-poll-runner.test.ts index 9f19c4f5813..bff4e78bbdc 100644 --- a/src/renderer/src/components/right-sidebar/coalesced-poll-runner.test.ts +++ b/src/renderer/src/components/right-sidebar/coalesced-poll-runner.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it, vi } from 'vitest' -import { createCoalescedPollRunner } from './coalesced-poll-runner' +import { type CoalescedPollRunner, createCoalescedPollRunner } from './coalesced-poll-runner' function deferred(): { promise: Promise @@ -88,6 +88,189 @@ describe('createCoalescedPollRunner', () => { vi.useRealTimers() }) + describe('slowTaskBackoff', () => { + const BACKOFF = { idleMultiplier: 5, changeSignalMultiplier: 1, maxIntervalMs: 300_000 } + + function makeSlowRunner(): { + runner: CoalescedPollRunner + task: ReturnType + calls: ReturnType[] + } { + const calls: ReturnType[] = [] + const task = vi.fn(() => { + const call = deferred() + calls.push(call) + return call.promise + }) + const runner = createCoalescedPollRunner(task, { + minIntervalMs: 3000, + slowTaskBackoff: BACKOFF + }) + return { runner, task, calls } + } + + async function completeRunOfDuration( + runner: CoalescedPollRunner, + calls: ReturnType[], + durationMs: number + ): Promise { + runner.run() + await vi.advanceTimersByTimeAsync(durationMs) + calls.at(-1)?.resolve() + await flushMicrotasks() + } + + it('scales the tick idle gap with the previous run duration', async () => { + vi.useFakeTimers() + const { runner, task, calls } = makeSlowRunner() + + // A 35s run (large-monorepo git status) with poll ticks arriving during it. + runner.run() + runner.run() + await vi.advanceTimersByTimeAsync(35_000) + calls[0]?.resolve() + await flushMicrotasks() + expect(task).toHaveBeenCalledTimes(1) + + // Ticks during the backoff window must not start a run early. + await vi.advanceTimersByTimeAsync(3000) + runner.run() + await vi.advanceTimersByTimeAsync(171_998) + runner.run() + expect(task).toHaveBeenCalledTimes(1) + + // 5x the 35s duration → next tick-driven run only after 175s idle. + await vi.advanceTimersByTimeAsync(2) + expect(task).toHaveBeenCalledTimes(2) + + calls[1]?.resolve() + await flushMicrotasks() + vi.useRealTimers() + }) + + it('caps the backoff at maxIntervalMs', async () => { + vi.useFakeTimers() + const { runner, task, calls } = makeSlowRunner() + + await completeRunOfDuration(runner, calls, 120_000) + expect(task).toHaveBeenCalledTimes(1) + + // 5 x 120s = 600s uncapped; the cap holds the gap to 300s. + runner.run() + await vi.advanceTimersByTimeAsync(299_999) + expect(task).toHaveBeenCalledTimes(1) + await vi.advanceTimersByTimeAsync(1) + expect(task).toHaveBeenCalledTimes(2) + + calls[1]?.resolve() + await flushMicrotasks() + vi.useRealTimers() + }) + + it('keeps minIntervalMs as the gap for fast tasks', async () => { + vi.useFakeTimers() + const { runner, task, calls } = makeSlowRunner() + + await completeRunOfDuration(runner, calls, 100) + expect(task).toHaveBeenCalledTimes(1) + + // max(3000, 5 x 100ms) = 3000 → normal-repo cadence is unchanged. + runner.run() + await vi.advanceTimersByTimeAsync(2999) + expect(task).toHaveBeenCalledTimes(1) + await vi.advanceTimersByTimeAsync(1) + expect(task).toHaveBeenCalledTimes(2) + + calls[1]?.resolve() + await flushMicrotasks() + vi.useRealTimers() + }) + + it('lets change signals run after the short backoff instead of the tick backoff', async () => { + vi.useFakeTimers() + const { runner, task, calls } = makeSlowRunner() + + await completeRunOfDuration(runner, calls, 35_000) + expect(task).toHaveBeenCalledTimes(1) + + // A change signal waits 1 x 35s, not 5 x 35s. + runner.run({ changeSignal: true }) + await vi.advanceTimersByTimeAsync(34_999) + expect(task).toHaveBeenCalledTimes(1) + await vi.advanceTimersByTimeAsync(1) + expect(task).toHaveBeenCalledTimes(2) + + calls[1]?.resolve() + await flushMicrotasks() + vi.useRealTimers() + }) + + it('pulls an already-scheduled tick run earlier when a change signal arrives', async () => { + vi.useFakeTimers() + const { runner, task, calls } = makeSlowRunner() + + await completeRunOfDuration(runner, calls, 35_000) + runner.run() // tick scheduled for +175s + await vi.advanceTimersByTimeAsync(10_000) + + runner.run({ changeSignal: true }) // reschedules for end + 35s + await vi.advanceTimersByTimeAsync(24_999) + expect(task).toHaveBeenCalledTimes(1) + await vi.advanceTimersByTimeAsync(1) + expect(task).toHaveBeenCalledTimes(2) + + // The superseded tick timer must not fire an extra run later. + calls[1]?.resolve() + await flushMicrotasks() + await vi.advanceTimersByTimeAsync(600_000) + expect(task).toHaveBeenCalledTimes(2) + vi.useRealTimers() + }) + + it('does not let a later tick delay a scheduled change-signal run', async () => { + vi.useFakeTimers() + const { runner, task, calls } = makeSlowRunner() + + await completeRunOfDuration(runner, calls, 35_000) + runner.run({ changeSignal: true }) // scheduled for +35s + await vi.advanceTimersByTimeAsync(10_000) + runner.run() // tick must not push the schedule to +175s + + await vi.advanceTimersByTimeAsync(24_999) + expect(task).toHaveBeenCalledTimes(1) + await vi.advanceTimersByTimeAsync(1) + expect(task).toHaveBeenCalledTimes(2) + + calls[1]?.resolve() + await flushMicrotasks() + vi.useRealTimers() + }) + + it('keeps change-signal pacing for a signal that arrived mid-run', async () => { + vi.useFakeTimers() + const { runner, task, calls } = makeSlowRunner() + + runner.run() + await vi.advanceTimersByTimeAsync(20_000) + runner.run({ changeSignal: true }) // during flight + await vi.advanceTimersByTimeAsync(15_000) + runner.run() // a later tick must not downgrade the pending signal + calls[0]?.resolve() + await flushMicrotasks() + expect(task).toHaveBeenCalledTimes(1) + + // Trailing rerun waits 1 x 35s (signal), not 5 x 35s (tick). + await vi.advanceTimersByTimeAsync(34_999) + expect(task).toHaveBeenCalledTimes(1) + await vi.advanceTimersByTimeAsync(1) + expect(task).toHaveBeenCalledTimes(2) + + calls[1]?.resolve() + await flushMicrotasks() + vi.useRealTimers() + }) + }) + it('cancels scheduled deferred run on dispose', async () => { vi.useFakeTimers() const calls: ReturnType[] = [] diff --git a/src/renderer/src/components/right-sidebar/coalesced-poll-runner.ts b/src/renderer/src/components/right-sidebar/coalesced-poll-runner.ts index b18eb634fe6..1040ed279e2 100644 --- a/src/renderer/src/components/right-sidebar/coalesced-poll-runner.ts +++ b/src/renderer/src/components/right-sidebar/coalesced-poll-runner.ts @@ -1,42 +1,95 @@ +export type CoalescedPollRunnerTrigger = { + // True when the caller has evidence something changed (file-watch event, + // repo metadata push signal, finished terminal command) rather than a bare + // timer tick. Change signals wait out a shorter slow-task backoff. + changeSignal?: boolean +} + export type CoalescedPollRunner = { - run: () => void + run: (trigger?: CoalescedPollRunnerTrigger) => void dispose: () => void } +export type SlowTaskBackoffOptions = { + // Idle gap before an evidence-free timer tick may re-run, as a multiple of + // the previous run's duration. + idleMultiplier: number + // Idle gap before a change-signal run may re-run. Kept low so slow repos + // still refresh promptly after real changes, while sustained churn can + // never keep the task running back-to-back. + changeSignalMultiplier: number + maxIntervalMs: number +} + +// Why: on large monorepos a poll task (git status) can take tens of seconds, +// so a fixed idle gap makes polling effectively continuous (#7983). Scale the +// gap with the previous run's duration — aggressively for evidence-free timer +// ticks, mildly for change signals — capped so results never go minutes-stale. export function createCoalescedPollRunner( task: () => Promise, - options?: { minIntervalMs?: number } + options?: { + minIntervalMs?: number + slowTaskBackoff?: SlowTaskBackoffOptions + } ): CoalescedPollRunner { let disposed = false let inFlight = false - let rerun = false + let rerunTrigger: CoalescedPollRunnerTrigger | null = null let lastRunEndedAt = -Infinity + let lastRunDurationMs = 0 let timeoutId: ReturnType | null = null + let timeoutFiresAt = Infinity + const minIntervalMs = options?.minIntervalMs ?? 0 - const run = (): void => { + const requiredIdleMsFor = (trigger?: CoalescedPollRunnerTrigger): number => { + const backoff = options?.slowTaskBackoff + if (!backoff) { + return minIntervalMs + } + const multiplier = trigger?.changeSignal + ? backoff.changeSignalMultiplier + : backoff.idleMultiplier + return Math.max(minIntervalMs, Math.min(lastRunDurationMs * multiplier, backoff.maxIntervalMs)) + } + + const clearScheduledRun = (): void => { + if (timeoutId !== null) { + clearTimeout(timeoutId) + timeoutId = null + timeoutFiresAt = Infinity + } + } + + const run = (trigger?: CoalescedPollRunnerTrigger): void => { if (disposed) { return } if (inFlight) { - rerun = true - return - } - if (timeoutId !== null) { + // Why: keep the strongest pending trigger so a change signal arriving + // mid-run is not downgraded to tick pacing by a later timer tick. + rerunTrigger = { changeSignal: rerunTrigger?.changeSignal || trigger?.changeSignal } return } const now = Date.now() - const timeSinceLastEnd = now - lastRunEndedAt - const minInterval = options?.minIntervalMs ?? 0 - if (timeSinceLastEnd < minInterval) { - const delay = minInterval - timeSinceLastEnd + const allowedAt = lastRunEndedAt + requiredIdleMsFor(trigger) + if (now < allowedAt) { + // Why: a change signal may pull an already-scheduled evidence-free run + // earlier; a weaker trigger must never push a scheduled run later. + if (allowedAt >= timeoutFiresAt) { + return + } + clearScheduledRun() + timeoutFiresAt = allowedAt timeoutId = setTimeout(() => { timeoutId = null - run() - }, delay) + timeoutFiresAt = Infinity + run(trigger) + }, allowedAt - now) return } + clearScheduledRun() inFlight = true void task() .catch(() => { @@ -46,10 +99,11 @@ export function createCoalescedPollRunner( .finally(() => { inFlight = false lastRunEndedAt = Date.now() - const shouldRerun = rerun && !disposed - rerun = false - if (shouldRerun) { - run() + lastRunDurationMs = lastRunEndedAt - now + const trailingTrigger = disposed ? null : rerunTrigger + rerunTrigger = null + if (trailingTrigger) { + run(trailingTrigger) } }) } @@ -58,11 +112,8 @@ export function createCoalescedPollRunner( run, dispose: () => { disposed = true - rerun = false - if (timeoutId !== null) { - clearTimeout(timeoutId) - timeoutId = null - } + rerunTrigger = null + clearScheduledRun() } } } diff --git a/src/renderer/src/components/right-sidebar/useGitStatusPolling.ts b/src/renderer/src/components/right-sidebar/useGitStatusPolling.ts index 895b69012a1..93cf5839d52 100644 --- a/src/renderer/src/components/right-sidebar/useGitStatusPolling.ts +++ b/src/renderer/src/components/right-sidebar/useGitStatusPolling.ts @@ -7,7 +7,7 @@ import { getConnectionId } from '@/lib/connection-context' import { getRuntimeGitConflictOperation } from '@/runtime/runtime-git-client' import { refreshGitStatusForWorktree } from './git-status-refresh' import { type CoalescedPollRunner, createCoalescedPollRunner } from './coalesced-poll-runner' -import { installWindowVisibilityInterval } from '@/lib/window-visibility-interval' +import { installWindowVisibilityInterval, isWindowVisible } from '@/lib/window-visibility-interval' import { hasInteractiveActiveGitStatusConsumer, shouldPollActiveGitStatus @@ -22,6 +22,16 @@ const INTERACTIVE_STATUS_POLL_INTERVAL_MS = MIN_STATUS_REFRESH_INTERVAL_MS // metadata watch, shell command completion) cover branch switches; the // terminal-only poll is a last-resort backstop for shells without either. const TERMINAL_ONLY_STATUS_POLL_INTERVAL_MS = 30_000 +// Why: on a large monorepo one status refresh can take tens of seconds, so the +// fixed 3s gap kept a git process running almost continuously while the +// workspace was idle (#7983). Evidence-free ticks wait 5x the previous poll +// duration (~1/6 duty cycle); change signals wait only 1x so real changes in a +// slow repo still surface promptly; the cap bounds worst-case staleness. +const SLOW_GIT_POLL_BACKOFF = { + idleMultiplier: 5, + changeSignalMultiplier: 1, + maxIntervalMs: 5 * 60_000 +} export function useGitStatusPolling(options: { enabled?: boolean } = {}): void { const enabled = options.enabled ?? true @@ -100,6 +110,12 @@ export function useGitStatusPolling(options: { enabled?: boolean } = {}): void { }, [allWorktrees, conflictOperationByWorktree, activeWorktreeId, repoMap]) const runFetchStatus = useCallback(async () => { + // Why: a backoff-deferred run can fire long after the window hides; skip + // the scan instead of running tens of seconds of git work nobody can see. + // The becoming-visible run catches up via the change-signal lane. + if (!isWindowVisible()) { + return + } if (!shouldPollActiveWorktreeGitStatus || !activeWorktreeId || !worktreePath) { return } @@ -144,7 +160,8 @@ export function useGitStatusPolling(options: { enabled?: boolean } = {}): void { const statusPollRunnerRef = useRef(null) useEffect(() => { const runner = createCoalescedPollRunner(() => runFetchStatusRef.current(), { - minIntervalMs: MIN_STATUS_REFRESH_INTERVAL_MS + minIntervalMs: MIN_STATUS_REFRESH_INTERVAL_MS, + slowTaskBackoff: SLOW_GIT_POLL_BACKOFF }) statusPollRunnerRef.current = runner return () => { @@ -157,24 +174,33 @@ export function useGitStatusPolling(options: { enabled?: boolean } = {}): void { statusPollRunnerRef.current?.run() }, []) + // Why: file-watch and push signals carry evidence something changed, so they + // take the runner's short-backoff lane instead of evidence-free tick pacing. + const fetchStatusOnChangeSignal = useCallback(() => { + statusPollRunnerRef.current?.run({ changeSignal: true }) + }, []) + useEffect(() => { if (!activeStatusPollScope) { return } // Why: this root-level poll should pause while hidden, but visible // unfocused windows still need fresh status for second-display workflows. + // Change signals are dropped while hidden, so the becoming-visible run + // rides the short-backoff lane to catch up on anything that was missed. return installWindowVisibilityInterval({ run: fetchStatus, + runOnVisible: fetchStatusOnChangeSignal, intervalMs: activeStatusPollIntervalMs }) - }, [activeStatusPollIntervalMs, activeStatusPollScope, fetchStatus]) + }, [activeStatusPollIntervalMs, activeStatusPollScope, fetchStatus, fetchStatusOnChangeSignal]) useGitStatusFileWatchRefresh({ activeConnectionId, activeRepoSupportsGit, activeWorktreeId, enabled, - fetchStatus, + fetchStatus: fetchStatusOnChangeSignal, gitStatusHugeByWorktree, isConnectionReady, openFiles, @@ -188,7 +214,7 @@ export function useGitStatusPolling(options: { enabled?: boolean } = {}): void { activeRepoId, activeWorktreeId, enabled: shouldPollActiveWorktreeGitStatus, - fetchStatus + fetchStatus: fetchStatusOnChangeSignal }) // Why: poll conflict operation for non-active worktrees that have a stale @@ -225,13 +251,18 @@ export function useGitStatusPolling(options: { enabled?: boolean } = {}): void { } // Why: remote conflict probes can exceed the 3s interval. Keep one poll in - // flight and coalesce skipped ticks into one trailing pass so stale badges - // catch up without stacking SSH/RPC work. - const pollRunner = createCoalescedPollRunner(pollStale) + // flight, coalesce skipped ticks into one trailing pass, and back off after + // slow probe chains so stale badges catch up without stacking SSH/RPC work. + const pollRunner = createCoalescedPollRunner(pollStale, { + slowTaskBackoff: SLOW_GIT_POLL_BACKOFF + }) // Why: conflict badges are visible sidebar state; keep them fresh in // visible unfocused windows, but do not poll disconnected hidden windows. + // The becoming-visible run rides the short-backoff lane so badges catch + // up promptly after a hidden stretch. const stopVisiblePoll = installWindowVisibilityInterval({ run: () => pollRunner.run(), + runOnVisible: () => pollRunner.run({ changeSignal: true }), intervalMs: MIN_STATUS_REFRESH_INTERVAL_MS }) return () => { diff --git a/src/renderer/src/lib/window-visibility-interval.test.ts b/src/renderer/src/lib/window-visibility-interval.test.ts index cd4a6eb8bc2..cb870794cd8 100644 --- a/src/renderer/src/lib/window-visibility-interval.test.ts +++ b/src/renderer/src/lib/window-visibility-interval.test.ts @@ -59,6 +59,51 @@ describe('installWindowVisibilityInterval', () => { ) }) + it('uses runOnVisible for the becoming-visible run and run for interval ticks', () => { + let visibilityState: DocumentVisibilityState = 'hidden' + const documentListeners = new Map void>() + const intervalCallbacks: (() => void)[] = [] + const setIntervalMock = vi.fn((callback: () => void) => { + intervalCallbacks.push(callback) + return 1 as unknown as ReturnType + }) + const run = vi.fn() + const runOnVisible = vi.fn() + + vi.stubGlobal('window', { + addEventListener: vi.fn(), + removeEventListener: vi.fn() + }) + vi.stubGlobal('document', { + get visibilityState() { + return visibilityState + }, + addEventListener: vi.fn((event: string, listener: () => void) => { + documentListeners.set(event, listener) + }), + removeEventListener: vi.fn() + }) + + const cleanup = installWindowVisibilityInterval({ + run, + runOnVisible, + intervalMs: 3000, + setIntervalFn: setIntervalMock, + clearIntervalFn: vi.fn() + }) + + visibilityState = 'visible' + documentListeners.get('visibilitychange')?.() + expect(runOnVisible).toHaveBeenCalledTimes(1) + expect(run).not.toHaveBeenCalled() + + intervalCallbacks.at(0)?.() + expect(run).toHaveBeenCalledTimes(1) + expect(runOnVisible).toHaveBeenCalledTimes(1) + + cleanup() + }) + it('starts while visible even when the window is not focused', () => { const run = vi.fn() const setIntervalMock = vi.fn(() => 1 as unknown as ReturnType) diff --git a/src/renderer/src/lib/window-visibility-interval.ts b/src/renderer/src/lib/window-visibility-interval.ts index 966fc91dc96..bdd250d5c16 100644 --- a/src/renderer/src/lib/window-visibility-interval.ts +++ b/src/renderer/src/lib/window-visibility-interval.ts @@ -10,6 +10,10 @@ export function isWindowVisible(): boolean { export function installWindowVisibilityInterval(args: { run: () => void + // Why: callers that drop refresh signals while hidden can treat the + // becoming-visible run as evidence-bearing (something may have been missed) + // instead of a bare interval tick. Defaults to `run`. + runOnVisible?: () => void intervalMs: number setIntervalFn?: (callback: () => void, intervalMs: number) => WindowVisibilityIntervalTimer clearIntervalFn?: (handle: WindowVisibilityIntervalTimer) => void @@ -33,7 +37,7 @@ export function installWindowVisibilityInterval(args: { if (intervalId || !isWindowVisible()) { return } - args.run() + ;(args.runOnVisible ?? args.run)() // Why: many callers shell out or cross IPC. Keep their interval alive only // while Orca can present the refreshed data, but still refresh a visible // unfocused window so status UI does not go stale on a second display.