perf(memory): skip unrelated boot worktree scans (#8204)
* perf(memory): scope boot worktree scans to live sessions * fix(memory): refresh daemon hydration snapshots * fix(memory): rescan repos referenced only by the re-read snapshot Repo selection previously came solely from the first daemon listing, so a session that only became visible on the post-enumeration re-read (e.g. a briefly unreachable adapter) could never be registered for the life of the process. Loop enumeration until the latest daemon snapshot references no unresolved repos, so selection and registration always share a snapshot. Co-authored-by: Orca <help@stably.ai> --------- Co-authored-by: Orca <help@stably.ai>
This commit is contained in:
Neil
GitHub
parent
2cfb028163
commit
e825df8218
|
|
@ -32,9 +32,9 @@ vi.mock('../daemon/daemon-init', () => ({
|
|||
getDaemonProvider: () => getDaemonProviderMock()
|
||||
}))
|
||||
|
||||
// Why: the hydrator builds its worktreeId → connectionId map by calling
|
||||
// listRepoWorktrees(repo) for every repo in the store. The git I/O is
|
||||
// out of scope for this unit; mock returns whatever the test wants.
|
||||
// Why: the hydrator builds its worktreeId → connectionId map through
|
||||
// listRepoWorktrees(repo). The git I/O is out of scope for this unit; the mock
|
||||
// also lets count tests prove repos without live sessions launch no Git work.
|
||||
const listRepoWorktreesMock = vi.fn()
|
||||
vi.mock('../repo-worktrees', () => ({
|
||||
listRepoWorktrees: (repo: unknown) => listRepoWorktreesMock(repo)
|
||||
|
|
@ -79,6 +79,17 @@ function makeLocalSessions(repoId: string, worktreePath: string, count: number):
|
|||
return sessions
|
||||
}
|
||||
|
||||
function createDeferred<T>(): {
|
||||
promise: Promise<T>
|
||||
resolve: (value: T) => void
|
||||
} {
|
||||
let resolve!: (value: T) => void
|
||||
const promise = new Promise<T>((resolver) => {
|
||||
resolve = resolver
|
||||
})
|
||||
return { promise, resolve }
|
||||
}
|
||||
|
||||
// Why: the module under test memoizes `hasHydrated` at module scope so it
|
||||
// only runs the git/RPC pass once per process. The pty-registry module
|
||||
// also stashes state in a module-level Map, so we have to load BOTH
|
||||
|
|
@ -127,6 +138,7 @@ describe('hydrateLocalPtyRegistryAtBoot', () => {
|
|||
await hydrate(makeStore([{ id: 'repo-a' }]))
|
||||
|
||||
expect(provider.listSessions).toHaveBeenCalledTimes(1)
|
||||
expect(listRepoWorktreesMock).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('catches provider.listSessions rejection and does not throw', async () => {
|
||||
|
|
@ -145,6 +157,7 @@ describe('hydrateLocalPtyRegistryAtBoot', () => {
|
|||
warnSpy.mockRestore()
|
||||
|
||||
expect(listRegisteredPtys()).toHaveLength(0)
|
||||
expect(listRepoWorktreesMock).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('does not clobber a pre-existing registry pid with a null pid from listSessions', async () => {
|
||||
|
|
@ -180,6 +193,69 @@ describe('hydrateLocalPtyRegistryAtBoot', () => {
|
|||
expect(entry).toBeDefined()
|
||||
expect(entry!.pid).toBe(12345)
|
||||
expect(entry!.paneKey).toBe('tab-1:1')
|
||||
expect(listRepoWorktreesMock).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('does not clobber a pty:spawn registration that arrives during worktree enumeration', async () => {
|
||||
const { hydrate, listRegisteredPtys, registerPty } = await loadFresh()
|
||||
const ptyId = 'repo-a::/local/Triton@@deadbeef'
|
||||
const provider = makeProvider([
|
||||
{ sessionId: ptyId, pid: null, cwd: '/local/Triton' } as unknown as SessionInfo
|
||||
])
|
||||
getDaemonProviderMock.mockReturnValue(provider)
|
||||
const worktrees =
|
||||
createDeferred<
|
||||
{ path: string; head: string; branch: string; isBare: boolean; isMainWorktree: boolean }[]
|
||||
>()
|
||||
listRepoWorktreesMock.mockReturnValue(worktrees.promise)
|
||||
|
||||
const hydration = hydrate(makeStore([{ id: 'repo-a', connectionId: null }]))
|
||||
await vi.waitFor(() => expect(listRepoWorktreesMock).toHaveBeenCalledTimes(1))
|
||||
registerPty({
|
||||
ptyId,
|
||||
worktreeId: 'repo-a::/local/Triton',
|
||||
sessionId: ptyId,
|
||||
paneKey: 'tab-1:1',
|
||||
pid: 12345
|
||||
})
|
||||
worktrees.resolve([
|
||||
{ path: '/local/Triton', head: '', branch: '', isBare: false, isMainWorktree: true }
|
||||
])
|
||||
await hydration
|
||||
|
||||
expect(listRegisteredPtys()).toEqual([
|
||||
expect.objectContaining({ ptyId, pid: 12345, paneKey: 'tab-1:1' })
|
||||
])
|
||||
})
|
||||
|
||||
it('does not resurrect a daemon session that exits during worktree enumeration', async () => {
|
||||
const { hydrate, listRegisteredPtys, unregisterPty } = await loadFresh()
|
||||
const ptyId = 'repo-a::/local/Triton@@deadbeef'
|
||||
const provider = {
|
||||
listSessions: vi
|
||||
.fn()
|
||||
.mockResolvedValueOnce([
|
||||
{ sessionId: ptyId, pid: 4242, cwd: '/local/Triton' } as unknown as SessionInfo
|
||||
])
|
||||
.mockResolvedValueOnce([])
|
||||
}
|
||||
getDaemonProviderMock.mockReturnValue(provider)
|
||||
const worktrees =
|
||||
createDeferred<
|
||||
{ path: string; head: string; branch: string; isBare: boolean; isMainWorktree: boolean }[]
|
||||
>()
|
||||
listRepoWorktreesMock.mockReturnValue(worktrees.promise)
|
||||
|
||||
const hydration = hydrate(makeStore([{ id: 'repo-a', connectionId: null }]))
|
||||
await vi.waitFor(() => expect(listRepoWorktreesMock).toHaveBeenCalledTimes(1))
|
||||
unregisterPty(ptyId)
|
||||
worktrees.resolve([
|
||||
{ path: '/local/Triton', head: '', branch: '', isBare: false, isMainWorktree: true }
|
||||
])
|
||||
await hydration
|
||||
|
||||
expect(provider.listSessions).toHaveBeenCalledTimes(2)
|
||||
expect(listRegisteredPtys()).toHaveLength(0)
|
||||
})
|
||||
|
||||
it('skips SSH repos before enumerating worktrees', async () => {
|
||||
|
|
@ -223,6 +299,99 @@ describe('hydrateLocalPtyRegistryAtBoot', () => {
|
|||
expect(entry!.worktreeId).toBe('repo-a::/local/Triton')
|
||||
})
|
||||
|
||||
it('does not register a daemon session whose worktree was removed', async () => {
|
||||
const { hydrate, listRegisteredPtys } = await loadFresh()
|
||||
const provider = makeProvider([
|
||||
{
|
||||
sessionId: 'repo-a::/local/removed@@deadbeef',
|
||||
pid: 4242,
|
||||
cwd: '/local/removed'
|
||||
} as unknown as SessionInfo
|
||||
])
|
||||
getDaemonProviderMock.mockReturnValue(provider)
|
||||
listRepoWorktreesMock.mockResolvedValue([
|
||||
{ path: '/local/current', head: '', branch: '', isBare: false, isMainWorktree: true }
|
||||
])
|
||||
|
||||
await hydrate(makeStore([{ id: 'repo-a', connectionId: null }]))
|
||||
|
||||
expect(listRepoWorktreesMock).toHaveBeenCalledTimes(1)
|
||||
expect(listRegisteredPtys()).toHaveLength(0)
|
||||
})
|
||||
|
||||
it('enumerates worktrees only for repos referenced by live daemon sessions', async () => {
|
||||
const { hydrate, listRegisteredPtys } = await loadFresh()
|
||||
const repos = Array.from({ length: 100 }, (_, index) => ({ id: `repo-${index}` }))
|
||||
const activeRepoIds = ['repo-17', 'repo-83']
|
||||
const provider = makeProvider(
|
||||
activeRepoIds.map(
|
||||
(repoId, index) =>
|
||||
({
|
||||
sessionId: `${repoId}::/local/${repoId}@@0000000${index}`,
|
||||
pid: 4000 + index,
|
||||
cwd: `/local/${repoId}`
|
||||
}) as unknown as SessionInfo
|
||||
)
|
||||
)
|
||||
getDaemonProviderMock.mockReturnValue(provider)
|
||||
listRepoWorktreesMock.mockImplementation(async (repo: Repo) => [
|
||||
{
|
||||
path: `/local/${repo.id}`,
|
||||
head: '',
|
||||
branch: '',
|
||||
isBare: false,
|
||||
isMainWorktree: true
|
||||
}
|
||||
])
|
||||
|
||||
await hydrate(makeStore(repos))
|
||||
|
||||
expect(listRepoWorktreesMock).toHaveBeenCalledTimes(activeRepoIds.length)
|
||||
expect(listRepoWorktreesMock.mock.calls.map(([repo]) => (repo as Repo).id)).toEqual(
|
||||
activeRepoIds
|
||||
)
|
||||
expect(listRegisteredPtys()).toHaveLength(activeRepoIds.length)
|
||||
})
|
||||
|
||||
it('scans a repo that only becomes visible on the post-enumeration re-read', async () => {
|
||||
const { hydrate, listRegisteredPtys } = await loadFresh()
|
||||
|
||||
const sessionA = {
|
||||
sessionId: 'repo-a::/local/repo-a@@00000001',
|
||||
pid: 4001,
|
||||
cwd: '/local/repo-a'
|
||||
} as unknown as SessionInfo
|
||||
const sessionB = {
|
||||
sessionId: 'repo-b::/local/repo-b@@00000002',
|
||||
pid: 4002,
|
||||
cwd: '/local/repo-b'
|
||||
} as unknown as SessionInfo
|
||||
// Why: a briefly unreachable adapter can omit a session from the first
|
||||
// listing; once it reappears on the re-read its repo must still be
|
||||
// scanned and the session registered instead of silently dropped.
|
||||
const listSessions = vi
|
||||
.fn()
|
||||
.mockResolvedValueOnce([sessionA])
|
||||
.mockResolvedValue([sessionA, sessionB])
|
||||
getDaemonProviderMock.mockReturnValue({ listSessions })
|
||||
listRepoWorktreesMock.mockImplementation(async (repo: Repo) => [
|
||||
{ path: `/local/${repo.id}`, head: '', branch: '', isBare: false, isMainWorktree: true }
|
||||
])
|
||||
|
||||
await hydrate(makeStore([{ id: 'repo-a' }, { id: 'repo-b' }]))
|
||||
|
||||
expect(listRepoWorktreesMock.mock.calls.map(([repo]) => (repo as Repo).id)).toEqual([
|
||||
'repo-a',
|
||||
'repo-b'
|
||||
])
|
||||
expect(listSessions).toHaveBeenCalledTimes(3)
|
||||
const registered = listRegisteredPtys()
|
||||
expect(registered.map((p) => p.ptyId).sort()).toEqual([
|
||||
'repo-a::/local/repo-a@@00000001',
|
||||
'repo-b::/local/repo-b@@00000002'
|
||||
])
|
||||
})
|
||||
|
||||
it('hydrates large daemon session lists', async () => {
|
||||
const { hydrate, listRegisteredPtys } = await loadFresh()
|
||||
|
||||
|
|
|
|||
|
|
@ -25,6 +25,7 @@ import type { SessionInfo } from '../daemon/types'
|
|||
import { listRegisteredPtys, registerPty } from './pty-registry'
|
||||
import { listRepoWorktrees } from '../repo-worktrees'
|
||||
import { parsePtySessionId } from '../../shared/pty-session-id-format'
|
||||
import { splitWorktreeId } from '../../shared/worktree-id'
|
||||
import type { Store } from '../persistence'
|
||||
|
||||
// Why: `attachMainWindowServices` runs on every macOS dock re-activation
|
||||
|
|
@ -70,34 +71,62 @@ export async function hydrateLocalPtyRegistryAtBoot(store: Pick<Store, 'getRepos
|
|||
// renderer-side union still covers that case.
|
||||
hasHydrated = true
|
||||
|
||||
// Why: build a worktree-id → connectionId map so we can SSH-gate each
|
||||
// session before registering. Live git enumeration matches the path
|
||||
// shape used by `mintPtySessionId` (`${repoId}::${path}`).
|
||||
const repos = store.getRepos()
|
||||
const repoConnectionIdByWorktreeId = new Map<string, string | null>()
|
||||
// Why: ask the daemon which repos matter before launching Git worktree
|
||||
// enumeration. Most configured repos have no preserved session at boot,
|
||||
// so scanning all of them creates pure background subprocess churn.
|
||||
const reposById = new Map(store.getRepos().map((repo) => [repo.id, repo]))
|
||||
// Why: live git enumeration verifies that a referenced local worktree
|
||||
// still exists instead of resurrecting removed worktrees.
|
||||
const liveLocalWorktreeIds = new Set<string>()
|
||||
const resolvedRepoIds = new Set<string>()
|
||||
|
||||
for (const repo of repos) {
|
||||
const connectionId = repo.connectionId ?? null
|
||||
if (connectionId) {
|
||||
// Why: SSH PTYs are never registered for local process sampling, so
|
||||
// avoid startup SSH/git enumeration for repos we will skip anyway.
|
||||
continue
|
||||
let sessionInfos = await collectSessionInfos(provider)
|
||||
let alreadyRegistered = new Set(listRegisteredPtys().map((p) => p.ptyId))
|
||||
|
||||
// Why: repo selection and registration must come from the same daemon
|
||||
// snapshot. Git enumeration can take seconds, so after each scan pass we
|
||||
// re-read daemon and registry state; sessions that exited or were
|
||||
// authoritatively registered meanwhile are not resurrected or overwritten,
|
||||
// and a session that only became visible during a slow scan (e.g. a
|
||||
// briefly unreachable legacy adapter) gets its repo scanned on the next
|
||||
// pass instead of being silently dropped. Terminates because every pass
|
||||
// permanently resolves at least one new repo id.
|
||||
for (;;) {
|
||||
const newlyReferencedRepos = new Map<string, ReturnType<(typeof store)['getRepos']>[number]>()
|
||||
for (const info of sessionInfos) {
|
||||
if (alreadyRegistered.has(info.sessionId)) {
|
||||
continue
|
||||
}
|
||||
const { worktreeId } = parsePtySessionId(info.sessionId)
|
||||
const parsedWorktreeId = worktreeId ? splitWorktreeId(worktreeId) : null
|
||||
if (!parsedWorktreeId || resolvedRepoIds.has(parsedWorktreeId.repoId)) {
|
||||
continue
|
||||
}
|
||||
const repo = reposById.get(parsedWorktreeId.repoId)
|
||||
if (!repo || (repo.connectionId ?? null)) {
|
||||
// Why: unknown repos can't be proven local, and SSH PTYs are never
|
||||
// registered for local process sampling — resolve without git
|
||||
// enumeration so neither can extend the loop.
|
||||
resolvedRepoIds.add(parsedWorktreeId.repoId)
|
||||
continue
|
||||
}
|
||||
newlyReferencedRepos.set(repo.id, repo)
|
||||
}
|
||||
const worktrees = await listRepoWorktrees(repo)
|
||||
for (const wt of worktrees) {
|
||||
const worktreeId = `${repo.id}::${wt.path}`
|
||||
repoConnectionIdByWorktreeId.set(worktreeId, connectionId)
|
||||
if (newlyReferencedRepos.size === 0) {
|
||||
break
|
||||
}
|
||||
|
||||
for (const repo of newlyReferencedRepos.values()) {
|
||||
resolvedRepoIds.add(repo.id)
|
||||
const worktrees = await listRepoWorktrees(repo)
|
||||
for (const wt of worktrees) {
|
||||
liveLocalWorktreeIds.add(`${repo.id}::${wt.path}`)
|
||||
}
|
||||
}
|
||||
|
||||
sessionInfos = await collectSessionInfos(provider)
|
||||
alreadyRegistered = new Set(listRegisteredPtys().map((p) => p.ptyId))
|
||||
}
|
||||
|
||||
// Why: SessionInfo is read through the adapter's listSessions() so we
|
||||
// get the pid alongside each id. Routing through every adapter
|
||||
// (current + legacy) keeps protocol coverage symmetric with the
|
||||
// orphan-cleanup path.
|
||||
const sessionInfos = await collectSessionInfos(provider)
|
||||
|
||||
const alreadyRegistered = new Set(listRegisteredPtys().map((p) => p.ptyId))
|
||||
|
||||
for (const info of sessionInfos) {
|
||||
// Why: pid-write ordering — `pty:spawn` is the authoritative
|
||||
// writer for in-session sessions; if that fired before this loop
|
||||
|
|
@ -115,10 +144,7 @@ export async function hydrateLocalPtyRegistryAtBoot(store: Pick<Store, 'getRepos
|
|||
// `src/main/ipc/pty.ts`. If the repo isn't in the store, skip the
|
||||
// session: we can't prove it's local, and the renderer-side union
|
||||
// still surfaces the session at the cost of a missing pid sample.
|
||||
if (!repoConnectionIdByWorktreeId.has(worktreeId)) {
|
||||
continue
|
||||
}
|
||||
if (repoConnectionIdByWorktreeId.get(worktreeId)) {
|
||||
if (!liveLocalWorktreeIds.has(worktreeId)) {
|
||||
continue
|
||||
}
|
||||
registerPty({
|
||||
|
|
|
|||
Loading…
Reference in New Issue