diff --git a/packages/server/src/utils/directory-suggestions.test.ts b/packages/server/src/utils/directory-suggestions.test.ts index 2b24ca2174..3e1c619026 100644 --- a/packages/server/src/utils/directory-suggestions.test.ts +++ b/packages/server/src/utils/directory-suggestions.test.ts @@ -8,6 +8,7 @@ import { unlinkSync, writeFileSync, } from "node:fs"; +import { readdir } from "node:fs/promises"; import { tmpdir } from "node:os"; import path from "node:path"; import { afterEach, beforeEach, describe, expect, it } from "vitest"; @@ -953,3 +954,79 @@ describe("relative typed-entry configuration", () => { expect(results).toEqual([{ path: "blankpage/editor", kind: "directory" }]); }); }); + +// A home-directory scan is dominated by per-entry bookkeeping, not by reading the tree. These +// budgets are expressed against a plain readdir walk of the same tree, so they mean the same +// thing on a slow CI runner as on a developer machine. +describe("home-tree scan cost", () => { + const ENTRIES = 5_000; + const DEPTH = 8; + // Long names cost more to normalize, and per-entry path work is spent normalizing them. + const SEGMENT_NAME_LENGTH = 48; + let scanRoot: string; + + function buildDeepTree(root: string): void { + const chains = Math.ceil(ENTRIES / DEPTH); + for (let chain = 0; chain < chains; chain += 1) { + const segments: string[] = []; + for (let level = 0; level < DEPTH; level += 1) { + segments.push(`d${chain * DEPTH + level}`.padEnd(SEGMENT_NAME_LENGTH, "x")); + } + mkdirSync(path.join(root, ...segments), { recursive: true }); + } + } + + async function readWholeTree(root: string): Promise { + let level = [root]; + while (level.length) { + const next: string[] = []; + for (const directory of level) { + const children = await readdir(directory, { withFileTypes: true }).catch(() => []); + for (const child of children) { + if (child.isDirectory()) next.push(path.join(directory, child.name)); + } + } + level = next; + } + } + + async function measure(run: () => Promise): Promise { + const startedAt = process.hrtime.bigint(); + await run(); + return Number(process.hrtime.bigint() - startedAt) / 1e6; + } + + // Nothing matches, so every scan spends its whole budget and the timings are comparable. + const scanDeepTree = () => + searchAbsoluteDirectoryPaths({ + homeDir: scanRoot, + query: "nomatchanywhereinthistree", + limit: 30, + maxDepth: DEPTH + 4, + maxDirectoriesScanned: ENTRIES, + }); + + beforeEach(() => { + scanRoot = realpathSync.native(mkdtempSync(path.join(tmpdir(), "directory-scan-cost-"))); + buildDeepTree(scanRoot); + }); + + afterEach(() => { + rmSync(scanRoot, { recursive: true, force: true }); + }); + + it("scans a deep tree within a bounded multiple of reading it", async () => { + const readMs = await measure(() => readWholeTree(scanRoot)); + await scanDeepTree(); + const scanMs = await measure(scanDeepTree); + + // Re-deriving containment and Git-ignore state for every ancestor of every entry, and + // sweeping the directory cache on every miss, put this past 12x. That is what made real + // home directories exceed the client request timeout. + const ratio = scanMs / readMs; + expect({ ratio: Math.round(ratio), withinBudget: ratio < 6 }).toEqual({ + ratio: expect.any(Number), + withinBudget: true, + }); + }, 60_000); +}); diff --git a/packages/server/src/utils/directory-suggestions.ts b/packages/server/src/utils/directory-suggestions.ts index aa24c4a6b9..ee673ded22 100644 --- a/packages/server/src/utils/directory-suggestions.ts +++ b/packages/server/src/utils/directory-suggestions.ts @@ -46,6 +46,7 @@ interface ChildEntry { name: string; resolvedPath: string; kind: DirectorySuggestionKind; + viaSymlink: boolean; } interface RawChildEntry { @@ -249,7 +250,7 @@ async function searchChildren(input: SearchInput): Promise { async function searchTree(input: SearchInput): Promise { if (!(input.maxEntriesScanned > 0)) return []; const roots = (await readChildren(input.root)).filter((entry) => - isPathInsideRoot(input.root, entry.resolvedPath), + staysInsideRoot(entry, input.root), ); const visited = new Set([input.root]); const branches = roots.flatMap((entry) => @@ -294,7 +295,7 @@ async function* walkBranch( return; visited.add(entry.resolvedPath); const children = (await readChildren(entry.resolvedPath)).filter((child) => - isPathInsideRoot(input.root, child.resolvedPath), + staysInsideRoot(child, input.root), ); const branches = children.flatMap((child) => shouldDiscover(child, input) @@ -329,8 +330,17 @@ async function* roundRobin(branches: Array>): AsyncGenerato } } +// Only a symlink can resolve outside the tree being walked. Every other child is its parent's +// path plus a name, and the parent was already proved inside the root. +function staysInsideRoot(entry: ChildEntry, root: string): boolean { + return !entry.viaSymlink || isPathInsideRoot(root, entry.resolvedPath); +} + function shouldDiscover(entry: ChildEntry, input: SearchInput): boolean { - if (isGitIgnoredPath(entry.resolvedPath, input)) return false; + // Traversal only descends through entries that already passed this check, so every ancestor + // is known discoverable and only the entry itself can be newly ignored. Walking the ancestor + // chain here would re-derive that answer once per level, for every entry scanned. + if (input.gitIgnoredPaths.has(entry.resolvedPath)) return false; if (entry.kind === "file") { return input.includeFiles && !entry.name.startsWith("."); } @@ -712,14 +722,14 @@ function toRawChildEntry(dirent: Dirent): RawChildEntry | null { async function resolveChild(directory: string, entry: RawChildEntry): Promise { const visiblePath = path.join(directory, entry.name); if (entry.kind !== "symlink") { - return { name: entry.name, resolvedPath: visiblePath, kind: entry.kind }; + return { name: entry.name, resolvedPath: visiblePath, kind: entry.kind, viaSymlink: false }; } const resolvedPath = await realpath(visiblePath).catch(() => null); if (!resolvedPath) return null; const info = await stat(resolvedPath).catch(() => null); const kind = getEntryKind(info); - return kind ? { name: entry.name, resolvedPath, kind } : null; + return kind ? { name: entry.name, resolvedPath, kind, viaSymlink: true } : null; } function getEntryKind(info: Stats | null): DirectorySuggestionKind | null { @@ -728,10 +738,9 @@ function getEntryKind(info: Stats | null): DirectorySuggestionKind | null { return null; } +// Reads already reject stale entries by expiry and by directory metadata, so this only has to +// bound the map. Sweeping it for expired keys would cost a full pass on every cache miss. function pruneCache(): void { - if (directoryListCache.size <= DIRECTORY_LIST_CACHE_MAX_ENTRIES) return; - for (const [key, entry] of directoryListCache) - if (entry.expiresAt <= Date.now()) directoryListCache.delete(key); while (directoryListCache.size > DIRECTORY_LIST_CACHE_MAX_ENTRIES) { const key = directoryListCache.keys().next().value; if (!key) return;