mirror of
https://github.com/stablyai/orca.git
synced 2026-09-29 16:02:50 +00:00
perf(remote): read the repo catalog once per publish, not once per worktree
`remoteWorkspace:setForConnectedTargets` costs 13 ms of main-thread time per call at 0.48 calls/sec — 0.63% of wall on a real session, the second most expensive IPC handler in the main process. Almost all of it is one line. `exportRemoteWorkspaceSession` asks `isTargetWorktree(worktreeId)` once per worktree in the session, and that callback called `targetForWorktree(store, ...)`, which called `store.getRepos()` — and `getRepos()` maps `hydrateRepo` over every repo row. So publishing to one SSH target re-hydrated the whole repo catalog once per worktree, then threw a fresh `createRepoRowExecutionHostLookup` (which itself `filter`s the catalog per lookup) away each time. The lookup is now built once per handler invocation and shared across targets: the rows cannot change inside one synchronous projection, and they are the same for every target. On the session that surfaced this — 413 worktrees, 13 repos, 1 connected target — that is 413 catalog hydrations (5369 `hydrateRepo` calls) per publish reduced to 1 (13 calls). The repo normaliser reached through `hydrateRepo` was the #2 self-time function in a 30 s main-process CPU profile at 0.25%. No user-facing trade-off: identical ownership resolution, identical exported session, identical stale-revision handling.
This commit is contained in:
@@ -154,9 +154,10 @@ describe('remoteWorkspace:setForConnectedTargets', () => {
|
||||
const getRepoMock = vi.fn<Store['getRepo']>()
|
||||
const getWorkspaceSessionMock = vi.fn<Store['getWorkspaceSession']>()
|
||||
// Ownership resolution reads the catalog, not one id-keyed row, so the fake has to project one.
|
||||
const getReposMock = vi.fn(() => [getRepoMock('repo-target-1')].filter(Boolean))
|
||||
const store = {
|
||||
getRepo: getRepoMock,
|
||||
getRepos: () => [getRepoMock('repo-target-1')].filter(Boolean),
|
||||
getRepos: getReposMock,
|
||||
getWorkspaceSession: getWorkspaceSessionMock
|
||||
} as unknown as Store
|
||||
|
||||
@@ -176,6 +177,7 @@ describe('remoteWorkspace:setForConnectedTargets', () => {
|
||||
getTarget: (targetId: string) => targets.find((target) => target.id === targetId)
|
||||
})
|
||||
getRepoMock.mockReset()
|
||||
getReposMock.mockClear()
|
||||
getWorkspaceSessionMock.mockReset()
|
||||
getWorkspaceSessionMock.mockReturnValue(baseSession)
|
||||
getRepoMock.mockImplementation((repoId: string) =>
|
||||
@@ -251,6 +253,31 @@ describe('remoteWorkspace:setForConnectedTargets', () => {
|
||||
return observed as RemoteWorkspaceObservedSnapshot
|
||||
}
|
||||
|
||||
it('reads the repo catalog once per publish, not once per worktree', async () => {
|
||||
// `store.getRepos()` re-hydrates every repo row. The export asks "is this worktree mine?" once
|
||||
// per worktree, so reading the catalog inside that callback multiplied hydration by the
|
||||
// worktree count — 413 on the session that surfaced this.
|
||||
const worktrees = Object.fromEntries(
|
||||
Array.from({ length: 12 }, (_, index) => [`repo-target-1::/remote/repo-${index}`, []])
|
||||
)
|
||||
getWorkspaceSessionMock.mockReturnValue({
|
||||
...baseSession,
|
||||
tabsByWorktree: worktrees
|
||||
} as WorkspaceSessionState)
|
||||
const observed = await observeTarget('target-1')
|
||||
getReposMock.mockClear()
|
||||
|
||||
await callSetForConnectedTargets({
|
||||
hydratedTargetIds: ['target-1'],
|
||||
expectedRevisionsByTargetId: { 'target-1': observed.snapshot?.revision ?? 7 },
|
||||
expectedHostObservationTokensByTargetId: {
|
||||
'target-1': observed.hostObservationToken
|
||||
}
|
||||
})
|
||||
|
||||
expect(getReposMock).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
|
||||
it('does not write without an explicit non-empty hydrated target set', async () => {
|
||||
await expect(callSetForConnectedTargets({ session: baseSession })).resolves.toEqual([])
|
||||
await expect(
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
import { ipcMain, type BrowserWindow } from 'electron'
|
||||
import type { Store } from '../persistence'
|
||||
import type { Repo } from '../../shared/repo-types'
|
||||
import { getActiveMultiplexer, getSshConnectionStore } from './ssh'
|
||||
import { exportRemoteWorkspaceSession } from '../../shared/remote-workspace-session-projection'
|
||||
import {
|
||||
@@ -107,7 +108,7 @@ function getExpectedHostObservationTokens(
|
||||
}
|
||||
|
||||
function targetForWorktree(
|
||||
store: Store,
|
||||
repoLookup: ReturnType<typeof createRepoRowExecutionHostLookup<Repo>>,
|
||||
worktreeId: string,
|
||||
executionHostId?: string
|
||||
): string | null {
|
||||
@@ -115,21 +116,27 @@ function targetForWorktree(
|
||||
// `getRepo(id)?.connectionId`, which is host-blind — the same repo id can name rows on several
|
||||
// hosts, so a session could be published to a machine that never owned the worktree (#11163).
|
||||
// Unresolvable ownership exports to nobody rather than guessing.
|
||||
const resolution = resolveWorktreeExecutionHost(
|
||||
createRepoRowExecutionHostLookup(store.getRepos()),
|
||||
{ repoId: getRepoIdFromWorktreeId(worktreeId), hostId: executionHostId ?? null }
|
||||
)
|
||||
const resolution = resolveWorktreeExecutionHost(repoLookup, {
|
||||
repoId: getRepoIdFromWorktreeId(worktreeId),
|
||||
hostId: executionHostId ?? null
|
||||
})
|
||||
return resolution.kind === 'resolved' ? resolution.connectionId : null
|
||||
}
|
||||
|
||||
/**
|
||||
* Why the lookup is built by the caller: `isTargetWorktree` runs once per worktree in the session,
|
||||
* and `store.getRepos()` re-hydrates every repo row on each call. Reading the repo list once per
|
||||
* export — the rows cannot change inside one synchronous projection — keeps that hydration off the
|
||||
* per-worktree path.
|
||||
*/
|
||||
function exportSessionForTarget(
|
||||
store: Store,
|
||||
repoLookup: ReturnType<typeof createRepoRowExecutionHostLookup<Repo>>,
|
||||
targetId: string,
|
||||
session: WorkspaceSessionState
|
||||
): RemoteWorkspaceSession {
|
||||
return exportRemoteWorkspaceSession(session, {
|
||||
isTargetWorktree: (worktreeId, executionHostId) =>
|
||||
targetForWorktree(store, worktreeId, executionHostId) === targetId
|
||||
targetForWorktree(repoLookup, worktreeId, executionHostId) === targetId
|
||||
})
|
||||
}
|
||||
|
||||
@@ -246,11 +253,13 @@ export function registerRemoteWorkspaceHandlers(
|
||||
) ?? []
|
||||
|
||||
const workspaceSession = args.session ?? store.getWorkspaceSession()
|
||||
// One repo read for every target: the rows are the same for all of them.
|
||||
const repoLookup = createRepoRowExecutionHostLookup(store.getRepos())
|
||||
const results = await Promise.all(
|
||||
targets.map(async (target) => {
|
||||
// Why: each target has its own revision stream. Keep same-target
|
||||
// writes queued, but do not let one slow relay block others.
|
||||
const session = exportSessionForTarget(store, target.id, workspaceSession)
|
||||
const session = exportSessionForTarget(repoLookup, target.id, workspaceSession)
|
||||
const result = await queueRemoteWorkspacePatch(target.id, async () => {
|
||||
const current =
|
||||
getCachedRemoteWorkspaceSnapshot(target.id) ?? (await getRemoteSnapshot(target))
|
||||
|
||||
Reference in New Issue
Block a user