8000 lib: convert WeakMaps in cjs loader with private symbol properties · nodejs/node@3a2d8bf · GitHub
[go: up one dir, main page]
Skip to content

Commit 3a2d8bf

Browse files
legendecasmarco-ippolito
legendecas
authored and
marco-ippolito
committed
lib: convert WeakMaps in cjs loader with private symbol properties
Symbol properties are typically more GC-efficient than using WeakMaps, since WeakMap requires ephemeron GC. `module[kModuleExportNames]` would be easier to read than `importedCJSCache.get(module).exportNames` as well. PR-URL: #52095 Backport-PR-URL: #53500 Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
1 parent 51b88fa commit 3a2d8bf

File tree

3 files changed

+79
-54
lines changed

3 files changed

+79
-54
lines changed

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

Lines changed: 64 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,6 @@ const {
5050
ReflectSet,
5151
RegExpPrototypeExec,
5252
SafeMap,
53-
SafeWeakMap,
5453
String,
5554
StringPrototypeCharAt,
5655
StringPrototypeCharCodeAt,
@@ -62,25 +61,50 @@ const {
6261
StringPrototypeStartsWith,
6362
Symbol,
6463
} = primordials;
64+
const {
65+
privateSymbols: {
66+
module_source_private_symbol,
67+
module_export_names_private_symbol,
68+
module_circular_visited_private_symbol,
69+
module_export_private_symbol,
70+
module_parent_private_symbol,
71+
},
72+
} = internalBinding('util');
6573

6674
const { kEvaluated } = internalBinding('module_wrap');
6775

68-
// Map used to store CJS parsing data or for ESM loading.
69-
const importedCJSCache = new SafeWeakMap();
76+
// Internal properties for Module instances.
77+
/**
78+
* Cached {@link Module} source string.
79+
*/
80+
const kModuleSource = module_source_private_symbol;
81+
/**
82+
* Cached {@link Module} export names for ESM loader.
83+
*/
84+
const kModuleExportNames = module_export_names_private_symbol;
85+
/**
86+
* {@link Module} circular dependency visited flag.
87+
*/
88+
const kModuleCircularVisited = module_circular_visited_private_symbol;
7089
/**
71-
* Map of already-loaded CJS modules to use.
90+
* {@link Module} export object snapshot for ESM loader.
7291
*/
73-
const cjsExportsCache = new SafeWeakMap();
74-
const requiredESMSourceCache = new SafeWeakMap();
92+
const kModuleExport = module_export_private_symbol;
93+
/**
94+
* {@link Module} parent module.
95+
*/
96+
const kModuleParent = module_parent_private_symbol;
7597

7698
const kIsMainSymbol = Symbol('kIsMainSymbol');
7799
const kIsCachedByESMLoader = Symbol('kIsCachedByESMLoader');
78100
const kRequiredModuleSymbol = Symbol('kRequiredModuleSymbol');
79101
const kIsExecuting = Symbol('kIsExecuting');
80102
// Set first due to cycle with ESM loader functions.
81103
module.exports = {
82-
cjsExportsCache,
83-
importedCJSCache,
104+
kModuleSource,
105+
kModuleExport,
106+
kModuleExportNames,
107+
kModuleCircularVisited,
84108
initializeCJS,
85109
Module,
86110
wrapSafe,
@@ -256,8 +280,6 @@ function reportModuleNotFoundToWatchMode(basePath, extensions) {
256280
}
257281
}
258282

259-
/** @type {Map<Module, Module>} */
260-
const moduleParentCache = new SafeWeakMap();
261283
/**
262284
* Create a new module instance.
263285
* @param {string} id
@@ -267,7 +289,7 @@ function Module(id = '', parent) {
267289
this.id = id;
268290
this.path = path.dirname(id);
269291
setOwnProperty(this, 'exports', {});
270-
moduleParentCache.set(this, parent);
292+
this[kModuleParent] = parent;
271293
updateChildren(parent, this, false);
272294
this.filename = null;
273295
this.loaded = false;
@@ -355,17 +377,19 @@ ObjectDefineProperty(BuiltinModule.prototype, 'isPreloading', isPreloadingDesc);
355377

356378
/**
357379
* Get the parent of the current module from our cache.
380+
* @this {Module}
358381
*/
359382
function getModuleParent() {
360-
return moduleParentCache.get(this);
383+
return this[kModuleParent];
361384
}
362385

363386
/**
364387
* Set the parent of the current module in our cache.
388+
* @this {Module}
365389
* @param {Module} value
366390
*/
367391
function setModuleParent(value) {
368-
moduleParentCache.set(this, value);
392+
this[kModuleParent] = value;
369393
}
370394

371395
let debug = require('internal/util/debuglog').debuglog('module', (fn) => {
@@ -955,7 +979,7 @@ function getExportsForCircularRequire(module) {
955979
const requiredESM = module[kRequiredModuleSymbol];
956980
if (requiredESM && requiredESM.getStatus() !== kEvaluated) {
957981
let message = `Cannot require() ES Module ${module.id} in a cycle.`;
958-
const parent = moduleParentCache.get(module);
982+
const parent = module[kModuleParent];
959983
if (parent) {
960984
message += ` (from ${parent.filename})`;
961985
}
@@ -1028,25 +1052,24 @@ Module._load = function(request, parent, isMain) {
10281052
const cachedModule = Module._cache[filename];
10291053
if (cachedModule !== undefined) {
10301054
updateChildren(parent, cachedModule, true);
1031-
if (!cachedModule.loaded) {
1032-
// If it's not cached by the ESM loader, the loading request
1033-
// comes from required CJS, and we can consider it a circular
1034-
// dependency when it's cached.
1035-
if (!cachedModule[kIsCachedByESMLoader]) {
1036-
return getExportsForCircularRequire(cachedModule);
1037-
}
1038-
// If it's cached by the ESM loader as a way to indirectly pass
1039-
// the module in to avoid creating it twice, the loading request
1040-
// come from imported CJS. In that case use the importedCJSCache
1041-
// to determine if it's loading or not.
1042-
const importedCJSMetadata = importedCJSCache.get(cachedModule);
1043-
if (importedCJSMetadata.loading) {
1044-
return getExportsForCircularRequire(cachedModule);
1045-
}
1046-
importedCJSMetadata.loading = true;
1047-
} else {
1055+
if (cachedModule.loaded) {
10481056
return cachedModule.exports;
10491057
}
1058+
// If it's not cached by the ESM loader, the loading request
1059+
// comes from required CJS, and we can consider it a circular
1060+
// dependency when it's cached.
1061+
if (!cachedModule[kIsCachedByESMLoader]) {
1062+
return getExportsForCircularRequire(cachedModule);
1063+
}
1064+
// If it's cached by the ESM loader as a way to indirectly pass
1065+
// the module in to avoid creating it twice, the loading request
1066+
// come from imported CJS. In that case use the kModuleCircularVisited
1067+
// to determine if it's loading or not.
1068+
if (cachedModule[kModuleCircularVisited]) {
1069+
return getExportsForCircularRequire(cachedModule);
1070+
}
1071+
// This is an ESM loader created cache entry, mark it as visited and fallthrough to loading the module.
1072+
cachedModule[kModuleCircularVisited] = true;
10501073
}
10511074

10521075
if (BuiltinModule.canBeRequiredWithoutScheme(filename)) {
@@ -1190,7 +1213,7 @@ Module._resolveFilename = function(request, parent, isMain, options) {
11901213
const requireStack = [];
11911214
for (let cursor = parent;
11921215
cursor;
1193-
cursor = moduleParentCache.get(cursor)) {
1216+
cursor = cursor[kModuleParent]) {
11941217
ArrayPrototypePush(requireStack, cursor.filename || cursor.id);
11951218
}
11961219
let message = `Cannot find module '${request}'`;
@@ -1268,9 +1291,7 @@ Module.prototype.load = function(filename) {
12681291
// Create module entry at load time to snapshot exports correctly
12691292
const exports = this.exports;
12701293
// Preemptively cache for ESM loader.
1271-
if (!cjsExportsCache.has(this)) {
1272-
cjsExportsCache.set(this, exports);
1273-
}
1294+
this[kModuleExport] = exports;
12741295
};
12751296

12761297
/**
@@ -1313,7 +1334,7 @@ function loadESMFromCJS(mod, filename) {
13131334
const isMain = mod[kIsMainSymbol];
13141335
// TODO(joyeecheung): we may want to invent optional special handling for default exports here.
13151336
// For now, it's good enough to be identical to what `import()` returns.
1316-
mod.exports = cascadedLoader.importSyncForRequire(mod, filename, source, isMain, moduleParentCache.get(mod));
1337+
mod.exports = cascadedLoader.importSyncForRequire(mod, filename, source, isMain, mod[kModuleParent]);
13171338
}
13181339

13191340
/**
@@ -1399,7 +1420,7 @@ Module.prototype._compile = function(content, filename, loadAsESM = false) {
13991420
// Only modules being require()'d really need to avoid TLA.
14001421
if (loadAsESM) {
14011422
// Pass the source into the .mjs extension handler indirectly through the cache.
1402-
requiredESMSourceCache.set(this, content);
1423+
this[kModuleSource] = content;
14031424
loadESMFromCJS(this, filename);
14041425
return;
14051426
}
@@ -1460,15 +1481,15 @@ Module.prototype._compile = function(content, filename, loadAsESM = false) {
14601481
* @returns {string}
14611482
*/
14621483
function getMaybeCachedSource(mod, filename) {
1463-
const cached = importedCJSCache.get(mod);
1484+
// If already analyzed the source, then it will be cached.
14641485
let content;
1465-
if (cached?.source) {
1466-
content = cached.source;
1467-
cached.source = undefined;
1486+
if (mod[kModuleSource] !== undefined) {
1487+
content = mod[kModuleSource];
1488+
mod[kModuleSource] = undefined;
14681489
} else {
14691490
// TODO(joyeecheung): we can read a buffer instead to speed up
14701491
// compilation.
1471-
content = requiredESMSourceCache.get(mod) ?? fs.readFileSync(filename, 'utf8');
1492+
content = fs.readFileSync(filename, 'utf8');
14721493
}
14731494
return content;
14741495
}
@@ -1492,7 +1513,7 @@ Module._extensions['.js'] = function(module, filename) {
14921513
}
14931514

14941515
// This is an error path because `require` of a `.js` file in a `"type": "module"` scope is not allowed.
1495-
const parent = moduleParentCache.get(module);
1516+
const parent = module[kModuleParent];
14961517
const parentPath = parent?.filename;
14971518
const packageJsonPath = path.resolve(pkg.path, 'package.json');
14981519
const usesEsm = containsModuleSyntax(content, filename);

‎lib/internal/modules/esm/translators.js‎

Lines changed: 10 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -43,10 +43,11 @@ const {
4343
stripBOM,
4444
} = require('internal/modules/helpers');
4545
const {
46-
cjsExportsCache,
47-
importedCJSCache,
4846
kIsCachedByESMLoader,
4947
Module: CJSModule,
48+
kModuleSource,
49+
kModuleExport,
50+
kModuleExportNames,
5051
} = require('internal/modules/cjs/loader');
5152
const { fileURLToPath, pathToFileURL, URL } = require('internal/url');
5253
let debug = require('internal/util/debuglog').debuglog('esm', (fn) => {
@@ -285,9 +286,9 @@ function createCJSModuleWrap(url, source, isMain, loadCJS = loadCJSModule) {
285286
}
286287

287288
let exports;
288-
if (cjsExportsCache.has(module)) {
289-
exports = cjsExportsCache.get(module);
290-
cjsExportsCache.delete(module);
289+
if (module[kModuleExport] !== undefined) {
290+
exports = module[kModuleExport];
291+
module[kModuleExport] = undefined;
291292
} else {
292293
({ exports } = module);
293294
}
@@ -366,18 +367,16 @@ translators.set('commonjs', async function commonjsStrategy(url, source,
366367
function cjsPreparseModuleExports(filename, source) {
367368
// TODO: Do we want to keep hitting the user mutable CJS loader here?
368369
let module = CJSModule._cache[filename];
369-
if (module) {
370-
const cached = importedCJSCache.get(module);
371-
if (cached) {
372-
return { module, exportNames: cached.exportNames };
373-
}
370+
if (module && module[kModuleExportNames] !== undefined) {
371+
return { module, exportNames: module[kModuleExportNames] };
374372
}
375373
const loaded = Boolean(module);
376374
if (!loaded) {
377375
module = new CJSModule(filename);
378376
module.filename = filename;
< C859 /td>
379377
module.paths = CJSModule._nodeModulePaths(module.path);
380378
module[kIsCachedByESMLoader] = true;
379+
module[kModuleSource] = source;
381380
CJSModule._cache[filename] = module;
382381
}
383382

@@ -392,7 +391,7 @@ function cjsPreparseModuleExports(filename, source) {
392391
const exportNames = new SafeSet(new SafeArrayIterator(exports));
393392

394393
// Set first for cycles.
395-
importedCJSCache.set(module, { source, exportNames });
394+
module[kModuleExportNames] = exportNames;
396395

397396
if (reexports.length) {
398397
module.filename = filename;

‎src/env_properties.h‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,11 @@
2222
V(contextify_context_private_symbol, "node:contextify:context") \
2323
V(decorated_private_symbol, "node:decorated") \
2424
V(host_defined_option_symbol, "node:host_defined_option_symbol") \
25+
V(module_source_private_symbol, "node:module_source") \
26+
V(module_export_names_private_symbol, "node:module_export_names") \
27+
V(module_circular_visited_private_symbol, "node:module_circular_visited") \
28+
V(module_export_private_symbol, "node:module_export") \
29+
V(module_parent_private_symbol, "node:module_parent") \
2530
V(napi_type_tag, "node:napi:type_tag") \
2631
V(napi_wrapper, "node:napi:wrapper") \
2732
V(untransferable_object_private_symbol, "node:untransferableObject") \

0 commit comments

Comments
 (0)
0