Skip to content

Commit de44671

Browse files
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 `cjsParseCache.get(module).exportNames` as well.
1 parent 6f4d601 commit de44671

File tree

3 files changed

+70
-40
lines changed

3 files changed

+70
-40
lines changed

lib/internal/modules/cjs/loader.js

Lines changed: 55 additions & 29 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,18 +61,44 @@ 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

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

7396
// Set first due to cycle with ESM loader functions.
7497
module.exports = {
75-
cjsExportsCache,
76-
cjsSourceCache,
98+
kModuleSource,
99+
kModuleExport,
100+
kModuleExportNames,
101+
kModuleCircularVisited,
77102
initializeCJS,
78103
Module,
79104
wrapSafe,
@@ -246,8 +271,6 @@ function reportModuleNotFoundToWatchMode(basePath, extensions) {
246271
}
247272
}
248273

249-
/** @type {Map<Module, Module>} */
250-
const moduleParentCache = new SafeWeakMap();
251274
/**
252275
* Create a new module instance.
253276
* @param {string} id
@@ -257,7 +280,7 @@ function Module(id = '', parent) {
257280
this.id = id;
258281
this.path = path.dirname(id);
259282
setOwnProperty(this, 'exports', {});
260-
moduleParentCache.set(this, parent);
283+
this[kModuleParent] = parent;
261284
updateChildren(parent, this, false);
262285
this.filename = null;
263286
this.loaded = false;
@@ -345,17 +368,19 @@ ObjectDefineProperty(BuiltinModule.prototype, 'isPreloading', isPreloadingDesc);
345368

346369
/**
347370
* Get the parent of the current module from our cache.
371+
* @this {Module}
348372
*/
349373
function getModuleParent() {
350-
return moduleParentCache.get(this);
374+
return this[kModuleParent];
351375
}
352376

353377
/**
354378
* Set the parent of the current module in our cache.
379+
* @this {Module}
355380
* @param {Module} value
356381
*/
357382
function setModuleParent(value) {
358-
moduleParentCache.set(this, value);
383+
this[kModuleParent] = value;
359384
}
360385

