mirror of
https://github.com/stablyai/orca.git
synced 2026-09-22 08:02:28 +00:00
perf(ipc): index worktree owners instead of rescanning the repo list per lookup (#18416)
Two hot lookups rescanned a whole table once per repo. `getLocalRepoForRegisteredWorktree` (59 IPC call sites, including Quick Open keystrokes and every File Explorer expand) walked the entire worktree-meta table once per repo. One pass now collects the owning repo ids, built lazily so a repo whose own path matches still never touches the table. `createRepoRowExecutionHostLookup` re-filtered the repo array on every `byId` / `byHost` call. Rows are grouped into a Map once at construction, preserving repo-list order so `rows[0]` still picks the same owner.
This commit is contained in:
@@ -0,0 +1,142 @@
|
||||
import { beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import { WORKTREE_ID_SEPARATOR, type ParsedWorktreeId } from '../../shared/worktree/id'
|
||||
import type * as WorktreeIdModule from '../../shared/worktree/id'
|
||||
|
||||
const counter = vi.hoisted(() => ({ splitCalls: 0 }))
|
||||
|
||||
// Why: `splitWorktreeId` (and the `Object.keys` snapshot around it) is the per-row work the repo
|
||||
// loop used to repeat once per repo. Counting it makes the O(repos x rows) regression observable.
|
||||
vi.mock('../../shared/worktree/id', async (importOriginal) => {
|
||||
const actual = await importOriginal<typeof WorktreeIdModule>()
|
||||
return {
|
||||
...actual,
|
||||
splitWorktreeId: (worktreeId: string): ParsedWorktreeId | null => {
|
||||
counter.splitCalls += 1
|
||||
return actual.splitWorktreeId(worktreeId)
|
||||
}
|
||||
}
|
||||
})
|
||||
|
||||
const { getLocalRepoForRegisteredWorktree } = await import('./local-worktree-runtime-options')
|
||||
|
||||
type TestRepo = { id: string; path: string; connectionId?: string }
|
||||
|
||||
const makeStore = (
|
||||
repos: readonly TestRepo[],
|
||||
worktreeIds: readonly string[]
|
||||
): { store: never; metaScans: () => number } => {
|
||||
let metaScans = 0
|
||||
const meta = Object.fromEntries(worktreeIds.map((id) => [id, {}]))
|
||||
const store = {
|
||||
getRepos: () => repos,
|
||||
getAllWorktreeMeta: () => {
|
||||
metaScans += 1
|
||||
return meta
|
||||
}
|
||||
}
|
||||
return { store: store as never, metaScans: () => metaScans }
|
||||
}
|
||||
|
||||
const worktreeId = (repoId: string, path: string): string =>
|
||||
`${repoId}${WORKTREE_ID_SEPARATOR}${path}`
|
||||
|
||||
beforeEach(() => {
|
||||
counter.splitCalls = 0
|
||||
})
|
||||
|
||||
describe('getLocalRepoForRegisteredWorktree', () => {
|
||||
it('walks the worktree meta table once, not once per repo', () => {
|
||||
// Worst case: the owning repo is last, so every earlier repo used to force a full rescan.
|
||||
const repoCount = 10
|
||||
const rowCount = 200
|
||||
const repos = Array.from({ length: repoCount }, (_, i) => ({
|
||||
id: `repo-${i}`,
|
||||
path: `/repos/repo-${i}`
|
||||
}))
|
||||
const target = '/repos/repo-9/wt-last'
|
||||
const worktreeIds = Array.from({ length: rowCount }, (_, i) =>
|
||||
worktreeId(`repo-${i % repoCount}`, `/repos/wt-${i}`)
|
||||
)
|
||||
worktreeIds[rowCount - 1] = worktreeId(`repo-${repoCount - 1}`, target)
|
||||
const { store, metaScans } = makeStore(repos, worktreeIds)
|
||||
|
||||
expect(getLocalRepoForRegisteredWorktree(store, target, target)?.id).toBe('repo-9')
|
||||
expect(metaScans()).toBe(1)
|
||||
expect(counter.splitCalls).toBe(rowCount)
|
||||
})
|
||||
|
||||
it('never touches the meta table when a repo path matches directly', () => {
|
||||
const { store, metaScans } = makeStore([{ id: 'repo-a', path: '/repos/a' }], [])
|
||||
expect(getLocalRepoForRegisteredWorktree(store, '/repos/a', '/repos/a')?.id).toBe('repo-a')
|
||||
expect(metaScans()).toBe(0)
|
||||
})
|
||||
|
||||
describe('equivalence with the per-repo scan', () => {
|
||||
const repos: TestRepo[] = [
|
||||
{ id: 'first', path: '/repos/first' },
|
||||
{ id: 'middle', path: '/repos/middle' },
|
||||
{ id: 'last', path: '/repos/last' }
|
||||
]
|
||||
|
||||
it('finds a worktree owned by the first repo', () => {
|
||||
const { store } = makeStore(repos, [worktreeId('first', '/wt/one')])
|
||||
expect(getLocalRepoForRegisteredWorktree(store, '/wt/one', '/wt/one')?.id).toBe('first')
|
||||
})
|
||||
|
||||
it('finds a worktree owned by the last repo', () => {
|
||||
const { store } = makeStore(repos, [worktreeId('last', '/wt/one')])
|
||||
expect(getLocalRepoForRegisteredWorktree(store, '/wt/one', '/wt/one')?.id).toBe('last')
|
||||
})
|
||||
|
||||
it('returns undefined when no repo owns the worktree', () => {
|
||||
const { store } = makeStore(repos, [worktreeId('other', '/wt/elsewhere')])
|
||||
expect(getLocalRepoForRegisteredWorktree(store, '/wt/one', '/wt/one')).toBeUndefined()
|
||||
})
|
||||
|
||||
it('keeps getRepos precedence when two repos both own the path', () => {
|
||||
const { store } = makeStore(repos, [
|
||||
worktreeId('last', '/wt/shared'),
|
||||
worktreeId('middle', '/wt/shared')
|
||||
])
|
||||
// getRepos order decides, not the meta table's insertion order.
|
||||
expect(getLocalRepoForRegisteredWorktree(store, '/wt/shared', '/wt/shared')?.id).toBe(
|
||||
'middle'
|
||||
)
|
||||
})
|
||||
|
||||
it('excludes an SSH repo even when it owns the registered worktree', () => {
|
||||
const { store } = makeStore(
|
||||
[{ id: 'remote', path: '/repos/remote', connectionId: 'm4air' }, ...repos],
|
||||
[worktreeId('remote', '/wt/one'), worktreeId('middle', '/wt/one')]
|
||||
)
|
||||
expect(getLocalRepoForRegisteredWorktree(store, '/wt/one', '/wt/one')?.id).toBe('middle')
|
||||
|
||||
const onlyRemote = makeStore(
|
||||
[{ id: 'remote', path: '/repos/remote', connectionId: 'm4air' }],
|
||||
[worktreeId('remote', '/wt/one')]
|
||||
)
|
||||
expect(
|
||||
getLocalRepoForRegisteredWorktree(onlyRemote.store, '/wt/one', '/wt/one')
|
||||
).toBeUndefined()
|
||||
})
|
||||
|
||||
it('matches the resolved path spelling as well as the raw one', () => {
|
||||
const { store } = makeStore(repos, [worktreeId('middle', '/wt/one')])
|
||||
expect(getLocalRepoForRegisteredWorktree(store, '/wt/other', '/wt/one')?.id).toBe('middle')
|
||||
})
|
||||
|
||||
it('returns undefined for a folder workspace that is not a registered worktree', () => {
|
||||
const { store } = makeStore(repos, [worktreeId('middle', '/wt/one')])
|
||||
expect(
|
||||
getLocalRepoForRegisteredWorktree(store, '/folders/notes', '/folders/notes')
|
||||
).toBeUndefined()
|
||||
})
|
||||
|
||||
it('tolerates a store without getRepos or getAllWorktreeMeta', () => {
|
||||
expect(getLocalRepoForRegisteredWorktree({} as never, '/wt/one', '/wt/one')).toBeUndefined()
|
||||
expect(
|
||||
getLocalRepoForRegisteredWorktree({ getRepos: () => repos } as never, '/wt/one', '/wt/one')
|
||||
).toBeUndefined()
|
||||
})
|
||||
})
|
||||
})
|
||||
@@ -19,20 +19,21 @@ function getCandidateLocalWorktreePaths(
|
||||
return new Set([worktreePath, resolvedWorktreePath].map(comparableLocalPath))
|
||||
}
|
||||
|
||||
function hasRegisteredWorktreeMetaForRepo(
|
||||
/** Repos owning a registered worktree at one of `candidatePaths`, in one pass over the meta table. */
|
||||
function collectRepoIdsWithRegisteredWorktreeMeta(
|
||||
store: Store,
|
||||
repoId: string,
|
||||
candidatePaths: Set<string>
|
||||
): boolean {
|
||||
): Set<string> {
|
||||
const worktreeMeta =
|
||||
typeof store.getAllWorktreeMeta === 'function' ? store.getAllWorktreeMeta() : {}
|
||||
const repoIds = new Set<string>()
|
||||
for (const worktreeId of Object.keys(worktreeMeta)) {
|
||||
const parsed = splitWorktreeId(worktreeId)
|
||||
if (parsed?.repoId === repoId && candidatePaths.has(comparableLocalPath(parsed.worktreePath))) {
|
||||
return true
|
||||
if (parsed && candidatePaths.has(comparableLocalPath(parsed.worktreePath))) {
|
||||
repoIds.add(parsed.repoId)
|
||||
}
|
||||
}
|
||||
return false
|
||||
return repoIds
|
||||
}
|
||||
|
||||
export function getLocalRepoForRegisteredWorktree(
|
||||
@@ -45,13 +46,18 @@ export function getLocalRepoForRegisteredWorktree(
|
||||
}
|
||||
|
||||
const candidatePaths = getCandidateLocalWorktreePaths(worktreePath, resolvedWorktreePath)
|
||||
// Built at most once, and only when a repo actually needs it, so the meta table is never
|
||||
// rescanned per repo — 59 IPC call sites hit this, some per keystroke.
|
||||
let repoIdsWithMeta: Set<string> | undefined
|
||||
return store
|
||||
.getRepos()
|
||||
.find(
|
||||
(repo) =>
|
||||
!repo.connectionId &&
|
||||
(candidatePaths.has(comparableLocalPath(repo.path)) ||
|
||||
hasRegisteredWorktreeMetaForRepo(store, repo.id, candidatePaths))
|
||||
(repoIdsWithMeta ??= collectRepoIdsWithRegisteredWorktreeMeta(store, candidatePaths)).has(
|
||||
repo.id
|
||||
))
|
||||
)
|
||||
}
|
||||
|
||||
|
||||
@@ -1,7 +1,8 @@
|
||||
import { describe, expect, it } from 'vitest'
|
||||
import {
|
||||
createRepoRowExecutionHostLookup,
|
||||
resolveWorktreeExecutionHost
|
||||
resolveWorktreeExecutionHost,
|
||||
type ExecutionHostOwnerRow
|
||||
} from './worktree-execution-host-resolution'
|
||||
|
||||
// Why (#11163, #17799): main's terminal launch scope and the renderer's owner index both answer
|
||||
@@ -165,3 +166,67 @@ describe('resolveWorktreeExecutionHost', () => {
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
describe('createRepoRowExecutionHostLookup', () => {
|
||||
/** Rows whose `id` reads are counted, so a rescan of the repo list is observable. */
|
||||
const countingRepos = (
|
||||
rows: readonly ExecutionHostOwnerRow[]
|
||||
): { repos: ExecutionHostOwnerRow[]; idReads: () => number } => {
|
||||
let idReads = 0
|
||||
const repos = rows.map(({ id, ...rest }) => ({
|
||||
...rest,
|
||||
get id(): string {
|
||||
idReads += 1
|
||||
return id
|
||||
}
|
||||
}))
|
||||
return { repos, idReads: () => idReads }
|
||||
}
|
||||
|
||||
it('scans the repo list once for the factory, never again per lookup', () => {
|
||||
const { repos, idReads } = countingRepos([
|
||||
{ id: 'a' },
|
||||
{ id: 'b', connectionId: 'm4air' },
|
||||
{ id: 'c' }
|
||||
])
|
||||
const lookup = createRepoRowExecutionHostLookup(repos)
|
||||
// One grouping pass over the list — a Map get plus a set per row — and then never again.
|
||||
const afterBuild = idReads()
|
||||
expect(afterBuild).toBeLessThanOrEqual(repos.length * 2)
|
||||
|
||||
for (let i = 0; i < 50; i++) {
|
||||
lookup.byId('a')
|
||||
lookup.byId('missing')
|
||||
lookup.byHost('b', 'ssh:m4air')
|
||||
}
|
||||
expect(idReads()).toBe(afterBuild)
|
||||
})
|
||||
|
||||
it('answers missing, ambiguous and resolved exactly as a per-call scan would', () => {
|
||||
expect(createRepoRowExecutionHostLookup([]).byId('r')).toEqual({ kind: 'missing' })
|
||||
|
||||
const openclaw = { id: 'r', connectionId: 'openclaw' }
|
||||
const m4air = { id: 'r', connectionId: 'm4air' }
|
||||
expect(createRepoRowExecutionHostLookup([openclaw, m4air]).byId('r')).toEqual({
|
||||
kind: 'ambiguous'
|
||||
})
|
||||
|
||||
// Two rows agreeing on one host still resolve to the first in repo-list order.
|
||||
const first: ExecutionHostOwnerRow = { id: 'r', connectionId: 'm4air' }
|
||||
const second: ExecutionHostOwnerRow = { id: 'r', executionHostId: 'ssh:m4air' }
|
||||
expect(createRepoRowExecutionHostLookup([first, second]).byId('r')).toEqual({
|
||||
kind: 'resolved',
|
||||
owner: first
|
||||
})
|
||||
})
|
||||
|
||||
it('keeps byHost hits, misses and repo-list order', () => {
|
||||
const openclaw = { id: 'r', connectionId: 'openclaw' }
|
||||
const m4air = { id: 'r', connectionId: 'm4air' }
|
||||
const lookup = createRepoRowExecutionHostLookup([openclaw, m4air])
|
||||
expect(lookup.byHost('r', 'ssh:m4air')).toBe(m4air)
|
||||
expect(lookup.byHost('r', 'ssh:openclaw')).toBe(openclaw)
|
||||
expect(lookup.byHost('r', 'local')).toBeNull()
|
||||
expect(lookup.byHost('other', 'local')).toBeNull()
|
||||
})
|
||||
})
|
||||
|
||||
@@ -88,20 +88,37 @@ export function resolveWorktreeExecutionHost<T extends ExecutionHostOwnerRow>(
|
||||
}
|
||||
}
|
||||
|
||||
/** Array-backed lookup for callers holding the whole repo list (main's store). */
|
||||
const EMPTY_ROWS: readonly never[] = []
|
||||
|
||||
/**
|
||||
* Array-backed lookup for callers holding the whole repo list (main's store). Grouped once at
|
||||
* construction — a lookup is hit once per worktree key per target, so a per-call `filter` was an
|
||||
* O(repos) rescan each time. Rows keep repo-list order, which `byId` depends on for `rows[0]`.
|
||||
*/
|
||||
export function createRepoRowExecutionHostLookup<T extends ExecutionHostOwnerRow>(
|
||||
repos: readonly T[]
|
||||
): ExecutionHostOwnerLookup<T> {
|
||||
const rowsFor = (repoId: string): T[] => repos.filter((repo) => repo.id === repoId)
|
||||
const rowsById = new Map<string, T[]>()
|
||||
for (const repo of repos) {
|
||||
const rows = rowsById.get(repo.id)
|
||||
if (rows) {
|
||||
rows.push(repo)
|
||||
} else {
|
||||
rowsById.set(repo.id, [repo])
|
||||
}
|
||||
}
|
||||
const rowsFor = (repoId: string): readonly T[] => rowsById.get(repoId) ?? EMPTY_ROWS
|
||||
return {
|
||||
byId: (repoId) => {
|
||||
const rows = rowsFor(repoId)
|
||||
if (rows.length === 0) {
|
||||
const owner = rows[0]
|
||||
if (!owner) {
|
||||
return { kind: 'missing' }
|
||||
}
|
||||
const hostIds = new Set(rows.map((repo) => getRepoExecutionHostId(repo)))
|
||||
const owner = rows[0]
|
||||
return hostIds.size > 1 || !owner ? { kind: 'ambiguous' } : { kind: 'resolved', owner }
|
||||
const ownerHostId = getRepoExecutionHostId(owner)
|
||||
return rows.some((repo) => getRepoExecutionHostId(repo) !== ownerHostId)
|
||||
? { kind: 'ambiguous' }
|
||||
: { kind: 'resolved', owner }
|
||||
},
|
||||
byHost: (repoId, hostId) =>
|
||||
rowsFor(repoId).find((repo) => getRepoExecutionHostId(repo) === hostId) ?? null
|
||||
|
||||
Reference in New Issue
Block a user