Skip to content

Commit f6145aa

Browse files
legendecasaduh95
authored andcommitted
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 Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
1 parent 44ee04c commit f6145aa

3 files changed

Lines changed: 79 additions & 54 deletions

File tree

‎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-
constimportedCJSCache=newSafeWeakMap();
76+
// Internal properties for Module instances.
77+
/**
78+
* Cached {@link Module} source string.
79+
*/
80+
constkModuleSource=module_source_private_symbol;
81+
/**
82+
* Cached {@link Module} export names for ESM loader.
83+
*/
84+
constkModuleExportNames=module_export_names_private_symbol;
85+
/**
86+
* {@link Module} circular dependency visited flag.
87+
*/
88+
constkModuleCircularVisited=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-
constcjsExportsCache=newSafeWeakMap();
74-
constrequiredESMSourceCache=newSafeWeakMap();
92+
constkModuleExport=module_export_private_symbol;
93+
/**
94+
* {@link Module} parent module.
95+
*/
96+
constkModuleParent=module_parent_private_symbol;
7597

7698
constkIsMainSymbol=Symbol('kIsMainSymbol');
7799
constkIsCachedByESMLoader=Symbol('kIsCachedByESMLoader');
78100
constkRequiredModuleSymbol=Symbol('kRequiredModuleSymbol');
79101
constkIsExecuting=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
entryPointSource: undefined,// Set below.
86110
Module,
@@ -257,8 +281,6 @@ function reportModuleNotFoundToWatchMode(basePath, extensions) {
257281
}
258282
}
259283

260-
/** @type {Map<Module, Module>} */
261-
constmoduleParentCache=newSafeWeakMap();
262284
/**
263285
* Create a new module instance.
264286
* @param {string} id
@@ -268,7 +290,7 @@ function Module(id = '', parent) {
268290
this.id=id;
269291
this.path=path.dirname(id);
270292
setOwnProperty(this,'exports',{});
271-
moduleParentCache.set(this,parent);
293+
this[kModuleParent]=parent;
272294
updateChildren(parent,this,false);
273295
this.filename=null;
274296
this.loaded=false;
@@ -356,17 +378,19 @@ ObjectDefineProperty(BuiltinModule.prototype, 'isPreloading', isPreloadingDesc);
356378

357379
/**
358380
* Get the parent of the current module from our cache.
381+
* @this {Module}
359382
*/
360383
functiongetModuleParent(){
361-
returnmoduleParentCache.get(this);
384+
returnthis[kModuleParent];
362385
}
363386

364387
/**
365388
* Set the parent of the current module in our cache.
389+
* @this {Module}
366390
* @param {Module} value
367391
*/
368392
functionsetModuleParent(value){
369-
moduleParentCache.set(this,value);
393+
this[kModuleParent]=value;
370394
}
371395

372396
letdebug=require('internal/util/debuglog').debuglog('module',(fn)=>{
@@ -955,7 +979,7 @@ function getExportsForCircularRequire(module) {
955979
constrequiredESM=module[kRequiredModuleSymbol];
956980
if(requiredESM&&requiredESM.getStatus()!==kEvaluated){
957981
letmessage=`Cannot require() ES Module ${module.id} in a cycle.`;
958-
constparent=moduleParentCache.get(module);
982+
constparent=module[kModuleParent];
959983
if(parent){
960984
message+=` (from ${parent.filename})`;
961985
}
@@ -1028,25 +1052,24 @@ Module._load = function(request, parent, isMain) {
10281052
constcachedModule=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-
returngetExportsForCircularRequire(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-
constimportedCJSMetadata=importedCJSCache.get(cachedModule);
1043-
if(importedCJSMetadata.loading){
1044-
returngetExportsForCircularRequire(cachedModule);
1045-
}
1046-
importedCJSMetadata.loading=true;
1047-
}else{
1055+
if(cachedModule.loaded){
10481056
returncachedModule.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+
returngetExportsForCircularRequire(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+
returngetExportsForCircularRequire(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
constrequireStack=[];
11911214
for(letcursor=parent;
11921215
cursor;
1193-
cursor=moduleParentCache.get(cursor)){
1216+
cursor=cursor[kModuleParent]){
11941217
ArrayPrototypePush(requireStack,cursor.filename||cursor.id);
11951218
}
11961219
letmessage=`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
constexports=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
constisMain=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
/**
@@ -1406,7 +1427,7 @@ Module.prototype._compile = function(content, filename, loadAsESM = false) {
14061427
// Only modules being require()'d really need to avoid TLA.
14071428
if(loadAsESM){
14081429
// Pass the source into the .mjs extension handler indirectly through the cache.
1409-
requiredESMSourceCache.set(this,content);
1430+
this[kModuleSource]=content;
14101431
loadESMFromCJS(this,filename);
14111432
return;
14121433
}
@@ -1467,15 +1488,15 @@ Module.prototype._compile = function(content, filename, loadAsESM = false) {
14671488
* @returns {string}
14681489
*/
14691490
functiongetMaybeCachedSource(mod,filename){
1470-
constcached=importedCJSCache.get(mod);
1491+
// If already analyzed the source, then it will be cached.
14711492
letcontent;
1472-
if(cached?.source){
1473-
content=cached.source;
1474-
cached.source=undefined;
1493+
if(mod[kModuleSource]!==undefined){
1494+
content=mod[kModuleSource];
1495+
mod[kModuleSource]=undefined;
14751496
}else{
14761497
// TODO(joyeecheung): we can read a buffer instead to speed up
14771498
// compilation.
1478-
content=requiredESMSourceCache.get(mod)??fs.readFileSync(filename,'utf8');
1499+
content=fs.readFileSync(filename,'utf8');
14791500
}
14801501
returncontent;
14811502
}
@@ -1499,7 +1520,7 @@ Module._extensions['.js'] = function(module, filename) {
14991520
}
15001521

15011522
// This is an error path because `require` of a `.js` file in a `"type": "module"` scope is not allowed.
1502-
constparent=moduleParentCache.get(module);
1523+
constparent=module[kModuleParent];
15031524
constparentPath=parent?.filename;
15041525
constpackageJsonPath=path.resolve(pkg.path,'package.json');
15051526
constusesEsm=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
letdebug=require('internal/util/debuglog').debuglog('esm',(fn)=>{
@@ -285,9 +286,9 @@ function createCJSModuleWrap(url, source, isMain, loadCJS = loadCJSModule) {
285286
}
286287

287288
letexports;
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
functioncjsPreparseModuleExports(filename,source){
367368
// TODO: Do we want to keep hitting the user mutable CJS loader here?
368369
letmodule=CJSModule._cache[filename];
369-
if(module){
370-
constcached=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
constloaded=Boolean(module);
376374
if(!loaded){
377375
module=newCJSModule(filename);
378376
module.filename=filename;
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
constexportNames=newSafeSet(newSafeArrayIterator(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
@@ -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)