Skip to content

Commit

Permalink
lib: convert WeakMaps in cjs loader with private symbol properties
Browse files Browse the repository at this point in the history 
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: nodejs#52095
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
  • Loading branch information
@legendecas @joyeecheung
legendecas authored and joyeecheung committed Jul 30, 2024
1 parent 269b145 commit 99c718f
Show file tree
Hide file tree
Showing 3 changed files with 79 additions and 54 deletions.
107 changes: 64 additions & 43 deletions lib/internal/modules/cjs/loader.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,6 @@ const {
ReflectSet,
RegExpPrototypeExec,
SafeMap,
SafeWeakMap,
String,
StringPrototypeCharAt,
StringPrototypeCharCodeAt,
Expand All @@ -62,25 +61,50 @@ const {
StringPrototypeStartsWith,
Symbol,
} = primordials;
const {
privateSymbols: {
module_source_private_symbol,
module_export_names_private_symbol,
module_circular_visited_private_symbol,
module_export_private_symbol,
module_parent_private_symbol,
},
} = internalBinding('util');

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

// Map used to store CJS parsing data or for ESM loading.
const importedCJSCache = new SafeWeakMap();
// Internal properties for Module instances.
/**
* Cached {@link Module} source string.
*/
const kModuleSource = module_source_private_symbol;
/**
* Cached {@link Module} export names for ESM loader.
*/
const kModuleExportNames = module_export_names_private_symbol;
/**
* {@link Module} circular dependency visited flag.
*/
const kModuleCircularVisited = module_circular_visited_private_symbol;
/**
* Map of already-loaded CJS modules to use.
* {@link Module} export object snapshot for ESM loader.
*/
const cjsExportsCache = new SafeWeakMap();
const requiredESMSourceCache = new SafeWeakMap();
const kModuleExport = module_export_private_symbol;
/**
* {@link Module} parent module.
*/
const kModuleParent = module_parent_private_symbol;

const kIsMainSymbol = Symbol('kIsMainSymbol');
const kIsCachedByESMLoader = Symbol('kIsCachedByESMLoader');
const kRequiredModuleSymbol = Symbol('kRequiredModuleSymbol');
const kIsExecuting = Symbol('kIsExecuting');
// Set first due to cycle with ESM loader functions.
module.exports = {
cjsExportsCache,
importedCJSCache,
kModuleSource,
kModuleExport,
kModuleExportNames,
kModuleCircularVisited,
initializeCJS,
Module,
wrapSafe,
Expand Down Expand Up @@ -256,8 +280,6 @@ function reportModuleNotFoundToWatchMode(basePath, extensions) {
}
}

/** @type {Map<Module, Module>} */
const moduleParentCache = new SafeWeakMap();
/**
* Create a new module instance.
* @param {string} id
Expand All @@ -267,7 +289,7 @@ function Module(id = '', parent) {
this.id = id;
this.path = path.dirname(id);
setOwnProperty(this, 'exports', {});
moduleParentCache.set(this, parent);
this[kModuleParent] = parent;
updateChildren(parent, this, false);
this.filename = null;
this.loaded = false;
Expand Down Expand Up @@ -355,17 +377,19 @@ ObjectDefineProperty(BuiltinModule.prototype, 'isPreloading', isPreloadingDesc);

/**
* Get the parent of the current module from our cache.
* @this {Module}
*/
function getModuleParent() {
return moduleParentCache.get(this);
return this[kModuleParent];
}

/**
* Set the parent of the current module in our cache.
* @this {Module}
* @param {Module} value
*/
function setModuleParent(value) {
moduleParentCache.set(this, value);
this[kModuleParent] = value;
}

let debug = require('internal/util/debuglog').debuglog('module', (fn) => {
Expand Down Expand Up @@ -955,7 +979,7 @@ function getExportsForCircularRequire(module) {
const requiredESM = module[kRequiredModuleSymbol];
if (requiredESM && requiredESM.getStatus() !== kEvaluated) {
let message = `Cannot require() ES Module ${module.id} in a cycle.`;
const parent = moduleParentCache.get(module);
const parent = module[kModuleParent];
if (parent) {
message += ` (from ${parent.filename})`;
}
Expand Down Expand Up @@ -1028,25 +1052,24 @@ Module._load = function(request, parent, isMain) {
const cachedModule = Module._cache[filename];
if (cachedModule !== undefined) {
updateChildren(parent, cachedModule, true);
if (!cachedModule.loaded) {
// If it's not cached by the ESM loader, the loading request
// comes from required CJS, and we can consider it a circular
// dependency when it's cached.
if (!cachedModule[kIsCachedByESMLoader]) {
return getExportsForCircularRequire(cachedModule);
}
// If it's cached by the ESM loader as a way to indirectly pass
// the module in to avoid creating it twice, the loading request
// come from imported CJS. In that case use the importedCJSCache
// to determine if it's loading or not.
const importedCJSMetadata = importedCJSCache.get(cachedModule);
if (importedCJSMetadata.loading) {
return getExportsForCircularRequire(cachedModule);
}
importedCJSMetadata.loading = true;
} else {
if (cachedModule.loaded) {
return cachedModule.exports;
}
// If it's not cached by the ESM loader, the loading request
// comes from required CJS, and we can consider it a circular
// dependency when it's cached.
if (!cachedModule[kIsCachedByESMLoader]) {
return getExportsForCircularRequire(cachedModule);
}
// If it's cached by the ESM loader as a way to indirectly pass
// the module in to avoid creating it twice, the loading request
// come from imported CJS. In that case use the kModuleCircularVisited
// to determine if it's loading or not.
if (cachedModule[kModuleCircularVisited]) {
return getExportsForCircularRequire(cachedModule);
}
// This is an ESM loader created cache entry, mark it as visited and fallthrough to loading the module.
cachedModule[kModuleCircularVisited] = true;
}

if (BuiltinModule.canBeRequiredWithoutScheme(filename)) {
Expand Down Expand Up @@ -1190,7 +1213,7 @@ Module._resolveFilename = function(request, parent, isMain, options) {
const requireStack = [];
for (let cursor = parent;
cursor;
cursor = moduleParentCache.get(cursor)) {
cursor = cursor[kModuleParent]) {
ArrayPrototypePush(requireStack, cursor.filename || cursor.id);
}
let message = `Cannot find module '${request}'`;
Expand Down Expand Up @@ -1268,9 +1291,7 @@ Module.prototype.load = function(filename) {
// Create module entry at load time to snapshot exports correctly
const exports = this.exports;
// Preemptively cache for ESM loader.
if (!cjsExportsCache.has(this)) {
cjsExportsCache.set(this, exports);
}
this[kModuleExport] = exports;
};

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

/**
Expand Down Expand Up @@ -1399,7 +1420,7 @@ Module.prototype._compile = function(content, filename, loadAsESM = false) {
// Only modules being require()'d really need to avoid TLA.
if (loadAsESM) {
// Pass the source into the .mjs extension handler indirectly through the cache.
requiredESMSourceCache.set(this, content);
this[kModuleSource] = content;
loadESMFromCJS(this, filename);
return;
}
Expand Down Expand Up @@ -1460,15 +1481,15 @@ Module.prototype._compile = function(content, filename, loadAsESM = false) {
* @returns {string}
*/
function getMaybeCachedSource(mod, filename) {
const cached = importedCJSCache.get(mod);
// If already analyzed the source, then it will be cached.
let content;
if (cached?.source) {
content = cached.source;
cached.source = undefined;
if (mod[kModuleSource] !== undefined) {
content = mod[kModuleSource];
mod[kModuleSource] = undefined;
} else {
// TODO(joyeecheung): we can read a buffer instead to speed up
// compilation.
content = requiredESMSourceCache.get(mod) ?? fs.readFileSync(filename, 'utf8');
content = fs.readFileSync(filename, 'utf8');
}
return content;
}
Expand All @@ -1492,7 +1513,7 @@ Module._extensions['.js'] = function(module, filename) {
}

// This is an error path because `require` of a `.js` file in a `"type": "module"` scope is not allowed.
const parent = moduleParentCache.get(module);
const parent = module[kModuleParent];
const parentPath = parent?.filename;
const packageJsonPath = path.resolve(pkg.path, 'package.json');
const usesEsm = containsModuleSyntax(content, filename);
Expand Down
21 changes: 10 additions & 11 deletions lib/internal/modules/esm/translators.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,10 +43,11 @@ const {
stripBOM,
} = require('internal/modules/helpers');
const {
cjsExportsCache,
importedCJSCache,
kIsCachedByESMLoader,
Module: CJSModule,
kModuleSource,
kModuleExport,
kModuleExportNames,
} = require('internal/modules/cjs/loader');
const { fileURLToPath, pathToFileURL, URL } = require('internal/url');
let debug = require('internal/util/debuglog').debuglog('esm', (fn) => {
Expand Down Expand Up @@ -285,9 +286,9 @@ function createCJSModuleWrap(url, source, isMain, loadCJS = loadCJSModule) {
}

let exports;
if (cjsExportsCache.has(module)) {
exports = cjsExportsCache.get(module);
cjsExportsCache.delete(module);
if (module[kModuleExport] !== undefined) {
exports = module[kModuleExport];
module[kModuleExport] = undefined;
} else {
({ exports } = module);
}
Expand Down Expand Up @@ -366,18 +367,16 @@ translators.set('commonjs', async function commonjsStrategy(url, source,
function cjsPreparseModuleExports(filename, source) {
// TODO: Do we want to keep hitting the user mutable CJS loader here?
let module = CJSModule._cache[filename];
if (module) {
const cached = importedCJSCache.get(module);
if (cached) {
return { module, exportNames: cached.exportNames };
}
if (module && module[kModuleExportNames] !== undefined) {
return { module, exportNames: module[kModuleExportNames] };
}
const loaded = Boolean(module);
if (!loaded) {
module = new CJSModule(filename);
module.filename = filename;
module.paths = CJSModule._nodeModulePaths(module.path);
module[kIsCachedByESMLoader] = true;
module[kModuleSource] = source;
CJSModule._cache[filename] = module;
}

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

// Set first for cycles.
importedCJSCache.set(module, { source, exportNames });
module[kModuleExportNames] = exportNames;

if (reexports.length) {
module.filename = filename;
Expand Down
5 changes: 5 additions & 0 deletions src/env_properties.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,11 @@
V(contextify_context_private_symbol, "node:contextify:context") \
V(decorated_private_symbol, "node:decorated") \
V(host_defined_option_symbol, "node:host_defined_option_symbol") \
V(module_source_private_symbol, "node:module_source") \
V(module_export_names_private_symbol, "node:module_export_names") \
V(module_circular_visited_private_symbol, "node:module_circular_visited") \
V(module_export_private_symbol, "node:module_export") \
V(module_parent_private_symbol, "node:module_parent") \
V(napi_type_tag, "node:napi:type_tag") \
V(napi_wrapper, "node:napi:wrapper") \
V(untransferable_object_private_symbol, "node:untransferableObject") \
Expand Down

0 comments on commit 99c718f

Please sign in to comment.