diff --git a/src/renderer/src/store/project-host-setup-selector.test.ts b/src/renderer/src/store/project-host-setup-selector.test.ts new file mode 100644 index 00000000000..610e44c1feb --- /dev/null +++ b/src/renderer/src/store/project-host-setup-selector.test.ts @@ -0,0 +1,157 @@ +import { describe, expect, it } from 'vitest' +import type { Project, ProjectHostSetup } from '../../../shared/project-types' +import type { Repo } from '../../../shared/repo-types' +import { getProjectHostSetupProjectionFromState } from './project-host-setup-selector' + +type CollectionCounters = { + map: number + flatMap: number + iterator: number + length: number +} + +function countCollectionReads(items: readonly T[]): { + value: readonly T[] + counters: CollectionCounters +} { + const target = [...items] + const counters: CollectionCounters = { map: 0, flatMap: 0, iterator: 0, length: 0 } + const value = new Proxy(target, { + get(array, property) { + if (property === 'map' || property === 'flatMap') { + counters[property] += 1 + const method = Reflect.get(array, property) as (...args: unknown[]) => unknown + return method.bind(array) + } + if (property === Symbol.iterator) { + counters.iterator += 1 + return array[Symbol.iterator].bind(array) + } + if (property === 'length') { + counters.length += 1 + } + return Reflect.get(array, property) + } + }) + return { value, counters } +} + +function makeRepo(id: string): Repo { + return { + id, + path: `/repo/${id}`, + displayName: id, + badgeColor: '#737373', + addedAt: 1, + kind: 'git' + } +} + +function makeProject(id: string, repoId: string): Project { + return { + id, + displayName: id, + badgeColor: '#737373', + sourceRepoIds: [repoId], + createdAt: 1, + updatedAt: 1 + } +} + +function makeSetup(repoId: string, projectId: string): ProjectHostSetup { + return { + id: repoId, + projectId, + hostId: 'local', + repoId, + path: `/repo/${repoId}`, + displayName: repoId, + setupState: 'ready', + setupMethod: 'legacy-repo', + createdAt: 1, + updatedAt: 1 + } +} + +describe('project host setup selector', () => { + it('does not rescan stable catalog collections across 1,000 reads', () => { + const repos = Array.from({ length: 100 }, (_, index) => makeRepo(`repo-${index}`)) + const projects = repos.map((repo) => makeProject(`repo:${repo.id}`, repo.id)) + const setups = repos.map((repo) => makeSetup(repo.id, `repo:${repo.id}`)) + const countedRepos = countCollectionReads(repos) + const countedProjects = countCollectionReads(projects) + const countedSetups = countCollectionReads(setups) + const state = { + repos: countedRepos.value, + projects: countedProjects.value, + projectHostSetups: countedSetups.value + } + + const first = getProjectHostSetupProjectionFromState(state) + expect(countedRepos.counters.map).toBeGreaterThan(0) + expect(countedRepos.counters.iterator).toBeGreaterThan(0) + expect(countedProjects.counters.flatMap).toBeGreaterThan(0) + expect(countedSetups.counters.map).toBeGreaterThan(0) + countedRepos.counters.map = 0 + countedRepos.counters.flatMap = 0 + countedRepos.counters.iterator = 0 + countedRepos.counters.length = 0 + countedProjects.counters.map = 0 + countedProjects.counters.flatMap = 0 + countedProjects.counters.iterator = 0 + countedProjects.counters.length = 0 + countedSetups.counters.map = 0 + countedSetups.counters.flatMap = 0 + countedSetups.counters.iterator = 0 + countedSetups.counters.length = 0 + + for (let read = 0; read < 1_000; read += 1) { + expect(getProjectHostSetupProjectionFromState(state)).toBe(first) + } + + expect(countedRepos.counters).toEqual({ map: 0, flatMap: 0, iterator: 0, length: 0 }) + expect(countedProjects.counters).toEqual({ map: 0, flatMap: 0, iterator: 0, length: 0 }) + expect(countedSetups.counters).toEqual({ map: 0, flatMap: 0, iterator: 0, length: 0 }) + }) + + it('invalidates the projection when hydrated setup identity changes', () => { + const repo = makeRepo('repo-1') + const repos = [repo] + const projects = [makeProject('repo:repo-1', repo.id)] + const setups = [makeSetup(repo.id, 'repo:repo-1')] + const first = getProjectHostSetupProjectionFromState({ + repos, + projects, + projectHostSetups: setups + }) + const replacementSetups = [...setups] + + const next = getProjectHostSetupProjectionFromState({ + repos, + projects, + projectHostSetups: replacementSetups + }) + + expect(next).not.toBe(first) + expect(next.projects).toBe(projects) + expect(next.setups).toBe(replacementSetups) + }) + + it('invalidates the projection when the repo identity changes', () => { + const repo = makeRepo('repo-1') + const repos = [repo] + const projects = [makeProject('repo:repo-1', repo.id)] + const setups = [makeSetup(repo.id, 'repo:repo-1')] + getProjectHostSetupProjectionFromState({ repos, projects, projectHostSetups: setups }) + + const replacementRepos = [makeRepo('repo-2')] + const next = getProjectHostSetupProjectionFromState({ + repos: replacementRepos, + projects, + projectHostSetups: setups + }) + + expect(next.projects.map((project) => project.id)).toContain('repo:repo-2') + expect(next.setups.map((setup) => setup.id)).toContain('repo-2') + }) +}) diff --git a/src/renderer/src/store/project-host-setup-selector.ts b/src/renderer/src/store/project-host-setup-selector.ts index 55ba345f5ba..f56b3a6e0b3 100644 --- a/src/renderer/src/store/project-host-setup-selector.ts +++ b/src/renderer/src/store/project-host-setup-selector.ts @@ -19,6 +19,38 @@ const normalizedProjectHostSetupProjectionCache = new WeakMap< AppState['repos'], WeakMap> >() +// Catalog writers replace these readonly arrays; their identities are the invalidation boundary. +type StateProjectHostSetupProjectionCache = { + repos: AppState['repos'] + projects: AppState['projects'] + setups: AppState['projectHostSetups'] + projection: ProjectHostSetupProjection +} + +let stateProjectHostSetupProjectionCache: StateProjectHostSetupProjectionCache | null = null + +function getCachedStateProjectHostSetupProjection( + repos: AppState['repos'], + projects: AppState['projects'], + setups: AppState['projectHostSetups'] +): ProjectHostSetupProjection | undefined { + const cached = stateProjectHostSetupProjectionCache + return cached && + cached.repos === repos && + cached.projects === projects && + cached.setups === setups + ? cached.projection + : undefined +} + +function cacheStateProjectHostSetupProjection( + repos: AppState['repos'], + projects: AppState['projects'], + setups: AppState['projectHostSetups'], + projection: ProjectHostSetupProjection +): void { + stateProjectHostSetupProjectionCache = { repos, projects, setups, projection } +} function getCachedProjectHostSetupProjection(repos: AppState['repos']): ProjectHostSetupProjection { const cachedProjection = projectHostSetupProjectionCache.get(repos) @@ -135,10 +167,21 @@ function getCachedNormalizedProjectHostSetupProjection( export function getProjectHostSetupProjectionFromState( state: Pick & Partial> ): ProjectHostSetupProjection { - if (state.projects && state.projectHostSetups) { + const projects = state.projects + const projectHostSetups = state.projectHostSetups + if (projects && projectHostSetups) { + const cachedProjection = getCachedStateProjectHostSetupProjection( + state.repos, + projects, + projectHostSetups + ) + if (cachedProjection) { + return cachedProjection + } + const repoIds = new Set(state.repos.map((repo) => repo.id)) const coveredRepoIds = new Set() - for (const setup of state.projectHostSetups) { + for (const setup of projectHostSetups) { const repoId = typeof setup.repoId === 'string' ? setup.repoId : '' if (repoIds.has(repoId)) { coveredRepoIds.add(repoId) @@ -147,36 +190,39 @@ export function getProjectHostSetupProjectionFromState( coveredRepoIds.add(setup.id) } } + let projection: ProjectHostSetupProjection if (state.repos.length > 0 && coveredRepoIds.size < repoIds.size) { - return mergeProjectHostSetupProjection( + projection = mergeProjectHostSetupProjection( state.repos, - state.projects as Project[], - state.projectHostSetups as ProjectHostSetup[] + projects as Project[], + projectHostSetups as ProjectHostSetup[] + ) + } else { + const derived = getCachedProjectHostSetupProjection(state.repos) + const normalized = normalizeHydratedProjectHostSetupProjection( + state.repos, + projects as Project[], + projectHostSetups as ProjectHostSetup[], + derived ) - } - const derived = getCachedProjectHostSetupProjection(state.repos) - const normalized = normalizeHydratedProjectHostSetupProjection( - state.repos, - state.projects as Project[], - state.projectHostSetups as ProjectHostSetup[], - derived - ) - if (normalized.changed) { // Why: this is a zustand selector compared with Object.is, so the merged // result must be reference-stable per (repos, projects, setups) input or // every render returns a fresh object and triggers a re-render storm. - return getCachedNormalizedProjectHostSetupProjection( - state.repos, - state.projects as Project[], - state.projectHostSetups as ProjectHostSetup[], - derived, - normalized - ) + projection = normalized.changed + ? getCachedNormalizedProjectHostSetupProjection( + state.repos, + projects as Project[], + projectHostSetups as ProjectHostSetup[], + derived, + normalized + ) + : getCachedProvidedProjectHostSetupProjection( + projects as Project[], + projectHostSetups as ProjectHostSetup[] + ) } - return getCachedProvidedProjectHostSetupProjection( - state.projects as Project[], - state.projectHostSetups as ProjectHostSetup[] - ) + cacheStateProjectHostSetupProjection(state.repos, projects, projectHostSetups, projection) + return projection } return getCachedProjectHostSetupProjection(state.repos) }