From 784945580c6ef5c8b4f5cb497574cfd93ece9706 Mon Sep 17 00:00:00 2001 From: Maxime David Date: Wed, 22 Jul 2026 19:19:32 +0000 Subject: [PATCH 1/3] module: cache negative stat results in the CJS loader The CommonJS loader keeps a per-require-tree `statCache` to avoid re-stat-ing the same path while resolving a module tree, but it only caches successful stats. Negative results (e.g. -ENOENT) fall through and are re-probed every time the same missing path is looked up again within the same top-level require. These misses are extremely common during resolution and recur across sibling and descendant modules: `tryExtensions` probes .js/.json/.node in order (every extension before the real one is a miss), and bare specifiers walk the node_modules chain upward through many non-existent ancestor directories. None of these negatives were cached, so they were re-stat-ed repeatedly within a single resolution pass. Cache negative stat results alongside positive ones. The staleness window is identical and already accepted for positive results: the cache is tree-scoped, created when a top-level require begins (requireDepth === 0) and cleared when it completes, so a stale entry can only survive the duration of one top-level require. Signed-off-by: Maxime David --- lib/internal/modules/cjs/loader.js | 12 +++++-- .../test-module-negative-stat-cache.js | 34 +++++++++++++++++++ 2 files changed, 44 insertions(+), 2 deletions(-) create mode 100644 test/parallel/test-module-negative-stat-cache.js diff --git a/lib/internal/modules/cjs/loader.js b/lib/internal/modules/cjs/loader.js index bb466d0b68d5..abddb349d0c6 100644 --- a/lib/internal/modules/cjs/loader.js +++ b/lib/internal/modules/cjs/loader.js @@ -279,8 +279,16 @@ function stat(filename) { if (result !== undefined) { return result; } } const result = internalFsBinding.internalModuleStat(filename); - if (statCache !== null && result >= 0) { - // Only set cache when `internalModuleStat(filename)` succeeds. + if (statCache !== null) { + // Cache both successful results (0 = file, 1 = directory) and negative + // results (libuv error codes, e.g. -ENOENT). Negative results are common + // and repeated during resolution: `tryExtensions` probes several + // non-existent extensions, and bare specifiers walk the `node_modules` + // chain upward stat-ing many parent directories that do not exist. The + // cache is scoped to a single top-level `require` tree (created and torn + // down around `requireDepth === 0`), so caching a negative result carries + // the same bounded staleness window that caching a positive one already + // does. statCache.set(filename, result); } return result; diff --git a/test/parallel/test-module-negative-stat-cache.js b/test/parallel/test-module-negative-stat-cache.js new file mode 100644 index 000000000000..24a02812766c --- /dev/null +++ b/test/parallel/test-module-negative-stat-cache.js @@ -0,0 +1,34 @@ +'use strict'; +require('../common'); + +// This tests that the CommonJS loader's per-require-tree stat cache also caches +// negative (not-found) results, so a path that is missing when first probed is +// not re-stat-ed for the rest of the require tree. + +const assert = require('assert'); +const fs = require('fs'); +const tmpdir = require('../common/tmpdir'); + +tmpdir.refresh(); + +// A module path that does not exist yet. +const generated = tmpdir.resolve('generated.js'); + +// First probe: the file does not exist -> negative stat, cached. +assert.throws( + () => require(generated), + { code: 'MODULE_NOT_FOUND' }, + 'expected the module to be missing before it is created', +); + +// Create the file mid-traversal, in the same require tree. +fs.writeFileSync(generated, 'module.exports = 1;'); + +// Second probe, still in the same tree: the negative result is cached, so the +// loader must serve the cached miss instead of re-stat-ing and observing the +// freshly-created file. +assert.throws( + () => require(generated), + { code: 'MODULE_NOT_FOUND' }, + 'a negative stat result must be cached for the rest of the require tree', +); From 7d9cb3b539c666a82cd4963eb8f8d6004f98bbb8 Mon Sep 17 00:00:00 2001 From: Maxime David Date: Wed, 29 Jul 2026 17:44:25 +0000 Subject: [PATCH 2/3] module: limit negative stat caching to resolution probes Caching every negative stat reverted the behaviour of https://github.com/nodejs/node/pull/36642 and broke parallel/test-module-cache: a module missing on a failed require() and then created was no longer picked up by a later require() in the same tree. Narrow the negative caching to speculative probes -- paths resolution guesses at rather than paths the user named: the extension candidates tried by `tryExtensions` and the node_modules ancestors walked for bare specifiers. A cached negative is likewise only read back by a speculative probe, so a stat of a user-named path always re-stats. That restores the #36642 behaviour while keeping the repeated misses, which are where the win comes from, out of the filesystem. Rewrite test-module-negative-stat-cache to assert both halves of the scoped behaviour, and add a benchmark. The benchmark spawns its workload as a child process's main module because `benchmark/common.js` invokes main() from a process.nextTick callback, by which point statCache is already null. At deps=200 depth=12 it shows ~8% improvement. Signed-off-by: Maxime David --- benchmark/module/module-resolve-misses.js | 58 +++++++++++++ lib/internal/modules/cjs/loader.js | 47 +++++++---- .../test-module-negative-stat-cache.js | 84 +++++++++++++------ 3 files changed, 149 insertions(+), 40 deletions(-) create mode 100644 benchmark/module/module-resolve-misses.js diff --git a/benchmark/module/module-resolve-misses.js b/benchmark/module/module-resolve-misses.js new file mode 100644 index 000000000000..659e73238131 --- /dev/null +++ b/benchmark/module/module-resolve-misses.js @@ -0,0 +1,58 @@ +'use strict'; + +const fs = require('fs'); +const path = require('path'); +const { spawnSync } = require('child_process'); +const common = require('../common.js'); + +const tmpdir = require('../../test/common/tmpdir'); +const benchmarkDirectory = tmpdir.resolve('nodejs-benchmark-module'); + +const bench = common.createBenchmark(main, { + depth: [4, 12], + deps: [200], + n: [30], +}); + +function main({ depth, deps, n }) { + tmpdir.refresh(); + + // The only `node_modules` that exists: at the root, above the requirer. + const nodeModules = path.join(benchmarkDirectory, 'node_modules'); + for (let i = 0; i < deps; i++) { + const dep = path.join(nodeModules, `dep${i}`); + fs.mkdirSync(dep, { recursive: true }); + fs.writeFileSync( + path.join(dep, 'package.json'), + `{"name":"dep${i}","main":"index.js"}`, + ); + fs.writeFileSync(path.join(dep, 'index.js'), 'module.exports = {};'); + } + + // The requirer, nested `depth` levels down. Resolution walks up from here and + // finds no `node_modules` until the root. + const nested = path.join(benchmarkDirectory, ...'x'.repeat(depth).split('')); + fs.mkdirSync(nested, { recursive: true }); + + let entrySource = ''; + for (let i = 0; i < deps; i++) { + entrySource += `require('dep${i}');\n`; + } + const entry = path.join(nested, 'entry.js'); + fs.writeFileSync(entry, entrySource); + + const cmd = process.execPath || process.argv[0]; + const warmup = 3; + for (let i = -warmup; i < n; i++) { + if (i === 0) { + bench.start(); + } + const child = spawnSync(cmd, [entry]); + if (child.status !== 0) { + throw new Error(`Child process stopped with exit code ${child.status}`); + } + } + bench.end(n); + + tmpdir.refresh(); +} diff --git a/lib/internal/modules/cjs/loader.js b/lib/internal/modules/cjs/loader.js index abddb349d0c6..7b73657f8783 100644 --- a/lib/internal/modules/cjs/loader.js +++ b/lib/internal/modules/cjs/loader.js @@ -266,29 +266,37 @@ function wrapModuleLoad(request, parent, isMain, options) { /** * Get a path's properties, using an in-memory cache to minimize lookups. + * + * Successful results (0 = file, 1 = directory) are always cached. Negative + * results (libuv error codes, e.g. -ENOENT) are only cached for speculative + * probes, i.e. paths that resolution guesses at rather than paths the user + * actually asked for: the extension candidates tried by `tryExtensions` and + * the `node_modules` ancestor directories walked for bare specifiers. Those + * misses are numerous and repeat across sibling and descendant modules, so + * caching them is where the win is. + * + * A cached negative is also only *read back* by a speculative probe. A stat of + * a path the user named directly ignores it and re-stats. That keeps the + * behaviour of https://github.com/nodejs/node/pull/36642 intact: a module that + * is missing on a failed `require()` and then created can still be picked up by + * a later `require()` in the same tree. * @param {string} filename Absolute path to the file + * @param {boolean} [isSpeculativeProbe] Whether this is a resolution guess + * rather than a path the user named, which makes the negative result + * cacheable and lets a previously cached negative be reused. * @returns {number} */ -function stat(filename) { +function stat(filename, isSpeculativeProbe = false) { // Guard against internal bugs where a non-string filename is passed in by mistake. assert(typeof filename === 'string'); filename = path.toNamespacedPath(filename); if (statCache !== null) { const result = statCache.get(filename); - if (result !== undefined) { return result; } + if (result !== undefined && (result >= 0 || isSpeculativeProbe)) { return result; } } const result = internalFsBinding.internalModuleStat(filename); - if (statCache !== null) { - // Cache both successful results (0 = file, 1 = directory) and negative - // results (libuv error codes, e.g. -ENOENT). Negative results are common - // and repeated during resolution: `tryExtensions` probes several - // non-existent extensions, and bare specifiers walk the `node_modules` - // chain upward stat-ing many parent directories that do not exist. The - // cache is scoped to a single top-level `require` tree (created and torn - // down around `requireDepth === 0`), so caching a negative result carries - // the same bounded staleness window that caching a positive one already - // does. + if (statCache !== null && (result >= 0 || isSpeculativeProbe)) { statCache.set(filename, result); } return result; @@ -598,10 +606,12 @@ function tryPackage(requestPath, exts, isMain, originalPath) { * `--preserve-symlinks-main` and `isMain` is true , keep symlinks intact, otherwise resolve to the absolute realpath. * @param {string} requestPath The path to the file to load. * @param {boolean} isMain Whether the file is the main module. + * @param {boolean} [isSpeculativeProbe] Whether `requestPath` is a resolution + * guess rather than a path the user named. See {@link stat}. * @returns {string|undefined} */ -function tryFile(requestPath, isMain) { - const rc = _stat(requestPath); +function tryFile(requestPath, isMain, isSpeculativeProbe = false) { + const rc = _stat(requestPath, isSpeculativeProbe); if (rc !== 0) { return; } if (getOptionValue(isMain ? '--preserve-symlinks-main' : '--preserve-symlinks')) { return path.resolve(requestPath); @@ -611,6 +621,8 @@ function tryFile(requestPath, isMain) { /** * Given a path, check if the file exists with any of the set extensions. + * Each candidate is a speculative probe: the extension is appended by + * resolution, not named by the user, so its misses are cacheable. * @param {string} basePath The path and filename without extension * @param {string[]} exts The extensions to try * @param {boolean} isMain Whether the module is the main module @@ -618,7 +630,7 @@ function tryFile(requestPath, isMain) { */ function tryExtensions(basePath, exts, isMain) { for (let i = 0; i < exts.length; i++) { - const filename = tryFile(basePath + exts[i], isMain); + const filename = tryFile(basePath + exts[i], isMain, true); if (filename) { return filename; @@ -829,7 +841,10 @@ Module._findPath = function(request, paths, isMain, conditions = getCjsCondition if (typeof curPath !== 'string') { throw new ERR_INVALID_ARG_TYPE('paths', 'array of strings', paths); } - if (insidePath && curPath && _stat(curPath) < 1) { + // A candidate lookup directory (typically a `node_modules` ancestor) is a + // speculative probe: most of the chain does not exist, and every bare + // specifier in the tree re-walks the same missing ancestors. + if (insidePath && curPath && _stat(curPath, true) < 1) { continue; } diff --git a/test/parallel/test-module-negative-stat-cache.js b/test/parallel/test-module-negative-stat-cache.js index 24a02812766c..e941ca9ad5ac 100644 --- a/test/parallel/test-module-negative-stat-cache.js +++ b/test/parallel/test-module-negative-stat-cache.js @@ -1,34 +1,70 @@ 'use strict'; require('../common'); -// This tests that the CommonJS loader's per-require-tree stat cache also caches -// negative (not-found) results, so a path that is missing when first probed is -// not re-stat-ed for the rest of the require tree. +// This tests that the CommonJS loader's per-require-tree stat cache caches +// negative (not-found) results for *speculative* probes: the extension +// candidates tried by `tryExtensions` and the `node_modules` ancestor +// directories walked for bare specifiers. +// +// A path the user named directly is not negatively cached, so the behaviour of +// https://github.com/nodejs/node/pull/36642 is preserved: a module that is +// missing on a failed `require()` and then created is still picked up by a +// later `require()` in the same tree. That case is covered by +// test-module-cache.js and asserted again at the end of this file. +// +// The stat cache is populated and read internally by the loader, so it is not +// directly observable from user code. These tests make it observable by +// mutating the filesystem between two probes of the same path within one +// require tree. const assert = require('assert'); const fs = require('fs'); +const path = require('path'); const tmpdir = require('../common/tmpdir'); tmpdir.refresh(); -// A module path that does not exist yet. -const generated = tmpdir.resolve('generated.js'); - -// First probe: the file does not exist -> negative stat, cached. -assert.throws( - () => require(generated), - { code: 'MODULE_NOT_FOUND' }, - 'expected the module to be missing before it is created', -); - -// Create the file mid-traversal, in the same require tree. -fs.writeFileSync(generated, 'module.exports = 1;'); - -// Second probe, still in the same tree: the negative result is cached, so the -// loader must serve the cached miss instead of re-stat-ing and observing the -// freshly-created file. -assert.throws( - () => require(generated), - { code: 'MODULE_NOT_FOUND' }, - 'a negative stat result must be cached for the rest of the require tree', -); +// An extensionless specifier is resolved by probing `.js`, `.json`, +// `.node`, ... in order. Those candidate paths are appended by resolution +// rather than named by the user, so their misses are cached for the rest of the +// tree. +{ + const dir = tmpdir.resolve('speculative'); + fs.mkdirSync(dir); + + const specifier = path.join(dir, 'mod'); + + // Nothing exists yet: every extension candidate misses and is cached. + assert.throws( + () => require(specifier), + { code: 'MODULE_NOT_FOUND' }, + 'expected the module to be missing before it is created', + ); + + // Create one of the candidates that was just probed and missed. + fs.writeFileSync(`${specifier}.js`, 'module.exports = "late";'); + + // Resolving the same extensionless specifier probes `mod.js` again, but that + // negative result is cached, so the freshly-created file is not observed. + assert.throws( + () => require(specifier), + { code: 'MODULE_NOT_FOUND' }, + 'a negative result for a speculative extension probe must be cached', + ); +} + +// A path the user named directly is not negatively cached, so creating the file +// mid-tree makes a later require in the same tree resolve it. +{ + const explicit = tmpdir.resolve('explicit.js'); + + assert.throws( + () => require(explicit), + { code: 'MODULE_NOT_FOUND' }, + ); + + fs.writeFileSync(explicit, 'module.exports = "created";'); + + // A negative result for a user-named path must not be cached. + assert.strictEqual(require(explicit), 'created'); +} From 9903df55e72f06134ad3008bacd59954e2f95cbc Mon Sep 17 00:00:00 2001 From: Maxime David Date: Mon, 3 Aug 2026 13:18:47 +0000 Subject: [PATCH 3/3] module: use options bag for stat and tryFile probe flags Address review feedback: pass the resolution-probe flag as a named option instead of a positional boolean, so callsites read tryFile(basePath + exts[i], { isMain, isSpeculativeProbe: true }); _stat(curPath, { isSpeculativeProbe: true }); `isMain` moves into the bag as well rather than staying positional, and `stat` gets the same treatment since that is where the flag is consumed. Both default to `kEmptyObject`, matching the existing idiom in this file. No behaviour change: the negative-caching scope is identical and the existing tests pass unmodified. Signed-off-by: Maxime David --- lib/internal/modules/cjs/loader.js | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/lib/internal/modules/cjs/loader.js b/lib/internal/modules/cjs/loader.js index 7b73657f8783..7c4685571547 100644 --- a/lib/internal/modules/cjs/loader.js +++ b/lib/internal/modules/cjs/loader.js @@ -281,12 +281,13 @@ function wrapModuleLoad(request, parent, isMain, options) { * is missing on a failed `require()` and then created can still be picked up by * a later `require()` in the same tree. * @param {string} filename Absolute path to the file - * @param {boolean} [isSpeculativeProbe] Whether this is a resolution guess - * rather than a path the user named, which makes the negative result + * @param {object} [options] + * @param {boolean} [options.isSpeculativeProbe] Whether this is a resolution + * guess rather than a path the user named, which makes the negative result * cacheable and lets a previously cached negative be reused. * @returns {number} */ -function stat(filename, isSpeculativeProbe = false) { +function stat(filename, { isSpeculativeProbe = false } = kEmptyObject) { // Guard against internal bugs where a non-string filename is passed in by mistake. assert(typeof filename === 'string'); @@ -573,7 +574,7 @@ function tryPackage(requestPath, exts, isMain, originalPath) { } const filename = path.resolve(requestPath, pkg); - let actual = tryFile(filename, isMain) || + let actual = tryFile(filename, { isMain }) || tryExtensions(filename, exts, isMain) || tryExtensions(path.resolve(filename, 'index'), exts, isMain); if (actual === false) { @@ -605,13 +606,14 @@ function tryPackage(requestPath, exts, isMain, originalPath) { * Check if the file exists and is not a directory if using `--preserve-symlinks` and `isMain` is false or * `--preserve-symlinks-main` and `isMain` is true , keep symlinks intact, otherwise resolve to the absolute realpath. * @param {string} requestPath The path to the file to load. - * @param {boolean} isMain Whether the file is the main module. - * @param {boolean} [isSpeculativeProbe] Whether `requestPath` is a resolution - * guess rather than a path the user named. See {@link stat}. + * @param {object} [options] + * @param {boolean} [options.isMain] Whether the file is the main module. + * @param {boolean} [options.isSpeculativeProbe] Whether `requestPath` is a + * resolution guess rather than a path the user named. See {@link stat}. * @returns {string|undefined} */ -function tryFile(requestPath, isMain, isSpeculativeProbe = false) { - const rc = _stat(requestPath, isSpeculativeProbe); +function tryFile(requestPath, { isMain = false, isSpeculativeProbe = false } = kEmptyObject) { + const rc = _stat(requestPath, { isSpeculativeProbe }); if (rc !== 0) { return; } if (getOptionValue(isMain ? '--preserve-symlinks-main' : '--preserve-symlinks')) { return path.resolve(requestPath); @@ -630,7 +632,7 @@ function tryFile(requestPath, isMain, isSpeculativeProbe = false) { */ function tryExtensions(basePath, exts, isMain) { for (let i = 0; i < exts.length; i++) { - const filename = tryFile(basePath + exts[i], isMain, true); + const filename = tryFile(basePath + exts[i], { isMain, isSpeculativeProbe: true }); if (filename) { return filename; @@ -844,7 +846,7 @@ Module._findPath = function(request, paths, isMain, conditions = getCjsCondition // A candidate lookup directory (typically a `node_modules` ancestor) is a // speculative probe: most of the chain does not exist, and every bare // specifier in the tree re-walks the same missing ancestors. - if (insidePath && curPath && _stat(curPath, true) < 1) { + if (insidePath && curPath && _stat(curPath, { isSpeculativeProbe: true }) < 1) { continue; }