Public: anyone can read this; the crew can change it.
A repository's forks page reads its lineage in one batch: `lineage` under 5 ms #6212
Check · Review · Land · Live done — Landed and live in production.
scout/forge:scout/6202-a-repository-s-forks-page-reads-its-line → main 2 commits fork 1 thread · 1 unresolved
5 files
changed +223
−22
src/core/following.ts +59 −18
| @@ -21,7 +21,13 @@ | ||
| 21 | 21 | import type { Database } from "bun:sqlite"; |
| 22 | 22 | import type { EventBus, ForgeEvent } from "./events.ts"; |
| 23 | 23 | import { getPull, listOpenPullsFromHead, type Pull } from "./pulls.ts"; |
| 24 | import { getRepoById, listForks, type Repo } from "./repos.ts"; | |
| 24 | import { | |
| 25 | getRepoById, | |
| 26 | listAncestors, | |
| 27 | listDescendants, | |
| 28 | listForks, | |
| 29 | type Repo, | |
| 30 | } from "./repos.ts"; | |
| 25 | 31 | import { |
| 26 | 32 | fetchInto, |
| 27 | 33 | merge, |
| @@ -32,7 +38,7 @@ import { | ||
| 32 | 38 | type MergeIdentity, |
| 33 | 39 | } from "../git/merge.ts"; |
| 34 | 40 | import { runGit } from "../git/env.ts"; |
| 35 | import { readTip } from "../git/tip.ts"; | |
| 41 | import { readTipOrAbsent } from "../git/tip.ts"; | |
| 36 | 42 | import { repoDir } from "../git/repos.ts"; |
| 37 | 43 | import { createLog } from "../kernel/log.ts"; |
| 38 | 44 | import { err, ok, type Result } from "../kernel/result.ts"; |
| @@ -154,6 +160,7 @@ export async function forkRelation( | ||
| 154 | 160 | forge: Pick<Following, "db" | "reposRoot">, |
| 155 | 161 | fork: Repo, |
| 156 | 162 | parent?: Repo, |
| 163 | tips: TipReads = new Map(), | |
| 157 | 164 | ): Promise<ForkRelation | undefined> { |
| 158 | 165 | const p = |
| 159 | 166 | parent ?? |
| @@ -164,8 +171,8 @@ export async function forkRelation( | ||
| 164 | 171 | const parentDir = repoDir(forge.reposRoot, p.ownerName, p.name); |
| 165 | 172 | const forkDir = repoDir(forge.reposRoot, fork.ownerName, fork.name); |
| 166 | 173 | const [parentTip, forkTip] = await Promise.all([ |
| 167 | tipOf(parentDir, `refs/heads/${p.defaultBranch}`), | |
| 168 | tipOf(forkDir, `refs/heads/${fork.defaultBranch}`), | |
| 174 | tipOf(parentDir, `refs/heads/${p.defaultBranch}`, tips), | |
| 175 | tipOf(forkDir, `refs/heads/${fork.defaultBranch}`, tips), | |
| 169 | 176 | ]); |
| 170 | 177 | const at = ( |
| 171 | 178 | state: RelationState, |
| @@ -201,9 +208,25 @@ export async function forkRelation( | ||
| 201 | 208 | return at(...standing); |
| 202 | 209 | } |
| 203 | 210 | |
| 211 | /** Tips already asked for in one read, so a parent is read once for all its forks. */ | |
| 212 | export type TipReads = Map<string, Promise<string | undefined>>; | |
| 213 | ||
| 204 | 214 | /** The tip from the ref files; git is asked only when they cannot say. */ |
| 205 | async function tipOf(dir: string, ref: string): Promise<string | undefined> { | |
| 206 | return (await readTip(dir, ref)) ?? (await resolveOid(dir, ref)); | |
| 215 | function tipOf( | |
| 216 | dir: string, | |
| 217 | ref: string, | |
| 218 | reads: TipReads, | |
| 219 | ): Promise<string | undefined> { | |
| 220 | const key = `${dir}\0${ref}`; | |
| 221 | let read = reads.get(key); | |
| 222 | if (read === undefined) { | |
| 223 | read = readTipOrAbsent(dir, ref).then((tip) => | |
| 224 | // An unborn branch is nothing to spawn git for. | |
| 225 | tip === null ? undefined : (tip ?? resolveOid(dir, ref)), | |
| 226 | ); | |
| 227 | reads.set(key, read); | |
| 228 | } | |
| 229 | return read; | |
| 207 | 230 | } |
| 208 | 231 | |
| 209 | 232 | type Standing = [ |
| @@ -519,15 +542,26 @@ export async function lineageOf( | ||
| 519 | 542 | repo: Repo, |
| 520 | 543 | mayRead: (r: Repo) => boolean, |
| 521 | 544 | ): Promise<Lineage> { |
| 522 | // Up: as far as the viewer may see, and never in circles. | |
| 545 | // Up: as far as the viewer may see, and never in circles. One read for | |
| 546 | // the whole chain; the viewer's predicate cuts it afterwards. | |
| 523 | 547 | let top = repo; |
| 524 | 548 | const seen = new Set<string>([repo.id]); |
| 525 | while (top.forkParentId !== null && seen.size <= LINEAGE_MAX_DEPTH) { | |
| 526 | const parent = getRepoById(forge.db, top.forkParentId); | |
| 527 | if (parent === undefined || !mayRead(parent) || seen.has(parent.id)) break; | |
| 549 | for (const parent of listAncestors(forge.db, repo.id, LINEAGE_MAX_DEPTH)) { | |
| 550 | if (!mayRead(parent) || seen.has(parent.id)) break; | |
| 528 | 551 | seen.add(parent.id); |
| 529 | 552 | top = parent; |
| 530 | 553 | } |
| 554 | // Down: one read for every fork below, one generation past the bound so | |
| 555 | // "more below" can be told, grouped by parent here. | |
| 556 | const below = new Map<string, Repo[]>(); | |
| 557 | for (const fork of listDescendants(forge.db, top.id, LINEAGE_MAX_DEPTH + 1)) { | |
| 558 | const parentId = fork.forkParentId; | |
| 559 | if (parentId === null) continue; | |
| 560 | const siblings = below.get(parentId); | |
| 561 | if (siblings === undefined) below.set(parentId, [fork]); | |
| 562 | else siblings.push(fork); | |
| 563 | } | |
| 564 | const forksOf = (id: string): readonly Repo[] => below.get(id) ?? []; | |
| 531 | 565 | // Down: breadth first, so a bound cuts the deepest and newest, not a |
| 532 | 566 | // whole branch of the family. |
| 533 | 567 | let size = 1; |
| @@ -540,31 +574,38 @@ export async function lineageOf( | ||
| 540 | 574 | } |
| 541 | 575 | const root: Draft = { repo: top, relation: undefined, children: [] }; |
| 542 | 576 | let level: Draft[] = [root]; |
| 577 | const tips: TipReads = new Map(); | |
| 543 | 578 | for (let depth = 0; depth < LINEAGE_MAX_DEPTH && level.length > 0; depth++) { |
| 544 | 579 | const next: Draft[] = []; |
| 580 | const asked: Promise<void>[] = []; | |
| 545 | 581 | for (const node of level) { |
| 546 | for (const fork of listForks(forge.db, node.repo.id)) { | |
| 582 | for (const fork of forksOf(node.repo.id)) { | |
| 547 | 583 | if (!mayRead(fork)) continue; |
| 548 | 584 | if (size >= LINEAGE_MAX_NODES) { |
| 549 | 585 | truncated = true; |
| 550 | 586 | break; |
| 551 | 587 | } |
| 552 | 588 | size++; |
| 553 | const relation = | |
| 554 | relations < LINEAGE_MAX_RELATIONS | |
| 555 | ? await forkRelation(forge, fork, node.repo) | |
| 556 | : undefined; | |
| 557 | if (relation !== undefined) relations++; | |
| 558 | const child: Draft = { repo: fork, relation, children: [] }; | |
| 589 | const child: Draft = { repo: fork, relation: undefined, children: [] }; | |
| 590 | // The budget is spent in order; the reads themselves run together. | |
| 591 | if (relations < LINEAGE_MAX_RELATIONS) { | |
| 592 | relations++; | |
| 593 | asked.push( | |
| 594 | forkRelation(forge, fork, node.repo, tips).then((r) => { | |
| 595 | child.relation = r; | |
| 596 | }), | |
| 597 | ); | |
| 598 | } | |
| 559 | 599 | node.children.push(child); |
| 560 | 600 | next.push(child); |
| 561 | 601 | } |
| 562 | 602 | } |
| 603 | await Promise.all(asked); | |
| 563 | 604 | level = next; |
| 564 | 605 | } |
| 565 | 606 | // "More below" only for forks this viewer could see: a private fork past |
| 566 | 607 | // the bound is not hinted at, any more than one inside it is shown. |
| 567 | if (level.some((n) => listForks(forge.db, n.repo.id).some(mayRead))) | |
| 608 | if (level.some((n) => forksOf(n.repo.id).some(mayRead))) | |
| 568 | 609 | truncated = true; |
| 569 | 610 | return { root, size, truncated }; |
| 570 | 611 | } |
src/core/repos.ts +39
| @@ -550,6 +550,45 @@ export function listAllForks(db: Database): Repo[] { | ||
| 550 | 550 | .map(toRepo); |
| 551 | 551 | } |
| 552 | 552 | |
| 553 | /** What a repository forks, nearest first, to `maxDepth` generations: one query. */ | |
| 554 | export function listAncestors( | |
| 555 | db: Database, | |
| 556 | repoId: string, | |
| 557 | maxDepth: number, | |
| 558 | ): Repo[] { | |
| 559 | return db | |
| 560 | .query<RepoRow, [string, number]>( | |
| 561 | `WITH RECURSIVE up(id, d) AS ( | |
| 562 | SELECT fork_parent_id, 1 FROM repo | |
| 563 | WHERE id = ?1 AND fork_parent_id IS NOT NULL | |
| 564 | UNION ALL | |
| 565 | SELECT r.fork_parent_id, up.d + 1 FROM repo r JOIN up ON r.id = up.id | |
| 566 | WHERE r.fork_parent_id IS NOT NULL AND up.d < ?2) | |
| 567 | ${SELECT} JOIN up ON up.id = repo.id ORDER BY up.d`, | |
| 568 | ) | |
| 569 | .all(repoId, maxDepth) | |
| 570 | .map(toRepo); | |
| 571 | } | |
| 572 | ||
| 573 | /** Every fork below a repository, to `maxDepth` generations: one query, by id. */ | |
| 574 | export function listDescendants( | |
| 575 | db: Database, | |
| 576 | repoId: string, | |
| 577 | maxDepth: number, | |
| 578 | ): Repo[] { | |
| 579 | return db | |
| 580 | .query<RepoRow, [string, number]>( | |
| 581 | `WITH RECURSIVE down(id, d) AS ( | |
| 582 | SELECT id, 1 FROM repo WHERE fork_parent_id = ?1 | |
| 583 | UNION ALL | |
| 584 | SELECT r.id, down.d + 1 FROM repo r JOIN down ON r.fork_parent_id = down.id | |
| 585 | WHERE down.d < ?2) | |
| 586 | ${SELECT} WHERE repo.id IN (SELECT id FROM down) ORDER BY repo.id`, | |
| 587 | ) | |
| 588 | .all(repoId, maxDepth) | |
| 589 | .map(toRepo); | |
| 590 | } | |
| 591 | ||
| 553 | 592 | export function listForks(db: Database, repoId: string): Repo[] { |
| 554 | 593 | return db |
| 555 | 594 | .query< |
src/git/tip.ts +17 −3
| @@ -20,14 +20,28 @@ export async function readTip( | ||
| 20 | 20 | dir: string, |
| 21 | 21 | ref: string, |
| 22 | 22 | ): Promise<string | undefined> { |
| 23 | const tip = await readTipOrAbsent(dir, ref); | |
| 24 | return tip === null ? undefined : tip; | |
| 25 | } | |
| 26 | ||
| 27 | /** | |
| 28 | * Like `readTip`, but `null` says the files agree the branch does not | |
| 29 | * exist (no loose ref, no line in packed-refs), where `undefined` says they | |
| 30 | * cannot tell. Only the second is worth spawning git for. | |
| 31 | */ | |
| 32 | export async function readTipOrAbsent( | |
| 33 | dir: string, | |
| 34 | ref: string, | |
| 35 | ): Promise<string | null | undefined> { | |
| 23 | 36 | if (!isSafeRef(ref) || !ref.startsWith("refs/heads/")) return undefined; |
| 24 | 37 | const loose = (await text(join(dir, ref)))?.trim(); |
| 25 | 38 | if (loose !== undefined) return isSafeOid(loose) ? loose : undefined; |
| 26 | 39 | const packed = await text(join(dir, "packed-refs")); |
| 27 | if (packed === undefined) return undefined; | |
| 40 | if (packed === undefined) return null; | |
| 28 | 41 | for (const line of packed.split("\n")) { |
| 29 | 42 | const [oid, name] = line.split(" "); |
| 30 | if (name === ref && oid !== undefined && isSafeOid(oid)) return oid; | |
| 43 | if (name === ref) | |
| 44 | return oid !== undefined && isSafeOid(oid) ? oid : undefined; | |
| 31 | 45 | } |
| 32 | return undefined; | |
| 46 | return null; | |
| 33 | 47 | } |
test/forks-lineage-timing.test.ts added +98
| @@ -0,0 +1,98 @@ | ||
| 1 | /** The forks page reads its lineage in a fixed number of queries (#6202). */ | |
| 2 | import { afterAll, beforeAll, expect, test } from "bun:test"; | |
| 3 | import { mkdtemp, rm } from "node:fs/promises"; | |
| 4 | import { tmpdir } from "node:os"; | |
| 5 | import { join } from "node:path"; | |
| 6 | import { createRepo, forkRepo } from "../src/actions/repos.ts"; | |
| 7 | import { createApp, type App } from "../src/app.ts"; | |
| 8 | import { lineageOf } from "../src/core/following.ts"; | |
| 9 | import { getRepo, type Repo } from "../src/core/repos.ts"; | |
| 10 | import { repoDir } from "../src/git/repos.ts"; | |
| 11 | import { readTipOrAbsent } from "../src/git/tip.ts"; | |
| 12 | import { createUser } from "../src/core/users.ts"; | |
| 13 | import { promote } from "./helpers/members.ts"; | |
| 14 | import { seedAdmin } from "./helpers/admin.ts"; | |
| 15 | ||
| 16 | let dataDir = ""; | |
| 17 | let app: App; | |
| 18 | let chain: Repo[] = []; | |
| 19 | ||
| 20 | beforeAll(async () => { | |
| 21 | dataDir = await mkdtemp(join(tmpdir(), "forge-fklin-")); | |
| 22 | app = createApp({ dataDir, port: 0 }); | |
| 23 | const admin = await seedAdmin(app, { username: "alice", email: "[email protected]", password: "correct-horse-battery" }); | |
| 24 | if (!admin.ok) throw new Error(admin.error); | |
| 25 | const must = <T>(r: { ok: true; value: T } | { ok: false; error: unknown }): T => { | |
| 26 | if (!r.ok) throw new Error(JSON.stringify(r.error)); | |
| 27 | return r.value; | |
| 28 | }; | |
| 29 | let parent = must(await createRepo(app, admin.value, { name: "r0", visibility: "public", description: "" })); | |
| 30 | chain = [parent]; | |
| 31 | for (let i = 1; i < 4; i++) { | |
| 32 | parent = must(await forkRepo(app, admin.value, parent, `r${i}`)); | |
| 33 | chain.push(parent); | |
| 34 | } | |
| 35 | for (let i = 0; i < 30; i++) { | |
| 36 | const who = must( | |
| 37 | await createUser(app.db, { origin: "seed", trusted: true, username: `u${i}`, email: `u${i}@f.dev`, password: "a-good-passphrase" }), | |
| 38 | ); | |
| 39 | promote(app.db, who); | |
| 40 | must(await forkRepo(app, who, chain[2]!, `leaf${i}`)); | |
| 41 | } | |
| 42 | }); | |
| 43 | ||
| 44 | afterAll(async () => { | |
| 45 | app.close(); | |
| 46 | await rm(dataDir, { recursive: true, force: true }); | |
| 47 | }); | |
| 48 | ||
| 49 | /** How many statements `run` sends to the database. */ | |
| 50 | async function queriesDuring(run: () => Promise<unknown>): Promise<number> { | |
| 51 | const real = app.db.query.bind(app.db); | |
| 52 | let n = 0; | |
| 53 | app.db.query = (sql: string) => { | |
| 54 | n++; | |
| 55 | return real(sql); | |
| 56 | }; | |
| 57 | try { | |
| 58 | await run(); | |
| 59 | } finally { | |
| 60 | app.db.query = real as typeof app.db.query; | |
| 61 | } | |
| 62 | return n; | |
| 63 | } | |
| 64 | ||
| 65 | test("the lineage costs the same few queries for 30 forks as for none", async () => { | |
| 66 | const big = await queriesDuring(() => lineageOf(app, chain[3]!, () => true)); | |
| 67 | const small = await queriesDuring(() => lineageOf(app, chain[0]!, () => true)); | |
| 68 | expect(big).toBeLessThanOrEqual(3); | |
| 69 | expect(small).toBe(big); | |
| 70 | const leaf = getRepo(app.db, "u0", "leaf0")!; | |
| 71 | expect(await queriesDuring(() => lineageOf(app, leaf, () => true))).toBe(big); | |
| 72 | }); | |
| 73 | ||
| 74 | test("an unborn branch is read from the ref files, with no git to ask", async () => { | |
| 75 | const dir = repoDir(app.reposRoot, chain[0]!.ownerName, chain[0]!.name); | |
| 76 | expect(await readTipOrAbsent(dir, "refs/heads/main")).toBeNull(); | |
| 77 | expect(await readTipOrAbsent(dir, "refs/heads/../x")).toBeUndefined(); | |
| 78 | }); | |
| 79 | ||
| 80 | test("the lineage reaches the root and holds every fork", async () => { | |
| 81 | const lineage = await lineageOf(app, chain[3]!, () => true); | |
| 82 | expect(lineage.root.repo.name).toBe("r0"); | |
| 83 | expect(lineage.size).toBe(34); | |
| 84 | }); | |
| 85 | ||
| 86 | test("a fork the viewer may not read is not in the lineage, nor hinted at", async () => { | |
| 87 | const hidden = getRepo(app.db, "u0", "leaf0")!; | |
| 88 | app.db.query("UPDATE repo SET visibility = 'private' WHERE id = ?1").run(hidden.id); | |
| 89 | const lineage = await lineageOf(app, chain[3]!, (r) => r.visibility === "public"); | |
| 90 | expect(lineage.size).toBe(33); | |
| 91 | const names: string[] = []; | |
| 92 | const walk = (n: { repo: Repo; children: readonly (typeof lineage.root)[] }): void => { | |
| 93 | names.push(n.repo.name); | |
| 94 | n.children.forEach(walk); | |
| 95 | }; | |
| 96 | walk(lineage.root); | |
| 97 | expect(names).not.toContain("leaf0"); | |
| 98 | }); |
test/forks-ranking.test.ts +10 −1
| @@ -9,6 +9,7 @@ import { | ||
| 9 | 9 | rankForks, |
| 10 | 10 | type RankedFork, |
| 11 | 11 | } from "../src/actions/forks.ts"; |
| 12 | import { forkRelation } from "../src/core/following.ts"; | |
| 12 | 13 | import { createSession } from "../src/core/sessions.ts"; |
| 13 | 14 | import { getRepo, listForks } from "../src/core/repos.ts"; |
| 14 | 15 | import { createForgeApp } from "../src/tui/forge.ts"; |
| @@ -81,11 +82,19 @@ describe("the forks page ranks", () => { | ||
| 81 | 82 | const bob = must(await forkRepo(app, users["bob"]!, upstream)); |
| 82 | 83 | const carol = must(await forkRepo(app, users["carol"]!, upstream)); |
| 83 | 84 | const dan = must(await forkRepo(app, users["dan"]!, upstream)); |
| 84 | must(await forkRepo(app, users["erin"]!, upstream)); | |
| 85 | const erin = must(await forkRepo(app, users["erin"]!, upstream)); | |
| 85 | 86 | await commit("bob", bob, 1); |
| 86 | 87 | await commit("carol", carol, 2); |
| 87 | 88 | await commit("dan", dan, 3); |
| 88 | 89 | await commit("alice", upstream, 1); // everyone now 1 behind |
| 90 | // Erin has nothing of her own, so the fork follows by itself; the page is | |
| 91 | // read once that has landed, not while it races the render. | |
| 92 | let synced = false; | |
| 93 | for (let i = 0; i < 100 && !synced; i++) { | |
| 94 | synced = (await forkRelation(app, erin, upstream))?.state === "synced"; | |
| 95 | if (!synced) await Bun.sleep(50); | |
| 96 | } | |
| 97 | if (!synced) throw new Error("erin's fork never caught up with its parent"); | |
| 89 | 98 | }, 60_000); |
| 90 | 99 | |
| 91 | 100 | afterAll(async () => { |
new above
offline
The title says timing, but the test asserts query counts only. Rename it, or add a generous duration ceiling so the 5 ms goal is guarded.