Public: anyone can read this; the crew can change it.
grepRepos answers at the code budget: the first batch's hits always kept, later batches cut off, abandoned greps still holding their slots #7251
Check · Review · Land · Live done — Landed and live in production.
torque/forge:torque/7118-greprepos-answers-at-the-code-budget-the → main 2 commits fork 1 thread · 1 unresolved
2 files
changed +244
−9
src/core/search.ts +56 −9
| @@ -507,12 +507,29 @@ async function grepCached( | ||
| 507 | 507 | return hits; |
| 508 | 508 | } |
| 509 | 509 | |
| 510 | /** Whose slot a grep takes; `grep` and `slots` are for tests to inject. */ | |
| 510 | /** Whose slot a grep takes; `grep`, `slots`, `now` and `timer` are for tests to inject. */ | |
| 511 | 511 | export interface GrepWho { |
| 512 | 512 | /** The signed-in actor's id; undefined is a stranger. */ |
| 513 | 513 | readonly memberId?: string | undefined; |
| 514 | 514 | readonly slots?: GrepSlots; |
| 515 | 515 | readonly grep?: typeof grepRun; |
| 516 | /** Milliseconds on a monotonic clock; `performance.now` by default. */ | |
| 517 | readonly now?: () => number; | |
| 518 | /** Resolves after `ms`; `clear` stops it. A `setTimeout` by default. */ | |
| 519 | readonly timer?: (ms: number) => BudgetTimer; | |
| 520 | } | |
| 521 | ||
| 522 | export interface BudgetTimer { | |
| 523 | readonly done: Promise<void>; | |
| 524 | clear(): void; | |
| 525 | } | |
| 526 | ||
| 527 | function budgetTimer(ms: number): BudgetTimer { | |
| 528 | let id: ReturnType<typeof setTimeout> | undefined; | |
| 529 | const done = new Promise<void>((resolve) => { | |
| 530 | id = setTimeout(resolve, ms); | |
| 531 | }); | |
| 532 | return { done, clear: () => clearTimeout(id) }; | |
| 516 | 533 | } |
| 517 | 534 | |
| 518 | 535 | /** |
| @@ -520,13 +537,20 @@ export interface GrepWho { | ||
| 520 | 537 | * fifty small greps do not become fifty simultaneous processes, and the walk |
| 521 | 538 | * stops the moment the cap is reached — a common word in a monorepo costs |
| 522 | 539 | * one repo's work, not fifty. |
| 540 | * | |
| 541 | * With a finite budget the first batch is always waited for, so a cold search | |
| 542 | * shows what it finds however slow git is. Each later batch races the time | |
| 543 | * left: when the budget runs out first the answer goes back with `more`, and | |
| 544 | * the batch's greps are left to finish on their own. They keep the slot | |
| 545 | * `search/grep_slots.ts` gave them until git exits — that is the only cap on | |
| 546 | * abandoned greps, and it is charged to the caller who started them. | |
| 523 | 547 | */ |
| 524 | 548 | export async function grepRepos( |
| 525 | 549 | reposRoot: string, |
| 526 | 550 | repos: readonly Repo[], |
| 527 | 551 | query: string, |
| 528 | 552 | limit = MAX_CODE_HITS, |
| 529 | /** No new batch starts after this many ms; the rest is reported as `more`. */ | |
| 553 | /** No new batch starts after this many ms, nor is waited on past it; the rest is `more`. */ | |
| 530 | 554 | budgetMs = Infinity, |
| 531 | 555 | who: GrepWho = {}, |
| 532 | 556 | ): Promise<{ hits: CodeHit[]; truncated: boolean; more: boolean; skipped: boolean }> { |
| @@ -537,16 +561,20 @@ export async function grepRepos( | ||
| 537 | 561 | // Within a batch a member's greps run in pairs, so a search never |
| 538 | 562 | // starves itself of the member's own slots. |
| 539 | 563 | const width = who.memberId === undefined ? batch : GREP_LIMITS.perMember; |
| 540 | const started = performance.now(); | |
| 564 | const now = who.now ?? (() => performance.now()); | |
| 565 | const started = now(); | |
| 541 | 566 | const timeoutMs = budgetMs === Infinity ? GREP_TIMEOUT_MS : BUDGETED_GREP_MS; |
| 542 | 567 | let gaveUp = false; |
| 543 | for (let i = 0; i < considered.length; i += batch) { | |
| 544 | if (i > 0 && performance.now() - started >= budgetMs) { | |
| 545 | return { hits, truncated: repos.length > MAX_SEARCH_REPOS, more: true, skipped }; | |
| 546 | } | |
| 547 | const slice = considered.slice(i, i + batch); | |
| 568 | // Set when this search answers at its budget: the batch it left behind | |
| 569 | // starts no further pair. Local to this call, never shared between callers. | |
| 570 | let answered = false; | |
| 571 | const more = () => { | |
| 572 | answered = true; | |
| 573 | return { hits, truncated: repos.length > MAX_SEARCH_REPOS, more: true, skipped }; | |
| 574 | }; | |
| 575 | const grepBatch = async (slice: readonly Repo[]) => { | |
| 548 | 576 | const found: (RawHit[] | "skipped" | "timeout")[] = []; |
| 549 | for (let k = 0; k < slice.length; k += width) { | |
| 577 | for (let k = 0; k < slice.length && !answered; k += width) { | |
| 550 | 578 | found.push( |
| 551 | 579 | ...(await Promise.all( |
| 552 | 580 | slice |
| @@ -563,6 +591,25 @@ export async function grepRepos( | ||
| 563 | 591 | )), |
| 564 | 592 | ); |
| 565 | 593 | } |
| 594 | return found; | |
| 595 | }; | |
| 596 | for (let i = 0; i < considered.length; i += batch) { | |
| 597 | const slice = considered.slice(i, i + batch); | |
| 598 | let found: (RawHit[] | "skipped" | "timeout")[]; | |
| 599 | if (i === 0 || budgetMs === Infinity) { | |
| 600 | found = await grepBatch(slice); | |
| 601 | } else { | |
| 602 | const left = budgetMs - (now() - started); | |
| 603 | if (left <= 0) return more(); | |
| 604 | const running = grepBatch(slice); | |
| 605 | // An abandoned batch settles on its own; a rejection then is nobody's. | |
| 606 | running.catch(() => {}); | |
| 607 | const timer = (who.timer ?? budgetTimer)(left); | |
| 608 | const raced = await Promise.race([running, timer.done.then(() => "cut" as const)]); | |
| 609 | timer.clear(); | |
| 610 | if (raced === "cut") return more(); | |
| 611 | found = raced; | |
| 612 | } | |
| 566 | 613 | for (const [j, repo] of slice.entries()) { |
| 567 | 614 | const mine = found[j] ?? []; |
| 568 | 615 | if (mine === "skipped") { |
test/search-batch-race.test.ts added +188
| @@ -0,0 +1,188 @@ | ||
| 1 | /** | |
| 2 | * `grepRepos` answers at the code budget (#7118): the first batch is always | |
| 3 | * waited for, a later batch races the time left, and an abandoned grep keeps | |
| 4 | * its slot until git exits. Every grep and the budget's timer are deferred | |
| 5 | * promises the test settles by hand — no PATH shim, no sleep, no margin. | |
| 6 | */ | |
| 7 | import { expect, test } from "bun:test"; | |
| 8 | import { createGrepSlots } from "../src/search/grep_slots.ts"; | |
| 9 | import { grepRepos, type BudgetTimer } from "../src/core/search.ts"; | |
| 10 | import type { Repo } from "../src/core/repos.ts"; | |
| 11 | ||
| 12 | type Raw = { path: string; line: number; text: string }[] | null; | |
| 13 | ||
| 14 | const repo = (i: number) => | |
| 15 | ({ id: `r${i}`, ownerName: "u", name: `r${i}`, defaultBranch: "main", archivedAt: null }) as Repo; | |
| 16 | ||
| 17 | /** A grep per repo dir, each held until the test answers it. */ | |
| 18 | function deferredGreps() { | |
| 19 | const pending = new Map<string, (v: Raw) => void>(); | |
| 20 | const waiters: (() => void)[] = []; | |
| 21 | const grep = (dir: string) => | |
| 22 | new Promise<Raw>((done) => { | |
| 23 | pending.set(dir.replace(/^.*\/(r\d+)\.git$/, "$1"), done); | |
| 24 | for (const w of waiters.splice(0)) w(); | |
| 25 | }); | |
| 26 | /** Resolves once `name`'s grep has started. */ | |
| 27 | const started = async (name: string) => { | |
| 28 | while (!pending.has(name)) await new Promise<void>((r) => waiters.push(r)); | |
| 29 | }; | |
| 30 | const answer = (name: string, hits: Raw = [{ path: "a.txt", line: 1, text: name }]) => { | |
| 31 | pending.get(name)!(hits); | |
| 32 | pending.delete(name); | |
| 33 | }; | |
| 34 | return { grep: grep as never, started, answer, running: () => [...pending.keys()] }; | |
| 35 | } | |
| 36 | ||
| 37 | /** A budget timer the test fires; records whether one was ever asked for. */ | |
| 38 | function handTimer() { | |
| 39 | let fire!: () => void; | |
| 40 | let asked = 0; | |
| 41 | let cleared = 0; | |
| 42 | const timer = (_ms: number): BudgetTimer => { | |
| 43 | asked++; | |
| 44 | const done = new Promise<void>((r) => (fire = r)); | |
| 45 | return { done, clear: () => void cleared++ }; | |
| 46 | }; | |
| 47 | return { timer, fire: () => fire(), asked: () => asked, cleared: () => cleared }; | |
| 48 | } | |
| 49 | ||
| 50 | test("a spent budget still waits for batch 0 and keeps its hits", async () => { | |
| 51 | const g = deferredGreps(); | |
| 52 | const slots = createGrepSlots(); | |
| 53 | let t = 0; | |
| 54 | const search = grepRepos("/nowhere", [0, 1, 2, 3, 4, 5].map(repo), "q", 200, 10, { | |
| 55 | slots, | |
| 56 | grep: g.grep, | |
| 57 | now: () => t, | |
| 58 | }); | |
| 59 | for (const n of ["r0", "r1", "r2", "r3"]) await g.started(n); | |
| 60 | // The whole budget passes while batch 0 is still in git. | |
| 61 | t = 1_000; | |
| 62 | for (const n of ["r0", "r1", "r2", "r3"]) g.answer(n); | |
| 63 | const out = await search; | |
| 64 | expect(out.hits.map((h) => h.repo.name)).toEqual(["r0", "r1", "r2", "r3"]); | |
| 65 | expect(out.more).toBe(true); | |
| 66 | // Batch 1 never started: the budget was gone before it. | |
| 67 | expect(g.running()).toEqual([]); | |
| 68 | expect(slots.held().total).toBe(0); | |
| 69 | }); | |
| 70 | ||
| 71 | test("a never-settling later batch does not hold the answer, and keeps its slots", async () => { | |
| 72 | const g = deferredGreps(); | |
| 73 | const slots = createGrepSlots(); | |
| 74 | const clock = handTimer(); | |
| 75 | const search = grepRepos("/nowhere", [0, 1, 2, 3, 4, 5].map(repo), "q", 200, 10, { | |
| 76 | slots, | |
| 77 | grep: g.grep, | |
| 78 | now: () => 0, | |
| 79 | timer: clock.timer, | |
| 80 | }); | |
| 81 | for (const n of ["r0", "r1", "r2", "r3"]) await g.started(n); | |
| 82 | for (const n of ["r0", "r1", "r2", "r3"]) g.answer(n); | |
| 83 | await g.started("r4"); | |
| 84 | await g.started("r5"); | |
| 85 | // Batch 1 hangs; the budget runs out. | |
| 86 | clock.fire(); | |
| 87 | const out = await search; | |
| 88 | expect(out.more).toBe(true); | |
| 89 | expect(out.hits.map((h) => h.repo.name)).toEqual(["r0", "r1", "r2", "r3"]); | |
| 90 | expect(clock.cleared()).toBe(1); | |
| 91 | // The abandoned greps still hold their slots until git exits. | |
| 92 | expect(slots.held()).toEqual({ total: 2, strangers: 2 }); | |
| 93 | g.answer("r4"); | |
| 94 | g.answer("r5"); | |
| 95 | // The slot frees in the grep's own `finally`, a few microtasks on. | |
| 96 | for (let k = 0; k < 10 && slots.held().total > 0; k++) await Promise.resolve(); | |
| 97 | expect(slots.held()).toEqual({ total: 0, strangers: 0 }); | |
| 98 | }); | |
| 99 | ||
| 100 | test("a later batch that answers inside the budget is kept, and the timer cleared", async () => { | |
| 101 | const g = deferredGreps(); | |
| 102 | const clock = handTimer(); | |
| 103 | const search = grepRepos("/nowhere", [0, 1, 2, 3, 4].map(repo), "q", 200, 10, { | |
| 104 | slots: createGrepSlots(), | |
| 105 | grep: g.grep, | |
| 106 | now: () => 0, | |
| 107 | timer: clock.timer, | |
| 108 | }); | |
| 109 | for (const n of ["r0", "r1", "r2", "r3"]) await g.started(n); | |
| 110 | for (const n of ["r0", "r1", "r2", "r3"]) g.answer(n); | |
| 111 | await g.started("r4"); | |
| 112 | g.answer("r4"); | |
| 113 | const out = await search; | |
| 114 | expect(out.more).toBe(false); | |
| 115 | expect(out.hits.map((h) => h.repo.name)).toEqual(["r0", "r1", "r2", "r3", "r4"]); | |
| 116 | expect(clock.asked()).toBe(1); | |
| 117 | expect(clock.cleared()).toBe(1); | |
| 118 | }); | |
| 119 | ||
| 120 | test("one caller's abandoned greps do not stop another caller's search", async () => { | |
| 121 | const slots = createGrepSlots(); | |
| 122 | const a = deferredGreps(); | |
| 123 | const clock = handTimer(); | |
| 124 | // Member a's search abandons its second pair of greps at the budget. | |
| 125 | const first = grepRepos("/nowhere", [0, 1, 2, 3, 4, 5].map(repo), "q", 200, 10, { | |
| 126 | memberId: "a", | |
| 127 | slots, | |
| 128 | grep: a.grep, | |
| 129 | now: () => 0, | |
| 130 | timer: clock.timer, | |
| 131 | }); | |
| 132 | for (const pair of [["r0", "r1"], ["r2", "r3"]]) { | |
| 133 | for (const n of pair) await a.started(n); | |
| 134 | for (const n of pair) a.answer(n); | |
| 135 | } | |
| 136 | await a.started("r4"); | |
| 137 | await a.started("r5"); | |
| 138 | clock.fire(); | |
| 139 | expect((await first).more).toBe(true); | |
| 140 | expect(slots.held().total).toBe(2); | |
| 141 | ||
| 142 | // Member b, and a stranger, search the whole lot while a's greps hang. | |
| 143 | for (const memberId of ["b", undefined]) { | |
| 144 | const b = deferredGreps(); | |
| 145 | const second = grepRepos("/nowhere", [0, 1, 2, 3, 4, 5].map(repo), "q", 200, 10, { | |
| 146 | memberId, | |
| 147 | slots, | |
| 148 | grep: b.grep, | |
| 149 | now: () => 0, | |
| 150 | timer: handTimer().timer, | |
| 151 | }); | |
| 152 | const width = memberId === undefined ? 4 : 2; | |
| 153 | const names = ["r0", "r1", "r2", "r3", "r4", "r5"]; | |
| 154 | for (let k = 0; k < names.length; k += width) { | |
| 155 | const group = names.slice(k, k + width); | |
| 156 | for (const n of group) await b.started(n); | |
| 157 | for (const n of group) b.answer(n); | |
| 158 | } | |
| 159 | const out = await second; | |
| 160 | expect(out.skipped).toBe(false); | |
| 161 | expect(out.more).toBe(false); | |
| 162 | expect(out.hits.length).toBe(6); | |
| 163 | } | |
| 164 | ||
| 165 | a.answer("r4"); | |
| 166 | a.answer("r5"); | |
| 167 | }); | |
| 168 | ||
| 169 | test("a search with no budget never asks for a timer and waits for every batch", async () => { | |
| 170 | const g = deferredGreps(); | |
| 171 | const clock = handTimer(); | |
| 172 | let t = 0; | |
| 173 | const search = grepRepos("/nowhere", [0, 1, 2, 3, 4].map(repo), "q", 200, Infinity, { | |
| 174 | slots: createGrepSlots(), | |
| 175 | grep: g.grep, | |
| 176 | now: () => t, | |
| 177 | timer: clock.timer, | |
| 178 | }); | |
| 179 | for (const n of ["r0", "r1", "r2", "r3"]) await g.started(n); | |
| 180 | t = 1e9; | |
| 181 | for (const n of ["r0", "r1", "r2", "r3"]) g.answer(n); | |
| 182 | await g.started("r4"); | |
| 183 | g.answer("r4"); | |
| 184 | const out = await search; | |
| 185 | expect(out.more).toBe(false); | |
| 186 | expect(out.hits.length).toBe(5); | |
| 187 | expect(clock.asked()).toBe(0); | |
| 188 | }); |
new above
offline
budgetTimeris never run by a test, because every test injects its own timer. Add one small test with a real 1 ms budget so the default path is covered.