From fa2baa7bd3b1366d23d13f8894b8a2b31a1f06c2 Mon Sep 17 00:00:00 2001 From: Keyhan Vakil Date: Tue, 30 Jan 2024 22:28:28 +0000 Subject: [PATCH] module: compute module.paths lazily Make module.paths a getter which computes the property when it's requested. The paths property is entirely deterministically computed from the path property, so it's not necessary to keep the relatively large paths array in memory. On an internal application, this reduces the used heap by around 5.5%. Furthermore (although it was not a goal of this PR) it speeds up the execution time of `vite --version` by ~2% according to hyperfine. A similar speed improvement is observed on the internal application. Notably, this introduces two minor changes which may be considered breaking. The obvious one is that `paths` is now a getter/setter and not a property. The other one is more subtle: the lazy computation means that if a user changes `mod.path = X` and then later invokes `mod.paths`, the results will reflect the updated path as opposed to the current behavior where it would reflect the original path. One way to fix the latter would be to force capturing `paths` before `path` is changed, I'm open to doing that (or some cleaner approach?) if the reviewers deem it necessary. --- lib/internal/modules/cjs/loader.js | 27 ++++++++++++++++++++----- lib/internal/modules/esm/resolve.js | 1 - lib/internal/modules/esm/translators.js | 2 -- lib/internal/util/embedding.js | 1 - 4 files changed, 22 insertions(+), 9 deletions(-) diff --git a/lib/internal/modules/cjs/loader.js b/lib/internal/modules/cjs/loader.js index e5b47d8874aeb7..8b7291dd93192e 100644 --- a/lib/internal/modules/cjs/loader.js +++ b/lib/internal/modules/cjs/loader.js @@ -248,6 +248,18 @@ function reportModuleNotFoundToWatchMode(basePath, extensions) { } } +const kPaths = Symbol('kPaths'); +const lazyPathsGetter = { + __proto__: null, + enumerable: true, + get() { return this[kPaths] ?? Module._nodeModulePaths(this.path); }, + set(v) { + // We set paths directly in various places. + this[kPaths] = v; + }, +}; + + /** @type {Map} */ const moduleParentCache = new SafeWeakMap(); /** @@ -265,6 +277,11 @@ function Module(id = '', parent) { this.loaded = false; this.children = []; let redirects; + + // Lazily compute paths to save on memory. Defined as an own-property for + // backwards compatibility reasons. + ObjectDefineProperty(this, 'paths', lazyPathsGetter); + const manifest = policy()?.manifest; if (manifest) { const moduleURL = pathToFileURL(id); @@ -856,9 +873,10 @@ Module._resolveLookupPaths = function(request, parent) { /** @type {string[]} */ let paths; - if (parent?.paths?.length) { + const parentPaths = parent?.paths; + if (parentPaths?.length) { paths = ArrayPrototypeSlice(modulePaths); - ArrayPrototypeUnshiftApply(paths, parent.paths); + ArrayPrototypeUnshiftApply(paths, parentPaths); } else { paths = modulePaths; } @@ -1083,7 +1101,7 @@ Module._resolveFilename = function(request, parent, isMain, options) { for (let i = 0; i < options.paths.length; i++) { const path = options.paths[i]; - fakeParent.paths = Module._nodeModulePaths(path); + fakeParent.path = path; const lookupPaths = Module._resolveLookupPaths(request, fakeParent); for (let j = 0; j < lookupPaths.length; j++) { @@ -1201,7 +1219,6 @@ Module.prototype.load = function(filename) { assert(!this.loaded); this.filename = filename; - this.paths = Module._nodeModulePaths(path.dirname(filename)); const extension = findLongestRegisteredExtension(filename); // allow .mjs to be overridden @@ -1492,7 +1509,6 @@ function createRequireFromPath(filename) { const m = new Module(proxyPath); m.filename = proxyPath; - m.paths = Module._nodeModulePaths(m.path); return makeRequireFunction(m, null); } @@ -1579,6 +1595,7 @@ Module._preloadModules = function(requests) { isPreloading = false; throw e; } + parent.paths = []; } for (let n = 0; n < requests.length; n++) { internalRequire(parent, requests[n]); diff --git a/lib/internal/modules/esm/resolve.js b/lib/internal/modules/esm/resolve.js index 54cdbe8e6175ad..cd68670493c761 100644 --- a/lib/internal/modules/esm/resolve.js +++ b/lib/internal/modules/esm/resolve.js @@ -918,7 +918,6 @@ function resolveAsCommonJS(specifier, parentURL) { try { const parent = fileURLToPath(parentURL); const tmpModule = new CJSModule(parent, null); - tmpModule.paths = CJSModule._nodeModulePaths(parent); let found = CJSModule._resolveFilename(specifier, tmpModule, false); diff --git a/lib/internal/modules/esm/translators.js b/lib/internal/modules/esm/translators.js index 4ecf862be9ea7b..e284a08795de65 100644 --- a/lib/internal/modules/esm/translators.js +++ b/lib/internal/modules/esm/translators.js @@ -391,7 +391,6 @@ function cjsPreparseModuleExports(filename, source) { if (!loaded) { module = new CJSModule(filename); module.filename = filename; - module.paths = CJSModule._nodeModulePaths(module.path); CJSModule._cache[filename] = module; } @@ -410,7 +409,6 @@ function cjsPreparseModuleExports(filename, source) { if (reexports.length) { module.filename = filename; - module.paths = CJSModule._nodeModulePaths(module.path); for (let i = 0; i < reexports.length; i++) { const reexport = reexports[i]; let resolved; diff --git a/lib/internal/util/embedding.js b/lib/internal/util/embedding.js index be310f401ad115..745dab08099bcf 100644 --- a/lib/internal/util/embedding.js +++ b/lib/internal/util/embedding.js @@ -24,7 +24,6 @@ function embedderRunCjs(contents) { const customModule = new Module(filename, null); customModule.filename = filename; - customModule.paths = Module._nodeModulePaths(customModule.path); const customExports = customModule.exports;