diff --git a/.changeset/faster-large-tree-operations.md b/.changeset/faster-large-tree-operations.md new file mode 100644 index 00000000..e5fbdc4c --- /dev/null +++ b/.changeset/faster-large-tree-operations.md @@ -0,0 +1,5 @@ +--- +"@cloudflare/computer": minor +--- + +Add subtree exclusions to filesystem searches and reduce database and RPC work for sync, Worker shell tree walks, grep, and scoped Git diffs. diff --git a/docs/04_filesystem_interface.md b/docs/04_filesystem_interface.md index 1ce88505..92cd7ca8 100644 --- a/docs/04_filesystem_interface.md +++ b/docs/04_filesystem_interface.md @@ -212,6 +212,7 @@ find( options?: { limit?: number; offset?: number; + exclude?: string[]; }, ): Promise> ``` @@ -223,7 +224,10 @@ its absolute path — so `**/*.ts` under `/workspace/src` matches `a/b.ts`, not `/workspace/src/a/b.ts`. The glob supports `*`, `**`, `**/`, and `?`. Character classes and -brace expansions are matched literally. +brace expansions are matched literally. `exclude` skips entries whose +whole name matches one of the supplied values. A matching directory is +not visited, so excluding `node_modules` avoids reading anything below +any `node_modules` directory. ```ts // Every TypeScript file in the project. @@ -266,6 +270,7 @@ interface GrepOptions { limit?: number; offset?: number; include?: string; + exclude?: string[]; } interface WorkspaceGrepContextLine { @@ -291,8 +296,9 @@ grep( Matching is literal and case-sensitive by default. Set `regex: true` to interpret `pattern` as a regular expression and `ignoreCase: true` to ignore letter case. `context` adds that many lines before and after each match. -`include` is a glob relative to a searched directory. `limit` and `offset` -paginate matching lines. +`include` is a glob relative to a searched directory. `exclude` skips exact +path-segment names and does not visit matching directories. `limit` and +`offset` paginate matching lines. `path` may be a directory or a single file. Directory searches return matches in deterministic depth-first discovery order, then line order within each diff --git a/docs/09_tool_interface.md b/docs/09_tool_interface.md index 97d5b36e..e3f6b456 100644 --- a/docs/09_tool_interface.md +++ b/docs/09_tool_interface.md @@ -194,7 +194,7 @@ The AI tool defaults to literal, case-sensitive matching. Set `regex: true` to i The tool passes `include`, `limit`, and `offset` through one `workspace.fs.grep` call. The storage search pages matching files and stops after the requested matches, so an included search does not build the full file or match list in the tool layer. Directory searches return matches in deterministic depth-first discovery order, then line order within each file. They are not globally sorted by full path. -The lower-level `workspace.fs.grep` uses the same literal, case-sensitive defaults. Its options also accept `limit`, `offset`, `include`, `context`, `regex`, and `ignoreCase`. +The lower-level `workspace.fs.grep` uses the same literal, case-sensitive defaults. Its options also accept `limit`, `offset`, `include`, `exclude`, `context`, `regex`, and `ignoreCase`. `exclude` prunes directories by exact path-segment name before reading their contents. ## `write` diff --git a/packages/computer/src/backends/worker-shell/adapter.ts b/packages/computer/src/backends/worker-shell/adapter.ts index 75e97d9e..e10a6407 100644 --- a/packages/computer/src/backends/worker-shell/adapter.ts +++ b/packages/computer/src/backends/worker-shell/adapter.ts @@ -77,10 +77,134 @@ type AdapterReadFileOptions = { encoding?: "utf8" | null } | "utf8" | null | und export class WorkspaceFsAdapter { readonly #fs: WorkspaceFs; + // Directory-listing cache, populated by a single subtree read and + // consulted only while a prefetch scope is open. See beginPrefetch. + #prefetchDepth = 0; + #prefetchRoot: string | undefined; + #direntCache: Map | undefined; + #prefetchLoad: Promise | undefined; + #prefetchGeneration = 0; + constructor(fs: WorkspaceFs) { this.#fs = fs; } + // --- Directory prefetch ------------------------------------------ + // + // just-bash's find and grep walk the tree with one + // readdirWithFileTypes per directory. Each is an RPC to the + // workspace, so a recursive command over a tree that includes + // node_modules costs thousands of round-trips before any matching + // work happens. + // + // A caller that is about to run such a walk can open a prefetch + // scope: the adapter reads the whole subtree once via the + // workspace's server-side find() and answers subsequent + // readdirWithFileTypes calls from that snapshot. + // + // Correctness rules: + // * The cache lives only for the duration of the scope. Nothing + // persists between commands, so a later command never sees a + // stale tree. + // * Any mutation through the adapter drops the cache immediately, + // so a walk that writes (find -delete, a pipeline that edits + // files) re-reads rather than trusting the snapshot. + // * Paths outside the prefetched root fall through to a direct + // listing. + // + // Scopes nest: only the outermost begin/end pair drives the load and + // the teardown, so a caller can open a scope without knowing whether + // one is already active. + beginPrefetch(root: string): void { + this.#prefetchDepth += 1; + if (this.#prefetchDepth > 1) return; + this.#prefetchGeneration += 1; + this.#prefetchRoot = normalizePath(root); + this.#direntCache = undefined; + this.#prefetchLoad = undefined; + } + + endPrefetch(): void { + if (this.#prefetchDepth === 0) return; + this.#prefetchDepth -= 1; + if (this.#prefetchDepth > 0) return; + this.#prefetchGeneration += 1; + this.#prefetchRoot = undefined; + this.#direntCache = undefined; + this.#prefetchLoad = undefined; + } + + // Drop the snapshot. Called from every mutating method so a write + // inside a prefetch scope is never masked by cached listings. + #invalidatePrefetch(): void { + this.#prefetchGeneration += 1; + this.#direntCache = undefined; + this.#prefetchLoad = undefined; + } + + // True when `path` sits inside the active prefetch root. + #withinPrefetch(path: string): boolean { + if (this.#prefetchDepth === 0 || this.#prefetchRoot === undefined) return false; + if (this.#prefetchRoot === "/") return true; + return path === this.#prefetchRoot || path.startsWith(`${this.#prefetchRoot}/`); + } + + // Build the directory -> entries map for the whole prefetched + // subtree from one find() call. Concurrent callers share the load. + async #ensurePrefetch(): Promise | undefined> { + const root = this.#prefetchRoot; + if (root === undefined) return undefined; + if (this.#direntCache !== undefined) return this.#direntCache; + if (this.#prefetchLoad === undefined) { + const generation = this.#prefetchGeneration; + this.#prefetchLoad = (async () => { + const found = await this.#fs.find(root); + const cache = new Map(); + // The root itself always exists as a (possibly empty) bucket so + // an empty directory reads as empty rather than missing. + cache.set(root, []); + for (const entry of found) { + const slash = entry.path.lastIndexOf("/"); + const parent = slash <= 0 ? "/" : entry.path.slice(0, slash); + const name = entry.path.slice(slash + 1); + let bucket = cache.get(parent); + if (bucket === undefined) { + bucket = []; + cache.set(parent, bucket); + } + // dofs's find reports "symlink" at runtime even though the + // published WorkspaceFoundEntry union is narrower, so widen + // here rather than mislabelling links as regular files. + const type = entry.type as "file" | "dir" | "symlink"; + const isDirectory = type === "dir"; + const isSymbolicLink = type === "symlink"; + bucket.push({ + name, + isFile: !isDirectory && !isSymbolicLink, + isDirectory, + isSymbolicLink, + }); + if (isDirectory && !cache.has(entry.path)) cache.set(entry.path, []); + } + // A mutation may have invalidated this load while find() was + // in flight. Only the current generation may publish a cache. + if (this.#prefetchGeneration === generation && this.#prefetchRoot === root) { + this.#direntCache = cache; + } + })(); + } + const load = this.#prefetchLoad; + try { + await load; + } catch { + // A failed prefetch must not fail the command: fall back to + // direct listings for the rest of the scope. + if (this.#prefetchLoad === load) this.#prefetchLoad = undefined; + return undefined; + } + return this.#direntCache; + } + // --- Reads ------------------------------------------------------- async readFile(path: string, _options?: AdapterReadFileOptions): Promise { @@ -130,6 +254,18 @@ export class WorkspaceFsAdapter { if (isDevDir(path)) { return [{ name: "null", isFile: true, isDirectory: false, isSymbolicLink: false }]; } + const normalized = normalizePath(path); + if (this.#withinPrefetch(normalized)) { + const cache = await this.#ensurePrefetch(); + const hit = cache?.get(normalized); + if (hit !== undefined) return hit.map((entry) => ({ ...entry })); + if (cache !== undefined) { + // Inside the prefetched subtree but absent from the snapshot: + // the directory does not exist. Surface the same error a direct + // listing would. + throw createWorkspaceError("ENOENT", "no such file or directory", path); + } + } const entries = await this.#fs.readdir(path); return entries.map((e) => ({ name: e.name, @@ -150,11 +286,13 @@ export class WorkspaceFsAdapter { async writeFile(path: string, content: string | Uint8Array, _options?: unknown): Promise { if (isDevNull(path)) return; + this.#invalidatePrefetch(); await this.#fs.writeFile(path, content); } async appendFile(path: string, content: string | Uint8Array, _options?: unknown): Promise { if (isDevNull(path)) return; + this.#invalidatePrefetch(); let existing: Uint8Array; try { const stream = await this.#fs.readFile(path); @@ -172,16 +310,19 @@ export class WorkspaceFsAdapter { async mkdir(path: string, options?: { recursive?: boolean }): Promise { if (isDevDir(path)) return; + this.#invalidatePrefetch(); await this.#fs.mkdir(path, options); } async rm(path: string, options?: { recursive?: boolean; force?: boolean }): Promise { if (isVirtualDevPath(path)) return; + this.#invalidatePrefetch(); await this.#fs.rm(path, options); } async chmod(path: string, mode: number): Promise { if (isVirtualDevPath(path)) return; + this.#invalidatePrefetch(); await this.#fs.chmod(path, mode); } @@ -189,12 +330,14 @@ export class WorkspaceFsAdapter { if (isVirtualDevPath(linkPath)) { throw createWorkspaceError("EEXIST", "file exists", linkPath); } + this.#invalidatePrefetch(); await this.#fs.symlink(target, linkPath); } // --- Composites -------------------------------------------------- async cp(src: string, dest: string, options?: { recursive?: boolean }): Promise { + this.#invalidatePrefetch(); const recursive = options?.recursive === true; const s = await this.#fs.stat(src); if (s.isDirectory) { diff --git a/packages/computer/src/backends/worker-shell/dirent-prefetch.test.ts b/packages/computer/src/backends/worker-shell/dirent-prefetch.test.ts new file mode 100644 index 00000000..95a1cd8c --- /dev/null +++ b/packages/computer/src/backends/worker-shell/dirent-prefetch.test.ts @@ -0,0 +1,260 @@ +// Tests for the adapter's directory prefetch cache. +// +// just-bash's find/grep walk the tree with one readdirWithFileTypes per +// directory. Against the workspace stub each is an RPC, so a recursive +// command over a tree containing node_modules costs thousands of round +// trips. The adapter can serve that walk from a single subtree read. +// +// The cache is only safe if it can never return stale data, so the +// invalidation contract is tested as carefully as the speedup. + +import { SQLiteTestStorage } from "@cloudflare/dofs/testing"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +import type { BackendHandle, WorkspaceBackend } from "../../backend.js"; +import { WorkspaceFilesystemStub } from "../../stub.js"; +import { Workspace } from "../../workspace.js"; +import { WorkspaceFsAdapter } from "./adapter.js"; + +function noopBackend(): WorkspaceBackend { + return { + id: "noop", + async connect(): Promise { + return { + rpc: { sync: {} as never, shell: {} as never }, + sync: "none", + close: async () => {}, + }; + }, + }; +} + +let workspace: Workspace; +let stub: WorkspaceFilesystemStub; +let adapter: WorkspaceFsAdapter; + +beforeEach(async () => { + workspace = new Workspace({ + storage: new SQLiteTestStorage() as never, + backends: [noopBackend()], + }); + await workspace.ready(); + stub = new WorkspaceFilesystemStub(workspace); + adapter = new WorkspaceFsAdapter(stub); +}); + +afterEach(async () => { + await workspace.close(); +}); + +async function seedTree(): Promise { + for (let p = 0; p < 12; p++) { + await workspace.fs.mkdir(`/node_modules/pkg-${p}/dist`, { recursive: true }); + await workspace.fs.writeFile(`/node_modules/pkg-${p}/package.json`, "{}\n"); + await workspace.fs.writeFile(`/node_modules/pkg-${p}/dist/index.js`, "vendor\n"); + } + await workspace.fs.mkdir("/src/deep", { recursive: true }); + await workspace.fs.writeFile("/src/app.ts", "source\n"); + await workspace.fs.writeFile("/src/deep/mod.ts", "deep\n"); +} + +describe("directory prefetch", () => { + it("serves a recursive walk from a single underlying call", async () => { + await seedTree(); + const readdir = vi.spyOn(stub, "readdir"); + const find = vi.spyOn(stub, "find"); + + adapter.beginPrefetch("/"); + try { + // Walk the whole tree the way just-bash's find does. + const seen: string[] = []; + const walk = async (dir: string): Promise => { + for (const entry of await adapter.readdirWithFileTypes(dir)) { + const child = dir === "/" ? `/${entry.name}` : `${dir}/${entry.name}`; + seen.push(child); + if (entry.isDirectory) await walk(child); + } + }; + await walk("/"); + expect(seen).toContain("/src/app.ts"); + expect(seen).toContain("/node_modules/pkg-0/dist/index.js"); + } finally { + adapter.endPrefetch(); + } + + // One subtree read replaces the per-directory listings. + expect(find).toHaveBeenCalledTimes(1); + expect(readdir).not.toHaveBeenCalled(); + }); + + it("returns exactly what readdirWithFileTypes returns uncached", async () => { + await seedTree(); + const uncached = await adapter.readdirWithFileTypes("/src"); + + adapter.beginPrefetch("/"); + const cached = await adapter.readdirWithFileTypes("/src"); + adapter.endPrefetch(); + + const sort = (xs: Array<{ name: string }>) => + [...xs].sort((a, b) => a.name.localeCompare(b.name)); + expect(sort(cached)).toEqual(sort(uncached)); + }); + + it("reports directories, files and symlinks with the same flags", async () => { + await workspace.fs.mkdir("/d/sub", { recursive: true }); + await workspace.fs.writeFile("/d/file.txt", "x"); + await workspace.fs.symlink("/d/file.txt", "/d/link"); + + const uncached = await adapter.readdirWithFileTypes("/d"); + adapter.beginPrefetch("/"); + const cached = await adapter.readdirWithFileTypes("/d"); + adapter.endPrefetch(); + + const byName = (xs: Array<{ name: string }>) => Object.fromEntries(xs.map((e) => [e.name, e])); + expect(byName(cached)).toEqual(byName(uncached)); + }); + + it("does not cache across prefetch scopes", async () => { + await workspace.fs.writeFile("/a.txt", "one"); + adapter.beginPrefetch("/"); + expect((await adapter.readdirWithFileTypes("/")).map((e) => e.name)).toEqual(["a.txt"]); + adapter.endPrefetch(); + + // A write between scopes must be visible in the next scope. + await workspace.fs.writeFile("/b.txt", "two"); + adapter.beginPrefetch("/"); + const names = (await adapter.readdirWithFileTypes("/")).map((e) => e.name).sort(); + adapter.endPrefetch(); + expect(names).toEqual(["a.txt", "b.txt"]); + }); + + it("invalidates the cache when a write happens inside the scope", async () => { + await workspace.fs.writeFile("/a.txt", "one"); + adapter.beginPrefetch("/"); + try { + expect((await adapter.readdirWithFileTypes("/")).map((e) => e.name)).toEqual(["a.txt"]); + // A mutation through the adapter must drop the cached listing so + // the next read observes it. + await adapter.writeFile("/b.txt", "two"); + const names = (await adapter.readdirWithFileTypes("/")).map((e) => e.name).sort(); + expect(names).toEqual(["a.txt", "b.txt"]); + } finally { + adapter.endPrefetch(); + } + }); + + it("does not restore an in-flight snapshot after a write", async () => { + await workspace.fs.writeFile("/a.txt", "one"); + const originalFind = stub.find.bind(stub); + let releaseFind: (() => void) | undefined; + const findGate = new Promise((resolve) => { + releaseFind = resolve; + }); + const find = vi.spyOn(stub, "find").mockImplementation(async (...args) => { + const snapshot = await originalFind(...args); + await findGate; + return snapshot; + }); + + adapter.beginPrefetch("/"); + try { + const firstRead = adapter.readdirWithFileTypes("/"); + await vi.waitFor(() => expect(find).toHaveBeenCalledTimes(1)); + await adapter.writeFile("/b.txt", "two"); + releaseFind?.(); + await firstRead; + + const names = (await adapter.readdirWithFileTypes("/")).map((e) => e.name).sort(); + expect(names).toEqual(["a.txt", "b.txt"]); + expect(find).toHaveBeenCalledTimes(2); + } finally { + releaseFind?.(); + adapter.endPrefetch(); + } + }); + + it("invalidates on rm, mkdir and symlink too", async () => { + await workspace.fs.writeFile("/keep.txt", "x"); + await workspace.fs.writeFile("/gone.txt", "x"); + + adapter.beginPrefetch("/"); + try { + expect((await adapter.readdirWithFileTypes("/")).length).toBe(2); + await adapter.rm("/gone.txt", {}); + expect((await adapter.readdirWithFileTypes("/")).map((e) => e.name)).toEqual(["keep.txt"]); + await adapter.mkdir("/newdir", {}); + expect((await adapter.readdirWithFileTypes("/")).map((e) => e.name).sort()).toEqual([ + "keep.txt", + "newdir", + ]); + await adapter.symlink("/keep.txt", "/alias"); + expect((await adapter.readdirWithFileTypes("/")).map((e) => e.name).sort()).toEqual([ + "alias", + "keep.txt", + "newdir", + ]); + } finally { + adapter.endPrefetch(); + } + }); + + it("is a no-op when no prefetch scope is active", async () => { + await seedTree(); + const find = vi.spyOn(stub, "find"); + const readdir = vi.spyOn(stub, "readdir"); + await adapter.readdirWithFileTypes("/src"); + expect(find).not.toHaveBeenCalled(); + expect(readdir).toHaveBeenCalledTimes(1); + }); + + it("falls back to a direct listing for paths outside the prefetched root", async () => { + await seedTree(); + adapter.beginPrefetch("/src"); + try { + const readdir = vi.spyOn(stub, "readdir"); + const entries = await adapter.readdirWithFileTypes("/node_modules"); + expect(entries.length).toBe(12); + expect(readdir).toHaveBeenCalledTimes(1); + } finally { + adapter.endPrefetch(); + } + }); + + it("reports an empty directory as empty rather than missing", async () => { + await workspace.fs.mkdir("/empty", { recursive: true }); + adapter.beginPrefetch("/"); + try { + expect(await adapter.readdirWithFileTypes("/empty")).toEqual([]); + } finally { + adapter.endPrefetch(); + } + }); + + it("still throws ENOENT for a missing directory inside a scope", async () => { + adapter.beginPrefetch("/"); + try { + await expect(adapter.readdirWithFileTypes("/missing")).rejects.toMatchObject({ + code: "ENOENT", + }); + } finally { + adapter.endPrefetch(); + } + }); + + it("endPrefetch is safe to call without a matching begin", () => { + expect(() => adapter.endPrefetch()).not.toThrow(); + }); + + it("nested scopes keep the outermost prefetch until fully unwound", async () => { + await seedTree(); + const find = vi.spyOn(stub, "find"); + adapter.beginPrefetch("/"); + adapter.beginPrefetch("/src"); + await adapter.readdirWithFileTypes("/src"); + adapter.endPrefetch(); + // Inner scope closing must not discard the outer cache. + await adapter.readdirWithFileTypes("/node_modules"); + adapter.endPrefetch(); + expect(find).toHaveBeenCalledTimes(1); + }); +}); diff --git a/packages/computer/src/backends/worker-shell/entrypoint.ts b/packages/computer/src/backends/worker-shell/entrypoint.ts index 492d2c99..21bb44e5 100644 --- a/packages/computer/src/backends/worker-shell/entrypoint.ts +++ b/packages/computer/src/backends/worker-shell/entrypoint.ts @@ -19,11 +19,11 @@ import { WorkerEntrypoint } from "cloudflare:workers"; import { Bash, type CustomCommand, type SecureFetch } from "just-bash"; - import { WorkspaceFsAdapter } from "./adapter.js"; import { type ArtifactsCommandHost, defineArtifactsCommand } from "./artifacts-command.js"; import { type AssetsCommandHost, defineAssetsCommand } from "./assets-command.js"; import { defineGitCommand, type GitCommandHost } from "./git-command.js"; +import { prefetchRootFor } from "./prefetch-policy.js"; export interface ExecInput { command: string; @@ -218,10 +218,14 @@ export class ShellWorker< customCommands, }); } else { + const adapter = new WorkspaceFsAdapter(ws.fs); + // A recursive find/grep would otherwise walk the tree one + // readdir RPC per directory. Let the adapter answer that walk + // from a single subtree read for the duration of this command. + const prefetchRoot = prefetchRootFor(input.command, cwd); + if (prefetchRoot !== undefined) adapter.beginPrefetch(prefetchRoot); const bash = new Bash({ - fs: new WorkspaceFsAdapter(ws.fs) as unknown as NonNullable< - ConstructorParameters[0] - >["fs"], + fs: adapter as unknown as NonNullable[0]>["fs"], cwd, fetch: this.shellOptions.fetch === null @@ -239,14 +243,19 @@ export class ShellWorker< defenseInDepth: { enabled: false }, executionLimits: { maxOutputSize: MAX_OUTPUT_BYTES }, }); - result = await bash.exec(input.command, { - cwd, - env: input.env, - ...(input.stdin !== undefined - ? { stdin: latin1FromBytes(input.stdin), stdinKind: "bytes" as const } - : {}), - signal: controller.signal, - }); + try { + result = await bash.exec(input.command, { + cwd, + env: input.env, + ...(input.stdin !== undefined + ? { stdin: latin1FromBytes(input.stdin), stdinKind: "bytes" as const } + : {}), + signal: controller.signal, + }); + } finally { + // The snapshot must not outlive the command that opened it. + if (prefetchRoot !== undefined) adapter.endPrefetch(); + } } } catch (error) { const message = error instanceof Error ? error.message : String(error); diff --git a/packages/computer/src/backends/worker-shell/prefetch-policy.test.ts b/packages/computer/src/backends/worker-shell/prefetch-policy.test.ts new file mode 100644 index 00000000..a3b29083 --- /dev/null +++ b/packages/computer/src/backends/worker-shell/prefetch-policy.test.ts @@ -0,0 +1,75 @@ +// When should the shell open a directory-prefetch scope? +// +// Prefetching is only a win for commands that walk a subtree. Opening +// a scope for a command that touches one file wastes a full subtree +// read, so the policy has to be conservative in both directions: +// engage for recursive traversals, stay out of the way otherwise. + +import { describe, expect, it } from "vitest"; + +import { prefetchRootFor } from "./prefetch-policy.js"; + +describe("prefetchRootFor", () => { + const cwd = "/w"; + + it("engages for a recursive grep", () => { + expect(prefetchRootFor("grep -rn NEEDLE .", cwd)).toBe("/w"); + expect(prefetchRootFor("grep -r NEEDLE src", cwd)).toBe("/w/src"); + expect(prefetchRootFor("grep -R NEEDLE /abs", cwd)).toBe("/abs"); + }); + + it("engages for find", () => { + expect(prefetchRootFor("find .", cwd)).toBe("/w"); + expect(prefetchRootFor("find src -name '*.ts'", cwd)).toBe("/w/src"); + expect(prefetchRootFor("find /abs -type f", cwd)).toBe("/abs"); + }); + + it("defaults find with no path operand to the cwd", () => { + expect(prefetchRootFor("find -name '*.ts'", cwd)).toBe("/w"); + }); + + it("stays out of the way for a non-recursive grep", () => { + expect(prefetchRootFor("grep NEEDLE file.txt", cwd)).toBeUndefined(); + }); + + it("stays out of the way for unrelated commands", () => { + expect(prefetchRootFor("cat file.txt", cwd)).toBeUndefined(); + expect(prefetchRootFor("ls -la", cwd)).toBeUndefined(); + expect(prefetchRootFor("echo hello", cwd)).toBeUndefined(); + expect(prefetchRootFor("", cwd)).toBeUndefined(); + }); + + it("engages when a traversal appears anywhere in a pipeline", () => { + expect(prefetchRootFor("find . -name '*.ts' | head -5", cwd)).toBe("/w"); + expect(prefetchRootFor("ls && grep -r NEEDLE src", cwd)).toBe("/w/src"); + }); + + it("prefers the shallowest root when several traversals appear", () => { + // Two traversals in one line: the scope has to cover both, so the + // common ancestor is the safe choice. + expect(prefetchRootFor("grep -r A src; grep -r B lib", cwd)).toBe("/w"); + expect(prefetchRootFor("grep -r A src/x; grep -r B src/y", cwd)).toBe("/w/src"); + }); + + it("does not engage for a command that only writes", () => { + expect(prefetchRootFor("rm -rf node_modules", cwd)).toBeUndefined(); + expect(prefetchRootFor("cp -r a b", cwd)).toBeUndefined(); + }); + + it("skips traversals that mutate, where a snapshot could mislead", () => { + // -delete and -exec change the tree mid-walk; a cached listing + // would describe entries that no longer exist. + expect(prefetchRootFor("find . -name '*.log' -delete", cwd)).toBeUndefined(); + expect(prefetchRootFor("find . -exec rm {} ;", cwd)).toBeUndefined(); + }); + + it("ignores flag arguments when picking the start path", () => { + expect(prefetchRootFor("grep -r --color NEEDLE src", cwd)).toBe("/w/src"); + expect(prefetchRootFor("find src -maxdepth 2 -name x", cwd)).toBe("/w/src"); + }); + + it("normalises relative traversal roots", () => { + expect(prefetchRootFor("find ./src/../lib", cwd)).toBe("/w/lib"); + expect(prefetchRootFor("grep -r NEEDLE ..", cwd)).toBe("/"); + }); +}); diff --git a/packages/computer/src/backends/worker-shell/prefetch-policy.ts b/packages/computer/src/backends/worker-shell/prefetch-policy.ts new file mode 100644 index 00000000..0dd41194 --- /dev/null +++ b/packages/computer/src/backends/worker-shell/prefetch-policy.ts @@ -0,0 +1,138 @@ +// Decides whether a shell command is worth opening a directory +// prefetch scope for, and over which root. +// +// The adapter can answer a whole subtree walk from one server-side +// find(), but loading that snapshot costs a call, so it only pays off +// for commands that would otherwise issue one readdir per directory: +// `find` and recursive `grep`. +// +// This is a heuristic over the raw command string, deliberately kept +// simple. Getting it wrong in the "don't prefetch" direction costs +// nothing but the status quo; getting it wrong in the "do prefetch" +// direction costs one extra call and, for mutating traversals, could +// serve a listing that the command itself has invalidated. So the +// rules below are conservative: recognise the clear cases, decline +// everything else. + +function normalizePath(path: string): string { + const parts = path.split("/"); + const stack: string[] = []; + for (const part of parts) { + if (part === "" || part === ".") continue; + if (part === "..") { + stack.pop(); + continue; + } + stack.push(part); + } + return `/${stack.join("/")}`; +} + +function resolveAgainst(cwd: string, path: string): string { + if (path.startsWith("/")) return normalizePath(path); + return normalizePath(`${cwd}/${path}`); +} + +// Longest common ancestor of two absolute paths. +function commonAncestor(a: string, b: string): string { + if (a === b) return a; + const left = a.split("/").filter(Boolean); + const right = b.split("/").filter(Boolean); + const out: string[] = []; + for (let i = 0; i < Math.min(left.length, right.length); i++) { + if (left[i] !== right[i]) break; + out.push(left[i]); + } + return `/${out.join("/")}`; +} + +// Split a command line into pipeline/list segments so a traversal +// anywhere in the line is found. Quoting is not interpreted; a +// separator inside quotes only ever splits a segment into two, which +// at worst makes us decline to prefetch. +function segments(command: string): string[] { + return command + .split(/\|\||&&|[|;\n]/) + .map((part) => part.trim()) + .filter((part) => part.length > 0); +} + +// Tokenise on whitespace, stripping one layer of surrounding quotes. +function tokenize(segment: string): string[] { + const out: string[] = []; + const re = /"([^"]*)"|'([^']*)'|(\S+)/g; + let match = re.exec(segment); + while (match !== null) { + out.push(match[1] ?? match[2] ?? match[3] ?? ""); + match = re.exec(segment); + } + return out; +} + +// find expressions that change the tree as they walk. A snapshot taken +// before the walk would describe entries the command then removes, so +// decline rather than risk serving a stale listing. +const FIND_MUTATING = new Set(["-delete", "-exec", "-execdir", "-ok", "-okdir"]); + +function findRoot(tokens: string[], cwd: string): string | undefined { + for (const token of tokens) { + if (FIND_MUTATING.has(token)) return undefined; + } + // Start paths are the operands before the first expression token. + const starts: string[] = []; + for (let i = 1; i < tokens.length; i++) { + const token = tokens[i]; + if (token.startsWith("-") || token === "(" || token === ")" || token === "!") break; + starts.push(token); + } + if (starts.length === 0) return cwd; + let root = resolveAgainst(cwd, starts[0]); + for (const start of starts.slice(1)) root = commonAncestor(root, resolveAgainst(cwd, start)); + return root; +} + +function grepRoot(tokens: string[], cwd: string): string | undefined { + let recursive = false; + const operands: string[] = []; + for (let i = 1; i < tokens.length; i++) { + const token = tokens[i]; + if (token === "--") { + operands.push(...tokens.slice(i + 1)); + break; + } + if (token.startsWith("--")) continue; + if (token.startsWith("-") && token.length > 1) { + if (/[rR]/.test(token.slice(1))) recursive = true; + continue; + } + operands.push(token); + } + if (!recursive) return undefined; + // operands[0] is the pattern; the rest are search roots. + const paths = operands.slice(1); + if (paths.length === 0) return cwd; + let root = resolveAgainst(cwd, paths[0]); + for (const path of paths.slice(1)) root = commonAncestor(root, resolveAgainst(cwd, path)); + return root; +} + +/** + * The directory to prefetch for `command`, or undefined when the + * command would not benefit. When several traversals appear in one + * line the shallowest common ancestor is returned so a single scope + * covers them all. + */ +export function prefetchRootFor(command: string, cwd: string): string | undefined { + let root: string | undefined; + for (const segment of segments(command)) { + const tokens = tokenize(segment); + if (tokens.length === 0) continue; + const name = tokens[0]; + let candidate: string | undefined; + if (name === "find") candidate = findRoot(tokens, cwd); + else if (name === "grep") candidate = grepRoot(tokens, cwd); + if (candidate === undefined) continue; + root = root === undefined ? candidate : commonAncestor(root, candidate); + } + return root; +} diff --git a/packages/computer/src/git/diff.test.ts b/packages/computer/src/git/diff.test.ts index 923744fd..50f3dedb 100644 --- a/packages/computer/src/git/diff.test.ts +++ b/packages/computer/src/git/diff.test.ts @@ -31,6 +31,10 @@ async function init(): Promise { } async function commitFile(path: string, content: string, message: string): Promise { + const slash = path.lastIndexOf("/"); + if (slash > 0) { + await memfs.promises.mkdir(`${DIR}/${path.slice(0, slash)}`, { recursive: true }); + } await memfs.promises.writeFile(`${DIR}/${path}`, content); await git.add({ fs: memfs, dir: DIR, filepath: path }); return git.commit({ fs: memfs, dir: DIR, message, author: AUTHOR }); @@ -42,7 +46,7 @@ async function stageThenRemove(path: string): Promise { await memfs.promises.unlink(`${DIR}/${path}`); } -async function runDiff(opts: { ref?: string } = {}): Promise { +async function runDiff(opts: { ref?: string; paths?: string[] } = {}): Promise { return diffWith({ git: isomorphicGit, fs: memfs, @@ -50,6 +54,7 @@ async function runDiff(opts: { ref?: string } = {}): Promise { readFile: (path) => memfs.promises.readFile(path) as Promise, dir: DIR, ref: opts.ref, + paths: opts.paths, }); } @@ -156,6 +161,70 @@ describe("diffWith (real isomorphic-git + memfs)", () => { } }); + it("scopes the status walk to the requested paths", async () => { + await init(); + await commitFile("src/a.txt", "one\n", "init a"); + await commitFile("vendor/b.txt", "two\n", "init b"); + await memfs.promises.writeFile(`${DIR}/src/a.txt`, "one changed\n"); + await memfs.promises.writeFile(`${DIR}/vendor/b.txt`, "two changed\n"); + + const statusSpy = vi.spyOn(git, "statusMatrix"); + try { + const out = await runDiff({ paths: ["src"] }); + // The walk must be told to stay inside `src` rather than + // scanning the whole tree and filtering afterwards. + expect(statusSpy.mock.calls[0][0]).toMatchObject({ filepaths: ["src"] }); + // Result is unchanged by the scoping. + expect(out).toContain("+one changed"); + expect(out).not.toContain("two changed"); + } finally { + statusSpy.mockRestore(); + } + }); + + it("walks the whole tree when no paths are given", async () => { + await init(); + await commitFile("a.txt", "one\n", "init"); + await memfs.promises.writeFile(`${DIR}/a.txt`, "one changed\n"); + + const statusSpy = vi.spyOn(git, "statusMatrix"); + try { + await runDiff(); + // No scope requested: isomorphic-git's default ('.') must stand. + expect(statusSpy.mock.calls[0][0]).not.toHaveProperty("filepaths"); + } finally { + statusSpy.mockRestore(); + } + }); + + it("produces the same diff scoped and unscoped", async () => { + await init(); + await commitFile("src/a.txt", "one\n", "init a"); + await commitFile("vendor/b.txt", "two\n", "init b"); + await memfs.promises.writeFile(`${DIR}/src/a.txt`, "one changed\n"); + await memfs.promises.writeFile(`${DIR}/vendor/b.txt`, "two changed\n"); + + const scoped = await runDiff({ paths: ["src"] }); + const full = await runDiff(); + // The scoped run must equal the src-only slice of the full run. + expect(scoped).toContain("+one changed"); + expect(full).toContain("+one changed"); + expect(full).toContain("+two changed"); + expect(scoped).not.toContain("+two changed"); + }); + + it("scopes correctly for an exact file path", async () => { + await init(); + await commitFile("src/a.txt", "one\n", "init a"); + await commitFile("src/b.txt", "two\n", "init b"); + await memfs.promises.writeFile(`${DIR}/src/a.txt`, "one changed\n"); + await memfs.promises.writeFile(`${DIR}/src/b.txt`, "two changed\n"); + + const out = await runDiff({ paths: ["src/a.txt"] }); + expect(out).toContain("+one changed"); + expect(out).not.toContain("two changed"); + }); + it("respects the `ref` argument when diffing against an older commit", async () => { await init(); const first = await commitFile("a.txt", "v1\n", "v1"); diff --git a/packages/computer/src/git/diff.ts b/packages/computer/src/git/diff.ts index e71c7b5e..25fd304e 100644 --- a/packages/computer/src/git/diff.ts +++ b/packages/computer/src/git/diff.ts @@ -29,6 +29,8 @@ export interface IsomorphicGitDiffClient { dir: string; ref?: string; cache?: object; + /** Prunes the worktree walk to these paths. Omit to walk it all. */ + filepaths?: string[]; }): Promise; readBlob(args: { fs: object; @@ -166,7 +168,21 @@ async function collectDiffEntries(opts: DiffWithDeps): Promise { // requested commit rather than always HEAD. Without this the // `ref` argument would only affect blob reads, leaving the // status walk silently skewed. - const status = await opts.git.statusMatrix({ fs: opts.fs, dir, ref, cache: opts.cache }); + // Scope the walk when the caller named paths. isomorphic-git prunes + // the traversal to these prefixes instead of visiting the whole + // worktree and handing every path to `map`, which matters once the + // tree carries a synced node_modules. `filepaths` uses the same + // "exact path or directory prefix" rule as makePathFilter below, so + // the filter stays as the authority on what is emitted and this is + // purely a traversal hint. + const scopedPaths = normalizeFilepaths(opts.paths); + const status = await opts.git.statusMatrix({ + fs: opts.fs, + dir, + ref, + cache: opts.cache, + ...(scopedPaths === undefined ? {} : { filepaths: scopedPaths }), + }); const pathFilter = makePathFilter(opts.paths); const entries: DiffEntry[] = []; for (const [filepath, headStatus, workdirStatus] of status) { @@ -262,6 +278,17 @@ async function listFilesAt( return git.listFiles({ fs, dir, ref }); } +// The `filepaths` form isomorphic-git wants: normalized, deduplicated, +// and undefined when the caller asked for no scoping (so the library's +// own default of ['.'] stands). An empty string normalizes to the repo +// root, which would scope to everything — treat it as no scope. +function normalizeFilepaths(paths: string[] | undefined): string[] | undefined { + if (paths === undefined || paths.length === 0) return undefined; + const out = [...new Set(paths.map((p) => normalizePath(p)))]; + if (out.some((p) => p === "" || p === ".")) return undefined; + return out; +} + function makePathFilter(paths: string[] | undefined): (p: string) => boolean { if (paths === undefined || paths.length === 0) return () => true; // Match either an exact path or a directory prefix. Globs are diff --git a/packages/dofs/README.md b/packages/dofs/README.md index 94bd285f..5a1f0dac 100644 --- a/packages/dofs/README.md +++ b/packages/dofs/README.md @@ -47,7 +47,8 @@ export class WorkspaceDO extends DurableObject { - All filesystem primitives listed above are implemented and unit-tested. `readdir` returns size and modification time and supports stable `limit`/`offset` pages, including files held in pending write buffers. - `find` supports `*`, `**`, and `?` globs. `grep` supports bounded pages, + `find` supports `*`, `**`, and `?` globs. `find` and `grep` can prune + directories by exact path-segment name. `grep` supports bounded pages, regular expressions or fixed strings, explicit case handling, and numbered context lines. - `SQLiteWorkspaceProvider` (the `@platformatic/vfs` adapter) implemented and exported from the package entrypoint; consumed by `@cloudflare/computerd`. diff --git a/packages/dofs/src/fs/find.test.ts b/packages/dofs/src/fs/find.test.ts index b2b470ba..b6fb20c7 100644 --- a/packages/dofs/src/fs/find.test.ts +++ b/packages/dofs/src/fs/find.test.ts @@ -142,4 +142,115 @@ describe("find", () => { expect(paths).toEqual(["/a/file.ts"]); }); }); + + describe("exclude", () => { + async function tree(db: Parameters[0]) { + mkdir(db, "/node_modules/pkg/dist", { recursive: true }, () => 0); + mkdir(db, "/src/nested", { recursive: true }, () => 0); + await writeFile(db, "/node_modules/pkg/dist/index.js", "", {}, () => 0); + await writeFile(db, "/node_modules/pkg/package.json", "", {}, () => 0); + await writeFile(db, "/src/app.ts", "", {}, () => 0); + await writeFile(db, "/src/nested/deep.ts", "", {}, () => 0); + } + + it("omits an excluded directory and everything beneath it", async () => { + await withDB(async (db) => { + await tree(db); + const paths = find(db, "/", undefined, { exclude: ["node_modules"] }) + .map((e) => e.path) + .sort(); + expect(paths).toEqual(["/src", "/src/app.ts", "/src/nested", "/src/nested/deep.ts"]); + }); + }); + + it("matches an excluded segment at any depth", async () => { + await withDB(async (db) => { + mkdir(db, "/a/node_modules/deep", { recursive: true }, () => 0); + await writeFile(db, "/a/node_modules/deep/x.js", "", {}, () => 0); + await writeFile(db, "/a/keep.ts", "", {}, () => 0); + const paths = find(db, "/", undefined, { exclude: ["node_modules"] }) + .map((e) => e.path) + .sort(); + expect(paths).toEqual(["/a", "/a/keep.ts"]); + }); + }); + + it("excludes matching files as well as directories", async () => { + await withDB(async (db) => { + mkdir(db, "/d", {}, () => 0); + await writeFile(db, "/d/keep.ts", "", {}, () => 0); + await writeFile(db, "/d/skip.log", "", {}, () => 0); + const paths = find(db, "/", undefined, { exclude: ["skip.log"] }) + .map((e) => e.path) + .sort(); + expect(paths).toEqual(["/d", "/d/keep.ts"]); + }); + }); + + it("does not match a partial segment", async () => { + await withDB(async (db) => { + mkdir(db, "/node_modules_extra", {}, () => 0); + await writeFile(db, "/node_modules_extra/a.ts", "", {}, () => 0); + const paths = find(db, "/", undefined, { exclude: ["node_modules"] }) + .map((e) => e.path) + .sort(); + expect(paths).toEqual(["/node_modules_extra", "/node_modules_extra/a.ts"]); + }); + }); + + it("accepts several exclude patterns", async () => { + await withDB(async (db) => { + await tree(db); + mkdir(db, "/dist", {}, () => 0); + await writeFile(db, "/dist/out.js", "", {}, () => 0); + const paths = find(db, "/", undefined, { exclude: ["node_modules", "dist"] }) + .map((e) => e.path) + .sort(); + expect(paths).toEqual(["/src", "/src/app.ts", "/src/nested", "/src/nested/deep.ts"]); + }); + }); + + it("combines with a pattern", async () => { + await withDB(async (db) => { + await tree(db); + const paths = find(db, "/", "**/*.ts", { exclude: ["node_modules"] }) + .map((e) => e.path) + .sort(); + expect(paths).toEqual(["/src/app.ts", "/src/nested/deep.ts"]); + }); + }); + + it("an empty exclude list behaves like no exclude", async () => { + await withDB(async (db) => { + await tree(db); + const withEmpty = find(db, "/", undefined, { exclude: [] }).map((e) => e.path); + const without = find(db, "/").map((e) => e.path); + expect(withEmpty.sort()).toEqual(without.sort()); + }); + }); + + it("never descends into an excluded directory", async () => { + await withDB(async (db) => { + mkdir(db, "/node_modules/a/b/c", { recursive: true }, () => 0); + await writeFile(db, "/node_modules/a/b/c/deep.js", "", {}, () => 0); + await writeFile(db, "/keep.ts", "", {}, () => 0); + + let statements = 0; + const originalAll = db.all.bind(db); + (db as unknown as { all: unknown }).all = (...args: unknown[]) => { + statements += 1; + return (originalAll as (...a: unknown[]) => unknown)(...args); + }; + try { + find(db, "/", undefined, { exclude: ["node_modules"] }); + } finally { + (db as unknown as { all: unknown }).all = originalAll; + } + + // Pruning must stop the walk at /node_modules itself: one readChildren + // for the root, and nothing for the excluded subtree's four levels. + expect(statements).toBeLessThanOrEqual(2); + }); + }); + }); }); diff --git a/packages/dofs/src/fs/find.ts b/packages/dofs/src/fs/find.ts index 5580dcc2..2e70a156 100644 --- a/packages/dofs/src/fs/find.ts +++ b/packages/dofs/src/fs/find.ts @@ -8,17 +8,38 @@ export interface WorkspaceFoundEntry { type: "file" | "dir"; } +// Same as WorkspaceFoundEntry but carries the inode the traversal +// already read. Internal callers that need to touch the node's +// content (grep) use this to skip a redundant path re-resolve. +export interface FoundEntryWithInode extends WorkspaceFoundEntry { + inode: number; + /** Cached vfs_nodes.size; 0 for directories. */ + size: number; +} + export interface FindOptions { /** Maximum matching entries to return. */ limit?: number; /** Matching entries to skip in traversal order. */ offset?: number; + /** + * Whole-segment names to skip. A directory whose name matches is + * never descended into, so its subtree costs nothing; a file whose + * name matches is not yielded. Matching is exact per path segment — + * "node_modules" does not match "node_modules_extra". + * + * Pruning happens during descent rather than filtering emitted + * entries, which is the whole point: excluding node_modules has to + * avoid walking it, not walk it and discard the results. + */ + exclude?: string[]; } interface ChildRow { name: string; child_inode: number; type: "file" | "dir"; + size: number; } interface WalkStart { @@ -26,6 +47,7 @@ interface WalkStart { path: string; prefix: string; regex: RegExp | undefined; + exclude: ReadonlySet; } const CHILD_PAGE_SIZE = 128; @@ -36,7 +58,7 @@ export function find( pattern?: string, options: FindOptions = {}, ): WorkspaceFoundEntry[] { - const start = prepareWalk(db, directory, pattern); + const start = prepareWalk(db, directory, pattern, options.exclude); const limit = options.limit ?? Number.MAX_SAFE_INTEGER; if (!Number.isSafeInteger(limit) || limit < 0) { throw new TypeError("find limit must be a non-negative safe integer"); @@ -49,9 +71,11 @@ export function find( const out: WorkspaceFoundEntry[] = []; let seen = 0; - for (const entry of walk(db, start.inode, start.path, start.prefix, start.regex)) { + for (const entry of walk(db, start.inode, start.path, start.prefix, start.regex, start.exclude)) { if (seen >= offset) { - out.push(entry); + // The walk carries the inode for internal callers; the public + // find() contract is {path, type} only. + out.push({ path: entry.path, type: entry.type }); if (out.length >= limit) break; } seen += 1; @@ -63,12 +87,18 @@ export function* iterateFoundEntries( db: Database, directory: string, pattern?: string, -): IterableIterator { - const start = prepareWalk(db, directory, pattern); - yield* walk(db, start.inode, start.path, start.prefix, start.regex); + exclude?: string[], +): IterableIterator { + const start = prepareWalk(db, directory, pattern, exclude); + yield* walk(db, start.inode, start.path, start.prefix, start.regex, start.exclude); } -function prepareWalk(db: Database, directory: string, pattern: string | undefined): WalkStart { +function prepareWalk( + db: Database, + directory: string, + pattern: string | undefined, + exclude: string[] | undefined, +): WalkStart { const { path: canonical } = canonicalizePath(directory); const node = resolveInode(db, canonical); if (node === null) { @@ -87,29 +117,42 @@ function prepareWalk(db: Database, directory: string, pattern: string | undefine path: canonical, prefix: canonical === "/" ? "/" : `${canonical}/`, regex, + exclude: exclude === undefined || exclude.length === 0 ? EMPTY_EXCLUDE : new Set(exclude), }; } +const EMPTY_EXCLUDE: ReadonlySet = new Set(); + function* walk( db: Database, parentInode: number, parentPath: string, prefix: string, regex: RegExp | undefined, -): IterableIterator { + exclude: ReadonlySet, +): IterableIterator { let afterName = ""; while (true) { const children = readChildren(db, parentInode, afterName); if (children.length === 0) return; for (const child of children) { + // Prune before doing anything else: an excluded directory is + // neither yielded nor descended into, so its entire subtree + // costs zero statements. + if (exclude.size > 0 && exclude.has(child.name)) continue; const childPath = parentPath === "/" ? `/${child.name}` : `${parentPath}/${child.name}`; const relativePath = childPath.slice(prefix.length); if (regex === undefined || regex.test(relativePath)) { - yield { path: childPath, type: child.type }; + yield { + path: childPath, + type: child.type, + inode: child.child_inode, + size: child.size, + }; } if (child.type === "dir") { - yield* walk(db, child.child_inode, childPath, prefix, regex); + yield* walk(db, child.child_inode, childPath, prefix, regex, exclude); } } @@ -120,7 +163,7 @@ function* walk( function readChildren(db: Database, parentInode: number, afterName: string): ChildRow[] { return db.all( - `SELECT d.name AS name, d.child_inode AS child_inode, n.type AS type + `SELECT d.name AS name, d.child_inode AS child_inode, n.type AS type, n.size AS size FROM vfs_dirents d JOIN vfs_nodes n ON n.inode = d.child_inode WHERE d.parent_inode = ? AND d.name > ? diff --git a/packages/dofs/src/fs/grep.test.ts b/packages/dofs/src/fs/grep.test.ts index 9c6c2383..5b079378 100644 --- a/packages/dofs/src/fs/grep.test.ts +++ b/packages/dofs/src/fs/grep.test.ts @@ -195,4 +195,107 @@ describe("grep", () => { await expect(grep(db, "x", "/missing")).rejects.toMatchObject({ code: "ENOENT" }); }); }); + + describe("exclude", () => { + async function tree(db: Parameters[0]) { + mkdir(db, "/node_modules/pkg", { recursive: true }, () => 0); + mkdir(db, "/src", { recursive: true }, () => 0); + await writeFile(db, "/node_modules/pkg/index.js", "NEEDLE in vendor\n", {}, () => 0); + await writeFile(db, "/src/app.ts", "NEEDLE in source\n", {}, () => 0); + } + + it("skips files under an excluded directory", async () => { + await withDB(async (db) => { + await tree(db); + const matches = await grep(db, "NEEDLE", "/", { exclude: ["node_modules"] }); + expect(matches.map((m) => m.path)).toEqual(["/src/app.ts"]); + }); + }); + + it("without exclude, still searches everything", async () => { + await withDB(async (db) => { + await tree(db); + const matches = await grep(db, "NEEDLE", "/"); + expect(matches.map((m) => m.path).sort()).toEqual([ + "/node_modules/pkg/index.js", + "/src/app.ts", + ]); + }); + }); + + it("combines exclude with include", async () => { + await withDB(async (db) => { + await tree(db); + await writeFile(db, "/src/other.js", "NEEDLE elsewhere\n", {}, () => 0); + const matches = await grep(db, "NEEDLE", "/", { + include: "**/*.ts", + exclude: ["node_modules"], + }); + expect(matches.map((m) => m.path)).toEqual(["/src/app.ts"]); + }); + }); + + it("issues fewer statements per file than a path re-resolve would", async () => { + await withDB(async (db) => { + mkdir(db, "/many", {}, () => 0); + const FILES = 40; + for (let i = 0; i < FILES; i++) { + await writeFile(db, `/many/f${i}.ts`, "NEEDLE here\n", {}, () => 0); + } + + let statements = 0; + const originalAll = db.all.bind(db); + const originalOne = db.one.bind(db); + (db as unknown as { all: unknown }).all = (...args: unknown[]) => { + statements += 1; + return (originalAll as (...a: unknown[]) => unknown)(...args); + }; + (db as unknown as { one: unknown }).one = (...args: unknown[]) => { + statements += 1; + return (originalOne as (...a: unknown[]) => unknown)(...args); + }; + let matches: Awaited>; + try { + matches = await grep(db, "NEEDLE", "/many"); + } finally { + (db as unknown as { all: unknown }).all = originalAll; + (db as unknown as { one: unknown }).one = originalOne; + } + + expect(matches).toHaveLength(FILES); + // The traversal already knows each file's inode, so grep must not + // re-resolve the path. That leaves the chunk read (and a blob read + // when the cache misses) per file, plus the directory listing. + expect(statements).toBeLessThanOrEqual(2 * FILES + 4); + }); + }); + + it("never reads files inside an excluded directory", async () => { + await withDB(async (db) => { + mkdir(db, "/node_modules/deep/nested", { recursive: true }, () => 0); + for (let i = 0; i < 20; i++) { + await writeFile(db, `/node_modules/deep/nested/f${i}.js`, "NEEDLE\n", {}, () => 0); + } + await writeFile(db, "/keep.ts", "NEEDLE\n", {}, () => 0); + + let statements = 0; + const originalAll = db.all.bind(db); + (db as unknown as { all: unknown }).all = (...args: unknown[]) => { + statements += 1; + return (originalAll as (...a: unknown[]) => unknown)(...args); + }; + let matches: Awaited>; + try { + matches = await grep(db, "NEEDLE", "/", { exclude: ["node_modules"] }); + } finally { + (db as unknown as { all: unknown }).all = originalAll; + } + + expect(matches.map((m) => m.path)).toEqual(["/keep.ts"]); + // Reading the 20 excluded files would take far more than this; + // pruning keeps the walk to the root listing plus /keep.ts. + expect(statements).toBeLessThan(10); + }); + }); + }); }); diff --git a/packages/dofs/src/fs/grep.ts b/packages/dofs/src/fs/grep.ts index 2eae9392..751a289b 100644 --- a/packages/dofs/src/fs/grep.ts +++ b/packages/dofs/src/fs/grep.ts @@ -2,7 +2,7 @@ import { createWorkspaceError } from "../errors.js"; import { canonicalizePath } from "../path.js"; import type { Database } from "../storage.js"; import { iterateFoundEntries } from "./find.js"; -import { readFile } from "./readFile.js"; +import { readCommittedFileByInode, readFile } from "./readFile.js"; import { resolveInode } from "./resolve.js"; export interface WorkspaceGrepContextLine { @@ -31,6 +31,13 @@ export interface GrepOptions { offset?: number; /** Glob relative to a searched directory that limits files. */ include?: string; + /** + * Whole-segment names to skip during traversal. Excluded directories + * are never descended into, so their files are never read. Unlike + * `include`, which filters files after the walk has already visited + * them, this prunes the walk itself. + */ + exclude?: string[]; } interface ScanState { @@ -68,11 +75,14 @@ export async function grep( }); const matches: WorkspaceGrepMatch[] = []; const state: ScanState = { seen: 0, accepted: 0 }; - const filePaths = node.type === "file" ? [canonical] : filesUnder(db, canonical, options.include); - for (const filePath of filePaths) { + const filePaths: Iterable = + node.type === "file" + ? [{ path: canonical, inode: node.inode, size: node.size }] + : filesUnder(db, canonical, options.include, options.exclude); + for (const target of filePaths) { const complete = await scanFile( db, - filePath, + target, matcher, settings.context, settings.offset, @@ -113,13 +123,25 @@ function normalizeOptions(options: GrepOptions): { }; } +interface ScanTarget { + path: string; + // Undefined when the caller grepped a single file directly: that + // path still needs a normal resolve. Traversal-produced targets + // carry the inode the walk already read. + inode?: number; + size?: number; +} + function* filesUnder( db: Database, directory: string, include: string | undefined, -): Iterable { - for (const entry of iterateFoundEntries(db, directory, include)) { - if (entry.type === "file") yield entry.path; + exclude: string[] | undefined, +): Iterable { + for (const entry of iterateFoundEntries(db, directory, include, exclude)) { + if (entry.type === "file") { + yield { path: entry.path, inode: entry.inode, size: entry.size }; + } } } @@ -136,7 +158,7 @@ function compileMatcher(pattern: string, options: { regex: boolean; ignoreCase: async function scanFile( db: Database, - path: string, + target: ScanTarget, matcher: RegExp, context: number, offset: number, @@ -146,8 +168,9 @@ async function scanFile( ): Promise { const before: WorkspaceGrepContextLine[] = []; const pending: PendingMatch[] = []; + const path = target.path; - for await (const current of readLines(db, path)) { + for await (const current of readLines(db, target)) { const isMatch = matcher.test(current.text); const contextLine = { ...current, isMatch }; for (const item of pending) { @@ -188,8 +211,14 @@ function flushReady(pending: PendingMatch[], out: WorkspaceGrepMatch[]): void { } } -async function* readLines(db: Database, path: string): AsyncIterable { - const stream = await readFile(db, path); +async function* readLines(db: Database, target: ScanTarget): AsyncIterable { + // Traversal-produced targets carry the inode and size the walk + // already read, so the stream can skip a redundant path resolve. + // Both paths stay lazy so grep never holds a whole file in memory. + const stream = + target.inode !== undefined && target.size !== undefined + ? readCommittedFileByInode(db, target.inode, target.size, target.path) + : await readFile(db, target.path); const reader = stream.getReader(); const decoder = new TextDecoder("utf-8", { fatal: false }); let tail = ""; diff --git a/packages/dofs/src/fs/readFile.test.ts b/packages/dofs/src/fs/readFile.test.ts index 5487f5f8..3767e0d8 100644 --- a/packages/dofs/src/fs/readFile.test.ts +++ b/packages/dofs/src/fs/readFile.test.ts @@ -1,7 +1,8 @@ -import { describe, expect, it } from "vitest"; +import { describe, expect, it, vi } from "vitest"; +import { clearBlobCache } from "./blobCache.js"; import { mkdir } from "./mkdir.js"; -import { readFile } from "./readFile.js"; +import { readCommittedFileByInode, readFile } from "./readFile.js"; import { withDB } from "./with-db.js"; import { CHUNK_SIZE, @@ -77,6 +78,33 @@ describe("readFile", () => { }); }); + it("streams committed inode reads without loading later chunks", async () => { + await withDB(async (db) => { + const bytes = new Uint8Array(CHUNK_SIZE + 100); + bytes.fill(0x41, 0, CHUNK_SIZE); + bytes.fill(0x42, CHUNK_SIZE); + await writeFile(db, "/big", bytes, {}, () => 0); + const node = db.one<{ inode: number; size: number }>( + "SELECT inode, size FROM vfs_nodes WHERE type = 'file'", + ); + if (node === undefined) throw new Error("missing test file"); + + clearBlobCache(db); + const one = vi.spyOn(db, "one"); + const stream = readCommittedFileByInode(db, node.inode, node.size, "/big"); + const blobReads = () => + one.mock.calls.filter(([query]) => String(query).includes("vfs_blob_bytes")).length; + expect(stream).toBeInstanceOf(ReadableStream); + expect(blobReads()).toBe(0); + + const reader = stream.getReader(); + expect((await reader.read()).value?.byteLength).toBe(CHUNK_SIZE); + expect(blobReads()).toBe(1); + expect((await reader.read()).value?.byteLength).toBe(100); + expect(blobReads()).toBe(2); + }); + }); + it("streams only the requested byte range across chunk boundaries", async () => { await withDB(async (db) => { const bytes = new Uint8Array(CHUNK_SIZE + 8); diff --git a/packages/dofs/src/fs/readFile.ts b/packages/dofs/src/fs/readFile.ts index 2abd2ac7..a8b5d4fc 100644 --- a/packages/dofs/src/fs/readFile.ts +++ b/packages/dofs/src/fs/readFile.ts @@ -122,6 +122,55 @@ export async function readFile( }); } +// Read a whole committed file by inode, skipping path resolution. +// +// Traversal-driven callers (grep) already hold the inode and size that +// resolveInode would re-derive; re-resolving each path cost an extra +// recursive-CTE statement per file, which dominated a recursive grep +// once node_modules entered the tree. +// +// Only valid for inodes the caller just read from vfs_dirents. Open +// write buffers are still honoured so an in-flight write is not missed; +// pending *creates* have no inode yet and so cannot reach this path. +export function readCommittedFileByInode( + db: Database, + inode: number, + size: number, + path: string, +): ReadableStream { + const buffered = getWriteBuffer(db, inode); + if (buffered?.dirty) { + return streamBytes(buffered.buf.slice(0, buffered.size)); + } + if (size === 0) return streamBytes(new Uint8Array(0)); + + const lastIdx = Math.floor((size - 1) / CHUNK_SIZE); + const chunks = db.all( + `SELECT idx, hash, size + FROM vfs_chunks + WHERE inode = ? AND idx BETWEEN 0 AND ? + ORDER BY idx`, + inode, + lastIdx, + ); + assertDenseRange(chunks, 0, lastIdx, path); + + let index = 0; + return new ReadableStream( + { + pull(controller) { + if (index >= chunks.length) { + controller.close(); + return; + } + const chunk = chunks[index++]; + controller.enqueue(rangedChunkBytes(db, path, chunk, 0, size)); + }, + }, + { highWaterMark: 0 }, + ); +} + function validateReadWindow( path: string, byteOffset: number, @@ -156,9 +205,13 @@ function snapshotResult( const { start, end } = readWindow(size, byteOffset, byteLength); const snapshot = source.slice(start, end); if (wantString) return new TextDecoder().decode(snapshot); + return streamBytes(snapshot); +} + +function streamBytes(bytes: Uint8Array): ReadableStream { return new ReadableStream({ start(controller) { - if (snapshot.byteLength > 0) controller.enqueue(snapshot); + if (bytes.byteLength > 0) controller.enqueue(bytes); controller.close(); }, }); diff --git a/packages/dofs/src/schema/core.ts b/packages/dofs/src/schema/core.ts index bc77dd62..27af6644 100644 --- a/packages/dofs/src/schema/core.ts +++ b/packages/dofs/src/schema/core.ts @@ -11,9 +11,12 @@ // composite-PK lookups now read straight from the PK b-tree leaf // with no rowid indirection, and `child_inode` lives in the // dirents leaf so the (parent, name) resolve read is covering -// (no separate index needed). See `schema/migrations.ts` for the -// migration list; `sync.ts` carries the fresh-install DDL. -export const SCHEMA_VERSION = 5; +// (no separate index needed). Bumped to 6 when `vfs_changes` gained +// `vfs_changes_by_op_rev`, so the push tick's tombstone scan can +// restrict on the rev window instead of reading the whole table. +// See `schema/migrations.ts` for the migration list; `sync.ts` +// carries the fresh-install DDL. +export const SCHEMA_VERSION = 6; export const ROOT_INODE = 1; export const CORE_STATEMENTS = [ diff --git a/packages/dofs/src/schema/index.test.ts b/packages/dofs/src/schema/index.test.ts index f07b597e..9b749694 100644 --- a/packages/dofs/src/schema/index.test.ts +++ b/packages/dofs/src/schema/index.test.ts @@ -444,6 +444,74 @@ describe("initializeSchema", () => { expect(norm(tableSql("vfs_chunks"))).toBe(norm(freshSql("vfs_chunks"))); }); + it("creates vfs_changes_by_op_rev on a fresh DB and uses it for the tombstone scan", () => { + const storage = new SQLiteTestStorage(); + const db = new Database(storage); + initializeSchema(db, () => 0); + + const indexNames = db + .all<{ name: string }>("SELECT name FROM sqlite_master WHERE type = 'index'") + .map((r) => r.name); + expect(indexNames).toContain("vfs_changes_by_op_rev"); + + // The coalesce tombstone query must be able to restrict on rev + // rather than scanning the whole table. Before this index the plan + // was `SCAN vfs_changes USING INDEX vfs_changes_by_path`, which + // ignores the rev predicate entirely. + const plan = db + .all<{ detail: string }>( + `EXPLAIN QUERY PLAN + SELECT path, MAX(rev) AS rev FROM vfs_changes + WHERE rev > ? AND op = 'delete' GROUP BY path`, + 0, + ) + .map((r) => r.detail) + .join(" | "); + expect(plan).toContain("vfs_changes_by_op_rev"); + expect(plan).not.toMatch(/SCAN vfs_changes(?! USING)/); + }); + + it("adds vfs_changes_by_op_rev on the v5 -> v6 upgrade, preserving tombstones", () => { + // Stage a v5-shape database: everything current except the new + // index. The migrator must add it without disturbing existing rows. + const storage = new SQLiteTestStorage(); + const db = new Database(storage); + + initializeSchema(db, () => 0); + db.run("INSERT INTO vfs_changes (rev, path, op) VALUES (?, ?, 'delete')", 5, "/a.txt"); + db.run("INSERT INTO vfs_changes (rev, path, op) VALUES (?, ?, 'delete')", 7, "/b.txt"); + db.run("INSERT INTO vfs_changes (rev, path, op) VALUES (?, ?, 'delete')", 9, "/a.txt"); + const before = db.all("SELECT id, rev, path, op FROM vfs_changes ORDER BY id"); + + // Roll the database back to v5: drop the index and restamp. + db.run("DROP INDEX IF EXISTS vfs_changes_by_op_rev"); + db.run("UPDATE vfs_meta SET v = 5 WHERE k = 'schema_version'"); + expect( + db + .all<{ name: string }>("SELECT name FROM sqlite_master WHERE type = 'index'") + .map((r) => r.name), + ).not.toContain("vfs_changes_by_op_rev"); + + initializeSchema(db, () => 0); + + expect(db.one<{ v: number }>("SELECT v FROM vfs_meta WHERE k = 'schema_version'")?.v).toBe( + SCHEMA_VERSION, + ); + expect( + db + .all<{ name: string }>("SELECT name FROM sqlite_master WHERE type = 'index'") + .map((r) => r.name), + ).toContain("vfs_changes_by_op_rev"); + expect(db.all("SELECT id, rev, path, op FROM vfs_changes ORDER BY id")).toEqual(before); + + // The pre-existing path index must survive the upgrade too. + expect( + db + .all<{ name: string }>("SELECT name FROM sqlite_master WHERE type = 'index'") + .map((r) => r.name), + ).toContain("vfs_changes_by_path"); + }); + it("is idempotent across repeat calls", () => { const storage = new SQLiteTestStorage(); const db = new Database(storage); diff --git a/packages/dofs/src/schema/migrations.ts b/packages/dofs/src/schema/migrations.ts index d2425ada..510c38db 100644 --- a/packages/dofs/src/schema/migrations.ts +++ b/packages/dofs/src/schema/migrations.ts @@ -142,11 +142,27 @@ function v4_to_v5_without_rowid(db: Database): void { db.run(`CREATE INDEX vfs_chunks_by_hash ON vfs_chunks(hash)`); } +// v5 → v6 — add `vfs_changes_by_op_rev`. The push tick's tombstone +// query filters `rev > ? AND op = 'delete'` and groups by path; +// without an (op, rev) index the planner scans vfs_changes in path +// order and never applies the rev predicate, reading the whole table +// to return the few rows inside the watermark window. +// +// Index-only migration: no table is rewritten and no row is touched, +// so existing tombstones carry through untouched. The CREATE is +// duplicated in `sync.ts`'s fresh-install DDL; keep the two in +// lockstep. IF NOT EXISTS keeps this safe if a database somehow +// already has the index. +function v5_to_v6_changes_op_rev_index(db: Database): void { + db.run(`CREATE INDEX IF NOT EXISTS vfs_changes_by_op_rev ON vfs_changes(op, rev)`); +} + export const MIGRATIONS: readonly Migration[] = [ { from: 1, to: 2, migrator: v1_to_v2_add_mounts_mode }, { from: 2, to: 3, migrator: v2_to_v3_add_size_column }, { from: 3, to: 4, migrator: v3_to_v4_watermark_backend_column }, { from: 4, to: 5, migrator: v4_to_v5_without_rowid }, + { from: 5, to: 6, migrator: v5_to_v6_changes_op_rev_index }, ] as const; // Apply every migration whose `from` matches the current version, diff --git a/packages/dofs/src/schema/sync.ts b/packages/dofs/src/schema/sync.ts index fdbac101..090c0be8 100644 --- a/packages/dofs/src/schema/sync.ts +++ b/packages/dofs/src/schema/sync.ts @@ -23,6 +23,23 @@ export const SYNC_STATEMENTS = [ // index. Used on every recordDelete and on every push-tick that // processes tombstones. `CREATE INDEX IF NOT EXISTS vfs_changes_by_path ON vfs_changes(path, id DESC)`, + // coalesceChanges scans tombstones with + // `WHERE rev > ? AND op = 'delete' GROUP BY path`. Neither of the + // indexes above serves that: vfs_changes_by_rev(rev) can drive the + // range but leaves GROUP BY path to a sort, so the planner instead + // scans vfs_changes_by_path in path order and ignores the rev + // predicate entirely — reading the whole table to return the handful + // of rows in the watermark window. + // + // (op, rev) puts the equality column first and the range column + // second, which is the shape SQLite can drive both halves from. The + // GROUP BY still needs a temp b-tree, but the scan is now bounded by + // the rev window instead of the table. + // + // This only started to matter once node_modules entered the sync set: + // every reinstall appends thousands of tombstones and the table only + // grows. Added at schema v6; `schema/migrations.ts` owns the upgrade. + `CREATE INDEX IF NOT EXISTS vfs_changes_by_op_rev ON vfs_changes(op, rev)`, // Watermarks are keyed by (k, backend) so a workspace hosting // multiple backends keeps each backend's sync cursors // independent. The `backend` column was added at schema v3; diff --git a/packages/dofs/src/sync/coalesce.ts b/packages/dofs/src/sync/coalesce.ts index bfd4b304..d6aeb62e 100644 --- a/packages/dofs/src/sync/coalesce.ts +++ b/packages/dofs/src/sync/coalesce.ts @@ -1,7 +1,7 @@ import type { Database } from "../storage.js"; import { type ChangeEntry, materialiseChange } from "./changes.js"; import { isIgnored } from "./ignore.js"; -import { pathsOf } from "./paths.js"; +import { pathsOfMany } from "./paths.js"; import { type ChangeCursor, compareChangeCursors } from "./watermarks.js"; // Yield one ChangeEntry per path touched after `after`. Per-path @@ -63,11 +63,19 @@ export async function* coalesceChanges( lowerRev, through.rev, ); + // Resolve every touched inode to its path(s) in one batched pass. + // Doing this per inode issued O(N x depth) statements, which + // dominated the tick once node_modules entered the sync set. + const pathsByInode = pathsOfMany( + db, + touched.map((row) => row.inode), + ); + for (const { inode, rev } of touched) { // One inode can carry several hardlink names; every name has to // become a candidate so the wire materialises each, not just the // arbitrary one pathOf would return. - for (const path of pathsOf(db, inode)) { + for (const path of pathsByInode.get(inode) ?? []) { if (!inCursorWindow({ rev, path }, cursor, through)) continue; if (isIgnored(path, ignore)) continue; const prior = candidates.get(path); diff --git a/packages/dofs/src/sync/paths.test.ts b/packages/dofs/src/sync/paths.test.ts new file mode 100644 index 00000000..3dd156a4 --- /dev/null +++ b/packages/dofs/src/sync/paths.test.ts @@ -0,0 +1,141 @@ +import { describe, expect, it } from "vitest"; + +import { link } from "../fs/link.js"; +import { mkdir } from "../fs/mkdir.js"; +import { withDB } from "../fs/with-db.js"; +import { writeFile } from "../fs/writeFile.js"; +import { ROOT_INODE } from "../schema/index.js"; +import { type pathOf, pathsOf, pathsOfMany } from "./paths.js"; + +// Read back the inode a path currently names. Tests need this to feed +// pathsOfMany without going through resolveInode's symlink handling. +function inodeOf(db: Parameters[0], parentInode: number, name: string): number { + const row = db.one<{ child_inode: number }>( + "SELECT child_inode FROM vfs_dirents WHERE parent_inode = ? AND name = ?", + parentInode, + name, + ); + if (row === undefined) throw new Error(`no dirent ${name} under ${parentInode}`); + return row.child_inode; +} + +describe("pathsOfMany", () => { + it("returns an empty map for no inodes", async () => { + await withDB((db) => { + expect(pathsOfMany(db, [])).toEqual(new Map()); + }); + }); + + it("maps the root inode to /", async () => { + await withDB((db) => { + expect(pathsOfMany(db, [ROOT_INODE])).toEqual(new Map([[ROOT_INODE, ["/"]]])); + }); + }); + + it("resolves a top-level entry", async () => { + await withDB(async (db) => { + await writeFile(db, "/a.txt", "x", {}, () => 1); + const inode = inodeOf(db, ROOT_INODE, "a.txt"); + expect(pathsOfMany(db, [inode])).toEqual(new Map([[inode, ["/a.txt"]]])); + }); + }); + + it("resolves a deeply nested entry", async () => { + await withDB(async (db) => { + mkdir(db, "/a/b/c/d", { recursive: true }, () => 1); + await writeFile(db, "/a/b/c/d/deep.txt", "x", {}, () => 2); + const a = inodeOf(db, ROOT_INODE, "a"); + const b = inodeOf(db, a, "b"); + const c = inodeOf(db, b, "c"); + const d = inodeOf(db, c, "d"); + const file = inodeOf(db, d, "deep.txt"); + expect(pathsOfMany(db, [file])).toEqual(new Map([[file, ["/a/b/c/d/deep.txt"]]])); + }); + }); + + it("returns every hardlink name for an inode, sorted", async () => { + await withDB(async (db) => { + mkdir(db, "/dir", {}, () => 1); + await writeFile(db, "/one.txt", "x", {}, () => 2); + link(db, "/one.txt", "/two.txt"); + link(db, "/one.txt", "/dir/three.txt"); + const inode = inodeOf(db, ROOT_INODE, "one.txt"); + const got = pathsOfMany(db, [inode]).get(inode); + expect([...(got ?? [])].sort()).toEqual(["/dir/three.txt", "/one.txt", "/two.txt"]); + }); + }); + + it("omits inodes that are unreachable from the root", async () => { + await withDB((db) => { + // 99999 has no dirent row at all. + expect(pathsOfMany(db, [99999])).toEqual(new Map()); + }); + }); + + it("resolves many inodes in one call, matching pathsOf exactly", async () => { + await withDB(async (db) => { + const inodes: number[] = []; + mkdir(db, "/pkg", { recursive: true }, () => 1); + for (let i = 0; i < 25; i++) { + mkdir(db, `/pkg/p${i}/dist`, { recursive: true }, () => 2); + await writeFile(db, `/pkg/p${i}/dist/index.js`, "x", {}, () => 3); + } + for (const row of db.all<{ inode: number }>("SELECT inode FROM vfs_nodes")) { + inodes.push(row.inode); + } + + const batched = pathsOfMany(db, inodes); + for (const inode of inodes) { + const expected = pathsOf(db, inode); + const actual = batched.get(inode) ?? []; + expect([...actual].sort()).toEqual([...expected].sort()); + } + }); + }); + + it("agrees with pathsOf on a tree containing hardlinks", async () => { + await withDB(async (db) => { + mkdir(db, "/x/y", { recursive: true }, () => 1); + await writeFile(db, "/x/y/f.txt", "data", {}, () => 2); + link(db, "/x/y/f.txt", "/x/alias.txt"); + const inodes = db.all<{ inode: number }>("SELECT inode FROM vfs_nodes").map((r) => r.inode); + const batched = pathsOfMany(db, inodes); + for (const inode of inodes) { + expect([...(batched.get(inode) ?? [])].sort()).toEqual([...pathsOf(db, inode)].sort()); + } + }); + }); + + it("issues a bounded number of SQL statements regardless of inode count", async () => { + await withDB(async (db) => { + mkdir(db, "/big", { recursive: true }, () => 1); + for (let i = 0; i < 60; i++) { + await writeFile(db, `/big/f${i}.txt`, "x", {}, () => 2); + } + const inodes = db.all<{ inode: number }>("SELECT inode FROM vfs_nodes").map((r) => r.inode); + + let statements = 0; + const originalAll = db.all.bind(db); + const originalOne = db.one.bind(db); + (db as unknown as { all: unknown }).all = (...args: unknown[]) => { + statements += 1; + return (originalAll as (...a: unknown[]) => unknown)(...args); + }; + (db as unknown as { one: unknown }).one = (...args: unknown[]) => { + statements += 1; + return (originalOne as (...a: unknown[]) => unknown)(...args); + }; + try { + pathsOfMany(db, inodes); + } finally { + (db as unknown as { all: unknown }).all = originalAll; + (db as unknown as { one: unknown }).one = originalOne; + } + + // The batched resolver must not scale its statement count with the + // number of inodes. pathsOf would issue >= inodes.length here. + expect(statements).toBeLessThan(5); + expect(inodes.length).toBeGreaterThan(60); + }); + }); +}); diff --git a/packages/dofs/src/sync/paths.ts b/packages/dofs/src/sync/paths.ts index d68a1e14..cb75a3aa 100644 --- a/packages/dofs/src/sync/paths.ts +++ b/packages/dofs/src/sync/paths.ts @@ -44,3 +44,122 @@ export function pathsOf(db: Database, inode: number): string[] { } return paths; } + +// How many inodes to resolve per CTE round. SQLite's parameter limit +// (SQLITE_MAX_VARIABLE_NUMBER, 999 on conservative builds) caps a bound +// json array's practical size; we bind one JSON string, but keeping the +// batches bounded also keeps the recursive walk's working set small. +const PATHS_BATCH_SIZE = 512; + +// Batched `pathsOf`. Resolves every inode in `inodes` to all of its +// hardlink names in a fixed number of statements rather than one +// dirent lookup per ancestor per inode. +// +// coalesceChanges calls this once per push tick with the entire set of +// revved inodes. The per-inode version issued O(N x depth) statements +// — ~74k round-trips for a 20k-node node_modules tree, which dominated +// the tick (~30s) even though every one of those lookups was already a +// covering-index hit. The cost was the statement count, not the index. +// +// Shape: seed one row per (target, dirent) so hardlinks fan out, then +// walk `child_inode -> parent_inode` upward, carrying `target` along. +// Rows are ordered deepest-segment-first per target and reassembled in +// JS. An inode with no dirent row (unreachable, or the root) produces +// no seed row and is simply absent from the result — same contract as +// pathsOf returning [] / pathOf returning null. +export function pathsOfMany(db: Database, inodes: readonly number[]): Map { + const out = new Map(); + if (inodes.length === 0) return out; + + // Deduplicate and pull the root out; it has no dirent row. + const unique: number[] = []; + const seen = new Set(); + for (const inode of inodes) { + if (seen.has(inode)) continue; + seen.add(inode); + if (inode === ROOT_INODE) { + out.set(ROOT_INODE, ["/"]); + continue; + } + unique.push(inode); + } + if (unique.length === 0) return out; + + for (let start = 0; start < unique.length; start += PATHS_BATCH_SIZE) { + const batch = unique.slice(start, start + PATHS_BATCH_SIZE); + collectBatch(db, batch, out); + } + return out; +} + +interface SegmentRow { + target: number; + link: string; + depth: number; + name: string; + parent_inode: number; +} + +function collectBatch(db: Database, batch: number[], out: Map): void { + // `link` distinguishes the hardlink names of one target: each seed + // dirent starts a separate upward walk, and every row on that walk + // carries its seed's identity so segments regroup correctly. Two + // hardlinks can share a parent directory ("/one.txt" and "/two.txt" + // both sit under the root), so the seed's parent inode alone is not + // a unique key — the seed's (parent_inode, name) pair is. + const rows = db.all( + `WITH RECURSIVE + targets(inode) AS ( + SELECT value FROM json_each(?) + ), + walk(target, link, depth, name, parent_inode) AS ( + SELECT t.inode, d.parent_inode || '/' || d.name, 0, d.name, d.parent_inode + FROM targets t + JOIN vfs_dirents d ON d.child_inode = t.inode + UNION ALL + SELECT w.target, w.link, w.depth + 1, d.name, d.parent_inode + FROM walk w + JOIN vfs_dirents d ON d.child_inode = w.parent_inode + WHERE w.parent_inode <> ? + ) + SELECT target, link, depth, name, parent_inode + FROM walk + ORDER BY target, link, depth DESC`, + JSON.stringify(batch), + ROOT_INODE, + ); + + // Group by (target, link). Rows arrive root-most segment first, so + // appending in order builds the path left to right. + let currentTarget: number | undefined; + let currentLink: string | undefined; + let segments: string[] = []; + let reachedRoot = false; + + const flush = () => { + if (currentTarget === undefined) return; + // Only emit paths whose walk actually terminated at the root. A + // walk that ran out of dirent rows describes an unreachable inode. + if (reachedRoot && segments.length > 0) { + const path = `/${segments.join("/")}`; + const existing = out.get(currentTarget); + if (existing === undefined) out.set(currentTarget, [path]); + else existing.push(path); + } + segments = []; + reachedRoot = false; + }; + + for (const row of rows) { + if (row.target !== currentTarget || row.link !== currentLink) { + flush(); + currentTarget = row.target; + currentLink = row.link; + } + segments.push(row.name); + // The deepest row of a completed walk is the one whose parent is + // the root; depth 0 is the target's own dirent. + if (row.parent_inode === ROOT_INODE) reachedRoot = true; + } + flush(); +}