Skip to content

Commit 7849455

Browse files
committed
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 <maxday@amazon.com>
1 parent db3a8d8 commit 7849455

2 files changed

Lines changed: 44 additions & 2 deletions

File tree

‎lib/internal/modules/cjs/loader.js‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -279,8 +279,16 @@ function stat(filename) {
279279
if (result !== undefined) { return result; }
280280
}
281281
const result = internalFsBinding.internalModuleStat(filename);
282-
if (statCache !== null && result >= 0) {
283-
// Only set cache when `internalModuleStat(filename)` succeeds.
282+
if (statCache !== null) {
283+
// Cache both successful results (0 = file, 1 = directory) and negative
284+
// results (libuv error codes, e.g. -ENOENT). Negative results are common
285+
// and repeated during resolution: `tryExtensions` probes several
286+
// non-existent extensions, and bare specifiers walk the `node_modules`
287+
// chain upward stat-ing many parent directories that do not exist. The
288+
// cache is scoped to a single top-level `require` tree (created and torn
289+
// down around `requireDepth === 0`), so caching a negative result carries
290+
// the same bounded staleness window that caching a positive one already
291+
// does.
284292
statCache.set(filename, result);
285293
}
286294
return result;
Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
'use strict';
2+
require('../common');
3+
4+
// This tests that the CommonJS loader's per-require-tree stat cache also caches
5+
// negative (not-found) results, so a path that is missing when first probed is
6+
// not re-stat-ed for the rest of the require tree.
7+
8+
const assert = require('assert');
9+
const fs = require('fs');
10+
const tmpdir = require('../common/tmpdir');
11+
12+
tmpdir.refresh();
13+
14+
// A module path that does not exist yet.
15+
const generated = tmpdir.resolve('generated.js');
16+
17+
// First probe: the file does not exist -> negative stat, cached.
18+
assert.throws(
19+
() => require(generated),
20+
{ code: 'MODULE_NOT_FOUND' },
21+
'expected the module to be missing before it is created',
22+
);
23+
24+
// Create the file mid-traversal, in the same require tree.
25+
fs.writeFileSync(generated, 'module.exports = 1;');
26+
27+
// Second probe, still in the same tree: the negative result is cached, so the
28+
// loader must serve the cached miss instead of re-stat-ing and observing the
29+
// freshly-created file.
30+
assert.throws(
31+
() => require(generated),
32+
{ code: 'MODULE_NOT_FOUND' },
33+
'a negative stat result must be cached for the rest of the require tree',
34+
);

0 commit comments

Comments
 (0)