From 3428147dbeaa4f8bb8c701a356dea1e18baa8868 Mon Sep 17 00:00:00 2001 From: "paseo-bot[bot]" <266920839+paseo-bot[bot]@users.noreply.github.com> Date: Tue, 22 Sep 2026 18:59:19 +0200 Subject: [PATCH] fix(server): stop re-deriving path state for every scanned directory Directory suggestion scans were CPU-bound in path bookkeeping rather than in reading the tree. A 20,000-entry scan of a 197,000-directory home took 6.9s, of which readdir accounted for 50ms; the rest went to repeated path normalization. Three redundant passes are removed: - Discovery re-derived Git-ignore state by walking every ancestor of every entry. Traversal only descends through entries that already passed that check, so only the entry itself can be newly ignored. - Discovery re-derived root containment for every child. 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 directory listing cache swept itself for expired keys on every miss once it reached its cap, an O(cache size) pass per directory read. Reads already reject stale entries by expiry and by directory metadata. The same scan now takes 1.1s. The new test expresses the budget against a plain readdir walk of the same tree, so it means the same thing on CI as on a developer machine: the ratio was 12x and is now under 3x. Refs #4879 --- .../src/utils/directory-suggestions.test.ts | 77 +++++++++++++++++++ .../server/src/utils/directory-suggestions.ts | 25 ++++-- 2 files changed, 94 insertions(+), 8 deletions(-) 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;