From 49eec2957de7ac8767d8c58113fef655b97182b5 Mon Sep 17 00:00:00 2001 From: Chengzhong Wu Date: Fri, 15 Mar 2024 17:28:32 +0800 Subject: [PATCH] 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. --- lib/internal/modules/cjs/loader.js | 107 ++++++++++++++---------- lib/internal/modules/esm/translators.js | 21 +++-- src/env_properties.h | 5 ++ 3 files changed, 79 insertions(+), 54 deletions(-) diff --git a/lib/internal/modules/cjs/loader.js b/lib/internal/modules/cjs/loader.js index cacabb771f7115..a81f180958aebf 100644 --- a/lib/internal/modules/cjs/loader.js +++ b/lib/internal/modules/cjs/loader.js @@ -50,7 +50,6 @@ const { ReflectSet, RegExpPrototypeExec, SafeMap, - SafeWeakMap, String, StringPrototypeCharAt, StringPrototypeCharCodeAt, @@ -62,16 +61,39 @@ 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'); @@ -79,8 +101,10 @@ 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, entryPointSource: undefined, // Set below. Module, @@ -257,8 +281,6 @@ function reportModuleNotFoundToWatchMode(basePath, extensions) { } } -/** @type {Map} */ -const moduleParentCache = new SafeWeakMap(); /** * Create a new module instance. * @param {string} id @@ -268,7 +290,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; @@ -356,17 +378,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) => { @@ -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})`; } @@ -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)) { @@ -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}'`; @@ -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; }; /** @@ -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]); } /** @@ -1406,7 +1427,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; } @@ -1467,15 +1488,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; } @@ -1499,7 +1520,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); diff --git a/lib/internal/modules/esm/translators.js b/lib/internal/modules/esm/translators.js index 7cb4350b675ba9..4bdee1b843fbd5 100644 --- a/lib/internal/modules/esm/translators.js +++ b/lib/internal/modules/esm/translators.js @@ -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) => { @@ -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); } @@ -366,11 +367,8 @@ 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) { @@ -378,6 +376,7 @@ function cjsPreparseModuleExports(filename, source) { module.filename = filename; module.paths = CJSModule._nodeModulePaths(module.path); module[kIsCachedByESMLoader] = true; + module[kModuleSource] = source; CJSModule._cache[filename] = module; } @@ -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; diff --git a/src/env_properties.h b/src/env_properties.h index 55ca60d9f4992b..c28878486e4ac2 100644 --- a/src/env_properties.h +++ b/src/env_properties.h @@ -26,6 +26,11 @@ V(js_transferable_wrapper_private_symbol, "node:js_transferable_wrapper") \ V(entry_point_module_private_symbol, "node:entry_point_module") \ V(entry_point_promise_private_symbol, "node:entry_point_promise") \ + 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") \