mirror of
https://github.com/getpaseo/paseo.git
synced 2026-10-25 10:42:45 +00:00
Cut the cost of a directory-suggestion scan so Add Project search stops timing out (#5190)
* 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 * test(server): skip the scan-cost budget on Windows The tree it needs is deeper than MAX_PATH allows, and the directory listing cache it measures is disabled on Windows because directory metadata cannot validate it there. * test(server): count containment derivations instead of timing the scan The previous budget compared wall-clock scan time against a readdir walk of the same tree. packages/server/AGENTS.md requires tests to be deterministic, and a ratio between an I/O-bound walk and a CPU-bound scan moves with filesystem caching and runner load. isPathInsideRoot now counts its calls behind start/stopPathContainmentMetrics, matching the existing git command metrics. Scanning a symlink-free tree of 320 entries derived containment 1,760 times before this branch and derives it none now, which is the defect stated exactly rather than inferred from a stopwatch. A second test covers the other direction: a symlink that leaves the root is still checked, and still excluded from results. * fix(server): keep the Git-ignore ancestor walk for symlinked entries A symlink resolves to a path whose ancestors were never checked during traversal, so a link into an ignored directory was discovered. Named children keep the single-entry check. * test(server): declare the scan helper as a function --------- Co-authored-by: paseo-bot[bot] <266920839+paseo-bot[bot]@users.noreply.github.com>
This commit is contained in:
@@ -12,6 +12,7 @@ import { tmpdir } from "node:os";
|
||||
import path from "node:path";
|
||||
import { afterEach, beforeEach, describe, expect, it } from "vitest";
|
||||
import { isPlatform } from "../test-utils/platform.js";
|
||||
import { startPathContainmentMetrics, stopPathContainmentMetrics } from "./path.js";
|
||||
import { startGitCommandMetrics, stopGitCommandMetrics } from "./run-git-command.js";
|
||||
import {
|
||||
searchDirectoryEntries,
|
||||
@@ -169,6 +170,26 @@ describe("searchDirectoryEntries", () => {
|
||||
);
|
||||
});
|
||||
|
||||
it.skipIf(isWindows)(
|
||||
"prunes a symlink whose target is inside a Git-ignored directory",
|
||||
async () => {
|
||||
initGitRepo(searchRoot, "generated/\n");
|
||||
mkdirSync(path.join(searchRoot, "generated", "output"), { recursive: true });
|
||||
symlinkSync(path.join(searchRoot, "generated", "output"), path.join(searchRoot, "linked"));
|
||||
|
||||
await expect(
|
||||
searchDirectoryEntries({
|
||||
root: searchRoot,
|
||||
query: "linked",
|
||||
pathFormat: "relative",
|
||||
includeFiles: false,
|
||||
includeDirectories: true,
|
||||
respectGitIgnore: true,
|
||||
}),
|
||||
).resolves.toEqual([]);
|
||||
},
|
||||
);
|
||||
|
||||
it("configures raw blank queries independently from explicit root aliases", async () => {
|
||||
const rootEntries = [
|
||||
{ path: "projects", kind: "directory" as const },
|
||||
@@ -953,3 +974,62 @@ describe("relative typed-entry configuration", () => {
|
||||
expect(results).toEqual([{ path: "blankpage/editor", kind: "directory" }]);
|
||||
});
|
||||
});
|
||||
|
||||
// Reading a tree is cheap; re-deriving each entry's path state is not. A scan that asks whether
|
||||
// every ancestor of every entry is inside the root, or is Git-ignored, costs multiples of the
|
||||
// read and is what made large home directories exceed the client request timeout.
|
||||
describe("home-tree scan cost", () => {
|
||||
const DEPTH = 8;
|
||||
const CHAINS = 40;
|
||||
let scanRoot: string;
|
||||
|
||||
beforeEach(() => {
|
||||
scanRoot = realpathSync.native(mkdtempSync(path.join(tmpdir(), "directory-scan-cost-")));
|
||||
for (let chain = 0; chain < CHAINS; chain += 1) {
|
||||
const segments: string[] = [];
|
||||
for (let level = 0; level < DEPTH; level += 1) segments.push(`c${chain}l${level}`);
|
||||
mkdirSync(path.join(scanRoot, ...segments), { recursive: true });
|
||||
}
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
rmSync(scanRoot, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
// Nothing matches, so the scan walks the whole tree instead of stopping at a confident result.
|
||||
function scanWholeTree() {
|
||||
return searchAbsoluteDirectoryPaths({
|
||||
homeDir: scanRoot,
|
||||
query: "nomatch",
|
||||
maxDepth: DEPTH + 4,
|
||||
});
|
||||
}
|
||||
|
||||
async function countContainmentChecks(run: () => Promise<unknown>): Promise<number> {
|
||||
startPathContainmentMetrics();
|
||||
await run();
|
||||
return stopPathContainmentMetrics();
|
||||
}
|
||||
|
||||
it("derives no containment for a tree that cannot leave the root", async () => {
|
||||
const checks = await countContainmentChecks(scanWholeTree);
|
||||
|
||||
expect({ checks, scanned: CHAINS * DEPTH }).toEqual({ checks: 0, scanned: 320 });
|
||||
});
|
||||
|
||||
it.skipIf(isWindows)(
|
||||
"derives containment for symlinked entries, which can leave the root",
|
||||
async () => {
|
||||
const outside = realpathSync.native(mkdtempSync(path.join(tmpdir(), "directory-scan-away-")));
|
||||
symlinkSync(outside, path.join(scanRoot, "escape"), "dir");
|
||||
|
||||
const checks = await countContainmentChecks(scanWholeTree);
|
||||
|
||||
expect({ checked: checks > 0, escaped: (await scanWholeTree()).includes(outside) }).toEqual({
|
||||
checked: true,
|
||||
escaped: false,
|
||||
});
|
||||
rmSync(outside, { recursive: true, force: true });
|
||||
},
|
||||
);
|
||||
});
|
||||
|
||||
@@ -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<RankedEntry[]> {
|
||||
async function searchTree(input: SearchInput): Promise<RankedEntry[]> {
|
||||
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<string>([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,14 @@ async function* roundRobin<T>(branches: Array<AsyncGenerator<T>>): 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;
|
||||
if (isNewlyGitIgnored(entry, input)) return false;
|
||||
if (entry.kind === "file") {
|
||||
return input.includeFiles && !entry.name.startsWith(".");
|
||||
}
|
||||
@@ -339,6 +346,14 @@ function shouldDiscover(entry: ChildEntry, input: SearchInput): boolean {
|
||||
return input.hiddenDirectoryNames.has(entry.name);
|
||||
}
|
||||
|
||||
// Traversal only descends through entries that already passed this check, so every ancestor of
|
||||
// a named child is known discoverable and only the entry itself can be newly ignored. A symlink
|
||||
// resolves to a path with different ancestors, so it still needs the full walk.
|
||||
function isNewlyGitIgnored(entry: ChildEntry, input: SearchInput): boolean {
|
||||
if (entry.viaSymlink) return isGitIgnoredPath(entry.resolvedPath, input);
|
||||
return input.gitIgnoredPaths.has(entry.resolvedPath);
|
||||
}
|
||||
|
||||
function isGitIgnoredPath(absolutePath: string, input: SearchInput): boolean {
|
||||
let candidate = absolutePath;
|
||||
while (candidate !== input.root && isPathInsideRoot(input.root, candidate)) {
|
||||
@@ -712,14 +727,14 @@ function toRawChildEntry(dirent: Dirent): RawChildEntry | null {
|
||||
async function resolveChild(directory: string, entry: RawChildEntry): Promise<ChildEntry | null> {
|
||||
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 +743,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;
|
||||
|
||||
@@ -58,7 +58,22 @@ export function createRealpathAwarePathMatcher(target: string): (candidate: stri
|
||||
};
|
||||
}
|
||||
|
||||
let pathContainmentChecks: number | null = null;
|
||||
|
||||
// Containment is re-derived from scratch on every call. A tree walk that asks it per entry, or
|
||||
// per ancestor of every entry, costs more than reading the tree does, so tests count the calls.
|
||||
export function startPathContainmentMetrics(): void {
|
||||
pathContainmentChecks = 0;
|
||||
}
|
||||
|
||||
export function stopPathContainmentMetrics(): number {
|
||||
const checks = pathContainmentChecks ?? 0;
|
||||
pathContainmentChecks = null;
|
||||
return checks;
|
||||
}
|
||||
|
||||
export function isPathInsideRoot(root: string, candidate: string): boolean {
|
||||
if (pathContainmentChecks !== null) pathContainmentChecks += 1;
|
||||
return getRelativePathInsideRoot(root, candidate) !== null;
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user