From 9f9e68bf8b905cdbccb0fa7061299ae4aa97e01d Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Wed, 16 Sep 2026 18:47:17 +0000 Subject: [PATCH 1/5] node:fs glob: do not build strings or call path functions once per entry The walk memoizes Pattern.cacheKey() (every child() of a pattern shares the keys), joins the directory prefix once per directory and not twice per entry, and does not call path.resolve() per result or per queued path when there is no exclude array. The results and their order do not change. --- src/js/internal/fs/glob.ts | 62 ++++++++++++++++++++++-------------- test/js/node/fs/glob.test.ts | 36 ++++++++++++++++++++- 2 files changed, 73 insertions(+), 25 deletions(-) diff --git a/src/js/internal/fs/glob.ts b/src/js/internal/fs/glob.ts index bec1c1aeecd6..ffad65544f4d 100644 --- a/src/js/internal/fs/glob.ts +++ b/src/js/internal/fs/glob.ts @@ -15,7 +15,7 @@ // version on main). Replace this with Bun.Glob once those gaps are closed // natively. const { validateObject, validateString, validateBoolean, validateArray } = require("internal/validators"); -const { join, resolve, basename, dirname, isAbsolute } = require("node:path"); +const { join, resolve, basename, dirname, isAbsolute, sep } = require("node:path"); const { kEmptyObject } = require("internal/shared"); const isWindows = process.platform === "win32"; @@ -58,6 +58,10 @@ function makeMatchersExclude(matchers) { function statForFileTypes(cache, root, path) { return cache.statSync(isAbsolute(path) ? path : join(root, path)); } +// joinPrefix(dir) + name equals join(dir, name) for a name from readdir: it has no ".", ".." or separator. +function joinPrefix(dir) { + return dir === "." ? "" : join(dir, sep); +} const kStats = Symbol("stats"); let _DirentFromStats: any; @@ -298,14 +302,17 @@ class Cache { class Pattern { #pattern; #globStrings; + // cacheKey() by index. Every child() shares it. + #cacheKeys; indexes; symlinks; realpaths; last; - constructor(pattern, globStrings, indexes, symlinks, realpaths = new Set()) { + constructor(pattern, globStrings, indexes, symlinks, realpaths = new Set(), cacheKeys = []) { this.#pattern = pattern; this.#globStrings = globStrings; + this.#cacheKeys = cacheKeys; this.indexes = indexes; this.symlinks = symlinks; this.realpaths = realpaths; @@ -331,7 +338,7 @@ class Pattern { return this.#pattern.at(index); } child(indexes, symlinks = new Set(), realpaths = this.realpaths) { - return new Pattern(this.#pattern, this.#globStrings, indexes, symlinks, realpaths); + return new Pattern(this.#pattern, this.#globStrings, indexes, symlinks, realpaths, this.#cacheKeys); } test(index, path) { if (index > this.#pattern.length) { @@ -351,14 +358,7 @@ class Pattern { } cacheKey(index) { - let key = ""; - for (let i = index; i < this.#globStrings.length; i++) { - key += this.#globStrings[i]; - if (i !== this.#globStrings.length - 1) { - key += "/"; - } - } - return key; + return (this.#cacheKeys[index] ??= this.#globStrings.slice(index).join("/")); } } @@ -372,7 +372,7 @@ class ResultSet extends Set { } add(value): any { - if (this.#isExcluded(resolve(this.#root, value))) { + if (this.#isExcluded !== excludeNothing && this.#isExcluded(resolve(this.#root, value))) { return false; } super.add(value); @@ -515,14 +515,16 @@ class Glob { return real !== null && pattern.realpaths.has(real); } #addSubpattern(path, pattern) { - if (this.#isExcluded(path)) { - return; - } - const fullpath = resolve(this.#root, path); + if (this.#isExcluded !== excludeNothing) { + if (this.#isExcluded(path)) { + return; + } + const fullpath = resolve(this.#root, path); - // If path is a directory, add trailing slash and test patterns again. - if (this.#isExcluded(`${fullpath}/`) && this.#cache.statSync(fullpath).isDirectory()) { - return; + // If path is a directory, add trailing slash and test patterns again. + if (this.#isExcluded(`${fullpath}/`) && this.#cache.statSync(fullpath).isDirectory()) { + return; + } } if (this.#exclude) { @@ -532,7 +534,7 @@ class Glob { // process.cwd() instead of options.cwd (upstream passes `path` // here, which silently skips the exclude callback when cwd // differs). - const stat = this.#cache.statSync(fullpath); + const stat = this.#cache.statSync(resolve(this.#root, path)); if (stat !== null) { if (this.#exclude(stat)) { return; @@ -611,6 +613,9 @@ class Glob { const nextRealpaths = this.#nextRealpathsSync(fullpath, isDirectory, pattern); let children; + // Set when `children` comes from readdir; see joinPrefix(). + let entryPathPrefix; + let entryFullpathPrefix; const firstPattern = pattern.indexes.size === 1 && pattern.at(pattern.indexes.values().next().value); if (typeof firstPattern === "string") { const stat = this.#cache.statSync(join(fullpath, firstPattern)); @@ -622,12 +627,15 @@ class Glob { } } else { children = this.#cache.readdirSync(fullpath); + entryPathPrefix = joinPrefix(path); + entryFullpathPrefix = joinPrefix(fullpath); } for (let i = 0; i < children.length; i++) { const entry = children[i]; - const entryPath = join(path, entry.name); - const entryFullpath = join(fullpath, entry.name); + const entryPath = entryPathPrefix === undefined ? join(path, entry.name) : entryPathPrefix + entry.name; + const entryFullpath = + entryFullpathPrefix === undefined ? join(fullpath, entry.name) : entryFullpathPrefix + entry.name; this.#cache.addToStatCache(entryFullpath, entry); const entryIsDirectory = entry.isDirectory() || @@ -828,6 +836,9 @@ class Glob { const nextRealpaths = await this.#nextRealpaths(fullpath, isDirectory, pattern); let children; + // Set when `children` comes from readdir; see joinPrefix(). + let entryPathPrefix; + let entryFullpathPrefix; const firstPattern = pattern.indexes.size === 1 && pattern.at(pattern.indexes.values().next().value); if (typeof firstPattern === "string") { const stat = await this.#cache.stat(join(fullpath, firstPattern)); @@ -839,12 +850,15 @@ class Glob { } } else { children = await this.#cache.readdir(fullpath); + entryPathPrefix = joinPrefix(path); + entryFullpathPrefix = joinPrefix(fullpath); } for (let i = 0; i < children.length; i++) { const entry = children[i]; - const entryPath = join(path, entry.name); - const entryFullpath = join(fullpath, entry.name); + const entryPath = entryPathPrefix === undefined ? join(path, entry.name) : entryPathPrefix + entry.name; + const entryFullpath = + entryFullpathPrefix === undefined ? join(fullpath, entry.name) : entryFullpathPrefix + entry.name; this.#cache.addToStatCache(entryFullpath, entry); const entryIsDirectory = entry.isDirectory() || diff --git a/test/js/node/fs/glob.test.ts b/test/js/node/fs/glob.test.ts index 66392bf66513..bb8fdbc5d5ae 100644 --- a/test/js/node/fs/glob.test.ts +++ b/test/js/node/fs/glob.test.ts @@ -3,7 +3,7 @@ * tested elsewhere. These tests check API compatibility with Node.js. */ import { afterAll, beforeAll, describe, expect, it } from "bun:test"; -import { isWindows, tempDir, tempDirWithFiles } from "harness"; +import { bunEnv, bunExe, isWindows, tempDir, tempDirWithFiles } from "harness"; import fs from "node:fs"; let tmp: string; @@ -268,6 +268,40 @@ describe("fs.promises.glob", () => { }); }); // +describe("fs.glob walk", () => { + it.concurrent("does not call path.join() or path.resolve() once per entry", async () => { + const files: Record = {}; + for (let i = 0; i < 200; i++) files[`f${i}.txt`] = ""; + using flat = tempDir("fs-glob-flat", files); + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + ` + const path = require("node:path"); + const calls = { join: 0, resolve: 0 }; + for (const name of ["join", "resolve"]) { + const original = path[name]; + path[name] = (...args) => (calls[name]++, original(...args)); + } + const fs = require("node:fs"); + const cwd = process.argv[1]; + const sync = fs.globSync("*.txt", { cwd }).length; + const async = (await Array.fromAsync(fs.promises.glob("*.txt", { cwd }))).length; + console.log(JSON.stringify({ sync, async, perEntry: calls.join >= 200 || calls.resolve >= 200 })); + `, + String(flat), + ], + env: bunEnv, + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect(stderr).toBe(""); + expect(JSON.parse(stdout)).toEqual({ sync: 200, async: 200, perEntry: false }); + expect(exitCode).toBe(0); + }); +}); + describe("fs.globSync exclude with withFileTypes", () => { it("invokes the exclude callback with Dirents when cwd differs from process.cwd()", () => { using dir = tempDir("glob-exclude-dirent", { From f6e732f7bff62d1a2dce277ab3fce045ecad7a1c Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Wed, 16 Sep 2026 18:48:38 +0000 Subject: [PATCH 2/5] node:fs glob: the async walk reads the next directories ahead fs.promises.glob and fs.glob wait one thread pool round trip per directory, one after the other. The walk now starts the readdir of the next 8 queued directories before it visits them, and reads the root while it stats it. The walk awaits the same promises from the readdir cache, so the walk order and the results do not change. A rejection of a readdir that nothing awaits is marked handled. The consumer still gets it from the walk's own readdir. --- src/js/internal/fs/glob.ts | 48 +++++++++++++++- test/js/node/fs/glob.test.ts | 106 +++++++++++++++++++++++++++++++++++ 2 files changed, 152 insertions(+), 2 deletions(-) diff --git a/src/js/internal/fs/glob.ts b/src/js/internal/fs/glob.ts index ffad65544f4d..b4508d4904fe 100644 --- a/src/js/internal/fs/glob.ts +++ b/src/js/internal/fs/glob.ts @@ -167,6 +167,20 @@ function createMatcher(pattern, options = kEmptyObject) { return new (lazyMinimatch().Minimatch)(pattern, opts); } +// How many queued directories Glob.#prefetch() reads ahead of glob(). +const kPrefetchWindow = 8; + +// False when #iterateSubpatterns() would only stat a literal name and not list the directory. +function readsDirectory(patterns) { + for (let i = 0; i < patterns.length; i++) { + const { indexes } = patterns[i]; + if (indexes.size !== 1 || typeof patterns[i].at(indexes.values().next().value) !== "string") { + return true; + } + } + return false; +} + function cloneSet(values) { const cloned = new Set(); for (const value of values) { @@ -250,6 +264,11 @@ class Cache { addToStatCache(path, val) { this.#statsCache.set(path, val); } + // What a readdir of the parent directory gave for `path`, if anything. + cachedDirent(path) { + const cached = this.#statsCache.get(path); + return $isPromise(cached) ? undefined : cached; + } async readdir(path) { const cached = this.#readdirCache.get(path); if (cached) { @@ -267,6 +286,10 @@ class Cache { this.#readdirCache.set(path, promise); return promise; } + // Nothing awaits this promise, so its rejection must not count as unhandled. readdir(path) still rejects. + prefetchReaddir(path) { + this.readdir(path).$then(undefined, nullOnReject); + } readdirSync(path) { const cached = this.#readdirCache.get(path); if (cached) { @@ -385,7 +408,7 @@ class Glob { #exclude; #cache = new Cache(); #results = new ResultSet(); - #queue: Array<{ path: string; patterns: Pattern[] }> = []; + #queue: Array<{ path: string; patterns: Pattern[]; prefetched?: boolean }> = []; #subpatterns = new Map(); #patterns; #withFileTypes; @@ -753,15 +776,36 @@ class Glob { async *glob() { this.#queue.push({ __proto__: null, path: ".", patterns: this.#patterns }); + // The lstat and the readdir of the root run side by side; see #prefetch(). + if (readsDirectory(this.#patterns)) { + this.#cache.prefetchReaddir(resolve(this.#root, ".")); + } while (this.#queue.length > 0) { const item = this.#queue.pop()!; for (let i = 0; i < item.patterns.length; i++) { yield* this.#iterateSubpatterns(item.path, item.patterns[i]); } for (const [path, patterns] of this.#subpatterns) { - this.#queue.push({ __proto__: null, path, patterns }); + this.#queue.push({ __proto__: null, path, patterns, prefetched: false }); } this.#subpatterns.clear(); + this.#prefetch(); + } + } + // Starts the readdir of the directories that glob() visits next, so that the thread pool reads them meanwhile. + #prefetch() { + const queue = this.#queue; + const end = queue.length > kPrefetchWindow ? queue.length - kPrefetchWindow : 0; + for (let i = queue.length - 1; i >= end; i--) { + const item = queue[i]; + if (item.prefetched) { + continue; + } + item.prefetched = true; + const fullpath = resolve(this.#root, item.path); + if (this.#cache.cachedDirent(fullpath)?.isDirectory() && readsDirectory(item.patterns)) { + this.#cache.prefetchReaddir(fullpath); + } } } async *#iterateSubpatterns(path, pattern) { diff --git a/test/js/node/fs/glob.test.ts b/test/js/node/fs/glob.test.ts index bb8fdbc5d5ae..40f7c1bbe744 100644 --- a/test/js/node/fs/glob.test.ts +++ b/test/js/node/fs/glob.test.ts @@ -5,6 +5,7 @@ import { afterAll, beforeAll, describe, expect, it } from "bun:test"; import { bunEnv, bunExe, isWindows, tempDir, tempDirWithFiles } from "harness"; import fs from "node:fs"; +import path from "node:path"; let tmp: string; beforeAll(() => { @@ -302,6 +303,111 @@ describe("fs.glob walk", () => { }); }); +describe("fs.promises.glob on a tree", () => { + // The async walk starts the readdir of the next few directories before it + // visits them. The result must not depend on that: same entries, same order + // as the sync walk, also when a directory has more subdirectories than the + // walk reads ahead. + let cwd: string; + beforeAll(() => { + const files: Record = { "top.txt": "", ".hidden/h.txt": "" }; + for (let i = 0; i < 20; i++) { + files[`d${i}/a.txt`] = ""; + files[`d${i}/b.js`] = ""; + files[`d${i}/sub/c.txt`] = ""; + files[`d${i}/sub/deep/d.txt`] = ""; + } + cwd = tempDirWithFiles("fs-glob-tree", files); + }); + afterAll(() => fs.promises.rm(cwd, { recursive: true, force: true })); + + it.each(["**", "**/*.txt", "*/*/*.txt", "**/sub/**", "d1*/**/*.txt", "d3/sub/deep/*", "*/missing/*"])( + "%j gives what globSync gives", + async pattern => { + const expected = fs.globSync(pattern, { cwd }); + expect(await Array.fromAsync(fs.promises.glob(pattern, { cwd }))).toEqual(expected); + const dirents = (await Array.fromAsync(fs.promises.glob(pattern, { cwd, withFileTypes: true }))) as fs.Dirent[]; + expect(dirents.map(dirent => path.relative(cwd, path.join(dirent.parentPath, dirent.name)) || ".")).toEqual( + expected, + ); + }, + ); + + // The walk does not await the readdir that it starts ahead of time. A + // rejection of that readdir must still reach the consumer, and only the + // consumer: it is not an unhandled rejection. + it.concurrent.each([ + ["the root", "*.txt", "true"], + ["a queued directory", "*/*.txt", 'path.endsWith("d3")'], + ])("a readdir of %s that fails to sort rejects the iteration", async (_, pattern, fails) => { + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + ` + const fsp = require("node:fs/promises"); + const readdir = fsp.readdir; + fsp.readdir = async (path, options) => + ${fails} ? { sort() { throw new Error("cannot sort"); } } : readdir(path, options); + try { + for await (const entry of fsp.glob(${JSON.stringify(pattern)}, { cwd: process.argv[1] })) {} + console.log("no error"); + } catch (error) { + console.log("caught:", error.message); + } + `, + cwd, + ], + env: bunEnv, + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect({ stdout, stderr }).toEqual({ stdout: "caught: cannot sort\n", stderr: "" }); + expect(exitCode).toBe(0); + }); + + it.concurrent("reads the next directories while it matches the current one", async () => { + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + ` + const fsp = require("node:fs/promises"); + const readdir = fsp.readdir; + let inFlight = 0; + let maxInFlight = 0; + fsp.readdir = async (path, options) => { + maxInFlight = Math.max(maxInFlight, ++inFlight); + try { + return await readdir(path, options); + } finally { + inFlight--; + } + }; + const entries = await Array.fromAsync(fsp.glob("**/*.txt", { cwd: process.argv[1] })); + console.log(JSON.stringify({ entries: entries.length, readAhead: maxInFlight > 1, bounded: maxInFlight <= 16 })); + `, + cwd, + ], + env: bunEnv, + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect(stderr).toBe(""); + expect(JSON.parse(stdout)).toEqual({ entries: 61, readAhead: true, bounded: true }); + expect(exitCode).toBe(0); + }); + + it("stops without an error when the consumer breaks out early", async () => { + const seen: string[] = []; + for await (const entry of fs.promises.glob("**/*.txt", { cwd })) { + seen.push(entry); + if (seen.length === 3) break; + } + expect(seen).toHaveLength(3); + }); +}); + describe("fs.globSync exclude with withFileTypes", () => { it("invokes the exclude callback with Dirents when cwd differs from process.cwd()", () => { using dir = tempDir("glob-exclude-dirent", { From aaf77e04a4b8f1551f9aa0ae65fd9b81baf45aa6 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Wed, 16 Sep 2026 19:14:41 +0000 Subject: [PATCH 3/5] test: use describe.each, and expect the exact read-ahead window --- test/js/node/fs/glob.test.ts | 55 ++++++++++++++++++++---------------- 1 file changed, 30 insertions(+), 25 deletions(-) diff --git a/test/js/node/fs/glob.test.ts b/test/js/node/fs/glob.test.ts index 40f7c1bbe744..215291659198 100644 --- a/test/js/node/fs/glob.test.ts +++ b/test/js/node/fs/glob.test.ts @@ -321,30 +321,33 @@ describe("fs.promises.glob on a tree", () => { }); afterAll(() => fs.promises.rm(cwd, { recursive: true, force: true })); - it.each(["**", "**/*.txt", "*/*/*.txt", "**/sub/**", "d1*/**/*.txt", "d3/sub/deep/*", "*/missing/*"])( - "%j gives what globSync gives", - async pattern => { - const expected = fs.globSync(pattern, { cwd }); - expect(await Array.fromAsync(fs.promises.glob(pattern, { cwd }))).toEqual(expected); - const dirents = (await Array.fromAsync(fs.promises.glob(pattern, { cwd, withFileTypes: true }))) as fs.Dirent[]; - expect(dirents.map(dirent => path.relative(cwd, path.join(dirent.parentPath, dirent.name)) || ".")).toEqual( - expected, - ); + describe.each(["**", "**/*.txt", "*/*/*.txt", "**/sub/**", "d1*/**/*.txt", "d3/sub/deep/*", "*/missing/*"])( + "%j", + pattern => { + it("gives what globSync gives", async () => { + const expected = fs.globSync(pattern, { cwd }); + expect(await Array.fromAsync(fs.promises.glob(pattern, { cwd }))).toEqual(expected); + const dirents = (await Array.fromAsync(fs.promises.glob(pattern, { cwd, withFileTypes: true }))) as fs.Dirent[]; + expect(dirents.map(dirent => path.relative(cwd, path.join(dirent.parentPath, dirent.name)) || ".")).toEqual( + expected, + ); + }); }, ); // The walk does not await the readdir that it starts ahead of time. A // rejection of that readdir must still reach the consumer, and only the // consumer: it is not an unhandled rejection. - it.concurrent.each([ + describe.each([ ["the root", "*.txt", "true"], ["a queued directory", "*/*.txt", 'path.endsWith("d3")'], - ])("a readdir of %s that fails to sort rejects the iteration", async (_, pattern, fails) => { - await using proc = Bun.spawn({ - cmd: [ - bunExe(), - "-e", - ` + ])("a readdir of %s that fails to sort", (_, pattern, fails) => { + it.concurrent("rejects the iteration", async () => { + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + ` const fsp = require("node:fs/promises"); const readdir = fsp.readdir; fsp.readdir = async (path, options) => @@ -356,14 +359,15 @@ describe("fs.promises.glob on a tree", () => { console.log("caught:", error.message); } `, - cwd, - ], - env: bunEnv, - stderr: "pipe", + cwd, + ], + env: bunEnv, + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect({ stdout, stderr }).toEqual({ stdout: "caught: cannot sort\n", stderr: "" }); + expect(exitCode).toBe(0); }); - const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); - expect({ stdout, stderr }).toEqual({ stdout: "caught: cannot sort\n", stderr: "" }); - expect(exitCode).toBe(0); }); it.concurrent("reads the next directories while it matches the current one", async () => { @@ -385,7 +389,7 @@ describe("fs.promises.glob on a tree", () => { } }; const entries = await Array.fromAsync(fsp.glob("**/*.txt", { cwd: process.argv[1] })); - console.log(JSON.stringify({ entries: entries.length, readAhead: maxInFlight > 1, bounded: maxInFlight <= 16 })); + console.log(JSON.stringify({ entries: entries.length, maxInFlight })); `, cwd, ], @@ -394,7 +398,8 @@ describe("fs.promises.glob on a tree", () => { }); const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); expect(stderr).toBe(""); - expect(JSON.parse(stdout)).toEqual({ entries: 61, readAhead: true, bounded: true }); + // 8 is the read-ahead window. Each directory of this tree has at most one subdirectory, so no more are in flight. + expect(JSON.parse(stdout)).toEqual({ entries: 61, maxInFlight: 8 }); expect(exitCode).toBe(0); }); From bf5a2b2158046dad4b25be1936f85afbf60a9fad Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Wed, 16 Sep 2026 19:21:36 +0000 Subject: [PATCH 4/5] test: describe the read-ahead fixture correctly --- test/js/node/fs/glob.test.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/js/node/fs/glob.test.ts b/test/js/node/fs/glob.test.ts index 215291659198..6fbccc03e915 100644 --- a/test/js/node/fs/glob.test.ts +++ b/test/js/node/fs/glob.test.ts @@ -398,7 +398,8 @@ describe("fs.promises.glob on a tree", () => { }); const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); expect(stderr).toBe(""); - // 8 is the read-ahead window. Each directory of this tree has at most one subdirectory, so no more are in flight. + // 8 is the read-ahead window. Below the root a directory has at most one subdirectory, and the walk starts that + // one read after the read it awaited has settled, so the count never goes above the window. expect(JSON.parse(stdout)).toEqual({ entries: 61, maxInFlight: 8 }); expect(exitCode).toBe(0); }); From 5f3557b30a9184707f08647046dbe26bbecd44e4 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Wed, 16 Sep 2026 20:04:35 +0000 Subject: [PATCH 5/5] ci: retrigger