From 72fc614950c0805eb11231554231b701fcfdad34 Mon Sep 17 00:00:00 2001 From: Nathan Disidore Date: Fri, 3 Jul 2026 17:34:00 +0000 Subject: [PATCH 1/7] dofs: add filesystem micro-benchmark harness Measure the filesystem operations against a real Durable Object SqlStorage via vitest-pool-workers, not the node SQLiteTestStorage fixture (which caches prepared statements and would understate per-statement cost). Each scenario reports wall-clock ns/op and a deterministic statement and row count from a CountingStorage decorator over the real backend. Scenarios cover both stat surfaces across a path-depth sweep, present and missing exists, small and positional reads, readdir versus ls, recursive delete, a write-heavy create/edit/delete burst, single and subtree renames, and a dedup guard on database size. A signature gate asserts the deterministic counts so drift fails the run. Runs only through vitest.config.bench.ts, so it stays out of the regular suite; src/bench is excluded from the build as it imports the workers-only test module. Invoke with npm run bench. --- packages/dofs/package.json | 3 +- packages/dofs/src/bench/counting-storage.ts | 123 +++++ packages/dofs/src/bench/fs-ops.bench.ts | 572 ++++++++++++++++++++ packages/dofs/tsconfig.build.json | 2 +- packages/dofs/vitest.config.bench.ts | 24 + 5 files changed, 722 insertions(+), 2 deletions(-) create mode 100644 packages/dofs/src/bench/counting-storage.ts create mode 100644 packages/dofs/src/bench/fs-ops.bench.ts create mode 100644 packages/dofs/vitest.config.bench.ts diff --git a/packages/dofs/package.json b/packages/dofs/package.json index 5e198547..b15f1c59 100644 --- a/packages/dofs/package.json +++ b/packages/dofs/package.json @@ -18,7 +18,8 @@ "build": "tsc -p tsconfig.build.json", "typecheck": "tsc -p tsconfig.build.json --noEmit", "test": "vitest run", - "test:workers": "vitest run --config vitest.config.workers.ts" + "test:workers": "vitest run --config vitest.config.workers.ts", + "bench": "vitest run --config vitest.config.bench.ts" }, "devDependencies": { "@cloudflare/vitest-pool-workers": "^0.16.10", diff --git a/packages/dofs/src/bench/counting-storage.ts b/packages/dofs/src/bench/counting-storage.ts new file mode 100644 index 00000000..70797ae9 --- /dev/null +++ b/packages/dofs/src/bench/counting-storage.ts @@ -0,0 +1,123 @@ +// Statement/row counter that wraps a real storage backend. +// +// The benchmark harness runs against a genuine Durable Object +// SqlStorage (see vitest.config.bench.ts). Wall-clock alone can't +// prove *why* an operation is slow, so this decorator sits between the +// `Database` wrapper and the real backend and records every +// `sql.exec` call: how many statements ran, split into reads +// (SELECT/WITH) and writes (INSERT/UPDATE/DELETE/REPLACE), plus a +// best-effort tally of rows touched. +// +// Statement counts are deterministic and backend-independent — they +// are the primary signal for the O(depth) resolution fingerprint +// (`resolveInode` = 1 + 2D statements) and for write-amplification +// analysis (added rows per mutation). Row counts are read opportunist- +// ically off the cursor (the DO backend exposes rowsRead/rowsWritten) +// and are reported as a secondary, best-effort figure. +// +// The decorator forwards transactionSync/transaction straight through +// to the real backend, so real SQLite transaction semantics are +// preserved; only `sql.exec` is instrumented. + +import type { DurableObjectStorageLike, SQLCursorLike, SQLStorageLike } from "../types.js"; + +export interface StatementCounts { + statements: number; + reads: number; + writes: number; + other: number; + rowsRead: number; + rowsWritten: number; +} + +function readNumber(source: unknown, key: string): number | undefined { + const value = (source as Record | null)?.[key]; + return typeof value === "number" ? value : undefined; +} + +export class CountingStorage implements DurableObjectStorageLike { + statements = 0; + reads = 0; + writes = 0; + other = 0; + rowsRead = 0; + rowsWritten = 0; + + readonly sql: SQLStorageLike; + readonly transactionSync?: (closure: () => T) => T; + readonly transaction?: (closure: () => T | Promise) => T | Promise; + + constructor(inner: DurableObjectStorageLike) { + this.sql = { + exec: >( + query: string, + ...bindings: unknown[] + ): SQLCursorLike => { + this.statements += 1; + this.classify(query); + const cursor = inner.sql.exec(query, ...bindings); + // Writes report rowsWritten eagerly after exec on the DO + // backend; run() never iterates the cursor, so capture it here. + const written = readNumber(cursor, "rowsWritten"); + if (written !== undefined) { + this.rowsWritten += written; + } + return { + toArray: (): Row[] => { + const rows = cursor.toArray(); + // rowsRead is only meaningful once the cursor is drained, + // which all() does exactly once. + this.rowsRead += readNumber(cursor, "rowsRead") ?? rows.length; + return rows; + }, + }; + }, + }; + + if (inner.transactionSync !== undefined) { + const delegate = inner.transactionSync.bind(inner); + this.transactionSync = (closure: () => T): T => delegate(closure); + } + if (inner.transaction !== undefined) { + const delegate = inner.transaction.bind(inner); + this.transaction = (closure: () => T | Promise): T | Promise => delegate(closure); + } + } + + private classify(query: string): void { + const head = query.trimStart().slice(0, 6).toLowerCase(); + if (head.startsWith("select") || head.startsWith("with")) { + this.reads += 1; + } else if ( + head.startsWith("insert") || + head.startsWith("update") || + head.startsWith("delete") || + head.startsWith("replac") + ) { + this.writes += 1; + } else { + // SAVEPOINT/RELEASE/PRAGMA/DDL etc. Not part of per-op data cost. + this.other += 1; + } + } + + reset(): void { + this.statements = 0; + this.reads = 0; + this.writes = 0; + this.other = 0; + this.rowsRead = 0; + this.rowsWritten = 0; + } + + snapshot(): StatementCounts { + return { + statements: this.statements, + reads: this.reads, + writes: this.writes, + other: this.other, + rowsRead: this.rowsRead, + rowsWritten: this.rowsWritten, + }; + } +} diff --git a/packages/dofs/src/bench/fs-ops.bench.ts b/packages/dofs/src/bench/fs-ops.bench.ts new file mode 100644 index 00000000..58dc177b --- /dev/null +++ b/packages/dofs/src/bench/fs-ops.bench.ts @@ -0,0 +1,572 @@ +// dofs micro-benchmark harness. +// +// Runs under @cloudflare/vitest-pool-workers (see +// vitest.config.bench.ts) so every operation drives a REAL Durable +// Object SqlStorage. It deliberately does NOT use the node +// SQLiteTestStorage fixture: that backend caches prepared statements +// and would understate the per-statement cost this harness exists to +// measure. +// +// Each scenario is reported two ways: +// * ns/op wall-clock, measured against the raw DO SqlStorage. +// * statement + row counts, measured against the same backend +// wrapped in CountingStorage. Statement counts are deterministic +// and are the primary signal — the depth sweep should show +// fs.stat = 1 + 2D statements and provider.statSync = 3 + 4D, +// while the flat-path baseline stays constant (O(1)). +// +// Output is a set of tables plus a single-line JSON blob so before/ +// after deltas are easy to capture and diff. Run with: +// npm run bench --workspace @cloudflare/dofs +// (or: npx vitest run --config vitest.config.bench.ts, from the +// package dir). + +import { env, runInDurableObject } from "cloudflare:test"; +import { expect, it } from "vitest"; +import type { TestBindings } from "../../tests/worker.js"; +import { ls } from "../fs/ls.js"; +import { resolveInode } from "../fs/resolve.js"; +import { rm } from "../fs/rm.js"; +import { stat } from "../fs/stat.js"; +import { SQLiteWorkspaceProvider } from "../provider.js"; +import { initializeSchema } from "../schema/index.js"; +import { Database } from "../storage.js"; +import type { DurableObjectStorageLike } from "../types.js"; +import { CountingStorage } from "./counting-storage.js"; + +const NOW = (): number => 1000; + +interface RealStorage extends DurableObjectStorageLike { + readonly databaseSize?: number; +} + +function freshStub(): DurableObjectStub { + const ns = (env as unknown as TestBindings).TestStorage; + return ns.get(ns.newUniqueId()); +} + +// Raw real SqlStorage — used for clean wall-clock numbers with no +// counting overhead. +async function withRealDb( + fn: (db: Database, provider: SQLiteWorkspaceProvider, storage: RealStorage) => T, +): Promise { + const stub = freshStub(); + return runInDurableObject(stub, async (_instance: unknown, state: DurableObjectState) => { + const storage = state.storage as unknown as DurableObjectStorageLike; + const db = new Database(storage); + initializeSchema(db, NOW); + const provider = new SQLiteWorkspaceProvider(db, { now: NOW }); + return fn(db, provider, (storage as { sql: RealStorage }).sql as unknown as RealStorage); + }); +} + +// Counting wrapper over the same real backend — used for deterministic +// statement/row counts. Schema init is excluded via reset(). +async function withCountingDb( + fn: (db: Database, provider: SQLiteWorkspaceProvider, counting: CountingStorage) => T, +): Promise { + const stub = freshStub(); + return runInDurableObject(stub, async (_instance: unknown, state: DurableObjectState) => { + const counting = new CountingStorage(state.storage as unknown as DurableObjectStorageLike); + const db = new Database(counting); + initializeSchema(db, NOW); + counting.reset(); + const provider = new SQLiteWorkspaceProvider(db, { now: NOW }); + return fn(db, provider, counting); + }); +} + +type Build = (db: Database, provider: SQLiteWorkspaceProvider) => void; +type Op = (db: Database, provider: SQLiteWorkspaceProvider) => void; + +interface ReadResult { + name: string; + depth: number; + nsPerOp: number; + statements: number; + reads: number; + writes: number; +} + +interface MutationResult { + name: string; + items: number; + totalMs: number; + nsPerItem: number; + statements: number; + reads: number; + writes: number; + rowsWritten: number; +} + +// Repeatable read op: build the tree once, then time `op` over `iters`. +async function benchRead( + name: string, + depth: number, + build: Build, + op: Op, + iters: number, +): Promise { + const warmup = Math.min(200, iters); + const nsPerOp = await withRealDb((db, provider) => { + build(db, provider); + for (let i = 0; i < warmup; i++) { + op(db, provider); + } + const t0 = performance.now(); + for (let i = 0; i < iters; i++) { + op(db, provider); + } + const t1 = performance.now(); + return ((t1 - t0) * 1e6) / iters; + }); + const counts = await withCountingDb((db, provider, counting) => { + build(db, provider); + counting.reset(); + op(db, provider); + return counting.snapshot(); + }); + return { + name, + depth, + nsPerOp, + statements: counts.statements, + reads: counts.reads, + writes: counts.writes, + }; +} + +// One-shot mutation batch: build fresh state, run the whole batch once, +// report totals and per-item amortized cost. +async function benchMutation( + name: string, + build: Build, + batch: (db: Database, provider: SQLiteWorkspaceProvider) => number, +): Promise { + const timing = await withRealDb((db, provider) => { + build(db, provider); + const t0 = performance.now(); + const items = batch(db, provider); + const t1 = performance.now(); + return { totalMs: t1 - t0, items }; + }); + const counts = await withCountingDb((db, provider, counting) => { + build(db, provider); + counting.reset(); + batch(db, provider); + return counting.snapshot(); + }); + const items = Math.max(1, timing.items); + return { + name, + items: timing.items, + totalMs: timing.totalMs, + nsPerItem: (timing.totalMs * 1e6) / items, + statements: counts.statements, + reads: counts.reads, + writes: counts.writes, + rowsWritten: counts.rowsWritten, + }; +} + +// --- path helpers ------------------------------------------------- + +function chainOf(depth: number): { dir: string | null; file: string } { + const segs = Array.from({ length: depth }, (_, i) => `s${i + 1}`); + const file = `/${segs.join("/")}`; + const dir = depth > 1 ? `/${segs.slice(0, -1).join("/")}` : null; + return { dir, file }; +} + +function buildChainFile(provider: SQLiteWorkspaceProvider, depth: number, content = "x"): string { + const { dir, file } = chainOf(depth); + if (dir !== null) { + provider.mkdirSync(dir, { recursive: true }); + } + provider.writeFileSync(file, content); + return file; +} + +// --- formatting --------------------------------------------------- + +function ns(value: number): string { + if (value >= 1e6) { + return `${(value / 1e6).toFixed(3)}ms`; + } + if (value >= 1e3) { + return `${(value / 1e3).toFixed(2)}\u00b5s`; + } + return `${value.toFixed(0)}ns`; +} + +function pad(value: string | number, width: number): string { + return String(value).padStart(width); +} + +function padEnd(value: string | number, width: number): string { + return String(value).padEnd(width); +} + +// ------------------------------------------------------------------ + +it("dofs micro-benchmark (real DO SqlStorage)", async () => { + const lines: string[] = []; + const readResults: ReadResult[] = []; + const mutationResults: MutationResult[] = []; + + const depths = [1, 2, 4, 8, 16, 20]; + + // Group A — path-resolution depth sweep, both stat surfaces + a + // flat single-lookup baseline that should stay constant. + for (const depth of depths) { + readResults.push( + await benchRead( + "fs.stat", + depth, + (_db, provider) => { + buildChainFile(provider, depth); + }, + (db) => { + stat(db, chainOf(depth).file); + }, + 4000, + ), + ); + readResults.push( + await benchRead( + "provider.statSync", + depth, + (_db, provider) => { + buildChainFile(provider, depth); + }, + (_db, provider) => { + provider.statSync(chainOf(depth).file); + }, + 4000, + ), + ); + const holder = { inode: 0 }; + readResults.push( + await benchRead( + "flat-baseline(inode)", + depth, + (db, provider) => { + const file = buildChainFile(provider, depth); + holder.inode = resolveInode(db, file)?.inode ?? 0; + }, + (db) => { + db.one( + "SELECT inode, type, mode, mtime, size FROM vfs_nodes WHERE inode = ?", + holder.inode, + ); + }, + 4000, + ), + ); + } + + // Group B — exists, present vs missing leaf, at two depths. + for (const depth of [8, 16]) { + readResults.push( + await benchRead( + "exists(present)", + depth, + (_db, provider) => { + buildChainFile(provider, depth); + }, + (_db, provider) => { + provider.existsSync(chainOf(depth).file); + }, + 4000, + ), + ); + readResults.push( + await benchRead( + "exists(missing)", + depth, + (_db, provider) => { + const { dir } = chainOf(depth); + if (dir !== null) { + provider.mkdirSync(dir, { recursive: true }); + } + }, + (_db, provider) => { + provider.existsSync(chainOf(depth).file); + }, + 4000, + ), + ); + } + + // Group C — read paths at a representative depth. + { + const depth = 8; + const content = "x".repeat(4096); + readResults.push( + await benchRead( + "readFile(4KiB)", + depth, + (_db, provider) => { + buildChainFile(provider, depth, content); + }, + (_db, provider) => { + provider.readFileSync(chainOf(depth).file, "utf8"); + }, + 2000, + ), + ); + readResults.push( + await benchRead( + "readRange(1KiB)", + depth, + (_db, provider) => { + buildChainFile(provider, depth, content); + }, + (_db, provider) => { + provider.readRangeSync(chainOf(depth).file, 0, 1024); + }, + 2000, + ), + ); + } + + // Group D — directory listing: readdir (single dir) vs ls (whole-tree + // CTE, which seeds at root). + { + const width = 200; + const buildWide: Build = (_db, provider) => { + provider.mkdirSync("/wide", { recursive: true }); + for (let i = 0; i < width; i++) { + provider.writeFileSync(`/wide/f${i}.txt`, "x"); + } + }; + readResults.push( + await benchRead( + `readdir(${width})`, + 1, + buildWide, + (_db, provider) => { + provider.readdirSync("/wide"); + }, + 1000, + ), + ); + readResults.push( + await benchRead( + `ls(${width})`, + 1, + buildWide, + (db) => { + ls(db, "/wide"); + }, + 500, + ), + ); + } + + // Group E — recursive delete of a populated tree. + { + const files = 2000; + mutationResults.push( + await benchMutation( + `recursive-delete(${files})`, + (_db, provider) => { + provider.mkdirSync("/tree", { recursive: true }); + for (let i = 0; i < files; i++) { + provider.writeFileSync(`/tree/f${i}.txt`, "x"); + } + }, + (db) => { + rm(db, "/tree", { recursive: true, force: true }); + return files; + }, + ), + ); + } + + // Group F — write-heavy burst (agent edit session): create, then + // edit-in-place (overwrite), then delete N files at depth. + { + const depth = 8; + const count = 1000; + const { dir } = chainOf(depth); + const base = dir ?? ""; + const ensureDir: Build = (_db, provider) => { + if (dir !== null) { + provider.mkdirSync(dir, { recursive: true }); + } + }; + const createAll = (provider: SQLiteWorkspaceProvider, value: string): void => { + for (let i = 0; i < count; i++) { + provider.writeFileSync(`${base}/burst${i}.txt`, value); + } + }; + mutationResults.push( + await benchMutation(`write-burst:create(${count})`, ensureDir, (_db, provider) => { + createAll(provider, "x"); + return count; + }), + ); + mutationResults.push( + await benchMutation( + `write-burst:edit-in-place(${count})`, + (_db, provider) => { + ensureDir(_db, provider); + createAll(provider, "x"); + }, + (_db, provider) => { + createAll(provider, "yy"); + return count; + }, + ), + ); + mutationResults.push( + await benchMutation( + `write-burst:delete(${count})`, + (_db, provider) => { + ensureDir(_db, provider); + createAll(provider, "x"); + }, + (_db, provider) => { + for (let i = 0; i < count; i++) { + provider.unlinkSync(`${base}/burst${i}.txt`); + } + return count; + }, + ), + ); + } + + // Group G — rename: many single-file renames vs one subtree rename. + { + const count = 1000; + mutationResults.push( + await benchMutation( + `single-rename(${count})`, + (_db, provider) => { + provider.mkdirSync("/mv", { recursive: true }); + for (let i = 0; i < count; i++) { + provider.writeFileSync(`/mv/a${i}.txt`, "x"); + } + }, + (_db, provider) => { + for (let i = 0; i < count; i++) { + provider.renameSync(`/mv/a${i}.txt`, `/mv/b${i}.txt`); + } + return count; + }, + ), + ); + const descendants = 500; + mutationResults.push( + await benchMutation( + `subtree-rename(${descendants})`, + (_db, provider) => { + provider.mkdirSync("/sub/inner", { recursive: true }); + for (let i = 0; i < descendants; i++) { + provider.writeFileSync(`/sub/inner/f${i}.txt`, "x"); + } + }, + (_db, provider) => { + provider.renameSync("/sub", "/moved"); + return descendants; + }, + ), + ); + } + + // Group H — DB size / dedup guard: 100 identical 1 MiB files should + // dedup to ~1 MiB of blob bytes plus small metadata. + const dedup = await withRealDb((_db, provider, storage) => { + const oneMiB = "a".repeat(1024 * 1024); + provider.mkdirSync("/dup", { recursive: true }); + for (let i = 0; i < 100; i++) { + provider.writeFileSync(`/dup/f${i}.bin`, oneMiB); + } + return { bytes: storage.databaseSize ?? 0, files: 100, logicalMiB: 100 }; + }); + + // --- render report ------------------------------------------------ + + lines.push("=".repeat(96)); + lines.push( + "dofs micro-benchmark — backend: REAL Durable Object SqlStorage (vitest-pool-workers)", + ); + lines.push(`generated: ${new Date().toISOString()}`); + lines.push( + "note: statement counts are deterministic; ns/op is wall-clock under workerd. " + + "fs.stat=1+2D, provider.statSync=3+4D, flat-baseline=O(1).", + ); + lines.push("=".repeat(96)); + + lines.push(""); + lines.push("READ / RESOLVE OPS"); + lines.push( + `${padEnd("operation", 22)}${pad("depth", 6)}${pad("ns/op", 12)}${pad("stmts", 8)}${pad("reads", 7)}${pad("writes", 8)}`, + ); + lines.push("-".repeat(63)); + for (const r of readResults) { + lines.push( + `${padEnd(r.name, 22)}${pad(r.depth, 6)}${pad(ns(r.nsPerOp), 12)}${pad(r.statements, 8)}${pad(r.reads, 7)}${pad(r.writes, 8)}`, + ); + } + + lines.push(""); + lines.push("MUTATION BATCHES (per-item amortized)"); + lines.push( + `${padEnd("operation", 30)}${pad("items", 7)}${pad("total", 11)}${pad("ns/item", 12)}${pad("stmts", 8)}${pad("writes", 8)}${pad("rowsWr", 8)}`, + ); + lines.push("-".repeat(84)); + for (const m of mutationResults) { + lines.push( + `${padEnd(m.name, 30)}${pad(m.items, 7)}${pad(`${m.totalMs.toFixed(1)}ms`, 11)}${pad(ns(m.nsPerItem), 12)}${pad(m.statements, 8)}${pad(m.writes, 8)}${pad(m.rowsWritten, 8)}`, + ); + } + + lines.push(""); + lines.push("DB SIZE / DEDUP GUARD"); + lines.push( + `100 x 1 MiB identical files -> logical ${dedup.logicalMiB} MiB, on-disk ${(dedup.bytes / (1024 * 1024)).toFixed(2)} MiB (${dedup.bytes} bytes)`, + ); + + lines.push(""); + lines.push("JSON"); + lines.push( + JSON.stringify({ + backend: "durable-object-sqlstorage", + reads: readResults, + mutations: mutationResults, + dedup, + }), + ); + lines.push("=".repeat(96)); + + console.log(`\n${lines.join("\n")}\n`); + + // Signature gate. Statement counts are deterministic, so the O(depth) + // fingerprint doubles as the harness's contract: fs.stat = 1 + 2D, + // provider.statSync = 3 + 4D, flat baseline = O(1). Asserted AFTER the + // report is printed so the numbers stay visible even when the gate + // trips. A deliberate perf change is expected to update these, which + // is the point — silent drift becomes a failure. + const find = (name: string, depth: number): ReadResult => { + const row = readResults.find((r) => r.name === name && r.depth === depth); + if (row === undefined) { + throw new Error(`benchmark result missing: ${name} depth=${depth}`); + } + return row; + }; + for (const depth of depths) { + expect(find("fs.stat", depth).statements, `fs.stat depth=${depth}`).toBe(1 + 2 * depth); + expect(find("provider.statSync", depth).statements, `provider.statSync depth=${depth}`).toBe( + 3 + 4 * depth, + ); + expect(find("flat-baseline(inode)", depth).statements, `flat-baseline depth=${depth}`).toBe(1); + } + // exists: present path resolves fully (1 + 2D); a missing leaf stops at + // the final dirent miss (2D). + expect(find("exists(present)", 8).statements).toBe(17); + expect(find("exists(missing)", 8).statements).toBe(16); + // Dedup guarantee: 100 identical 1 MiB files must not balloon the DB. + expect(dedup.bytes).toBeLessThan(2 * 1024 * 1024); +}); diff --git a/packages/dofs/tsconfig.build.json b/packages/dofs/tsconfig.build.json index fbdaec7c..f9b10b93 100644 --- a/packages/dofs/tsconfig.build.json +++ b/packages/dofs/tsconfig.build.json @@ -8,5 +8,5 @@ "rootDir": "src" }, "include": ["src/**/*.ts"], - "exclude": ["src/**/*.test.ts", "src/fs/with-db.workers.ts"] + "exclude": ["src/**/*.test.ts", "src/fs/with-db.workers.ts", "src/bench/**"] } diff --git a/packages/dofs/vitest.config.bench.ts b/packages/dofs/vitest.config.bench.ts new file mode 100644 index 00000000..b9e25298 --- /dev/null +++ b/packages/dofs/vitest.config.bench.ts @@ -0,0 +1,24 @@ +import { cloudflareTest } from "@cloudflare/vitest-pool-workers"; +import { defineConfig } from "vitest/config"; + +// Benchmark runner. Reuses the workerd-backed pool (same wrangler +// config as the workers test project) so the harness drives a REAL +// Durable Object SqlStorage — NOT the node SQLiteTestStorage fixture, +// which caches prepared statements and would understate per-statement +// cost. Scoped to the *.bench.ts glob so it never runs during +// `npm test`; invoke explicitly via `npm run bench`. +export default defineConfig({ + plugins: [ + cloudflareTest({ + wrangler: { configPath: "./tests/wrangler.jsonc" }, + }), + ], + test: { + globals: true, + include: ["src/bench/**/*.bench.ts"], + // The harness builds large trees and loops tens of thousands of + // synchronous ops; the default 5s timeout is far too tight. + testTimeout: 600_000, + hookTimeout: 600_000, + }, +}); From 72a9ffee7f27669cc60b05afe0adae2b979e707c Mon Sep 17 00:00:00 2001 From: Nathan Disidore Date: Fri, 3 Jul 2026 18:06:37 +0000 Subject: [PATCH 2/7] dofs: drop redundant path resolves in statSync, rm, and writeSync MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit statSync resolved the path twice — once through statImpl for mode/size/mtime and again for the inode behind the link count. stat now returns the resolved inode, so the provider reuses it for both. nlink for hardlinked files and the inode 0 / nlink 1 of a pending-create file are unchanged. Recursive delete threads the parent inode and name the post-order walk already knows into unlinkDirent instead of re-resolving each parent from the root; the single-entry path reuses the parent it just resolved. Deleted dirents, reap-on-last-link, and tombstones are unchanged. writeSync stats only when appending, the only case that needs the offset. A non-append write is validated by writeRangeSyncImpl (ENOENT/EISDIR); a zero-length write keeps an explicit existence check since it short-circuits before that resolve. --- packages/dofs/src/bench/fs-ops.bench.ts | 14 +++-- packages/dofs/src/fs/rm.ts | 80 +++++++++++++++---------- packages/dofs/src/fs/stat.ts | 8 +++ packages/dofs/src/provider.ts | 24 ++++++-- 4 files changed, 85 insertions(+), 41 deletions(-) diff --git a/packages/dofs/src/bench/fs-ops.bench.ts b/packages/dofs/src/bench/fs-ops.bench.ts index 58dc177b..e34370eb 100644 --- a/packages/dofs/src/bench/fs-ops.bench.ts +++ b/packages/dofs/src/bench/fs-ops.bench.ts @@ -12,7 +12,7 @@ // * statement + row counts, measured against the same backend // wrapped in CountingStorage. Statement counts are deterministic // and are the primary signal — the depth sweep should show -// fs.stat = 1 + 2D statements and provider.statSync = 3 + 4D, +// fs.stat = 1 + 2D statements and provider.statSync = 2 + 2D, // while the flat-path baseline stays constant (O(1)). // // Output is a set of tables plus a single-line JSON blob so before/ @@ -495,7 +495,7 @@ it("dofs micro-benchmark (real DO SqlStorage)", async () => { lines.push(`generated: ${new Date().toISOString()}`); lines.push( "note: statement counts are deterministic; ns/op is wall-clock under workerd. " + - "fs.stat=1+2D, provider.statSync=3+4D, flat-baseline=O(1).", + "fs.stat=1+2D, provider.statSync=2+2D, flat-baseline=O(1).", ); lines.push("=".repeat(96)); @@ -544,11 +544,15 @@ it("dofs micro-benchmark (real DO SqlStorage)", async () => { console.log(`\n${lines.join("\n")}\n`); // Signature gate. Statement counts are deterministic, so the O(depth) - // fingerprint doubles as the harness's contract: fs.stat = 1 + 2D, - // provider.statSync = 3 + 4D, flat baseline = O(1). Asserted AFTER the + // fingerprint doubles as the harness's contract. Asserted AFTER the // report is printed so the numbers stay visible even when the gate // trips. A deliberate perf change is expected to update these, which // is the point — silent drift becomes a failure. + // + // fs.stat = 1 + 2D — resolveInode: 1 root read plus a + // dirent and node read per segment. + // provider.statSync = 2 + 2D — one resolve plus linkCount. + // flat-baseline = 1 — a single indexed inode lookup. const find = (name: string, depth: number): ReadResult => { const row = readResults.find((r) => r.name === name && r.depth === depth); if (row === undefined) { @@ -559,7 +563,7 @@ it("dofs micro-benchmark (real DO SqlStorage)", async () => { for (const depth of depths) { expect(find("fs.stat", depth).statements, `fs.stat depth=${depth}`).toBe(1 + 2 * depth); expect(find("provider.statSync", depth).statements, `provider.statSync depth=${depth}`).toBe( - 3 + 4 * depth, + 2 + 2 * depth, ); expect(find("flat-baseline(inode)", depth).statements, `flat-baseline depth=${depth}`).toBe(1); } diff --git a/packages/dofs/src/fs/rm.ts b/packages/dofs/src/fs/rm.ts index 9ce423fa..dfb7c7b5 100644 --- a/packages/dofs/src/fs/rm.ts +++ b/packages/dofs/src/fs/rm.ts @@ -20,23 +20,55 @@ interface DirChild { } // Walk a directory subtree post-order so we delete leaves before -// parents. Yields { path, inode, type } for each node to remove. The -// caller appends one tombstone per yielded path and clears -// vfs_chunks for file inodes. +// parents. Yields each node together with the parent inode and name +// the walk already knows, so the caller can unlink the dirent by +// (parent, name) without re-resolving the parent from root. The caller +// appends one tombstone per yielded path and clears vfs_chunks for +// file inodes. function* walkPostOrder( db: Database, rootInode: number, rootPath: string, -): Generator<{ path: string; inode: number; type: "file" | "dir" | "symlink" }> { + rootParentInode: number, + rootName: string, +): Generator<{ + path: string; + inode: number; + type: "file" | "dir" | "symlink"; + parentInode: number; + name: string; +}> { // Stack-based DFS to avoid recursion limits on deep trees. - type Frame = { inode: number; path: string; type: "file" | "dir" | "symlink"; expanded: boolean }; - const stack: Frame[] = [{ inode: rootInode, path: rootPath, type: "dir", expanded: false }]; + type Frame = { + inode: number; + path: string; + type: "file" | "dir" | "symlink"; + parentInode: number; + name: string; + expanded: boolean; + }; + const stack: Frame[] = [ + { + inode: rootInode, + path: rootPath, + type: "dir", + parentInode: rootParentInode, + name: rootName, + expanded: false, + }, + ]; while (stack.length > 0) { const top = stack[stack.length - 1]; if (top.type !== "dir" || top.expanded) { stack.pop(); - yield { path: top.path, inode: top.inode, type: top.type }; + yield { + path: top.path, + inode: top.inode, + type: top.type, + parentInode: top.parentInode, + name: top.name, + }; continue; } top.expanded = true; @@ -54,6 +86,8 @@ function* walkPostOrder( inode: child.child_inode, path: childPath, type: child.type, + parentInode: top.inode, + name: child.name, expanded: false, }); } @@ -118,9 +152,11 @@ export function rm(db: Database, path: string, options: RmOptions): void { // Single entry removal — file, symlink, or empty directory. A // file inode may have multiple dirents (hardlinks), so remove // only the requested name and reap chunks/node after the final - // link disappears. The tombstone is recorded at the resolved - // real path so sync sees the move-aware location. - removeEntry(db, realPath, node.inode, node.type); + // link disappears. `parent` is already resolved above, so unlink + // by (parent, name) directly rather than re-resolving. The + // tombstone is recorded at the resolved real path so sync sees + // the move-aware location. + unlinkDirent(db, parent.inode, name, node.inode, node.type); recordDelete(db, rev, realPath); return; } @@ -128,27 +164,11 @@ export function rm(db: Database, path: string, options: RmOptions): void { // Recursive directory removal. Walk leaves first so each delete // sees an empty parent by the time we get to it. File entries may // be hardlinked outside this subtree, so delete by path rather - // than by child inode. - for (const entry of walkPostOrder(db, node.inode, realPath)) { - removeEntry(db, entry.path, entry.inode, entry.type); + // than by child inode. The walk carries each node's parent inode + // and name, so unlinkDirent needs no per-node re-resolve from root. + for (const entry of walkPostOrder(db, node.inode, realPath, parent.inode, name)) { + unlinkDirent(db, entry.parentInode, entry.name, entry.inode, entry.type); recordDelete(db, rev, entry.path); } }); } - -function removeEntry( - db: Database, - path: string, - inode: number, - type: "file" | "dir" | "symlink", -): void { - const { parts, path: canonical } = canonicalizePath(path); - const name = parts[parts.length - 1]; - const parentPath = parts.length === 1 ? "/" : `/${parts.slice(0, -1).join("/")}`; - const parent = resolveInode(db, parentPath, { followSymlinks: false }); - if (parent === null || parent.type !== "dir") { - throw createWorkspaceError("ENOENT", `parent directory missing: ${canonical}`, canonical); - } - - unlinkDirent(db, parent.inode, name, inode, type); -} diff --git a/packages/dofs/src/fs/stat.ts b/packages/dofs/src/fs/stat.ts index cea38d86..4ce2734f 100644 --- a/packages/dofs/src/fs/stat.ts +++ b/packages/dofs/src/fs/stat.ts @@ -6,6 +6,10 @@ import { getPendingWriteBufferByPath, getWriteBuffer } from "./writeBuffer.js"; export interface WorkspaceStatResult { name: string; + // Inode of the resolved node, or 0 for a pending-create file that + // has no inode yet. Exposed so provider stat surfaces can read the + // inode from the same resolve instead of walking the path twice. + inode: number; mode: number; mtime: number; size: number; @@ -39,6 +43,9 @@ function statShared(db: Database, path: string, followFinal: boolean): Workspace if (pending !== undefined && pending.pending !== undefined) { return { name, + // A pending create has no inode until releaseWriteBufferSync + // commits it; report 0, which yields nlink 1 in the provider. + inode: 0, mode: pending.mode & 0o7777, mtime: pending.pending.mtime, size: pending.size, @@ -68,6 +75,7 @@ function statShared(db: Database, path: string, followFinal: boolean): Workspace return { name, + inode: node.inode, mode: node.mode, mtime: node.mtime, size, diff --git a/packages/dofs/src/provider.ts b/packages/dofs/src/provider.ts index 0746828f..e9dd5741 100644 --- a/packages/dofs/src/provider.ts +++ b/packages/dofs/src/provider.ts @@ -176,18 +176,19 @@ export class SQLiteWorkspaceProvider { } statSync(path: string, _options?: { bigint?: boolean }): VirtualStatsLike { + // statImpl resolves the path once (following symlinks) and returns + // the inode, so nlink comes from the same walk. A pending-create + // file reports inode 0, which yields nlink 1. const s = statImpl(this.db, path); - const node = resolveInode(this.db, path); - const ino = node?.inode ?? 0; return wrapStats({ mode: s.mode, size: s.size, mtimeMs: s.mtime, - ino, + ino: s.inode, isFile: s.isFile, isDirectory: s.isDirectory, isSymbolicLink: false, - nlink: linkCount(this.db, ino), + nlink: linkCount(this.db, s.inode), }); } @@ -604,8 +605,19 @@ export class SQLiteWorkspaceProvider { if (!state.writable) { throw createWorkspaceError("EBADF", `fd ${fd} is not writable`); } - const stat = this.statSync(state.path); - const startAt = state.append ? stat.size : (position ?? state.position); + // Append needs the current EOF, so stat only then. A non-append + // write of >0 bytes doesn't need it: writeRangeSyncImpl resolves the + // path and raises ENOENT/EISDIR. A zero-length write short-circuits + // before that resolve, so keep an explicit existence check for it. + let startAt: number; + if (state.append) { + startAt = this.statSync(state.path).size; + } else { + if (length === 0) { + this.statSync(state.path); + } + startAt = position ?? state.position; + } const view = buffer instanceof Buffer ? new Uint8Array(buffer.buffer, buffer.byteOffset + offset, length) From 3fa056649ba6ae1a3dd46fbb16d82a7696822892 Mon Sep 17 00:00:00 2001 From: Nathan Disidore Date: Fri, 3 Jul 2026 21:00:45 +0000 Subject: [PATCH 3/7] dofs: store vfs_dirents and vfs_chunks WITHOUT ROWID MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cluster the two composite-key tables on their primary keys so a lookup reads from the b-tree leaf with no rowid indirection: child_inode lives in the vfs_dirents PK leaf, so a path segment resolves with a single covering read, and a file's chunks sit in (inode, idx) order. Both are eligible — composite PK, no AUTOINCREMENT, not part of any foreign key. vfs_blob_bytes stays a rowid table; it holds the large blob payloads and the schema's only foreign key. Fresh databases get the shape from the baseline DDL; a v4 to v5 migration rebuilds existing ones, copying each table into a WITHOUT ROWID replacement and recreating its secondary index (the rename and drop destroy it, and the baseline CREATE INDEX does not re-run). The rebuild runs inside initializeSchema's transaction. A migration test stages a v4 database with nested dirs, a hardlink, multi-chunk files and dedup, and asserts the version bump, both tables WITHOUT ROWID, both indexes present, byte-for-byte data survival, and that a fresh install matches a migrated one. The direct-range-write test watches vfs_blobs.last_seen instead of a chunk rowid, which no longer exists. --- packages/dofs/src/fs/writeRange.test.ts | 33 +++-- packages/dofs/src/schema/core.ts | 29 +++- packages/dofs/src/schema/index.test.ts | 182 ++++++++++++++++++++++++ packages/dofs/src/schema/migrations.ts | 56 ++++++++ 4 files changed, 279 insertions(+), 21 deletions(-) diff --git a/packages/dofs/src/fs/writeRange.test.ts b/packages/dofs/src/fs/writeRange.test.ts index 1f952edf..b04a600a 100644 --- a/packages/dofs/src/fs/writeRange.test.ts +++ b/packages/dofs/src/fs/writeRange.test.ts @@ -38,15 +38,6 @@ async function readBytes(db: Database, path: string): Promise { return out; } -function chunkRowIds(db: Database, path: string): Array<{ idx: number; rowid: number }> { - const node = resolveInode(db, path); - if (node === null) throw new Error(`missing node: ${path}`); - return db.all<{ idx: number; rowid: number }>( - "SELECT idx, rowid FROM vfs_chunks WHERE inode = ? ORDER BY idx", - node.inode, - ); -} - function manifestHash(db: Database, path: string): Uint8Array | null { const node = resolveInode(db, path); if (node === null) throw new Error(`missing node: ${path}`); @@ -139,21 +130,33 @@ describe("direct range writes", () => { }); }); - it("keeps untouched chunk rowids stable across a small range write", async () => { + it("skips rewriting untouched chunks on a small range write", async () => { await withDB(async (db) => { const original = new Uint8Array(CHUNK_SIZE * 3); original.fill(1, 0, CHUNK_SIZE); original.fill(2, CHUNK_SIZE, CHUNK_SIZE * 2); original.fill(3, CHUNK_SIZE * 2, CHUNK_SIZE * 3); writeFileSync(db, "/large.bin", original, {}, () => 1000); - const beforeIds = chunkRowIds(db, "/large.bin"); + // Touch only the middle chunk, at a later mtime. writeRangeSync(db, "/large.bin", new Uint8Array([7]), CHUNK_SIZE + 10, {}, () => 1001); - const afterIds = chunkRowIds(db, "/large.bin"); - expect(afterIds[0].rowid).toBe(beforeIds[0].rowid); - expect(afterIds[2].rowid).toBe(beforeIds[2].rowid); - expect(afterIds[1].rowid).not.toBe(beforeIds[1].rowid); + // applyChunkedInodeUpdate skips SQL entirely for unchanged chunks, + // so their blobs are never re-upserted and keep the original + // last_seen; only the rewritten middle chunk's blob is stamped with + // the new mtime. (vfs_chunks is WITHOUT ROWID, so there is no rowid + // to watch. On the write path last_seen is bumped only by + // upsertChunkBlob, and this test performs no reads — reads also + // touch last_seen — so an unchanged last_seen proves the chunk row + // was skipped.) + const rows = chunkRows(db, "/large.bin"); + expect(rows).toHaveLength(3); + const lastSeen = (hash: Uint8Array): number | undefined => + db.one<{ last_seen: number }>("SELECT last_seen FROM vfs_blobs WHERE hash = ?", hash) + ?.last_seen; + expect(lastSeen(rows[0].hash)).toBe(1000); + expect(lastSeen(rows[1].hash)).toBe(1001); + expect(lastSeen(rows[2].hash)).toBe(1000); }); }); diff --git a/packages/dofs/src/schema/core.ts b/packages/dofs/src/schema/core.ts index 62885ddf..bc77dd62 100644 --- a/packages/dofs/src/schema/core.ts +++ b/packages/dofs/src/schema/core.ts @@ -6,10 +6,14 @@ // gained a cached `size` column so stat() doesn't have to SUM // chunks on every call. Bumped to 4 when `_vfs_watermark` gained // a `backend` column so a single workspace can host more than -// one backend with independent sync cursors. See -// `schema/migrations.ts` for the migration list; `sync.ts` -// carries the fresh-install DDL. -export const SCHEMA_VERSION = 4; +// one backend with independent sync cursors. Bumped to 5 when +// `vfs_dirents` and `vfs_chunks` became WITHOUT ROWID: their +// 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; export const ROOT_INODE = 1; export const CORE_STATEMENTS = [ @@ -29,12 +33,19 @@ export const CORE_STATEMENTS = [ link_target TEXT, size INTEGER NOT NULL DEFAULT 0 )`, + // WITHOUT ROWID: the row lives in the (parent_inode, name) PK + // b-tree leaf, so resolving a path segment reads child_inode + // directly from the leaf — no autoindex -> rowid hop, and no + // separate covering index. Legal here because the PK is composite + // and the table has no AUTOINCREMENT. Existing databases are + // rebuilt by the v4 -> v5 migration in schema/migrations.ts; keep + // this DDL and that migrator's CREATE in lockstep. `CREATE TABLE IF NOT EXISTS vfs_dirents ( parent_inode INTEGER NOT NULL, name TEXT NOT NULL, child_inode INTEGER NOT NULL, PRIMARY KEY (parent_inode, name) - )`, + ) WITHOUT ROWID`, `CREATE INDEX IF NOT EXISTS vfs_dirents_by_child ON vfs_dirents(child_inode)`, `CREATE INDEX IF NOT EXISTS vfs_nodes_by_rev ON vfs_nodes(rev)`, // gc/manifests checks every manifest row against vfs_nodes via a @@ -53,12 +64,18 @@ export const CORE_STATEMENTS = [ hash BLOB PRIMARY KEY REFERENCES vfs_blobs(hash) ON DELETE CASCADE, bytes BLOB NOT NULL )`, + // WITHOUT ROWID: clustered on (inode, idx) so a file's chunks are + // stored and scanned in index order straight from the PK leaf. + // Legal here — composite PK, no AUTOINCREMENT. The bytes live in + // vfs_blob_bytes (content-addressed), so these rows stay small, + // which is what WITHOUT ROWID wants. Rebuilt for existing DBs by + // the v4 -> v5 migration; keep in lockstep with that migrator. `CREATE TABLE IF NOT EXISTS vfs_chunks ( inode INTEGER NOT NULL, idx INTEGER NOT NULL, hash BLOB NOT NULL, size INTEGER NOT NULL, PRIMARY KEY (inode, idx) - )`, + ) WITHOUT ROWID`, `CREATE INDEX IF NOT EXISTS vfs_chunks_by_hash ON vfs_chunks(hash)`, ] as const; diff --git a/packages/dofs/src/schema/index.test.ts b/packages/dofs/src/schema/index.test.ts index 25a8485a..f07b597e 100644 --- a/packages/dofs/src/schema/index.test.ts +++ b/packages/dofs/src/schema/index.test.ts @@ -262,6 +262,188 @@ describe("initializeSchema", () => { expect(worker?.v).toBe(99); }); + it("rebuilds vfs_dirents and vfs_chunks as WITHOUT ROWID on the v4 -> v5 upgrade, preserving all data", () => { + // Stage a v4-shape database: vfs_dirents and vfs_chunks are plain + // rowid tables carrying their secondary indexes; vfs_nodes already + // has the size column. Populate a representative graph — nested + // dirs, a hardlink (one inode, two names), multi-chunk files, and + // content dedup (distinct files sharing blob hashes) — so the + // rebuild is proven lossless, not merely structurally correct. + const storage = new SQLiteTestStorage(); + const db = new Database(storage); + + const hashA = new Uint8Array(32).fill(0xaa); + const hashB = new Uint8Array(32).fill(0xbb); + + db.transactionSync(() => { + db.run(`CREATE TABLE vfs_meta (k TEXT PRIMARY KEY, v INTEGER NOT NULL)`); + db.run( + `CREATE TABLE vfs_nodes ( + inode INTEGER PRIMARY KEY AUTOINCREMENT, + type TEXT NOT NULL CHECK(type IN ('file','dir','symlink')), + mode INTEGER NOT NULL DEFAULT 493, + mtime INTEGER NOT NULL, + rev INTEGER NOT NULL DEFAULT 0, + mount_root TEXT, + stub_size INTEGER, + manifest_hash BLOB, + link_target TEXT, + size INTEGER NOT NULL DEFAULT 0 + )`, + ); + // Pre-migration shape: rowid tables plus their secondary indexes. + db.run( + `CREATE TABLE vfs_dirents ( + parent_inode INTEGER NOT NULL, + name TEXT NOT NULL, + child_inode INTEGER NOT NULL, + PRIMARY KEY (parent_inode, name) + )`, + ); + db.run(`CREATE INDEX vfs_dirents_by_child ON vfs_dirents(child_inode)`); + db.run( + `CREATE TABLE vfs_chunks ( + inode INTEGER NOT NULL, + idx INTEGER NOT NULL, + hash BLOB NOT NULL, + size INTEGER NOT NULL, + PRIMARY KEY (inode, idx) + )`, + ); + db.run(`CREATE INDEX vfs_chunks_by_hash ON vfs_chunks(hash)`); + db.run( + `CREATE TABLE vfs_blobs ( + hash BLOB PRIMARY KEY, + size INTEGER NOT NULL, + last_seen INTEGER NOT NULL + )`, + ); + db.run( + `CREATE TABLE vfs_blob_bytes ( + hash BLOB PRIMARY KEY REFERENCES vfs_blobs(hash) ON DELETE CASCADE, + bytes BLOB NOT NULL + )`, + ); + + // Graph: /(1) -> a(2) -> { f1(3), f2(4), b(5) }, b(5) -> deep(6). + // Hardlink: /a/hard is a second name for inode 3. + db.run( + `INSERT INTO vfs_nodes (inode, type, mode, mtime, rev, size) VALUES + (1, 'dir', 493, 0, 0, 0), + (2, 'dir', 493, 0, 1, 0), + (3, 'file', 420, 0, 2, 10), + (4, 'file', 420, 0, 3, 5), + (5, 'dir', 493, 0, 4, 0), + (6, 'file', 420, 0, 5, 5)`, + ); + db.run( + `INSERT INTO vfs_dirents (parent_inode, name, child_inode) VALUES + (1, 'a', 2), + (2, 'f1', 3), + (2, 'f2', 4), + (2, 'hard', 3), + (2, 'b', 5), + (5, 'deep', 6)`, + ); + // f1(3): hashA + hashB. f2(4): hashA (dedup). deep(6): hashB (dedup). + // -> 4 chunk rows referencing 2 distinct blobs. + db.run("INSERT INTO vfs_chunks (inode, idx, hash, size) VALUES (?, ?, ?, ?)", 3, 0, hashA, 5); + db.run("INSERT INTO vfs_chunks (inode, idx, hash, size) VALUES (?, ?, ?, ?)", 3, 1, hashB, 5); + db.run("INSERT INTO vfs_chunks (inode, idx, hash, size) VALUES (?, ?, ?, ?)", 4, 0, hashA, 5); + db.run("INSERT INTO vfs_chunks (inode, idx, hash, size) VALUES (?, ?, ?, ?)", 6, 0, hashB, 5); + db.run("INSERT INTO vfs_blobs (hash, size, last_seen) VALUES (?, ?, ?)", hashA, 5, 0); + db.run("INSERT INTO vfs_blobs (hash, size, last_seen) VALUES (?, ?, ?)", hashB, 5, 0); + db.run( + "INSERT INTO vfs_blob_bytes (hash, bytes) VALUES (?, ?)", + hashA, + new Uint8Array(5).fill(1), + ); + db.run( + "INSERT INTO vfs_blob_bytes (hash, bytes) VALUES (?, ?)", + hashB, + new Uint8Array(5).fill(2), + ); + + db.run("INSERT INTO vfs_meta (k, v) VALUES (?, ?)", "schema_version", 4); + }); + + // Snapshot the two rebuilt tables before migrating. + const direntsBefore = db.all<{ parent_inode: number; name: string; child_inode: number }>( + "SELECT parent_inode, name, child_inode FROM vfs_dirents ORDER BY parent_inode, name", + ); + const chunksBefore = db.all<{ inode: number; idx: number; hash: Uint8Array; size: number }>( + "SELECT inode, idx, hash, size FROM vfs_chunks ORDER BY inode, idx", + ); + + initializeSchema(db, () => 0); + + // (a) Version bumped. + expect(db.one<{ v: number }>("SELECT v FROM vfs_meta WHERE k = ?", "schema_version")?.v).toBe( + SCHEMA_VERSION, + ); + + const tableSql = (name: string): string => + db.one<{ sql: string }>( + "SELECT sql FROM sqlite_master WHERE type = 'table' AND name = ?", + name, + )?.sql ?? ""; + + // (b) Both targets are now WITHOUT ROWID; vfs_blob_bytes is untouched. + expect(tableSql("vfs_dirents").toUpperCase()).toContain("WITHOUT ROWID"); + expect(tableSql("vfs_chunks").toUpperCase()).toContain("WITHOUT ROWID"); + expect(tableSql("vfs_blob_bytes").toUpperCase()).not.toContain("WITHOUT ROWID"); + + // (c) Both secondary indexes survived the rebuild. + const indexNames = db + .all<{ name: string }>("SELECT name FROM sqlite_master WHERE type = 'index'") + .map((r) => r.name); + expect(indexNames).toContain("vfs_dirents_by_child"); + expect(indexNames).toContain("vfs_chunks_by_hash"); + + // The rebuild's temp tables are dropped — no leftovers. + const tableNames = db + .all<{ name: string }>("SELECT name FROM sqlite_master WHERE type = 'table'") + .map((r) => r.name); + expect(tableNames).not.toContain("vfs_dirents_v4"); + expect(tableNames).not.toContain("vfs_chunks_v4"); + + // (d) Data survived byte-for-byte. + expect( + db.all("SELECT parent_inode, name, child_inode FROM vfs_dirents ORDER BY parent_inode, name"), + ).toEqual(direntsBefore); + expect(db.all("SELECT inode, idx, hash, size FROM vfs_chunks ORDER BY inode, idx")).toEqual( + chunksBefore, + ); + // Hardlink preserved: inode 3 still reached by both names via the + // recreated child index. + expect( + db.all<{ parent_inode: number; name: string }>( + "SELECT parent_inode, name FROM vfs_dirents WHERE child_inode = ? ORDER BY name", + 3, + ), + ).toEqual([ + { parent_inode: 2, name: "f1" }, + { parent_inode: 2, name: "hard" }, + ]); + // Dedup intact: 4 chunk rows, 2 distinct blobs. + expect(db.one<{ c: number }>("SELECT COUNT(*) AS c FROM vfs_chunks")?.c).toBe(4); + expect(db.one<{ c: number }>("SELECT COUNT(*) AS c FROM vfs_blobs")?.c).toBe(2); + expect(db.one<{ c: number }>("SELECT COUNT(DISTINCT hash) AS c FROM vfs_chunks")?.c).toBe(2); + + // (e) A fresh install lands the identical table shape (modulo + // whitespace) as the migrated database. + const fresh = new Database(new SQLiteTestStorage()); + initializeSchema(fresh, () => 0); + const freshSql = (name: string): string => + fresh.one<{ sql: string }>( + "SELECT sql FROM sqlite_master WHERE type = 'table' AND name = ?", + name, + )?.sql ?? ""; + const norm = (sql: string): string => sql.replace(/\s+/g, " ").trim().toUpperCase(); + expect(norm(tableSql("vfs_dirents"))).toBe(norm(freshSql("vfs_dirents"))); + expect(norm(tableSql("vfs_chunks"))).toBe(norm(freshSql("vfs_chunks"))); + }); + 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 31bf0e15..d2425ada 100644 --- a/packages/dofs/src/schema/migrations.ts +++ b/packages/dofs/src/schema/migrations.ts @@ -87,10 +87,66 @@ function v3_to_v4_watermark_backend_column(db: Database): void { db.run(`DROP TABLE _vfs_watermark_v3`); } +// v4 → v5 — rebuild `vfs_dirents` and `vfs_chunks` as WITHOUT ROWID. +// SQLite can't convert a table to WITHOUT ROWID in place, so for each +// table: rename it aside, create the WITHOUT ROWID replacement, copy +// the rows, drop the old table. +// +// Both targets are FK-inert (neither is an FK parent or child; the +// schema's only foreign key is vfs_blob_bytes -> vfs_blobs) and have +// composite primary keys with no AUTOINCREMENT, so WITHOUT ROWID is +// legal and sqlite_sequence is untouched. `vfs_blob_bytes` is left +// alone on purpose — it holds the large blob payloads and the FK. +// +// A RENAME carries the table's secondary index along to the temp +// name, and the following DROP takes the index with it. The baseline +// `CREATE INDEX IF NOT EXISTS` in initializeSchema already ran, before +// migrations, and does not re-run — so this migrator must recreate +// vfs_dirents_by_child and vfs_chunks_by_hash itself, or upgraded +// databases silently lose them. Keep the CREATE bodies in lockstep +// with the fresh-install DDL in core.ts. +function v4_to_v5_without_rowid(db: Database): void { + // vfs_dirents + db.run(`ALTER TABLE vfs_dirents RENAME TO vfs_dirents_v4`); + db.run( + `CREATE TABLE vfs_dirents ( + parent_inode INTEGER NOT NULL, + name TEXT NOT NULL, + child_inode INTEGER NOT NULL, + PRIMARY KEY (parent_inode, name) + ) WITHOUT ROWID`, + ); + db.run( + `INSERT INTO vfs_dirents (parent_inode, name, child_inode) + SELECT parent_inode, name, child_inode FROM vfs_dirents_v4`, + ); + db.run(`DROP TABLE vfs_dirents_v4`); + db.run(`CREATE INDEX vfs_dirents_by_child ON vfs_dirents(child_inode)`); + + // vfs_chunks + db.run(`ALTER TABLE vfs_chunks RENAME TO vfs_chunks_v4`); + db.run( + `CREATE TABLE vfs_chunks ( + inode INTEGER NOT NULL, + idx INTEGER NOT NULL, + hash BLOB NOT NULL, + size INTEGER NOT NULL, + PRIMARY KEY (inode, idx) + ) WITHOUT ROWID`, + ); + db.run( + `INSERT INTO vfs_chunks (inode, idx, hash, size) + SELECT inode, idx, hash, size FROM vfs_chunks_v4`, + ); + db.run(`DROP TABLE vfs_chunks_v4`); + db.run(`CREATE INDEX vfs_chunks_by_hash ON vfs_chunks(hash)`); +} + 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 }, ] as const; // Apply every migration whose `from` matches the current version, From 1e52048ad4682490595bed9e6b90c222bdd569fc Mon Sep 17 00:00:00 2001 From: Nathan Disidore Date: Fri, 3 Jul 2026 22:10:06 +0000 Subject: [PATCH 4/7] dofs: resolve paths in one statement and cache path lookups MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Resolve the read path with a single recursive-CTE statement and add a per-Database path-to-inode cache, so repeat stat/exists/read of a path cost one statement, and one node re-read once warm, independent of depth. Both fast paths serve only follow-symlinks resolutions outside a transaction. A cache hit re-reads just the node row, so mode/size/mtime stay current — only the path-to-inode mapping is cached. A miss runs one recursive walk of vfs_dirents joined to vfs_nodes; it descends only through directories, so a missing segment or non-directory intermediate yields null exactly as the loop, and any symlink on the path falls back to the per-component loop (which follows links and enforces ELOOP). Symlink resolutions are not cached. lstat/readlink and every resolve inside a mutation keep the loop: those are shallow and hot, and the cache must not be populated mid-transaction so a rollback leaves no stale entry. The cache is invalidated at every dirent move — create, unlink, delete, rename, and the sync-apply structural cleanup — dropping one entry for a leaf and the whole subtree for a directory or symlink change. --- packages/dofs/src/bench/fs-ops.bench.ts | 96 +++++++-- packages/dofs/src/fs/link.ts | 4 + packages/dofs/src/fs/mkdir.ts | 5 + packages/dofs/src/fs/rename.ts | 13 ++ packages/dofs/src/fs/resolve.ts | 129 ++++++++++- packages/dofs/src/fs/resolveCache.test.ts | 248 ++++++++++++++++++++++ packages/dofs/src/fs/resolveCache.ts | 129 +++++++++++ packages/dofs/src/fs/rm.ts | 7 + packages/dofs/src/fs/symlink.ts | 5 + packages/dofs/src/fs/writeFile.ts | 58 +++-- packages/dofs/src/storage.ts | 15 ++ packages/dofs/src/sync/apply.ts | 7 + 12 files changed, 669 insertions(+), 47 deletions(-) create mode 100644 packages/dofs/src/fs/resolveCache.test.ts create mode 100644 packages/dofs/src/fs/resolveCache.ts diff --git a/packages/dofs/src/bench/fs-ops.bench.ts b/packages/dofs/src/bench/fs-ops.bench.ts index e34370eb..b63384da 100644 --- a/packages/dofs/src/bench/fs-ops.bench.ts +++ b/packages/dofs/src/bench/fs-ops.bench.ts @@ -11,9 +11,9 @@ // * ns/op wall-clock, measured against the raw DO SqlStorage. // * statement + row counts, measured against the same backend // wrapped in CountingStorage. Statement counts are deterministic -// and are the primary signal — the depth sweep should show -// fs.stat = 1 + 2D statements and provider.statSync = 2 + 2D, -// while the flat-path baseline stays constant (O(1)). +// and are the primary signal — a resolve is one statement +// regardless of depth (fs.stat = 1, provider.statSync = 2), and the +// cold-vs-warm group isolates the CTE cold walk from the cache hit. // // Output is a set of tables plus a single-line JSON blob so before/ // after deltas are easy to capture and diff. Run with: @@ -26,6 +26,7 @@ import { expect, it } from "vitest"; import type { TestBindings } from "../../tests/worker.js"; import { ls } from "../fs/ls.js"; import { resolveInode } from "../fs/resolve.js"; +import { clearResolveCache } from "../fs/resolveCache.js"; import { rm } from "../fs/rm.js"; import { stat } from "../fs/stat.js"; import { SQLiteWorkspaceProvider } from "../provider.js"; @@ -486,6 +487,54 @@ it("dofs micro-benchmark (real DO SqlStorage)", async () => { return { bytes: storage.databaseSize ?? 0, files: 100, logicalMiB: 100 }; }); + // Group I — cold vs warm resolve: isolate the CTE cold walk from the + // cache hit. Cold clears the cache before every timed op (a fresh + // single-statement CTE that reads D rows internally); warm leaves it + // primed (a single readNode, O(1)). Same fs.stat, shallow and deep. + interface ColdWarmResult { + name: string; + depth: number; + coldNsPerOp: number; + warmNsPerOp: number; + } + const coldWarmResults: ColdWarmResult[] = []; + for (const depth of [4, 20]) { + const measured = await withRealDb((db, provider) => { + buildChainFile(provider, depth); + const file = chainOf(depth).file; + const iters = 4000; + const warmup = 200; + for (let i = 0; i < warmup; i++) { + clearResolveCache(db); + stat(db, file); + } + const cold0 = performance.now(); + for (let i = 0; i < iters; i++) { + clearResolveCache(db); + stat(db, file); + } + const cold1 = performance.now(); + for (let i = 0; i < warmup; i++) { + stat(db, file); + } + const warm0 = performance.now(); + for (let i = 0; i < iters; i++) { + stat(db, file); + } + const warm1 = performance.now(); + return { + cold: ((cold1 - cold0) * 1e6) / iters, + warm: ((warm1 - warm0) * 1e6) / iters, + }; + }); + coldWarmResults.push({ + name: "fs.stat", + depth, + coldNsPerOp: measured.cold, + warmNsPerOp: measured.warm, + }); + } + // --- render report ------------------------------------------------ lines.push("=".repeat(96)); @@ -495,7 +544,8 @@ it("dofs micro-benchmark (real DO SqlStorage)", async () => { lines.push(`generated: ${new Date().toISOString()}`); lines.push( "note: statement counts are deterministic; ns/op is wall-clock under workerd. " + - "fs.stat=1+2D, provider.statSync=2+2D, flat-baseline=O(1).", + "resolve = 1 statement (cold CTE or warm cache), depth-independent. " + + "Depth-sweep ns/op below is cache-warm; see COLD-VS-WARM for the CTE cold walk.", ); lines.push("=".repeat(96)); @@ -523,6 +573,18 @@ it("dofs micro-benchmark (real DO SqlStorage)", async () => { ); } + lines.push(""); + lines.push("COLD (CTE walk) VS WARM (cached) RESOLVE — fs.stat"); + lines.push( + `${padEnd("operation", 22)}${pad("depth", 6)}${pad("cold ns/op", 12)}${pad("warm ns/op", 12)}`, + ); + lines.push("-".repeat(52)); + for (const c of coldWarmResults) { + lines.push( + `${padEnd(c.name, 22)}${pad(c.depth, 6)}${pad(ns(c.coldNsPerOp), 12)}${pad(ns(c.warmNsPerOp), 12)}`, + ); + } + lines.push(""); lines.push("DB SIZE / DEDUP GUARD"); lines.push( @@ -536,6 +598,7 @@ it("dofs micro-benchmark (real DO SqlStorage)", async () => { backend: "durable-object-sqlstorage", reads: readResults, mutations: mutationResults, + coldWarm: coldWarmResults, dedup, }), ); @@ -549,10 +612,13 @@ it("dofs micro-benchmark (real DO SqlStorage)", async () => { // trips. A deliberate perf change is expected to update these, which // is the point — silent drift becomes a failure. // - // fs.stat = 1 + 2D — resolveInode: 1 root read plus a - // dirent and node read per segment. - // provider.statSync = 2 + 2D — one resolve plus linkCount. - // flat-baseline = 1 — a single indexed inode lookup. + // fs.stat = 1 — one recursive-CTE resolve. + // provider.statSync = 2 — one resolve plus linkCount. + // flat-baseline = 1 — a single indexed inode lookup. + // + // The CTE still reads O(depth) rows internally, but the statement + // count — what the DO bills and round-trips — is depth-independent, + // and a warm cache hit re-reads just the one node row. const find = (name: string, depth: number): ReadResult => { const row = readResults.find((r) => r.name === name && r.depth === depth); if (row === undefined) { @@ -561,16 +627,14 @@ it("dofs micro-benchmark (real DO SqlStorage)", async () => { return row; }; for (const depth of depths) { - expect(find("fs.stat", depth).statements, `fs.stat depth=${depth}`).toBe(1 + 2 * depth); - expect(find("provider.statSync", depth).statements, `provider.statSync depth=${depth}`).toBe( - 2 + 2 * depth, - ); + expect(find("fs.stat", depth).statements, `fs.stat depth=${depth}`).toBe(1); + expect(find("provider.statSync", depth).statements, `provider.statSync depth=${depth}`).toBe(2); expect(find("flat-baseline(inode)", depth).statements, `flat-baseline depth=${depth}`).toBe(1); } - // exists: present path resolves fully (1 + 2D); a missing leaf stops at - // the final dirent miss (2D). - expect(find("exists(present)", 8).statements).toBe(17); - expect(find("exists(missing)", 8).statements).toBe(16); + // exists resolves in a single CTE statement too, whether the leaf is + // present or missing. + expect(find("exists(present)", 8).statements).toBe(1); + expect(find("exists(missing)", 8).statements).toBe(1); // Dedup guarantee: 100 identical 1 MiB files must not balloon the DB. expect(dedup.bytes).toBeLessThan(2 * 1024 * 1024); }); diff --git a/packages/dofs/src/fs/link.ts b/packages/dofs/src/fs/link.ts index bdeffa3c..c5e21d17 100644 --- a/packages/dofs/src/fs/link.ts +++ b/packages/dofs/src/fs/link.ts @@ -5,6 +5,7 @@ import { ROOT_INODE } from "../schema/index.js"; import type { Database } from "../storage.js"; import { assertNotReadOnly } from "./mount-guard.js"; import { resolveInode } from "./resolve.js"; +import { invalidateResolveExact } from "./resolveCache.js"; function resolveParent(db: Database, parts: string[], canonical: string): number { let parentInode = ROOT_INODE; @@ -76,5 +77,8 @@ export function link(db: Database, existingPath: string, newPath: string): void ); const rev = incrementRev(db); db.run("UPDATE vfs_nodes SET rev = ? WHERE inode = ?", rev, source.inode); + // A new hardlink name for an existing file: a leaf with no + // descendants, so drop just the (possibly negative) entry for it. + invalidateResolveExact(db, canonicalNew); }); } diff --git a/packages/dofs/src/fs/mkdir.ts b/packages/dofs/src/fs/mkdir.ts index f092a18f..4d664c61 100644 --- a/packages/dofs/src/fs/mkdir.ts +++ b/packages/dofs/src/fs/mkdir.ts @@ -4,6 +4,7 @@ import { incrementRev } from "../rev.js"; import { ROOT_INODE } from "../schema/index.js"; import type { Database } from "../storage.js"; import { assertNotReadOnly } from "./mount-guard.js"; +import { invalidateResolveExact } from "./resolveCache.js"; export interface MkdirOptions { recursive?: boolean; @@ -95,6 +96,9 @@ export function mkdir(db: Database, path: string, options: MkdirOptions, now: () throw createWorkspaceError("ENOENT", `parent directory missing: ${canonical}`, canonical); } parentInode = createDir(db, parentInode, name, 0o755, mtime, rev); + // A newly created directory is empty, so a cached negative for + // its own path is the only stale entry possible; drop it exact. + invalidateResolveExact(db, `/${parts.slice(0, i + 1).join("/")}`); continue; } if (existing.type !== "dir") { @@ -121,5 +125,6 @@ export function mkdir(db: Database, path: string, options: MkdirOptions, now: () } createDir(db, parentInode, leafName, mode, mtime, rev); + invalidateResolveExact(db, canonical); }); } diff --git a/packages/dofs/src/fs/rename.ts b/packages/dofs/src/fs/rename.ts index d9398a01..a7854521 100644 --- a/packages/dofs/src/fs/rename.ts +++ b/packages/dofs/src/fs/rename.ts @@ -6,6 +6,7 @@ import { recordDelete } from "../sync/changes.js"; import { pathOf } from "../sync/paths.js"; import { assertNotReadOnly } from "./mount-guard.js"; import { resolveInode } from "./resolve.js"; +import { invalidateResolveExact, invalidateResolveSubtree } from "./resolveCache.js"; import { unlinkDirent } from "./unlink.js"; interface DirChild { @@ -153,6 +154,18 @@ export function rename(db: Database, oldPath: string, newPath: string): void { db.run("UPDATE vfs_nodes SET rev = ? WHERE inode = ?", rev, entry.inode); recordDelete(db, rev, entry.path); } + + // Drop cached resolutions for both endpoints. A directory move + // changes every descendant's path, so both sides need a subtree + // drop; a file/symlink move only touches the two leaf paths. The + // destination drop also covers any entry displaced by an overwrite. + if (source.type === "dir") { + invalidateResolveSubtree(db, oldRealPath); + invalidateResolveSubtree(db, newRealPath); + } else { + invalidateResolveExact(db, oldRealPath); + invalidateResolveExact(db, newRealPath); + } }); } diff --git a/packages/dofs/src/fs/resolve.ts b/packages/dofs/src/fs/resolve.ts index dcf2d20e..defaa66c 100644 --- a/packages/dofs/src/fs/resolve.ts +++ b/packages/dofs/src/fs/resolve.ts @@ -2,6 +2,7 @@ import { createWorkspaceError } from "../errors.js"; import { canonicalizePath } from "../path.js"; import { ROOT_INODE } from "../schema/index.js"; import type { Database } from "../storage.js"; +import { lookupResolveCache, storeResolveCache } from "./resolveCache.js"; export interface ResolvedInode { inode: number; @@ -58,7 +59,133 @@ export function resolveInode( options: ResolveOptions = {}, ): ResolvedInode | null { const followFinal = options.followSymlinks !== false; - return resolveParts(db, canonicalizePath(path).parts, followFinal, 0); + const { parts, path: canonical } = canonicalizePath(path); + + // Cache + single-statement CTE serve only cache-eligible reads: + // follow-symlinks resolutions outside a transaction. Everything else + // uses the per-component loop: + // * followSymlinks:false (lstat / readlink / the provider's + // pre-mutation captures) — not cached, and the loop is cheaper + // for these shallow one-shot resolves than the recursive CTE. + // * inside a transaction (every mutation path) — resolves are + // shallow and hot, the CTE competes with the mutation's own + // statements for the plan cache (recompiling it is far dearer + // than the loop), and the cache must not be populated + // mid-transaction anyway (rollback safety). + // Mutations still invalidate the cache; that is independent of this. + if (!followFinal || db.inTransaction) { + return resolveParts(db, parts, followFinal, 0); + } + + // Repeat reads of the same path are served from the per-Database + // cache. Only the path -> inode mapping is cached; re-read the node + // row so mode/size/mtime/type are always current. A stale mapping + // (inode reaped without invalidation) reads back null and falls + // through to a full resolve that re-populates the cache. + const hit = lookupResolveCache(db, canonical); + if (hit !== undefined) { + if (hit.kind === "negative") { + return null; + } + const node = readNode(db, hit.inode); + if (node !== null) { + return toResolved(node); + } + } + + // One recursive-CTE statement resolves the common symlink-free + // case. Any symlink on the path falls back to the per-component loop, + // which follows links and enforces ELOOP; those resolutions are not + // cached (a followed path is an alias whose invalidation can't be + // reasoned about structurally). + const cte = resolveViaCte(db, parts); + if (cte.kind === "symlink") { + return resolveParts(db, parts, followFinal, 0); + } + storeResolveCache(db, canonical, cte.node === null ? null : cte.node.inode); + return cte.node; +} + +interface CteRow { + level: number; + inode: number; + type: "file" | "dir" | "symlink"; + mode: number; + mtime: number; + size: number; + link_target: string | null; +} + +type CteResolution = + // Walk completed with no symlink on the path: `node` is the resolved + // final node, or null when a segment was missing or an intermediate + // was not a directory (both map to null, exactly like the loop). + | { kind: "resolved"; node: ResolvedInode | null } + // A symlink was encountered anywhere on the path (intermediate or + // final). The CTE can't follow links, so the caller must fall back to + // the loop for byte-identical follow / ELOOP / dangling behaviour. + | { kind: "symlink" }; + +// Single-statement path walk. Binds the canonical path segments as a +// JSON array and walks vfs_dirents -> vfs_nodes from ROOT_INODE, one +// level per segment. Descends only through directories (WHERE +// w.type = 'dir'), so a file intermediate stalls the walk (ENOTDIR) +// and a missing dirent produces no row (ENOENT) — both surface as a +// missing level-D row, matching the loop's null. Every node the walk +// touches is returned so the caller can detect any symlink and fall +// back. +function resolveViaCte(db: Database, parts: string[]): CteResolution { + const rows = db.all( + `WITH RECURSIVE + segs(level, name) AS ( + SELECT key, value FROM json_each(?) + ), + walk(level, inode, type, mode, mtime, size, link_target) AS ( + SELECT 0, n.inode, n.type, n.mode, n.mtime, n.size, n.link_target + FROM vfs_nodes n + WHERE n.inode = ? + UNION ALL + SELECT w.level + 1, n.inode, n.type, n.mode, n.mtime, n.size, n.link_target + FROM walk w + JOIN segs s ON s.level = w.level + JOIN vfs_dirents d ON d.parent_inode = w.inode AND d.name = s.name + JOIN vfs_nodes n ON n.inode = d.child_inode + WHERE w.type = 'dir' + ) + SELECT level, inode, type, mode, mtime, size, link_target + FROM walk + ORDER BY level`, + JSON.stringify(parts), + ROOT_INODE, + ); + + const depth = parts.length; + let target: CteRow | undefined; + for (const row of rows) { + // Any symlink on the walk (root is level 0 and always a dir) means + // the loop must take over to follow it. + if (row.level >= 1 && row.type === "symlink") { + return { kind: "symlink" }; + } + if (row.level === depth) { + target = row; + } + } + return { + kind: "resolved", + node: target === undefined ? null : toResolved(target), + }; +} + +function toResolved(node: NodeRow): ResolvedInode { + return { + inode: node.inode, + type: node.type, + mode: node.mode, + mtime: node.mtime, + size: node.size, + linkTarget: node.link_target ?? undefined, + }; } function resolveParts( diff --git a/packages/dofs/src/fs/resolveCache.test.ts b/packages/dofs/src/fs/resolveCache.test.ts new file mode 100644 index 00000000..b3e61b52 --- /dev/null +++ b/packages/dofs/src/fs/resolveCache.test.ts @@ -0,0 +1,248 @@ +import { describe, expect, it } from "vitest"; +import { applyChangesSync } from "../sync/apply.js"; +import { link } from "./link.js"; +import { mkdir } from "./mkdir.js"; +import { rename } from "./rename.js"; +import { resolveInode } from "./resolve.js"; +import { invalidateResolveExact } from "./resolveCache.js"; +import { rm } from "./rm.js"; +import { symlink } from "./symlink.js"; +import { withDB } from "./with-db.js"; +import { writeFileSync } from "./writeFile.js"; + +const NOW = (): number => 1000; + +function bytesOf(text: string): Uint8Array { + return new TextEncoder().encode(text); +} + +// These prove the path->inode cache never serves a stale result across +// every mutation shape, and that the recursive-CTE resolve's symlink +// fallback matches the per-component loop. They run under both backends +// (node:sqlite and real DO SqlStorage) via withDB, so cache correctness +// is exercised on the shipping storage. +describe("resolve cache + CTE resolve", () => { + it("drops a negative entry the instant the path is created", async () => { + await withDB(async (db) => { + mkdir(db, "/d", { recursive: true }, NOW); + // Prime a negative entry. + expect(resolveInode(db, "/d/f")).toBeNull(); + writeFileSync(db, "/d/f", bytesOf("x"), {}, NOW); + // Must resolve to the new inode, not a stale ENOENT. + expect(resolveInode(db, "/d/f")?.type).toBe("file"); + }); + }); + + it("invalidates a positive entry on unlink, rename, and rmdir", async () => { + await withDB(async (db) => { + mkdir(db, "/p", { recursive: true }, NOW); + + writeFileSync(db, "/p/a", bytesOf("a"), {}, NOW); + expect(resolveInode(db, "/p/a")?.inode).toBeGreaterThan(0); // prime + rm(db, "/p/a", {}); + expect(resolveInode(db, "/p/a")).toBeNull(); + + writeFileSync(db, "/p/b", bytesOf("b"), {}, NOW); + expect(resolveInode(db, "/p/b")).not.toBeNull(); // prime + rename(db, "/p/b", "/p/c"); + expect(resolveInode(db, "/p/b")).toBeNull(); + expect(resolveInode(db, "/p/c")).not.toBeNull(); + + mkdir(db, "/p/sub", { recursive: true }, NOW); + expect(resolveInode(db, "/p/sub")).not.toBeNull(); // prime + rm(db, "/p/sub", {}); + expect(resolveInode(db, "/p/sub")).toBeNull(); + }); + }); + + it("invalidates every descendant path on a directory rename", async () => { + await withDB(async (db) => { + mkdir(db, "/src/inner", { recursive: true }, NOW); + writeFileSync(db, "/src/inner/deep.txt", bytesOf("d"), {}, NOW); + // Prime positives for the whole chain. + const deepInode = resolveInode(db, "/src/inner/deep.txt")?.inode; + expect(resolveInode(db, "/src/inner")).not.toBeNull(); + expect(deepInode).toBeGreaterThan(0); + + rename(db, "/src", "/dst"); + + // Old descendant paths must be gone, not stale positives. + expect(resolveInode(db, "/src/inner/deep.txt")).toBeNull(); + expect(resolveInode(db, "/src/inner")).toBeNull(); + expect(resolveInode(db, "/src")).toBeNull(); + // New paths resolve; the moved file keeps its inode. + expect(resolveInode(db, "/dst/inner/deep.txt")?.inode).toBe(deepInode); + }); + }); + + it("resolves a hardlink's second name to the shared inode", async () => { + await withDB(async (db) => { + mkdir(db, "/h", { recursive: true }, NOW); + writeFileSync(db, "/h/a", bytesOf("shared"), {}, NOW); + const inode = resolveInode(db, "/h/a")?.inode; + // Prime a negative for the not-yet-existing link path. + expect(resolveInode(db, "/h/b")).toBeNull(); + link(db, "/h/a", "/h/b"); + expect(resolveInode(db, "/h/b")?.inode).toBe(inode); + }); + }); + + it("reflects a sync-applied change through cached negative and positive paths", async () => { + await withDB(async (db) => { + mkdir(db, "/s", { recursive: true }, NOW); + writeFileSync(db, "/s/existing", bytesOf("e"), {}, NOW); + // Prime: negative for a dir we will create, positive for a file we + // will delete. + expect(resolveInode(db, "/s/newdir")).toBeNull(); + expect(resolveInode(db, "/s/existing")).not.toBeNull(); + + // applyChangesSync funnels through mkdir (create) and rm (delete), + // both of which invalidate the cache. + applyChangesSync( + db, + [ + { kind: "dir", rev: 1, path: "/s/newdir", mode: 0o755, mtime: 1000 }, + { kind: "delete", rev: 2, path: "/s/existing" }, + ], + new Map(), + ); + + expect(resolveInode(db, "/s/newdir")?.type).toBe("dir"); + expect(resolveInode(db, "/s/existing")).toBeNull(); + + // Apply a symlink over a POPULATED directory: exercises apply's own + // conflict-cleanup branch (removeReplaceableFinalEntry -> + // removeInodeTreeAtPath subtree invalidation) plus symlink-create + // invalidation, with a primed descendant positive. + mkdir(db, "/s/dir/child", { recursive: true }, NOW); + expect(resolveInode(db, "/s/dir/child")).not.toBeNull(); + expect(resolveInode(db, "/s/dir")?.type).toBe("dir"); + applyChangesSync( + db, + [ + { + kind: "symlink", + rev: 3, + path: "/s/dir", + target: "/elsewhere", + mode: 0o777, + mtime: 1000, + }, + ], + new Map(), + ); + // /s/dir is now a symlink; the former descendant no longer resolves. + expect(resolveInode(db, "/s/dir", { followSymlinks: false })?.type).toBe("symlink"); + expect(resolveInode(db, "/s/dir/child")).toBeNull(); + }); + }); + + it("leaves no stale entry after a rolled-back write", async () => { + await withDB(async (db) => { + mkdir(db, "/r", { recursive: true }, NOW); + const rDir = resolveInode(db, "/r")?.inode ?? 0; + writeFileSync(db, "/r/keep", bytesOf("k"), {}, NOW); + const keepInode = resolveInode(db, "/r/keep")?.inode; // prime positive + expect(keepInode).toBeGreaterThan(0); + + // A mutation is (structural write + cache invalidation) inside one + // transaction. This drives that shape with raw statements at a + // single transaction level — the DO backend forbids the nested + // savepoints an outer db.transactionSync around an fs op would use, + // and the cache's rollback-safety is backend-independent anyway. + + // Rolled-back delete: the invalidation drops the entry mid-txn; the + // rollback restores the dirent; the recompute finds it alive again + // (no stale ENOENT). + expect(() => + db.transactionSync(() => { + db.run("DELETE FROM vfs_dirents WHERE parent_inode = ? AND name = ?", rDir, "keep"); + invalidateResolveExact(db, "/r/keep"); + throw new Error("boom-delete"); + }), + ).toThrow("boom-delete"); + expect(resolveInode(db, "/r/keep")?.inode).toBe(keepInode); + + // Rolled-back create + inode reuse: population is gated inside a + // transaction, so the doomed inode is never cached for /r/new. A + // broken gate would cache it and, after AUTOINCREMENT reuses the + // number on the next committed create, alias /r/new to /r/other. + expect(resolveInode(db, "/r/new")).toBeNull(); // prime negative + expect(() => + db.transactionSync(() => { + db.run("INSERT INTO vfs_nodes (type, mode, mtime, rev) VALUES ('file', 420, 0, 0)"); + const inode = db.scalar("SELECT last_insert_rowid() AS v") ?? 0; + db.run( + "INSERT INTO vfs_dirents (parent_inode, name, child_inode) VALUES (?, ?, ?)", + rDir, + "new", + inode, + ); + invalidateResolveExact(db, "/r/new"); + // Reading inside the txn must not populate the cache (the gate). + expect(resolveInode(db, "/r/new")).not.toBeNull(); + throw new Error("boom-create"); + }), + ).toThrow("boom-create"); + writeFileSync(db, "/r/other", bytesOf("o"), {}, NOW); + expect(resolveInode(db, "/r/new")).toBeNull(); + expect(resolveInode(db, "/r/other")).not.toBeNull(); + }); + }); + + it("falls back to the loop for symlinks, resolving identically", async () => { + await withDB(async (db) => { + mkdir(db, "/target/sub", { recursive: true }, NOW); + writeFileSync(db, "/target/sub/file.txt", bytesOf("hello"), {}, NOW); + const realInode = resolveInode(db, "/target/sub/file.txt")?.inode; + + // Intermediate symlink: /link -> /target. + symlink(db, "/target", "/link", NOW); + // Following through the link reaches the real file inode. + expect(resolveInode(db, "/link/sub/file.txt")?.inode).toBe(realInode); + // Following the link itself lands on the directory it points at. + expect(resolveInode(db, "/link")?.type).toBe("dir"); + // lstat (no follow) lands on the symlink node itself. + const withoutFollow = resolveInode(db, "/link", { followSymlinks: false }); + expect(withoutFollow?.type).toBe("symlink"); + expect(withoutFollow?.linkTarget).toBe("/target"); + + // Dangling symlink: ENOENT when followed, symlink node when not. + symlink(db, "/nope", "/dangling", NOW); + expect(resolveInode(db, "/dangling")).toBeNull(); + expect(resolveInode(db, "/dangling", { followSymlinks: false })?.type).toBe("symlink"); + }); + }); + + it("drops a negative primed beneath a path that a new symlink makes resolvable", async () => { + await withDB(async (db) => { + mkdir(db, "/target", { recursive: true }, NOW); + writeFileSync(db, "/target/x", bytesOf("x"), {}, NOW); + const realInode = resolveInode(db, "/target/x")?.inode; + // Prime a negative for a path beneath where the link will land + // (no symlink on the path yet, so the negative is cached). + expect(resolveInode(db, "/link/x")).toBeNull(); + + // Creating the symlink makes "/link/x" resolvable through it; the + // subtree invalidation must drop the stale negative beneath it. + symlink(db, "/target", "/link", NOW); + expect(resolveInode(db, "/link/x")?.inode).toBe(realInode); + }); + }); + + it("does not cache a negative when a symlink on the path forces the loop", async () => { + await withDB(async (db) => { + mkdir(db, "/target", { recursive: true }, NOW); + symlink(db, "/target", "/link", NOW); + // Resolve through the link before the leaf exists: the CTE bails + // to the loop and must cache nothing for the aliased path. + expect(resolveInode(db, "/link/x")).toBeNull(); + + // Create the real leaf. This invalidates "/target/x", not the + // "/link/x" alias — so a negative wrongly cached by the bail would + // linger and this read would still see null. + writeFileSync(db, "/target/x", bytesOf("x"), {}, NOW); + expect(resolveInode(db, "/link/x")?.type).toBe("file"); + }); + }); +}); diff --git a/packages/dofs/src/fs/resolveCache.ts b/packages/dofs/src/fs/resolveCache.ts new file mode 100644 index 00000000..0b40f054 --- /dev/null +++ b/packages/dofs/src/fs/resolveCache.ts @@ -0,0 +1,129 @@ +// Per-Database path -> inode resolution cache. +// +// Maps a canonical absolute path (as produced by +// canonicalizePath().path) to the inode that resolveInode(path, +// { followSymlinks: true }) lands on, or a NEGATIVE marker when the +// path does not resolve. It turns repeat stat/exists/read of the same +// path into an O(1) lookup instead of an O(depth) walk. +// +// Deliberately narrow, for correctness: +// +// * Only the path -> inode MAPPING is cached. resolveInode always +// re-reads the node row on a hit, so content changes (chmod, +// size/mtime, type) are never served stale — only structural +// mutations that move a dirent can invalidate an entry. +// +// * Only symlink-free resolutions are cached. Following a symlink +// makes the cached path an alias of the target whose invalidation +// can't be reasoned about from the path alone, so resolveInode +// stores nothing when a symlink was traversed. +// +// * Population is gated on Database.inTransaction: entries are only +// written outside a transaction, so a rolled-back mutation can +// never leave a positive/negative entry reflecting uncommitted +// state. Mutations invalidate (drop) freely — dropping is safe +// under rollback because the worst case is a recompute. +// +// The cache is per-Database (WeakMap) and bounded (LRU by Map +// insertion order, same discipline as blobCache). + +import type { Database } from "../storage.js"; + +// Sentinel value for "this path resolves to nothing" (ENOENT/ENOTDIR). +const NEGATIVE = -1; + +// Upper bound on cached paths per Database. Entries are tiny (a string +// key and a number), so this caps memory at a few MB while covering +// the working set of a busy tree. +const MAX_ENTRIES = 8192; + +// Keyed by the Database instance, so correctness assumes exactly one +// Database wraps each SqlStorage. Two Databases over the same storage +// would hold independent caches and could serve each other stale +// results; the DO owns a single Database, which upholds this. +const caches = new WeakMap>(); + +function cacheFor(db: Database): Map { + let cache = caches.get(db); + if (cache === undefined) { + cache = new Map(); + caches.set(db, cache); + } + return cache; +} + +export type ResolveCacheHit = { kind: "inode"; inode: number } | { kind: "negative" }; + +// Look up a canonical path. Returns undefined on a miss, a positive +// inode hit, or a negative (known-absent) hit. Bumps LRU recency. +export function lookupResolveCache( + db: Database, + canonicalPath: string, +): ResolveCacheHit | undefined { + const cache = cacheFor(db); + const value = cache.get(canonicalPath); + if (value === undefined) { + return undefined; + } + // Move to most-recent position for LRU eviction. + cache.delete(canonicalPath); + cache.set(canonicalPath, value); + return value === NEGATIVE ? { kind: "negative" } : { kind: "inode", inode: value }; +} + +// Cache a resolution. `inode === null` records a negative entry. No-op +// while a transaction is active so the cache never reflects +// uncommitted state (rollback safety). +export function storeResolveCache(db: Database, canonicalPath: string, inode: number | null): void { + if (db.inTransaction) { + return; + } + const cache = cacheFor(db); + cache.set(canonicalPath, inode === null ? NEGATIVE : inode); + while (cache.size > MAX_ENTRIES) { + const oldest = cache.keys().next(); + if (oldest.done === true) { + break; + } + cache.delete(oldest.value); + } +} + +// Drop the entry for exactly `canonicalPath`. Use after a mutation +// that changes a single leaf's existence without affecting anything +// beneath it: creating/removing a file, symlink, hardlink, or an +// empty directory. O(1). +export function invalidateResolveExact(db: Database, canonicalPath: string): void { + const cache = caches.get(db); + cache?.delete(canonicalPath); +} + +// Drop `canonicalPath` and every entry beneath it (keys prefixed +// `canonicalPath + "/"`). Use when a mutation changes a whole subtree's +// resolution: a recursive delete, any directory rename (every +// descendant's path changes), a structural subtree replacement, or a +// symlink create (paths *through* the new link become resolvable, so +// stale negatives beneath it must go). Root ("/") clears everything. +export function invalidateResolveSubtree(db: Database, canonicalPath: string): void { + const cache = caches.get(db); + if (cache === undefined || cache.size === 0) { + return; + } + if (canonicalPath === "/") { + cache.clear(); + return; + } + cache.delete(canonicalPath); + const prefix = `${canonicalPath}/`; + for (const key of cache.keys()) { + if (key.startsWith(prefix)) { + cache.delete(key); + } + } +} + +// Drop the entire cache for a Database. Used by tests and available as +// a blunt reset. +export function clearResolveCache(db: Database): void { + caches.get(db)?.clear(); +} diff --git a/packages/dofs/src/fs/rm.ts b/packages/dofs/src/fs/rm.ts index dfb7c7b5..cf82fb8e 100644 --- a/packages/dofs/src/fs/rm.ts +++ b/packages/dofs/src/fs/rm.ts @@ -6,6 +6,7 @@ import { recordDelete } from "../sync/changes.js"; import { pathOf } from "../sync/paths.js"; import { assertNotReadOnly } from "./mount-guard.js"; import { resolveInode } from "./resolve.js"; +import { invalidateResolveExact, invalidateResolveSubtree } from "./resolveCache.js"; import { unlinkDirent } from "./unlink.js"; export interface RmOptions { @@ -158,6 +159,9 @@ export function rm(db: Database, path: string, options: RmOptions): void { // the move-aware location. unlinkDirent(db, parent.inode, name, node.inode, node.type); recordDelete(db, rev, realPath); + // A single removed entry is a file, symlink, or empty directory: + // no cached descendants to worry about, so drop it exact. + invalidateResolveExact(db, realPath); return; } @@ -170,5 +174,8 @@ export function rm(db: Database, path: string, options: RmOptions): void { unlinkDirent(db, entry.parentInode, entry.name, entry.inode, entry.type); recordDelete(db, rev, entry.path); } + // The whole subtree under realPath is gone; one subtree drop covers + // every descendant's cached resolution. + invalidateResolveSubtree(db, realPath); }); } diff --git a/packages/dofs/src/fs/symlink.ts b/packages/dofs/src/fs/symlink.ts index 695db349..cbd6f2e9 100644 --- a/packages/dofs/src/fs/symlink.ts +++ b/packages/dofs/src/fs/symlink.ts @@ -4,6 +4,7 @@ import { incrementRev } from "../rev.js"; import { ROOT_INODE } from "../schema/index.js"; import type { Database } from "../storage.js"; import { assertNotReadOnly } from "./mount-guard.js"; +import { invalidateResolveSubtree } from "./resolveCache.js"; // Create a symlink node. The target is stored as-is — it can be a // relative or absolute path, dangling or live. resolveInode follows @@ -72,5 +73,9 @@ export function symlink(db: Database, target: string, path: string, now: () => n leafName, inode, ); + // Subtree, not exact: paths *through* the new link (e.g. /s/x when + // /s -> a populated dir) now resolve, so any cached negative + // beneath the link must be dropped. + invalidateResolveSubtree(db, canonical); }); } diff --git a/packages/dofs/src/fs/writeFile.ts b/packages/dofs/src/fs/writeFile.ts index eca85dbe..1f045565 100644 --- a/packages/dofs/src/fs/writeFile.ts +++ b/packages/dofs/src/fs/writeFile.ts @@ -8,6 +8,7 @@ import { stageBlob } from "../sync/blobs.js"; import { buildManifest } from "../sync/manifests.js"; import { getBlobBytes } from "./blobCache.js"; import { assertNotReadOnly } from "./mount-guard.js"; +import { invalidateResolveExact } from "./resolveCache.js"; import { allocatePendingInode, deleteWriteBuffer, @@ -222,12 +223,7 @@ async function writeFileStreaming( db.run("DELETE FROM vfs_chunks WHERE inode = ?", inode); } else { inode = insertFileNode(db, mode, mtime); - db.run( - "INSERT INTO vfs_dirents (parent_inode, name, child_inode) VALUES (?, ?, ?)", - parentInode, - leafName, - inode, - ); + insertFileDirent(db, parentInode, leafName, inode, canonical); } for (let idx = 0; idx < chunkRefs.length; idx++) { const ref = chunkRefs[idx]; @@ -258,6 +254,28 @@ async function writeFileStreaming( // Allocate a fresh file inode row with the supplied mode and mtime, // using SQLite's RETURNING so the new rowid comes back in the same // statement instead of through a follow-up SELECT last_insert_rowid(). +// Link a freshly created file inode into its parent directory and drop +// any cached negative resolution for the new path. The single choke +// point for every new-file dirent, so the resolve cache stays correct +// on create without touching the overwrite path (which reuses the +// existing inode and dirent, so its resolution is unchanged). A new +// file is a leaf with no descendants, so exact invalidation suffices. +function insertFileDirent( + db: Database, + parentInode: number, + leafName: string, + childInode: number, + canonicalPath: string, +): void { + db.run( + "INSERT INTO vfs_dirents (parent_inode, name, child_inode) VALUES (?, ?, ?)", + parentInode, + leafName, + childInode, + ); + invalidateResolveExact(db, canonicalPath); +} + function insertFileNode(db: Database, mode: number, mtime: number): number { const row = db.one<{ inode: number }>( "INSERT INTO vfs_nodes (type, mode, mtime, rev) VALUES ('file', ?, ?, 0) RETURNING inode", @@ -476,12 +494,7 @@ export function createFileSync( rev, ); if (row === undefined) throw createWorkspaceError("EIO", "failed to allocate inode"); - db.run( - "INSERT INTO vfs_dirents (parent_inode, name, child_inode) VALUES (?, ?, ?)", - parentInode, - leafName, - row.inode, - ); + insertFileDirent(db, parentInode, leafName, row.inode, canonical); }); } @@ -651,12 +664,7 @@ function commitPendingBuffer(db: Database, entry: WriteBufferEntry, now: () => n if (row === undefined) { throw createWorkspaceError("EIO", "failed to allocate inode"); } - db.run( - "INSERT INTO vfs_dirents (parent_inode, name, child_inode) VALUES (?, ?, ?)", - parentInode, - leafName, - row.inode, - ); + insertFileDirent(db, parentInode, leafName, row.inode, canonicalPath); if (entry.size > 0) { const inode = row.inode; const chunkCount = Math.ceil(entry.size / CHUNK_SIZE); @@ -935,12 +943,7 @@ export function writeFileSync( db.run("DELETE FROM vfs_chunks WHERE inode = ?", inode); } else { inode = insertFileNode(db, mode, mtime); - db.run( - "INSERT INTO vfs_dirents (parent_inode, name, child_inode) VALUES (?, ?, ?)", - parentInode, - leafName, - inode, - ); + insertFileDirent(db, parentInode, leafName, inode, canonical); } const rev = incrementRev(db); @@ -1010,12 +1013,7 @@ export function writeFileRangesSync( oldChunks = existingChunkRefs(db, inode); } else { inode = insertFileNode(db, mode, mtime); - db.run( - "INSERT INTO vfs_dirents (parent_inode, name, child_inode) VALUES (?, ?, ?)", - parentInode, - leafName, - inode, - ); + insertFileDirent(db, parentInode, leafName, inode, canonical); } const rev = incrementRev(db); diff --git a/packages/dofs/src/storage.ts b/packages/dofs/src/storage.ts index 7408ce79..becce5a7 100644 --- a/packages/dofs/src/storage.ts +++ b/packages/dofs/src/storage.ts @@ -58,6 +58,21 @@ export class Database { }; } + // True while a transactionSync closure is on the stack. The resolve + // cache uses this to refuse populating entries mid-transaction, so a + // rolled-back mutation can never leave the cache reflecting + // uncommitted state. (Invalidation still runs freely inside a + // transaction — dropping an entry is always safe.) + // + // Invariant: #txDepth only tracks transactionSync. A raw + // BEGIN/SAVEPOINT issued through run() would open a transaction this + // flag can't see, letting the cache populate mid-transaction and + // survive a rollback — so transactionSync is the only sanctioned way + // to open one. + get inTransaction(): boolean { + return this.#txDepth > 0; + } + run(query: string, ...bindings: unknown[]): void { this.sql.exec(query, ...bindings); } diff --git a/packages/dofs/src/sync/apply.ts b/packages/dofs/src/sync/apply.ts index 19211190..4986c98e 100644 --- a/packages/dofs/src/sync/apply.ts +++ b/packages/dofs/src/sync/apply.ts @@ -1,6 +1,7 @@ import { mkdir } from "../fs/mkdir.js"; import { readOnlyRootFor } from "../fs/mount-guard.js"; import { resolveInode } from "../fs/resolve.js"; +import { invalidateResolveSubtree } from "../fs/resolveCache.js"; import { rm } from "../fs/rm.js"; import { symlink } from "../fs/symlink.js"; import { unlinkDirent } from "../fs/unlink.js"; @@ -96,6 +97,12 @@ function removeReplaceableFinalEntry( // the local shape without recording tombstones because the incoming // entry is the authoritative state for this path. function removeInodeTreeAtPath(db: Database, path: string, inode: number, type: NodeType): void { + // Structural subtree removal that bypasses rm() and calls unlinkDirent + // directly, so it must drop cached resolutions itself. One subtree + // drop at the root covers every descendant the walk unlinks. + // Canonicalize to the exact key readers cache under (every other hook + // already passes a canonical path; this one takes an entry path). + invalidateResolveSubtree(db, canonicalizePath(path).path); const root = direntForPath(db, path, inode); const stack: Array<{ path: string; From 49a48c04274a65fdfdada428de57aa07578ef7ff Mon Sep 17 00:00:00 2001 From: Nathan Disidore Date: Sat, 4 Jul 2026 15:23:37 +0000 Subject: [PATCH 5/7] dofs: batch directory-rename subtree writes and encode manifests once A directory rename stamps the shared rev across the moved inode subtree and records an old-path tombstone per edge with two set-based statements over one recursive-CTE walk: an UPDATE keyed on the subtree inodes and an INSERT ... SELECT of the old paths. The self-move guard reuses the same walk. The tombstone rows and rev stamps form the set the sync log expects, coalesced by rev and path independently of insertion order. File and symlink renames take the single-row path. buildManifest encodes the chunk list once and hashes those bytes, so the manifest hash and the stored encoding stay in step by construction. --- packages/dofs/src/fs/rename.ts | 130 +++++++++++++++------------- packages/dofs/src/provider.test.ts | 44 ++++++++++ packages/dofs/src/sync/manifests.ts | 26 +++--- 3 files changed, 126 insertions(+), 74 deletions(-) diff --git a/packages/dofs/src/fs/rename.ts b/packages/dofs/src/fs/rename.ts index a7854521..c2692b1e 100644 --- a/packages/dofs/src/fs/rename.ts +++ b/packages/dofs/src/fs/rename.ts @@ -9,18 +9,6 @@ import { resolveInode } from "./resolve.js"; import { invalidateResolveExact, invalidateResolveSubtree } from "./resolveCache.js"; import { unlinkDirent } from "./unlink.js"; -interface DirChild { - name: string; - child_inode: number; - type: NodeType; -} - -interface SubtreeEntry { - path: string; - inode: number; - type: NodeType; -} - type NodeType = "file" | "dir" | "symlink"; export function rename(db: Database, oldPath: string, newPath: string): void { @@ -101,17 +89,20 @@ export function rename(db: Database, oldPath: string, newPath: string): void { newName, ); - const oldEntries = - source.type === "dir" - ? collectSubtree(db, source.inode, oldRealPath) - : [{ path: oldRealPath, inode: source.inode, type: source.type }]; - if (source.type === "dir") { - // Authoritative directory self-move guard. It tests the *resolved* - // destination parent inode against the source subtree, so it - // catches a symlinked destination that lands inside the source and - // allows one that resolves outside it. A textual prefix test on the - // unresolved path cannot do either and is intentionally absent. - assertDestinationParentOutsideSource(oldEntries, newParent.inode, oldRealPath, newCanonical); + // Authoritative directory self-move guard. It tests the *resolved* + // destination parent inode against the source subtree, so it catches + // a symlinked destination that lands inside the source and allows one + // that resolves outside it. A textual prefix test on the unresolved + // path could do neither and is intentionally absent. + if ( + source.type === "dir" && + renamedSubtreeContains(db, source.inode, oldRealPath, newParent.inode) + ) { + throw createWorkspaceError( + "EINVAL", + `cannot rename a directory into itself: ${oldRealPath}`, + newCanonical, + ); } if (existing !== undefined) { @@ -149,10 +140,15 @@ export function rename(db: Database, oldPath: string, newPath: string): void { // live entries for the moved inode subtree, so stamp only that // subtree with the shared rev. Parent directory mtimes are left // unchanged on purpose; this diverges from POSIX rename(2), but - // avoids treating the old and new parents as content changes. - for (const entry of oldEntries) { - db.run("UPDATE vfs_nodes SET rev = ? WHERE inode = ?", rev, entry.inode); - recordDelete(db, rev, entry.path); + // avoids treating the old and new parents as content changes. A + // directory move stamps and tombstones its whole subtree in two + // set-based statements; a file or symlink touches one inode and + // one path. + if (source.type === "dir") { + stampRenamedSubtree(db, source.inode, oldRealPath, rev); + } else { + db.run("UPDATE vfs_nodes SET rev = ? WHERE inode = ?", rev, source.inode); + recordDelete(db, rev, oldRealPath); } // Drop cached resolutions for both endpoints. A directory move @@ -183,42 +179,52 @@ function assertCompatibleOverwrite( } } -function assertDestinationParentOutsideSource( - oldEntries: SubtreeEntry[], - newParentInode: number, - oldCanonical: string, - newCanonical: string, -): void { - if (!oldEntries.some((entry) => entry.inode === newParentInode)) return; - - throw createWorkspaceError( - "EINVAL", - `cannot rename a directory into itself: ${oldCanonical}`, - newCanonical, +// Recursive walk of a directory subtree seeded at an inode and its +// path. Descends through directory dirents only, so files and symlinks +// are leaves and each hardlink name yields its own row (matching the +// per-component collection it replaces). Bound as a reusable WITH +// clause whose two placeholders are the seed inode and path; callers +// append their own projection. +const SUBTREE_CTE = `WITH RECURSIVE subtree(inode, type, path) AS ( + SELECT ?, 'dir', ? + UNION ALL + SELECT n.inode, n.type, + CASE WHEN s.path = '/' THEN '/' || d.name ELSE s.path || '/' || d.name END + FROM subtree s + JOIN vfs_dirents d ON d.parent_inode = s.inode + JOIN vfs_nodes n ON n.inode = d.child_inode + WHERE s.type = 'dir' +)`; + +function renamedSubtreeContains( + db: Database, + rootInode: number, + rootPath: string, + targetInode: number, +): boolean { + const hit = db.one<{ hit: number }>( + `${SUBTREE_CTE} SELECT 1 AS hit FROM subtree WHERE inode = ? LIMIT 1`, + rootInode, + rootPath, + targetInode, ); + return hit !== undefined; } -function collectSubtree(db: Database, rootInode: number, rootPath: string): SubtreeEntry[] { - const entries: SubtreeEntry[] = [{ path: rootPath, inode: rootInode, type: "dir" }]; - for (let idx = 0; idx < entries.length; idx++) { - const entry = entries[idx]; - if (entry.type !== "dir") continue; - const children = db.all( - `SELECT d.name AS name, d.child_inode AS child_inode, n.type AS type - FROM vfs_dirents d - JOIN vfs_nodes n ON n.inode = d.child_inode - WHERE d.parent_inode = ? - ORDER BY d.name`, - entry.inode, - ); - for (const child of children) { - const childPath = entry.path === "/" ? `/${child.name}` : `${entry.path}/${child.name}`; - entries.push({ - path: childPath, - inode: child.child_inode, - type: child.type, - }); - } - } - return entries; +// Stamp the shared rev on every inode in the moved subtree and record +// an old-path tombstone for each entry, in two set-based statements +// over the same walk. +function stampRenamedSubtree(db: Database, rootInode: number, rootPath: string, rev: number): void { + db.run( + `${SUBTREE_CTE} UPDATE vfs_nodes SET rev = ? WHERE inode IN (SELECT inode FROM subtree)`, + rootInode, + rootPath, + rev, + ); + db.run( + `${SUBTREE_CTE} INSERT INTO vfs_changes (rev, path, op) SELECT ?, path, 'delete' FROM subtree ORDER BY path`, + rootInode, + rootPath, + rev, + ); } diff --git a/packages/dofs/src/provider.test.ts b/packages/dofs/src/provider.test.ts index bb25cae9..769086d7 100644 --- a/packages/dofs/src/provider.test.ts +++ b/packages/dofs/src/provider.test.ts @@ -315,6 +315,50 @@ describe("SQLiteWorkspaceProvider — renameSync overwrite matrix", () => { }); }); + it("subtree rename writes one tombstone per edge and one rev stamp per inode", async () => { + await withProviderAndDB(async (p, db) => { + // A nested subtree with a hardlink inside it: the file inode is + // reachable by two names, so both edges must be tombstoned while + // the single inode is stamped once. + p.mkdirSync("/src", {}); + p.writeFileSync("/src/a.txt", "a"); + p.mkdirSync("/src/sub", {}); + p.writeFileSync("/src/sub/b.txt", "b"); + p.linkSync("/src/a.txt", "/src/sub/a2.txt"); + const cursor = db.scalar("SELECT v FROM vfs_meta WHERE k = 'rev'") ?? 0; + + p.renameSync("/src", "/dst"); + + // Every edge of the moved subtree, tombstoned at its old path and + // sharing the rename's rev. + const tombstones = db.all<{ rev: number; path: string; op: string }>( + "SELECT rev, path, op FROM vfs_changes WHERE rev > ? ORDER BY path", + cursor, + ); + const rev = tombstones[0]?.rev; + expect(tombstones).toEqual([ + { rev, path: "/src", op: "delete" }, + { rev, path: "/src/a.txt", op: "delete" }, + { rev, path: "/src/sub", op: "delete" }, + { rev, path: "/src/sub/a2.txt", op: "delete" }, + { rev, path: "/src/sub/b.txt", op: "delete" }, + ]); + + // The shared rev lands on exactly the four subtree inodes (the + // hardlinked file counted once) and on nothing else. + const stamped = db + .all<{ inode: number }>("SELECT inode FROM vfs_nodes WHERE rev = ? ORDER BY inode", rev) + .map((r) => r.inode); + const expected = [ + p.statSync("/dst").ino, + p.statSync("/dst/a.txt").ino, + p.statSync("/dst/sub").ino, + p.statSync("/dst/sub/b.txt").ino, + ].sort((a, b) => a - b); + expect(stamped).toEqual(expected); + }); + }); + it("same-inode rename through a symlinked file path is a no-op", async () => { await withProviderAndDB(async (p, db) => { p.mkdirSync("/real", {}); diff --git a/packages/dofs/src/sync/manifests.ts b/packages/dofs/src/sync/manifests.ts index 3f47f90f..98dfcfbc 100644 --- a/packages/dofs/src/sync/manifests.ts +++ b/packages/dofs/src/sync/manifests.ts @@ -35,18 +35,24 @@ function sha256(bytes: Uint8Array): Uint8Array { return new Uint8Array(createHash("sha256").update(bytes).digest()); } +// Serialize a chunk list into the canonical manifest bytes. The +// hash is taken over these bytes and the same bytes are stored, so +// producing them once keeps the two in step. +function encodeManifest(chunks: ManifestChunk[]): Uint8Array { + const encoded: EncodedManifest = { + version: MANIFEST_VERSION, + chunks: chunks.map((c) => ({ hash: toHex(c.hash), size: c.size })), + }; + return new TextEncoder().encode(JSON.stringify(encoded)); +} + // Compute the manifest hash for a chunk list without touching the // DB. Used by the apply path to short-circuit when an upstream // entry already matches the local node — the manifest hash is // content-addressed so identical chunks always produce the same // hash. export function computeManifestHash(chunks: ManifestChunk[]): Uint8Array { - const encoded: EncodedManifest = { - version: MANIFEST_VERSION, - chunks: chunks.map((c) => ({ hash: toHex(c.hash), size: c.size })), - }; - const bytes = new TextEncoder().encode(JSON.stringify(encoded)); - return sha256(bytes); + return sha256(encodeManifest(chunks)); } // Build a manifest row for the given chunk list. Idempotent: a @@ -54,13 +60,9 @@ export function computeManifestHash(chunks: ManifestChunk[]): Uint8Array { // returned hash is what the caller writes onto // `vfs_nodes.manifest_hash`. export function buildManifest(db: Database, chunks: ManifestChunk[], now: number): Uint8Array { - const hash = computeManifestHash(chunks); + const bytes = encodeManifest(chunks); + const hash = sha256(bytes); const size = chunks.reduce((acc, c) => acc + c.size, 0); - const encoded: EncodedManifest = { - version: MANIFEST_VERSION, - chunks: chunks.map((c) => ({ hash: toHex(c.hash), size: c.size })), - }; - const bytes = new TextEncoder().encode(JSON.stringify(encoded)); db.run( "INSERT INTO vfs_manifests (hash, size, encoded, last_seen) VALUES (?, ?, ?, ?) ON CONFLICT(hash) DO UPDATE SET last_seen = excluded.last_seen", hash, From a3a73f2f6c200e85cda8983342021276db7bdb18 Mon Sep 17 00:00:00 2001 From: Nathan Disidore Date: Sat, 4 Jul 2026 19:51:49 +0000 Subject: [PATCH 6/7] dofs: resolve reads without restamping last_seen MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reads resolve chunk bytes by hash and leave vfs_blobs.last_seen untouched. A chunk being read is already linked to a node, so gc's orphan gate retains its blob independent of the timestamp and a read never needs to stamp it. last_seen remains the timer that guards blobs staged but not yet linked — the sync receiver, the streaming write, and manifests — and those write paths still stamp it. The read clock argument that fed the removed touch is dropped. --- packages/dofs/src/fs/filesystem.ts | 2 +- packages/dofs/src/fs/readFile.test.ts | 22 ++++++++++---- packages/dofs/src/fs/readFile.ts | 40 +++---------------------- packages/dofs/src/fs/writeRange.test.ts | 6 ++-- 4 files changed, 23 insertions(+), 47 deletions(-) diff --git a/packages/dofs/src/fs/filesystem.ts b/packages/dofs/src/fs/filesystem.ts index 7479eaa5..c48caa50 100644 --- a/packages/dofs/src/fs/filesystem.ts +++ b/packages/dofs/src/fs/filesystem.ts @@ -57,7 +57,7 @@ export class WorkspaceFilesystem { // Cast through the union overload of the free function; // the class's overloads above carry the precise return type // for each input shape back to the caller. - return readFile(this.db, path, optionsOrEncoding as ReadFileOptions, this.now); + return readFile(this.db, path, optionsOrEncoding as ReadFileOptions); } async stat(path: string): Promise { diff --git a/packages/dofs/src/fs/readFile.test.ts b/packages/dofs/src/fs/readFile.test.ts index d4c05f0c..c8d80c56 100644 --- a/packages/dofs/src/fs/readFile.test.ts +++ b/packages/dofs/src/fs/readFile.test.ts @@ -82,16 +82,26 @@ describe("readFile", () => { }); }); - it("touches vfs_blobs.last_seen when chunks are read", async () => { + it("does not modify vfs_blobs.last_seen when chunks are read", async () => { await withDB(async (db) => { const bytes = new Uint8Array(CHUNK_SIZE + 1); bytes.fill(0x61); await writeFile(db, "/x.txt", bytes, {}, () => 100); - const before = db.scalar("SELECT MIN(last_seen) FROM vfs_blobs"); - expect(before).toBe(100); - await readFile(db, "/x.txt", "utf8", () => 200); - const after = db.scalar("SELECT MIN(last_seen) FROM vfs_blobs"); - expect(after).toBe(200); + expect(db.scalar("SELECT MIN(last_seen) FROM vfs_blobs")).toBe(100); + + // String form reads every chunk and must leave last_seen alone. + await readFile(db, "/x.txt", "utf8"); + expect(db.scalar("SELECT MIN(last_seen) FROM vfs_blobs")).toBe(100); + + // Stream form: drain it so every chunk is pulled, then confirm + // no restamp happened in the pull callback. + const stream = await readFile(db, "/x.txt"); + const reader = stream.getReader(); + while (true) { + const { done } = await reader.read(); + if (done) break; + } + expect(db.scalar("SELECT MIN(last_seen) FROM vfs_blobs")).toBe(100); }); }); diff --git a/packages/dofs/src/fs/readFile.ts b/packages/dofs/src/fs/readFile.ts index 9dc2b40f..4d0c24e4 100644 --- a/packages/dofs/src/fs/readFile.ts +++ b/packages/dofs/src/fs/readFile.ts @@ -17,23 +17,16 @@ interface ChunkRow { // Overloads match docs/04_filesystem_interface.md exactly. export function readFile(db: Database, path: string): Promise>; -export function readFile( - db: Database, - path: string, - encoding: "utf8", - now?: () => number, -): Promise; +export function readFile(db: Database, path: string, encoding: "utf8"): Promise; export function readFile( db: Database, path: string, options: ReadFileOptions, - now?: () => number, ): Promise>; export async function readFile( db: Database, path: string, optionsOrEncoding?: "utf8" | ReadFileOptions, - now: () => number = Date.now, ): Promise> { const wantString = optionsOrEncoding === "utf8" || @@ -91,7 +84,6 @@ export async function readFile( const totalSize = chunks.reduce((acc, c) => acc + c.size, 0); const out = new Uint8Array(totalSize); let offset = 0; - const touched = now(); for (const chunk of chunks) { const bytes = getBlobBytes(db, chunk.hash); if (bytes === undefined) { @@ -100,15 +92,13 @@ export async function readFile( out.set(bytes, offset); offset += bytes.byteLength; } - if (chunks.length > 0) { - touchBlobs(db, chunks, touched); - } return new TextDecoder().decode(out); } // Stream form. We enqueue one Uint8Array per chunk, lazily pulled. - // last_seen is touched per chunk on read; that's the GC clock signal - // documented in 03_filesystem_schema.md. + // Reads resolve bytes by hash and never restamp last_seen: a chunk + // being read is already linked to a node, so gc's orphan gate keeps + // it. last_seen only guards blobs staged but not yet linked. let i = 0; return new ReadableStream({ pull(controller) { @@ -122,7 +112,6 @@ export async function readFile( controller.error(createWorkspaceError("EIO", `missing blob bytes for ${path}`, path)); return; } - db.run("UPDATE vfs_blobs SET last_seen = ? WHERE hash = ?", now(), chunk.hash); controller.enqueue(bytes); }, }); @@ -200,24 +189,3 @@ export function readRangeSync( } return written === out.byteLength ? out : out.subarray(0, written); } - -function touchBlobs(db: Database, chunks: ChunkRow[], at: number): void { - // Dedupe in case the same chunk hash appears multiple times in a - // single file — keeps the UPDATE count low without changing semantics. - const seen = new Set(); - for (const chunk of chunks) { - const key = bufferKey(chunk.hash); - if (seen.has(key)) continue; - seen.add(key); - db.run("UPDATE vfs_blobs SET last_seen = ? WHERE hash = ?", at, chunk.hash); - } -} - -function bufferKey(bytes: Uint8Array): string { - // crypto digests are 32 bytes; this is fine. - let key = ""; - for (const byte of bytes) { - key += byte.toString(16).padStart(2, "0"); - } - return key; -} diff --git a/packages/dofs/src/fs/writeRange.test.ts b/packages/dofs/src/fs/writeRange.test.ts index b04a600a..46d08548 100644 --- a/packages/dofs/src/fs/writeRange.test.ts +++ b/packages/dofs/src/fs/writeRange.test.ts @@ -145,10 +145,8 @@ describe("direct range writes", () => { // so their blobs are never re-upserted and keep the original // last_seen; only the rewritten middle chunk's blob is stamped with // the new mtime. (vfs_chunks is WITHOUT ROWID, so there is no rowid - // to watch. On the write path last_seen is bumped only by - // upsertChunkBlob, and this test performs no reads — reads also - // touch last_seen — so an unchanged last_seen proves the chunk row - // was skipped.) + // to watch. last_seen is bumped only by upsertChunkBlob, so an + // unchanged last_seen proves the chunk row was skipped.) const rows = chunkRows(db, "/large.bin"); expect(rows).toHaveLength(3); const lastSeen = (hash: Uint8Array): number | undefined => From 60ab69ce684cdb623ab9e7092a6dd2efdc2daf1e Mon Sep 17 00:00:00 2001 From: Nathan Disidore Date: Sat, 4 Jul 2026 20:23:51 +0000 Subject: [PATCH 7/7] dofs: scope ls to its subtree and batch hasObjects and range reads MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Directory listing resolves the prefix to an inode and seeds its recursive walk there, so it visits only the listed subtree rather than the whole store. The prefix walk follows dirents structurally and does not resolve symlinks, so a symlink prefix lists nothing. The output is unchanged: the same absolute file paths in path order. The sync object probe matches the requested hashes against vfs_blobs through an IN (…) list on the raw hash blobs, so the lookup rides the primary-key index instead of scanning; a probe larger than the window splits into a few bounded index lookups. The present subset comes back in input order with duplicates preserved. A positional read fetches its overlapping chunks in one indexed range scan ordered by index instead of a lookup per chunk. Missing indices are absent from the result, so the assembly compacts around them just as the per-chunk walk did, and the blob byte cache is consulted in the same order. --- packages/dofs/src/bench/fs-ops.bench.ts | 4 +- packages/dofs/src/fs/ls.test.ts | 26 +++++++++ packages/dofs/src/fs/ls.ts | 51 +++++++++++------ packages/dofs/src/fs/readFile.ts | 19 ++++--- packages/dofs/src/fs/readRange.test.ts | 74 +++++++++++++++++++++++++ packages/dofs/src/sync/fetch.test.ts | 24 ++++++++ packages/dofs/src/sync/fetch.ts | 35 +++++++++--- 7 files changed, 198 insertions(+), 35 deletions(-) diff --git a/packages/dofs/src/bench/fs-ops.bench.ts b/packages/dofs/src/bench/fs-ops.bench.ts index b63384da..38360b96 100644 --- a/packages/dofs/src/bench/fs-ops.bench.ts +++ b/packages/dofs/src/bench/fs-ops.bench.ts @@ -331,8 +331,8 @@ it("dofs micro-benchmark (real DO SqlStorage)", async () => { ); } - // Group D — directory listing: readdir (single dir) vs ls (whole-tree - // CTE, which seeds at root). + // Group D — directory listing: readdir (single dir) vs ls (resolves + // the prefix to its inode, then walks that subtree). { const width = 200; const buildWide: Build = (_db, provider) => { diff --git a/packages/dofs/src/fs/ls.test.ts b/packages/dofs/src/fs/ls.test.ts index 81d457d4..108c94ee 100644 --- a/packages/dofs/src/fs/ls.test.ts +++ b/packages/dofs/src/fs/ls.test.ts @@ -2,6 +2,7 @@ import { describe, expect, it } from "vitest"; import { ls } from "./ls.js"; import { mkdir } from "./mkdir.js"; +import { symlink } from "./symlink.js"; import { withDB } from "./with-db.js"; import { writeFile } from "./writeFile.js"; @@ -54,4 +55,29 @@ describe("ls", () => { expect(ls(db, "/no/such/prefix")).toEqual([]); }); }); + + it("lists a nested subdirectory without scanning sibling subtrees", async () => { + await withDB(async (db) => { + mkdir(db, "/a/deep", { recursive: true }, () => 0); + mkdir(db, "/b", { recursive: true }, () => 0); + await writeFile(db, "/a/deep/x.ts", "", {}, () => 0); + await writeFile(db, "/a/deep/y.ts", "", {}, () => 0); + await writeFile(db, "/a/top.ts", "", {}, () => 0); + await writeFile(db, "/b/other.ts", "", {}, () => 0); + expect(ls(db, "/a/deep")).toEqual(["/a/deep/x.ts", "/a/deep/y.ts"]); + }); + }); + + it("does not follow a symlink prefix", async () => { + await withDB(async (db) => { + mkdir(db, "/real", {}, () => 0); + await writeFile(db, "/real/f.ts", "", {}, () => 0); + symlink(db, "/real", "/link", () => 0); + // A symlink is a leaf with no dirents, so listing at or through + // it yields nothing; the real directory still lists normally. + expect(ls(db, "/link")).toEqual([]); + expect(ls(db, "/link/f.ts")).toEqual([]); + expect(ls(db, "/real")).toEqual(["/real/f.ts"]); + }); + }); }); diff --git a/packages/dofs/src/fs/ls.ts b/packages/dofs/src/fs/ls.ts index 761d3a06..6a5c2387 100644 --- a/packages/dofs/src/fs/ls.ts +++ b/packages/dofs/src/fs/ls.ts @@ -6,20 +6,17 @@ interface PathRow { path: string; } -// Recursive CTE that materializes every file path in the tree, then -// filters by the requested prefix. Files only (no directory entries) -// because that's the documented "flat list of file paths" semantics. +// Recursive CTE that materializes the file paths under one listing +// root. Files only (no directory entries) because that's the +// documented "flat list of file paths" semantics. // -// The CTE walks from ROOT_INODE: each row is (inode, path, type). The -// path is built by concatenating dirent names with '/' separators; -// root contributes the empty string so its children start with '/'. -// -// Prefix matching is exact: '/wsp' must not match '/workspace/x'. We -// require either path == prefix (file at the exact prefix) or -// path starts with prefix + '/' (descendants of a directory prefix). +// The walk is seeded at the listing root's inode: each row is +// (inode, path, type), built by concatenating dirent names with '/' +// separators onto the seed path. Scoping the seed to the requested +// directory keeps the walk O(subtree) instead of O(whole tree). const LS_QUERY = ` WITH RECURSIVE walk(inode, path, type) AS ( - SELECT inode, '', type FROM vfs_nodes WHERE inode = ? + SELECT inode, ?, type FROM vfs_nodes WHERE inode = ? UNION ALL SELECT n.inode, w.path || '/' || d.name, n.type FROM walk w @@ -28,13 +25,35 @@ const LS_QUERY = ` ) SELECT path FROM walk WHERE type = 'file' - AND (? = '/' OR path = ? OR path LIKE ? || '/%') ORDER BY path `; +// Walk dirents from the root to `parts` without following symlinks, so +// the seed matches the CTE's structural view: a symlink component has +// no dirents and thus lists nothing, and a missing or non-directory +// component resolves to null (an empty listing). Returns the root +// inode for an empty path. +function resolvePrefixInode(db: Database, parts: string[]): number | null { + let inode = ROOT_INODE; + for (const name of parts) { + const child = db.one<{ child_inode: number }>( + "SELECT child_inode FROM vfs_dirents WHERE parent_inode = ? AND name = ?", + inode, + name, + ); + if (child === undefined) return null; + inode = child.child_inode; + } + return inode; +} + export function ls(db: Database, prefix: string): string[] { - const { path: canonical } = canonicalizePath(prefix); - return db - .all(LS_QUERY, ROOT_INODE, canonical, canonical, canonical) - .map((row) => row.path); + const { parts, path: canonical } = canonicalizePath(prefix); + const inode = resolvePrefixInode(db, parts); + if (inode === null) return []; + // Root contributes the empty string so its children start with '/'; + // a non-root prefix seeds its own path so descendants read as + // absolute paths. + const seedPath = canonical === "/" ? "" : canonical; + return db.all(LS_QUERY, seedPath, inode).map((row) => row.path); } diff --git a/packages/dofs/src/fs/readFile.ts b/packages/dofs/src/fs/readFile.ts index 4d0c24e4..7f89de40 100644 --- a/packages/dofs/src/fs/readFile.ts +++ b/packages/dofs/src/fs/readFile.ts @@ -167,17 +167,20 @@ export function readRangeSync( const end = Math.min(offset + length, totalSize); const firstIdx = Math.floor(offset / CHUNK_SIZE); const lastIdx = Math.floor((end - 1) / CHUNK_SIZE); + // Pull every overlapping chunk in one indexed range scan. Missing + // indices (a sparse file) simply don't come back, so the assembly + // below compacts around the gaps exactly as a per-index walk would. + const chunks = db.all<{ idx: number; hash: Uint8Array }>( + "SELECT idx, hash FROM vfs_chunks WHERE inode = ? AND idx BETWEEN ? AND ? ORDER BY idx", + node.inode, + firstIdx, + lastIdx, + ); const out = new Uint8Array(end - offset); let written = 0; - for (let idx = firstIdx; idx <= lastIdx; idx++) { + for (const { idx, hash } of chunks) { const start = idx * CHUNK_SIZE; - const chunk = db.one<{ hash: Uint8Array }>( - "SELECT hash FROM vfs_chunks WHERE inode = ? AND idx = ?", - node.inode, - idx, - ); - if (chunk === undefined) continue; - const bytes = getBlobBytes(db, chunk.hash); + const bytes = getBlobBytes(db, hash); if (bytes === undefined) { throw createWorkspaceError("EIO", `missing blob bytes for ${path}`, path); } diff --git a/packages/dofs/src/fs/readRange.test.ts b/packages/dofs/src/fs/readRange.test.ts index bad73801..626b5ab9 100644 --- a/packages/dofs/src/fs/readRange.test.ts +++ b/packages/dofs/src/fs/readRange.test.ts @@ -1,6 +1,7 @@ import { describe, expect, it } from "vitest"; import { readRangeSync } from "./readFile.js"; +import { resolveInode } from "./resolve.js"; import { withDB } from "./with-db.js"; import { CHUNK_SIZE, writeFileSync } from "./writeFile.js"; @@ -58,4 +59,77 @@ describe("readRangeSync", () => { expect(readRangeSync(db, "/large.bin", CHUNK_SIZE + 1, 10).byteLength).toBe(0); }); }); + + it("assembles a read spanning multiple chunks with partial ends", async () => { + await withDB((db) => { + const original = new Uint8Array(CHUNK_SIZE * 3); + original.fill(1, 0, CHUNK_SIZE); + original.fill(2, CHUNK_SIZE, CHUNK_SIZE * 2); + original.fill(3, CHUNK_SIZE * 2); + writeFileSync(db, "/large.bin", original, {}, () => 1); + + // Start inside chunk 0 and end inside chunk 2, so the range query + // returns all three rows and they must assemble in idx order with + // correct partial-chunk trimming. + const start = CHUNK_SIZE - 3; + const len = CHUNK_SIZE + 6; + const slice = readRangeSync(db, "/large.bin", start, len); + expect(slice.byteLength).toBe(len); + expect(equalBytes(slice, original.subarray(start, start + len))).toBe(true); + }); + }); + + it("reads an entire multi-chunk file byte-for-byte", async () => { + await withDB((db) => { + const original = new Uint8Array(CHUNK_SIZE * 2 + 50); + for (let i = 0; i < original.byteLength; i++) original[i] = i % 251; + writeFileSync(db, "/large.bin", original, {}, () => 1); + + const slice = readRangeSync(db, "/large.bin", 0, original.byteLength); + expect(equalBytes(slice, original)).toBe(true); + }); + }); + + it("compacts around a missing chunk row rather than zero-filling", async () => { + await withDB((db) => { + const original = new Uint8Array(CHUNK_SIZE * 3); + original.fill(1, 0, CHUNK_SIZE); + original.fill(2, CHUNK_SIZE, CHUNK_SIZE * 2); + original.fill(3, CHUNK_SIZE * 2); + writeFileSync(db, "/large.bin", original, {}, () => 1); + const node = resolveInode(db, "/large.bin"); + // Drop the middle chunk row (node.size still reports three + // chunks). The read elides the gap and returns the present + // chunks concatenated, trimmed to what was actually read. + db.run("DELETE FROM vfs_chunks WHERE inode = ? AND idx = 1", node?.inode ?? 0); + + const slice = readRangeSync(db, "/large.bin", 0, CHUNK_SIZE * 3); + expect(slice.byteLength).toBe(CHUNK_SIZE * 2); + expect(slice[0]).toBe(1); + expect(slice[CHUNK_SIZE - 1]).toBe(1); + expect(slice[CHUNK_SIZE]).toBe(3); + expect(slice[CHUNK_SIZE * 2 - 1]).toBe(3); + }); + }); + + it("throws EIO when a referenced chunk's blob bytes are gone", async () => { + await withDB((db) => { + writeFileSync(db, "/inline.txt", new TextEncoder().encode("hello"), {}, () => 1); + // vfs_chunks still references the hash, but the bytes are gone + // (cascade from vfs_blobs) — a read must surface EIO. + db.run("DELETE FROM vfs_blobs"); + + expect(() => readRangeSync(db, "/inline.txt", 0, 5)).toThrowError( + expect.objectContaining({ code: "EIO" }), + ); + }); + }); }); + +function equalBytes(a: Uint8Array, b: Uint8Array): boolean { + if (a.byteLength !== b.byteLength) return false; + for (let i = 0; i < a.byteLength; i++) { + if (a[i] !== b[i]) return false; + } + return true; +} diff --git a/packages/dofs/src/sync/fetch.test.ts b/packages/dofs/src/sync/fetch.test.ts index 8d36c08d..3aa6ab37 100644 --- a/packages/dofs/src/sync/fetch.test.ts +++ b/packages/dofs/src/sync/fetch.test.ts @@ -98,4 +98,28 @@ describe("hasObjects", () => { expect(hasObjects(db, [])).toEqual([]); }); }); + + it("preserves input order and duplicates across mixed inputs", async () => { + await withDB(async (db) => { + await writeFile(db, "/a.txt", "alpha", {}, () => 1); + await writeFile(db, "/b.txt", "beta", {}, () => 2); + const entries = await drain(fetchChanges(db, 0)); + const hashOf = (path: string): Uint8Array => { + const e = entries.find((x) => x.kind === "file" && x.path === path); + return e?.kind === "file" ? e.chunks[0].hash : new Uint8Array(); + }; + const a = hashOf("/a.txt"); + const b = hashOf("/b.txt"); + const missing = new Uint8Array(32).fill(0xff); + + // All present, returned in input order. + expect(hasObjects(db, [a, b])).toEqual([a, b]); + // Mixed: only present hashes, still in input order. + expect(hasObjects(db, [missing, b, a])).toEqual([b, a]); + // A duplicated present hash keeps every occurrence. + expect(hasObjects(db, [a, a, missing])).toEqual([a, a]); + // Duplicated absent hashes drop entirely. + expect(hasObjects(db, [missing, missing])).toEqual([]); + }); + }); }); diff --git a/packages/dofs/src/sync/fetch.ts b/packages/dofs/src/sync/fetch.ts index 9d4e3c07..b372db42 100644 --- a/packages/dofs/src/sync/fetch.ts +++ b/packages/dofs/src/sync/fetch.ts @@ -24,21 +24,38 @@ export function fetchObjects( return pushObjects(db, hashes); } +// Stable hex key for JS-side membership tests. Only content matters +// here; the SQL match is on the raw hash blob. +function toHex(bytes: Uint8Array): string { + let out = ""; + for (const b of bytes) out += b.toString(16).padStart(2, "0"); + return out; +} + +// Largest hash list bound into one IN (…) probe. Comfortably under +// SQLite's bound-parameter limit, so a large probe splits into a few +// index-backed lookups instead of one oversized statement. +const PROBE_BATCH = 256; + // Subset-test the input hashes against vfs_blobs. Symmetric on both // sides: the DO probes the container before pushObjects, and the // container probes the DO before fetchObjects, so both sides ship // only the bytes the receiver lacks. // -// Single SQL round-trip: bind the hash list to a temp predicate via -// a JOIN on a values() table. The DO SQL flavour doesn't expose -// SQLite carray, so we serialise the input as a JSON array and join -// against json_each. +// Matches the raw hash blobs through an IN (…) list so the lookup +// rides the primary-key index. Present hashes are returned in input +// order, preserving any duplicates the caller passed. export function hasObjects(db: Database, hashes: Uint8Array[]): Uint8Array[] { if (hashes.length === 0) return []; - const out: Uint8Array[] = []; - for (const h of hashes) { - const row = db.one<{ hash: Uint8Array }>("SELECT hash FROM vfs_blobs WHERE hash = ?", h); - if (row !== undefined) out.push(row.hash); + const present = new Set(); + for (let i = 0; i < hashes.length; i += PROBE_BATCH) { + const window = hashes.slice(i, i + PROBE_BATCH); + const placeholders = window.map(() => "?").join(", "); + const rows = db.all<{ hash: Uint8Array }>( + `SELECT hash FROM vfs_blobs WHERE hash IN (${placeholders})`, + ...window, + ); + for (const row of rows) present.add(toHex(row.hash)); } - return out; + return hashes.filter((h) => present.has(toHex(h))); }