Commit d94f4c2
module: fix stat cache
The current caching logic broke by [0] because it used destructuring
on the module arguments. Since the exported property is a primitive
counting it up or down would not have any effect anymore in the module
that required that property.
The original implementation would cache all stat calls caused during
bootstrap. Afterwards it would clear the cache and lazy require calls
during runtime would create a new cascading cache for the then
loaded modules and clear the cache again.
This behavior is now restored. This is difficult to test without
exposing a lot of information and therfore the existing tests have
been removed (as they could not detect the issue).
With the broken implementation it caused each module compilation to
reset the cache and therefore minimizing the effect drastically.
[0] #19177
PR-URL: #26266
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>1 parent 2a66cd3 commit d94f4c2
5 files changed
Lines changed: 15 additions & 58 deletions
File tree
- lib/internal/modules/cjs
- test
- fixtures/module-require-depth
- parallel
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -11,12 +11,7 @@ function makeRequireFunction(mod) { | |||
| 11 | 11 | const Module = mod.constructor; | |
| 12 | 12 | ||
| 13 | 13 | function require(path) { | |
| 14 | - try { | ||
| 15 | - exports.requireDepth += 1; | ||
| 16 | - return mod.require(path); | ||
| 17 | - } finally { | ||
| 18 | - exports.requireDepth -= 1; | ||
| 19 | - } | ||
| 14 | + return mod.require(path); | ||
| 20 | 15 | } | |
| 21 | 16 | ||
| 22 | 17 | function resolve(request, options) { | |
@@ -139,7 +134,6 @@ module.exports = exports = { | |||
| 139 | 134 | builtinLibs, | |
| 140 | 135 | makeRequireFunction, | |
| 141 | 136 | normalizeReferrerURL, | |
| 142 | - requireDepth: 0, | ||
| 143 | 137 | stripBOM, | |
| 144 | 138 | stripShebang | |
| 145 | 139 | }; | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -37,7 +37,6 @@ const { safeGetenv } = internalBinding('credentials'); | |||
| 37 | 37 | const { | |
| 38 | 38 | makeRequireFunction, | |
| 39 | 39 | normalizeReferrerURL, | |
| 40 | - requireDepth, | ||
| 41 | 40 | stripBOM, | |
| 42 | 41 | stripShebang | |
| 43 | 42 | } = require('internal/modules/cjs/helpers'); | |
@@ -85,18 +84,17 @@ const { | |||
| 85 | 84 | ||
| 86 | 85 | const isWindows = process.platform === 'win32'; | |
| 87 | 86 | ||
| 87 | + let requireDepth = 0; | ||
| 88 | + let statCache = new Map(); | ||
| 88 | 89 | function stat(filename) { | |
| 89 | 90 | filename = path.toNamespacedPath(filename); | |
| 90 | - const cache = stat.cache; | ||
| 91 | - if (cache !== null) { | ||
| 92 | - const result = cache.get(filename); | ||
| 93 | - if (result !== undefined) return result; | ||
| 94 | - } | ||
| 95 | - const result = internalModuleStat(filename); | ||
| 96 | - if (cache !== null) cache.set(filename, result); | ||
| 91 | + if (statCache === null) statCache = new Map(); | ||
| 92 | + let result = statCache.get(filename); | ||
| 93 | + if (result !== undefined) return result; | ||
| 94 | + result = internalModuleStat(filename); | ||
| 95 | + statCache.set(filename, result); | ||
| 97 | 96 | return result; | |
| 98 | 97 | } | |
| 99 | - stat.cache = null; | ||
| 100 | 98 | ||
| 101 | 99 | function updateChildren(parent, child, scan) { | |
| 102 | 100 | var children = parent && parent.children; | |
@@ -702,7 +700,12 @@ Module.prototype.require = function(id) { | |||
| 702 | 700 | throw new ERR_INVALID_ARG_VALUE('id', id, | |
| 703 | 701 | 'must be a non-empty string'); | |
| 704 | 702 | } | |
| 705 | - return Module._load(id, this, /* isMain */ false); | ||
| 703 | + requireDepth++; | ||
| 704 | + try { | ||
| 705 | + return Module._load(id, this, /* isMain */ false); | ||
| 706 | + } finally { | ||
| 707 | + requireDepth--; | ||
| 708 | + } | ||
| 706 | 709 | }; | |
| 707 | 710 | ||
| 708 | 711 | ||
@@ -785,8 +788,6 @@ Module.prototype._compile = function(content, filename) { | |||
| 785 | 788 | } | |
| 786 | 789 | var dirname = path.dirname(filename); | |
| 787 | 790 | var require = makeRequireFunction(this); | |
| 788 | - var depth = requireDepth; | ||
| 789 | - if (depth === 0) stat.cache = new Map(); | ||
| 790 | 791 | var result; | |
| 791 | 792 | var exports = this.exports; | |
| 792 | 793 | var thisValue = exports; | |
@@ -798,7 +799,7 @@ Module.prototype._compile = function(content, filename) { | |||
| 798 | 799 | result = compiledWrapper.call(thisValue, exports, require, module, | |
| 799 | 800 | filename, dirname); | |
| 800 | 801 | } | |
| 801 | - if (depth === 0) stat.cache = null; | ||
| 802 | + if (requireDepth === 0) statCache = null; | ||
| 802 | 803 | return result; | |
| 803 | 804 | }; | |
| 804 | 805 | ||
This file was deleted.
This file was deleted.
This file was deleted.
0 commit comments