From 3f5c12a6d6edc0cffa4f57c1f3e66df24b1c0e83 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 `cjsParseCache.get(module).exportNames` as well. --- lib/internal/modules/cjs/loader.js | 78 ++++++++++++++++--------- lib/internal/modules/esm/translators.js | 21 ++++--- src/env_properties.h | 5 ++ 3 files changed, 65 insertions(+), 39 deletions(-) diff --git a/lib/internal/modules/cjs/loader.js b/lib/internal/modules/cjs/loader.js index 0d76fa0613b7a9..7036c55c81ce9b 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, @@ -61,18 +60,41 @@ const { StringPrototypeSplit, StringPrototypeStartsWith, } = 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'); -// Map used to store CJS parsing data. -const cjsParseCache = 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 kModuleExport = module_export_private_symbol; +const kModuleParent = module_parent_private_symbol; // Set first due to cycle with ESM loader functions. module.exports = { - cjsExportsCache, - cjsParseCache, + kModuleSource, + kModuleExport, + kModuleExportNames, + kModuleCircularVisited, initializeCJS, Module, wrapSafe, @@ -243,8 +265,6 @@ function reportModuleNotFoundToWatchMode(basePath, extensions) { } } -/** @type {Map} */ -const moduleParentCache = new SafeWeakMap(); /** * Create a new module instance. * @param {string} id @@ -254,7 +274,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; @@ -342,17 +362,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) => { @@ -986,15 +1008,18 @@ Module._load = function(request, parent, isMain) { const cachedModule = Module._cache[filename]; if (cachedModule !== undefined) { updateChildren(parent, cachedModule, true); - if (!cachedModule.loaded) { - const parseCachedModule = cjsParseCache.get(cachedModule); - if (!parseCachedModule || parseCachedModule.loaded) { - return getExportsForCircularRequire(cachedModule); - } - parseCachedModule.loaded = true; - } else { + if (cachedModule.loaded) { return cachedModule.exports; } + // If the cached module was finished loading, there are two possible conditions: + // 1. the cache entry was created by the ESM loader, or + // 2. it is circularly required. + // Return a proxy for the cached module's export object if the module is circularly required. + if (cachedModule[kModuleExportNames] === undefined || 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)) { @@ -1132,7 +1157,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}'`; @@ -1210,9 +1235,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; }; /** @@ -1368,11 +1391,10 @@ Module.prototype._compile = function(content, filename) { */ Module._extensions['.js'] = function(module, filename) { // If already analyzed the source, then it will be cached. - const cached = cjsParseCache.get(module); let content; - if (cached?.source) { - content = cached.source; - cached.source = undefined; + if (module[kModuleSource] !== undefined) { + content = module[kModuleSource]; + module[kModuleSource] = undefined; } else { content = fs.readFileSync(filename, 'utf8'); } @@ -1381,7 +1403,7 @@ Module._extensions['.js'] = function(module, filename) { // Function require shouldn't be used in ES modules. if (pkg?.data.type === 'module') { // 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 6772bbffd989d2..6fceb043843afd 100644 --- a/lib/internal/modules/esm/translators.js +++ b/lib/internal/modules/esm/translators.js @@ -44,8 +44,9 @@ const { } = require('internal/modules/helpers'); const { Module: CJSModule, - cjsParseCache, - cjsExportsCache, + kModuleSource, + kModuleExport, + kModuleExportNames, } = require('internal/modules/cjs/loader'); const { fileURLToPath, pathToFileURL, URL } = require('internal/url'); let debug = require('internal/util/debuglog').debuglog('esm', (fn) => { @@ -298,9 +299,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); } @@ -370,11 +371,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 = cjsParseCache.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) { @@ -395,7 +393,8 @@ function cjsPreparseModuleExports(filename, source) { const exportNames = new SafeSet(new SafeArrayIterator(exports)); // Set first for cycles. - cjsParseCache.set(module, { source, exportNames }); + module[kModuleExportNames] = exportNames; + module[kModuleSource] = source; if (reexports.length) { module.filename = filename; diff --git a/src/env_properties.h b/src/env_properties.h index 82d2a64e88e114..cbcbe6c76515c3 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") \