From 8140c8f4647907e24b9790c307bf418d613d6fa9 Mon Sep 17 00:00:00 2001 From: Jinjing <6427696+AmethystLiang@users.noreply.github.com> Date: Mon, 7 Sep 2026 23:39:27 -0700 Subject: [PATCH] perf(persistence): record durable pty-binding flushes per pane The global write generation is held back by any unrelated dirty state, causing bindings unchanged for minutes to appear unpersisted despite being on disk. Track per-pane durability to skip redundant flushes. --- ...sistence-flush-and-save-scheduling.test.ts | 44 +++++++++++++ .../pty-binding-durability-records.test.ts | 63 +++++++++++++++++++ .../pty-binding-durability-records.ts | 57 +++++++++++++++++ .../loading-store/pty-binding-persistence.ts | 21 ++++++- .../loading-store/store-runtime-state.ts | 2 + 5 files changed, 186 insertions(+), 1 deletion(-) create mode 100644 src/main/persistence/loading-store/pty-binding-durability-records.test.ts create mode 100644 src/main/persistence/loading-store/pty-binding-durability-records.ts diff --git a/src/main/persistence-flush-and-save-scheduling.test.ts b/src/main/persistence-flush-and-save-scheduling.test.ts index d054d6180ae..9e1b092e798 100644 --- a/src/main/persistence-flush-and-save-scheduling.test.ts +++ b/src/main/persistence-flush-and-save-scheduling.test.ts @@ -582,6 +582,50 @@ describe('Store', () => { expect(flushSpy).not.toHaveBeenCalled() }) + it('stays on the fast lane while unrelated state is dirty', async () => { + const store = await createStore() + store.setWorkspaceSession(boundSession()) + expect(store.persistPtyBinding(binding)).toBe(true) + // A workspace switch dirties unrelated state constantly; the binding is still on disk. + store.addRepo(makeRepo({ id: 'r-dirty', path: '/dirty' })) + const flushSpy = vi.spyOn(store, 'flushOrThrow') + + expect(store.persistPtyBinding(binding)).toBe(true) + expect(store.persistPtyBinding(binding)).toBe(true) + + expect(flushSpy).not.toHaveBeenCalled() + }) + + it('flushes again once the session object is replaced', async () => { + const store = await createStore() + store.setWorkspaceSession(boundSession()) + store.persistPtyBinding(binding) + // A renderer publish installs a fresh session object, so the record no longer describes it. + store.setWorkspaceSession({ ...store.getWorkspaceSession() }) + store.addRepo(makeRepo({ id: 'r-dirty', path: '/dirty' })) + const flushSpy = vi.spyOn(store, 'flushOrThrow') + + expect(store.persistPtyBinding(binding)).toBe(true) + + expect(flushSpy).toHaveBeenCalledTimes(1) + }) + + it('flushes a changed pty for a pane whose old binding was durable', async () => { + const store = await createStore() + store.setWorkspaceSession(boundSession()) + store.persistPtyBinding(binding) + store.addRepo(makeRepo({ id: 'r-dirty', path: '/dirty' })) + const flushSpy = vi.spyOn(store, 'flushOrThrow') + + expect(store.persistPtyBinding({ ...binding, ptyId: 'pty-next' })).toBe(true) + + expect(flushSpy).toHaveBeenCalledTimes(1) + const persisted = readDataFile() as { workspaceSession: WorkspaceSessionState } + expect( + persisted.workspaceSession.terminalLayoutsByTabId?.tab1?.ptyIdsByLeafId?.[TEST_LEAF_1] + ).toBe('pty-next') + }) + it('lets every pane of a split tab hit the fast lane', async () => { const store = await createStore() store.setWorkspaceSession( diff --git a/src/main/persistence/loading-store/pty-binding-durability-records.test.ts b/src/main/persistence/loading-store/pty-binding-durability-records.test.ts new file mode 100644 index 00000000000..6c3e44bb697 --- /dev/null +++ b/src/main/persistence/loading-store/pty-binding-durability-records.test.ts @@ -0,0 +1,63 @@ +import { describe, expect, it } from 'vitest' +import { getDefaultWorkspaceSession } from '../../../shared/constants' +import { + isBindingDurable, + recordDurableBinding, + type DurableBindingRecords +} from './pty-binding-durability-records' + +const PANE = 'tab1:leaf1' + +function seeded(): { + records: DurableBindingRecords + session: ReturnType +} { + const records: DurableBindingRecords = new Map() + const session = getDefaultWorkspaceSession() + recordDurableBinding(records, PANE, { + session, + ptyId: 'pty-1', + incarnationId: 'inc-1', + generation: 5 + }) + return { records, session } +} + +describe('durable binding records', () => { + it('accepts the recorded binding once its generation is durable', () => { + const { records, session } = seeded() + expect(isBindingDurable(records, PANE, session, 'pty-1', 'inc-1', 5)).toBe(true) + expect(isBindingDurable(records, PANE, session, 'pty-1', 'inc-1', 9)).toBe(true) + expect(isBindingDurable(records, PANE, session, 'pty-1', 'inc-1', 4)).toBe(false) + }) + + it('retires the record when the session object is replaced', () => { + const { records } = seeded() + expect(isBindingDurable(records, PANE, getDefaultWorkspaceSession(), 'pty-1', 'inc-1', 9)).toBe( + false + ) + }) + + it('rejects a different pty, incarnation, or pane', () => { + const { records, session } = seeded() + expect(isBindingDurable(records, PANE, session, 'pty-2', 'inc-1', 9)).toBe(false) + expect(isBindingDurable(records, PANE, session, 'pty-1', 'inc-2', 9)).toBe(false) + expect(isBindingDurable(records, PANE, session, 'pty-1', undefined, 9)).toBe(false) + expect(isBindingDurable(records, 'other:pane', session, 'pty-1', 'inc-1', 9)).toBe(false) + }) + + it('bounds growth by clearing rather than tracking recency', () => { + const records: DurableBindingRecords = new Map() + const session = getDefaultWorkspaceSession() + for (let i = 0; i < 5000; i++) { + recordDurableBinding(records, `tab:${i}`, { + session, + ptyId: 'pty', + incarnationId: undefined, + generation: 1 + }) + } + expect(records.size).toBeLessThanOrEqual(4096) + expect(isBindingDurable(records, 'tab:4999', session, 'pty', undefined, 1)).toBe(true) + }) +}) diff --git a/src/main/persistence/loading-store/pty-binding-durability-records.ts b/src/main/persistence/loading-store/pty-binding-durability-records.ts new file mode 100644 index 00000000000..fc9bcc0df13 --- /dev/null +++ b/src/main/persistence/loading-store/pty-binding-durability-records.ts @@ -0,0 +1,57 @@ +import type { WorkspaceSessionState } from '../../../shared/workspace-session-state-types' + +/** + * What the last durable write of one pane's binding contained. + * + * `session` is the object identity the binding was written into. Every session-replacing writer + * (`setWorkspaceSession`, `patchWorkspaceSession`) installs a fresh object, so an identity + * mismatch retires the record without those writers knowing this map exists. The two in-place + * binding writers are covered by the value fields instead: SSH lease cleanup only clears bindings, + * and SSH target migration rewrites the PTY id, so neither can leave a stale record matching a + * request. See the writer audit in orca-persistence-design-assessment.md. + */ +type DurableBindingRecord = { + readonly session: WorkspaceSessionState + readonly ptyId: string + readonly incarnationId: string | undefined + readonly generation: number +} + +export type DurableBindingRecords = Map + +/** Pane keys accumulate across a long session; a full clear is cheaper than tracking recency. */ +const MAX_DURABLE_BINDING_RECORDS = 4096 + +export function recordDurableBinding( + records: DurableBindingRecords, + paneKey: string, + record: DurableBindingRecord +): void { + if (records.size >= MAX_DURABLE_BINDING_RECORDS && !records.has(paneKey)) { + records.clear() + } + records.set(paneKey, record) +} + +/** + * Whether this exact binding is already on disk. The global write generation cannot answer that: + * any unrelated dirty state holds it below the current generation, and a workspace switch dirties + * unrelated state constantly, so a binding untouched for minutes would still look unpersisted. + */ +export function isBindingDurable( + records: DurableBindingRecords, + paneKey: string, + session: WorkspaceSessionState, + ptyId: string, + incarnationId: string | undefined, + lastDurableWriteGeneration: number +): boolean { + const record = records.get(paneKey) + return ( + record !== undefined && + record.session === session && + record.ptyId === ptyId && + record.incarnationId === incarnationId && + record.generation <= lastDurableWriteGeneration + ) +} diff --git a/src/main/persistence/loading-store/pty-binding-persistence.ts b/src/main/persistence/loading-store/pty-binding-persistence.ts index 27d62c8602b..73aca47298c 100644 --- a/src/main/persistence/loading-store/pty-binding-persistence.ts +++ b/src/main/persistence/loading-store/pty-binding-persistence.ts @@ -20,9 +20,11 @@ import { evaluatePtyBindingFastLane } from './pty-binding-fast-lane' import { ptyBindingIsRefused } from './pty-binding-refusals' import { startPtyBindingSpan, type PtyBindingOrigin } from './pty-binding-span' import { tabRowPtyIdAfterLeafBinding } from './terminal-tab-pty-ownership' +import { isBindingDurable, recordDurableBinding } from './pty-binding-durability-records' type PtyBindingPersistenceOperationsRuntime = Pick< StoreRuntimeState, + | 'durableBindingRecords' | 'flushOrThrow' | 'lastDurableWriteGeneration' | 'pendingWrite' @@ -94,7 +96,16 @@ export class PtyBindingPersistenceOperations { args, session, bindingWorktreeId, - !runtime.quitFlushStarted && runtime.lastDurableWriteGeneration >= runtime.writeGeneration + !runtime.quitFlushStarted && + (runtime.lastDurableWriteGeneration >= runtime.writeGeneration || + isBindingDurable( + runtime.durableBindingRecords, + paneKey, + session, + args.ptyId, + args.incarnationId, + runtime.lastDurableWriteGeneration + )) ) span.setEligibility(verdict) if (verdict.eligible) { @@ -131,6 +142,14 @@ function writePtyBinding( } applyPtyBinding(args, session, bindingWorktreeId, paneKey) runtime.flushOrThrow() + // Why: the global generation is held below by any unrelated dirty state, so remember what + // this flush put on disk for this pane alone. + recordDurableBinding(runtime.durableBindingRecords, paneKey, { + session, + ptyId: args.ptyId, + incarnationId: args.incarnationId, + generation: runtime.writeGeneration + }) } catch (err) { if (resolvedHostId === LOCAL_EXECUTION_HOST_ID) { runtime.state.workspaceSession = sessionBeforeBinding diff --git a/src/main/persistence/loading-store/store-runtime-state.ts b/src/main/persistence/loading-store/store-runtime-state.ts index b14ea8ce3a4..bf9d217ff71 100644 --- a/src/main/persistence/loading-store/store-runtime-state.ts +++ b/src/main/persistence/loading-store/store-runtime-state.ts @@ -1,4 +1,5 @@ import { removeStaleDurableWriteTempFiles } from '../../durable-file-write' +import type { DurableBindingRecords } from './pty-binding-durability-records' import type { GlobalSettings } from '../../../shared/global-settings-types' import type { PersistedState } from '../../../shared/persisted-state-types' import type { ActiveViewPreference } from '../../active-view-preference' @@ -45,6 +46,7 @@ export class StoreRuntimeState { quitFlushPromise: Promise | null = null lastWrittenStateHash: string | null = null lastDurableWriteGeneration = -1 + readonly durableBindingRecords: DurableBindingRecords = new Map() firstPendingSaveAt: number | null = null githubCacheDirty = false githubCacheGeneration = 0