mirror of
https://github.com/stablyai/orca.git
synced 2026-10-30 16:47:34 +00:00
perf(sidebar): stop building host projections the row model throws away
getMixedHostContextLabels built a label map for every visible worktree and then returned undefined unless two distinct hosts existed — so a single-host install, the common case, paid the whole build on every buildRows rebuild. Decide first, then build only when it is mixed. getHostWorktreeCounts repeated getHostWorktreeIds' dedupe walk to compute a number that is exactly the id list's length, and both loops built the identity string twice per row. Derive the counts and hoist the identity.
This commit is contained in:
@@ -0,0 +1,62 @@
|
||||
import { describe, expect, it } from 'vitest'
|
||||
import type { ExecutionHostId } from '../../../../../../shared/execution-host'
|
||||
import type { Repo } from '../../../../../../shared/repo-types'
|
||||
import type { Worktree } from '../../../../../../shared/worktree-types'
|
||||
import { getHostWorktreeCounts, getHostWorktreeIds } from './host-labels'
|
||||
|
||||
const LOCAL = 'local' as ExecutionHostId
|
||||
|
||||
function makeRepos(hostByRepo: Record<string, ExecutionHostId | undefined>): Map<string, Repo> {
|
||||
return new Map(
|
||||
Object.entries(hostByRepo).map(([id, executionHostId]) => [
|
||||
id,
|
||||
{ id, path: `/${id}`, ...(executionHostId ? { executionHostId } : {}) } as Repo
|
||||
])
|
||||
)
|
||||
}
|
||||
|
||||
function makeWorktrees(rows: { id: string; repoId: string }[]): Worktree[] {
|
||||
return rows.map((row) => row as Worktree)
|
||||
}
|
||||
|
||||
describe('host worktree counts and ids', () => {
|
||||
it('reports a count equal to the length of each host id list', () => {
|
||||
const repoMap = makeRepos({
|
||||
'repo-local': undefined,
|
||||
'repo-remote': 'ssh:other' as ExecutionHostId
|
||||
})
|
||||
const worktrees = makeWorktrees([
|
||||
{ id: 'a', repoId: 'repo-local' },
|
||||
{ id: 'b', repoId: 'repo-local' },
|
||||
{ id: 'c', repoId: 'repo-remote' }
|
||||
])
|
||||
|
||||
const counts = getHostWorktreeCounts(worktrees, repoMap, LOCAL)
|
||||
const ids = getHostWorktreeIds(worktrees, repoMap, LOCAL)
|
||||
|
||||
expect(ids).toBeDefined()
|
||||
expect(counts).toBeDefined()
|
||||
for (const [hostId, hostIds] of ids ?? []) {
|
||||
expect(counts?.get(hostId), `count for ${hostId}`).toBe(hostIds.length)
|
||||
}
|
||||
expect([...(counts?.keys() ?? [])].sort()).toEqual([...(ids?.keys() ?? [])].sort())
|
||||
})
|
||||
|
||||
it('counts a repeated host identity once', () => {
|
||||
const repoMap = makeRepos({ 'repo-local': undefined })
|
||||
const worktrees = makeWorktrees([
|
||||
{ id: 'a', repoId: 'repo-local' },
|
||||
{ id: 'a', repoId: 'repo-local' },
|
||||
{ id: 'b', repoId: 'repo-local' }
|
||||
])
|
||||
|
||||
expect(getHostWorktreeCounts(worktrees, repoMap, LOCAL)?.get(LOCAL)).toBe(2)
|
||||
expect(getHostWorktreeIds(worktrees, repoMap, LOCAL)?.get(LOCAL)).toEqual(['a', 'b'])
|
||||
})
|
||||
|
||||
it('returns undefined for an empty lane', () => {
|
||||
const repoMap = makeRepos({})
|
||||
expect(getHostWorktreeCounts([], repoMap, LOCAL)).toBeUndefined()
|
||||
expect(getHostWorktreeIds([], repoMap, LOCAL)).toBeUndefined()
|
||||
})
|
||||
})
|
||||
@@ -153,17 +153,17 @@ export function getHostWorktreeCounts(
|
||||
if (worktrees.length === 0) {
|
||||
return undefined
|
||||
}
|
||||
const counts = new Map<ExecutionHostId, number>()
|
||||
// Derived from the id map rather than repeating its dedupe walk: every caller asks for both,
|
||||
// and a host's count is exactly the length of its id list by construction.
|
||||
// Dedup by host, not by bare id: the same id on two hosts is two workspaces
|
||||
// and has to be counted under each of them (STA-4343).
|
||||
const seenIdentities = new Set<string>()
|
||||
for (const worktree of worktrees) {
|
||||
if (seenIdentities.has(getWorktreeHostIdentity(worktree))) {
|
||||
continue
|
||||
}
|
||||
seenIdentities.add(getWorktreeHostIdentity(worktree))
|
||||
const hostId = getWorktreeExecutionHostId(worktree, repoMap.get(worktree.repoId), defaultHostId)
|
||||
counts.set(hostId, (counts.get(hostId) ?? 0) + 1)
|
||||
const idsByHost = getHostWorktreeIds(worktrees, repoMap, defaultHostId)
|
||||
if (!idsByHost) {
|
||||
return undefined
|
||||
}
|
||||
const counts = new Map<ExecutionHostId, number>()
|
||||
for (const [hostId, ids] of idsByHost) {
|
||||
counts.set(hostId, ids.length)
|
||||
}
|
||||
return counts
|
||||
}
|
||||
@@ -179,10 +179,12 @@ export function getHostWorktreeIds(
|
||||
const idsByHost = new Map<ExecutionHostId, string[]>()
|
||||
const seenIdentities = new Set<string>()
|
||||
for (const worktree of worktrees) {
|
||||
if (seenIdentities.has(getWorktreeHostIdentity(worktree))) {
|
||||
// Hoisted: the identity string was built twice per row, once to test and once to record.
|
||||
const identity = getWorktreeHostIdentity(worktree)
|
||||
if (seenIdentities.has(identity)) {
|
||||
continue
|
||||
}
|
||||
seenIdentities.add(getWorktreeHostIdentity(worktree))
|
||||
seenIdentities.add(identity)
|
||||
const hostId = getWorktreeExecutionHostId(worktree, repoMap.get(worktree.repoId), defaultHostId)
|
||||
const ids = idsByHost.get(hostId) ?? []
|
||||
ids.push(worktree.id)
|
||||
|
||||
@@ -0,0 +1,53 @@
|
||||
import { describe, expect, it } from 'vitest'
|
||||
import type { ExecutionHostId } from '../execution-host'
|
||||
import { getMixedHostContextLabels } from './host-context-labels'
|
||||
|
||||
type Item = { id: string; hostId: ExecutionHostId }
|
||||
|
||||
function argsFor(items: Item[], counters: { hostIdReads: number; identityReads: number }) {
|
||||
return {
|
||||
getHostId: (item: Item): ExecutionHostId => {
|
||||
counters.hostIdReads += 1
|
||||
return item.hostId
|
||||
},
|
||||
getIdentity: (item: Item): string => {
|
||||
counters.identityReads += 1
|
||||
return item.id
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
describe('getMixedHostContextLabels', () => {
|
||||
it('returns undefined without building any labels when every item shares one host', () => {
|
||||
const items: Item[] = Array.from({ length: 50 }, (_, index) => ({
|
||||
id: `wt-${index}`,
|
||||
hostId: 'local' as ExecutionHostId
|
||||
}))
|
||||
const counters = { hostIdReads: 0, identityReads: 0 }
|
||||
|
||||
expect(getMixedHostContextLabels(items, argsFor(items, counters))).toBeUndefined()
|
||||
// The label map was built for all 50 and thrown away before; now nothing is built.
|
||||
expect(counters.identityReads).toBe(0)
|
||||
})
|
||||
|
||||
it('labels every item once a second host appears, wherever it appears', () => {
|
||||
for (const lateIndex of [1, 25, 49]) {
|
||||
const items: Item[] = Array.from({ length: 50 }, (_, index) => ({
|
||||
id: `wt-${index}`,
|
||||
hostId: (index === lateIndex ? 'ssh:other' : 'local') as ExecutionHostId
|
||||
}))
|
||||
const counters = { hostIdReads: 0, identityReads: 0 }
|
||||
|
||||
const labels = getMixedHostContextLabels(items, argsFor(items, counters))
|
||||
|
||||
expect(labels, `second host at index ${lateIndex}`).toBeDefined()
|
||||
expect(labels?.size).toBe(50)
|
||||
expect(labels?.has(`wt-${lateIndex}`)).toBe(true)
|
||||
}
|
||||
})
|
||||
|
||||
it('returns undefined for an empty list', () => {
|
||||
const counters = { hostIdReads: 0, identityReads: 0 }
|
||||
expect(getMixedHostContextLabels([], argsFor([], counters))).toBeUndefined()
|
||||
})
|
||||
})
|
||||
@@ -83,12 +83,30 @@ export function getMixedHostContextLabels<T>(
|
||||
sources?: HostContextLabelSources
|
||||
}
|
||||
): Map<string, string> | undefined {
|
||||
const labelsByIdentity = new Map<string, string>()
|
||||
const hostIds = new Set<ExecutionHostId>()
|
||||
// Why two passes: on a single-host install the map is built for every item and then discarded.
|
||||
// Deciding first costs one getHostId per item and skips the label work entirely.
|
||||
let firstHostId: ExecutionHostId | undefined
|
||||
let isMixed = false
|
||||
for (const item of items) {
|
||||
const hostId = args.getHostId(item)
|
||||
hostIds.add(hostId)
|
||||
labelsByIdentity.set(args.getIdentity(item), getHostContextLabel(hostId, args.sources))
|
||||
if (firstHostId === undefined) {
|
||||
firstHostId = hostId
|
||||
continue
|
||||
}
|
||||
if (hostId !== firstHostId) {
|
||||
isMixed = true
|
||||
break
|
||||
}
|
||||
}
|
||||
return hostIds.size > 1 ? labelsByIdentity : undefined
|
||||
if (!isMixed) {
|
||||
return undefined
|
||||
}
|
||||
const labelsByIdentity = new Map<string, string>()
|
||||
for (const item of items) {
|
||||
labelsByIdentity.set(
|
||||
args.getIdentity(item),
|
||||
getHostContextLabel(args.getHostId(item), args.sources)
|
||||
)
|
||||
}
|
||||
return labelsByIdentity
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user