361386
let debug = require('internal/util/debuglog').debuglog('module', (fn) => {
@@ -1007,15 +1032,18 @@ Module._load = function(request, parent, isMain) {
10071032
const cachedModule = Module._cache[filename];
10081033
if (cachedModule !== undefined) {
10091034
updateChildren(parent, cachedModule, true);
1010-
if (!cachedModule.loaded) {
1011-
const parseCachedModule = cjsSourceCache.get(cachedModule);
1012-
if (!parseCachedModule || parseCachedModule.loaded) {
1013-
return getExportsForCircularRequire(cachedModule);
1014-
}
1015-
parseCachedModule.loaded = true;
1016-
} else {
1035+
if (cachedModule.loaded) {
10171036
return cachedModule.exports;
10181037
}
1038+
// If the cached module was finished loading, there are two possible conditions:
1039+
// 1. the cache entry was created by the ESM loader, or
1040+
// 2. it is circularly required.
1041+
// Return a proxy for the cached module's export object if the module is circularly required.
1042+
if (cachedModule[kModuleExportNames] === undefined || cachedModule[kModuleCircularVisited]) {
1043+
return getExportsForCircularRequire(cachedModule);
1044+
}
1045+
// This is an ESM loader created cache entry, mark it as visited and fallthrough to loading the module.
1046+
cachedModule[kModuleCircularVisited] = true;
10191047
}
10201048

10211049
if (BuiltinModule.canBeRequiredWithoutScheme(filename)) {
@@ -1156,7 +1184,7 @@ Module._resolveFilename = function(request, parent, isMain, options) {
11561184
const requireStack = [];
11571185
for (let cursor = parent;
11581186
cursor;
1159-
cursor = moduleParentCache.get(cursor)) {
1187+
cursor = cursor[kModuleParent]) {
11601188
ArrayPrototypePush(requireStack, cursor.filename || cursor.id);
11611189
}
11621190
let message = `Cannot find module '${request}'`;
@@ -1234,9 +1262,7 @@ Module.prototype.load = function(filename) {
12341262
// Create module entry at load time to snapshot exports correctly
12351263
const exports = this.exports;
12361264
// Preemptively cache for ESM loader.
1237-
if (!cjsExportsCache.has(this)) {
1238-
cjsExportsCache.set(this, exports);
1239-
}
1265+
this[kModuleExport] = exports;
12401266
};
12411267

12421268
/**
@@ -1365,7 +1391,7 @@ Module.prototype._compile = function(content, filename, loadAsESM = false) {
13651391
// Only modules being require()'d really need to avoid TLA.
13661392
if (loadAsESM) {
13671393
// Pass the source into the .mjs extension handler indirectly through the cache.
1368-
cjsSourceCache.set(this, { source: content });
1394+
this[kModuleSource] = content;
13691395
loadESMFromCJS(this, filename);
13701396
return;
13711397
}
@@ -1424,11 +1450,11 @@ Module.prototype._compile = function(content, filename, loadAsESM = false) {
14241450
* @returns {string}
14251451
*/
14261452
function getMaybeCachedSource(mod, filename) {
1427-
const cached = cjsSourceCache.get(mod);
1453+
// If already analyzed the source, then it will be cached.
14281454
let content;
1429-
if (cached?.source) {
1430-
content = cached.source;
1431-
cached.source = undefined;
1455+
if (mod[kModuleSource] !== undefined) {
1456+
content = mod[kModuleSource];
1457+
mod[kModuleSource] = undefined;
14321458
} else {
14331459
// TODO(joyeecheung): we can read a buffer instead to speed up
14341460
// compilation.
@@ -1456,7 +1482,7 @@ Module._extensions['.js'] = function(module, filename) {
14561482
}
14571483

14581484
// This is an error path because `require` of a `.js` file in a `"type": "module"` scope is not allowed.
1459-
const parent = moduleParentCache.get(module);
1485+
const parent = module[kModuleParent];
14601486
const parentPath = parent?.filename;
14611487
const packageJsonPath = path.resolve(pkg.path, 'package.json');
14621488
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
@@ -44,8 +44,9 @@ const {
4444
} = require('internal/modules/helpers');
4545
const {
4646
Module: CJSModule,
47-
cjsSourceCache,
48-
cjsExportsCache,
47+
kModuleSource,
48+
kModuleExport,
49+
kModuleExportNames,
4950
} = require('internal/modules/cjs/loader');
5051
const { fileURLToPath, pathToFileURL, URL } = require('internal/url');
5152
let debug = require('internal/util/debuglog').debuglog('esm', (fn) => {
@@ -306,9 +307,9 @@ function createCJSModuleWrap(url, source, isMain, loadCJS = loadCJSModule) {
306307
}
307308

308309
let exports;
309-
if (cjsExportsCache.has(module)) {
310-
exports = cjsExportsCache.get(module);
311-
cjsExportsCache.delete(module);
310+
if (module[kModuleExport] !== undefined) {
311+
exports = module[kModuleExport];
312+
module[kModuleExport] = undefined;
312313
} else {
313314
({ exports } = module);
314315
}
@@ -388,17 +389,15 @@ translators.set('commonjs', async function commonjsStrategy(url, source,
388389
function cjsPreparseModuleExports(filename, source) {
389390
// TODO: Do we want to keep hitting the user mutable CJS loader here?
390391
let module = CJSModule._cache[filename];
391-
if (module) {
392-
const cached = cjsSourceCache.get(module);
393-
if (cached) {
394-
return { module, exportNames: cached.exportNames };
395-
}
392+
if (module && module[kModuleExportNames] !== undefined) {
393+
return { module, exportNames: module[kModuleExportNames] };
396394
}
397395
const loaded = Boolean(module);
398396
if (!loaded) {
399397
module = new CJSModule(filename);
400398
module.filename = filename;
401399
module.paths = CJSModule._nodeModulePaths(module.path);
400+
module[kModuleSource] = source;
402401
CJSModule._cache[filename] = module;
403402
}
404403

@@ -413,7 +412,7 @@ function cjsPreparseModuleExports(filename, source) {
413412
const exportNames = new SafeSet(new SafeArrayIterator(exports));
414413

415414
// Set first for cycles.
416-
cjsSourceCache.set(module, { source, exportNames });
415+
module[kModuleExportNames] = exportNames;
417416

418417
if (reexports.length) {
419418
module.filename = filename;

src/env_properties.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,11 @@
2626
V(js_transferable_wrapper_private_symbol, "node:js_transferable_wrapper") \
2727
V(entry_point_module_private_symbol, "node:entry_point_module") \
2828
V(entry_point_promise_private_symbol, "node:entry_point_promise") \
29+
V(module_source_private_symbol, "node:module_source") \
30+
V(module_export_names_private_symbol, "node:module_export_names") \
31+
V(module_circular_visited_private_symbol, "node:module_circular_visited") \
32+
V(module_export_private_symbol, "node:module_export") \
33+
V(module_parent_private_symbol, "node:module_parent") \
2934
V(napi_type_tag, "node:napi:type_tag") \
3035
V(napi_wrapper, "node:napi:wrapper") \
3136
V(untransferable_object_private_symbol, "node:untransferableObject") \

0 commit comments

Comments
 (0)