module: cache negative stat results in the CJS loader#64682
Conversation
|
Review requested:
|
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>
ea93d9e to
7849455
Compare
|
force pushing to add the missing : "Signed-off-by:" in the commit message |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #64682 +/- ##
==========================================
- Coverage 90.14% 90.11% -0.03%
==========================================
Files 741 741
Lines 242133 242121 -12
Branches 45568 45602 +34
==========================================
- Hits 218265 218192 -73
- Misses 15371 15431 +60
- Partials 8497 8498 +1
🚀 New features to boost your workflow:
|
|
This was the behaviour in days of yore (#36638) so this is effectively a reversion of that change, hence the failing tests. One would expect that the impact of the scenario described here is far more common than the need to modify the module tree on-the-fly after a failed const jsonModule = path.resolve('./test.json')
try {
const m = require(jsonModule)
console.debug(m)
} catch (e) {
console.error(e)
}
fs.writeFileSync(jsonModule, '{}\n')
try {
const m = require(jsonModule)
console.debug(m)
} catch (e) {
console.error(e)
}FWIW, there aren't any module benchmarks which really capture the scenario of repeated attempts at resolving the same module specifier(s) and/or having to walk several levels up the directory tree, which would probably help make the case for this PR. The original change was accepted in the context of a neutral impact on the module benchmark suite at the time. |
module: cache negative stat results in the CJS loader
Summary
The CommonJS loader keeps a per-require-tree
statCacheto avoid re-stat-ing the same path while resolving a module tree. Today 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-levelrequire.Failed probes are extremely common during resolution, and the same misses recur across sibling and descendant modules:
tryExtensionsresolves an extensionless specifier (e.g.require('./helper')) bystat-ing each candidate in order:./helper.js, then./helper.json, then./helper.node, until one exists. Every extension tried before the real one is a miss.require('foo')) walk thenode_moduleschain upward:…/a/b/node_modules,…/a/node_modules,…/node_modules. Most of those parent directories don't exist, so each is a negative stat and every bare require in the tree re-walks the same non-existent ancestors.Because negatives aren't cached, these misses are re-
stat-ed repeatedly within a single resolution pass.Why this change is safe
The
statCacheis tree-scoped: it is created when a top-levelrequirebegins (requireDepth === 0) and set back tonullwhen it completes. So the staleness window for any cached entry is bounded to the duration of a single top-levelrequire.That window already exists for positive results: a file cached as "exists" could be deleted mid-traversal and the cache wouldn't notice. Caching a negative result has the exact same bounded window: a file cached as "missing" could be created mid-traversal and the cache wouldn't notice.
Impact
The larger and deeper the dependency tree, the more the same missing paths get re-probed, so real-world
node_modulestrees are exactly where repeated negative stats add up. The gain scales with how resolution-heavy the tree is and how expensive each syscall is (it is largest when the OS filesystem cache is cold, e.g. the very first run after boot).It is especially impactful on AWS Lambda. Lambda cold starts are the worst-case for this cost: the filesystem cache is cold, the vCPU share is small so syscalls are relatively expensive, and startup latency is directly user-facing and billed. This is precisely the environment where eliminating repeated negative stats pays off most.
Measured on AWS Lambda (
provided:al2023, 128 MB, N=30 cold starts) with a resolution-heavy dependency tree (~1093 modules, each doing bare-specifiernode_moduleswalks plus extensionless/missingtryExtensionsprobes):Time spent in the top-level
require()call, measured by wrapping it inprocess.hrtime.bigint():Test
test/parallel/test-module-negative-stat-cache.jsverifies that negative (not-found) stat results are cached. The stat cache is populated and read internally by the loader, so it is not directly observable from user code. The test makes it observable by mutating the filesystem between two probes of the same path within one require tree.Fixes: #64681