From 2c5cd845a2ec3512cf983f62f10bd88eea61ee25 Mon Sep 17 00:00:00 2001 From: OrcaWin Date: Tue, 8 Sep 2026 19:40:10 -0700 Subject: [PATCH] perf: append to owned project source lists without repeated copying (#19465) * perf: append to owned project source lists without repeated copying * docs(projects): record draft-ownership rule for in-place project merge --------- Co-authored-by: m4air Co-authored-by: Neil <4138956+nwparker@users.noreply.github.com> --- .../project-host-setup-projection.test.ts | 30 ++++++++++++++ src/shared/project-host-setup-projection.ts | 40 ++++++++++--------- 2 files changed, 52 insertions(+), 18 deletions(-) diff --git a/src/shared/project-host-setup-projection.test.ts b/src/shared/project-host-setup-projection.test.ts index 67fca1ad6b5..f6eeb9a0bdb 100644 --- a/src/shared/project-host-setup-projection.test.ts +++ b/src/shared/project-host-setup-projection.test.ts @@ -684,3 +684,33 @@ describe('getProjectHostSetupWorktreeMeta host selection', () => { expect(getProjectHostSetupWorktreeMeta(setups, sshRepo).hostId).toBe('ssh:build-box') }) }) + +it('appends project source IDs without repeatedly copying an accumulating array', () => { + const repos = Array.from({ length: 2000 }, (_, i) => + repo({ + id: `source-${i}`, + path: `/repos/${i}`, + displayName: `Repo ${i}`, + upstream: { owner: 'acme', repo: 'project' } + }) + ) + let copied = 0 + const iterator = Array.prototype[Symbol.iterator] + Array.prototype[Symbol.iterator] = function (this: unknown[]) { + if (this[0] === 'source-0') { + copied += this.length + } + return iterator.call(this) + } + let result: ReturnType + try { + result = projectHostSetupProjectionFromRepos([...repos, repos[0]]) + } finally { + Array.prototype[Symbol.iterator] = iterator + } + expect(copied).toBeLessThan(10_000) + expect(result.projects).toHaveLength(1) + expect(result.projects[0].sourceRepoIds).toEqual(repos.map((repo) => repo.id)) + expect(result.setups).toHaveLength(2001) + expect(repos[0].displayName).toBe('Repo 0') +}) diff --git a/src/shared/project-host-setup-projection.ts b/src/shared/project-host-setup-projection.ts index 954208ee27d..794fd625b12 100644 --- a/src/shared/project-host-setup-projection.ts +++ b/src/shared/project-host-setup-projection.ts @@ -8,8 +8,15 @@ import { githubRepoIdentityKey, isDefaultGitHubHost } from './github/repository- import type { Project, ProjectHostSetup, ProjectProviderIdentity } from './project-types' import type { Repo } from './repo-types' +/** + * A projection-local draft. `project` is always freshly built by `createProjectFromRepo`, never a + * caller's or persisted row, because `mergeProjectRepo` writes to it in place; seeding it from an + * existing `Project` would leak those writes to whoever else holds that row. `sourceRepoIds` + * mirrors `project.sourceRepoIds` so membership is a lookup rather than a rescan. + */ type ProjectAccumulator = { project: Project + sourceRepoIds: Set } export type ProjectHostSetupProjection = { @@ -262,19 +269,16 @@ function createProjectFromRepo(repo: Repo): Project { } } -function mergeProjectRepo(project: Project, repo: Repo): Project { - const sourceRepoIds = project.sourceRepoIds.includes(repo.id) - ? project.sourceRepoIds - : [...project.sourceRepoIds, repo.id] - // Why unknown-aware on both sides: the accumulator itself carries 0 when the first repo of the - // project had no addedAt, so a plain min() would let repo order decide the project's createdAt. - const addedAt = catalogTimestampFromAddedAt(repo.addedAt) - return { - ...project, - sourceRepoIds, - createdAt: mergeCatalogCreatedAt(project.createdAt, addedAt), - updatedAt: mergeCatalogUpdatedAt(project.updatedAt, addedAt) +/** Mutates the draft in place — only ever call this on an accumulator this projection owns. */ +function mergeProjectRepo(accumulator: ProjectAccumulator, repo: Repo): void { + const { project, sourceRepoIds } = accumulator + if (!sourceRepoIds.has(repo.id)) { + sourceRepoIds.add(repo.id) + project.sourceRepoIds.push(repo.id) } + const addedAt = catalogTimestampFromAddedAt(repo.addedAt) + project.createdAt = mergeCatalogCreatedAt(project.createdAt, addedAt) + project.updatedAt = mergeCatalogUpdatedAt(project.updatedAt, addedAt) } function createSetupFromRepo(repo: Repo, projectId: string): ProjectHostSetup { @@ -312,15 +316,15 @@ export function projectHostSetupProjectionFromRepos( for (const repo of repos) { const projectId = getProjectId(repo) const existing = projectById.get(projectId) - const project = existing - ? mergeProjectRepo(existing.project, repo) - : createProjectFromRepo(repo) + if (existing) { + mergeProjectRepo(existing, repo) + } else { + const project = createProjectFromRepo(repo) + projectById.set(projectId, { project, sourceRepoIds: new Set(project.sourceRepoIds) }) + } // Why normalize here: a repo row is untrusted persisted/wire data too, and these // constructors copy repo.path / repo.id straight onto fields consumers call .trim() on. const setup = normalizeProjectHostSetupRow(createSetupFromRepo(repo, projectId)) - projectById.set(projectId, { - project - }) setups.push(setup) }