From 92e95f17b64838d4cf77343c1a814d4ebd795217 Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Sat, 5 Jan 2019 06:02:33 +0800 Subject: src: simplify NativeModule caching and remove redundant data - Remove `NativeModule._source` - the compilation is now entirely done in C++ and `process.binding('natives')` is implemented directly in the binding loader so there is no need to store additional source code strings. - Instead of using an object as `NativeModule._cached` and insert into it after compilation of each native module, simply prebuild a JS map filled with all the native modules and infer the state of compilation through `mod.loading`/`mod.loaded`. - Rename `NativeModule.nonInternalExists` to `NativeModule.canBeRequiredByUsers` and precompute that property for all the native modules during bootstrap instead of branching in every require call during runtime. This also fixes the bug where `worker_threads` can be made available with `--expose-internals`. - Rename `NativeModule.requireForDeps` to `NativeModule.requireWithFallbackInDeps`. - Add a test to make sure we do not accidentally leak any module to the global namespace. PR-URL: https://github.com/nodejs/node/pull/25352 Reviewed-By: Anna Henningsen --- lib/internal/bootstrap/cache.js | 23 +++--- lib/internal/bootstrap/loaders.js | 110 +++++++++++----------------- lib/internal/modules/cjs/loader.js | 14 ++-- lib/internal/modules/esm/default_resolve.js | 2 +- lib/internal/modules/esm/translators.js | 2 +- 5 files changed, 66 insertions(+), 85 deletions(-) (limited to 'lib/internal') diff --git a/lib/internal/bootstrap/cache.js b/lib/internal/bootstrap/cache.js index f69f1252ad..c89286ce0f 100644 --- a/lib/internal/bootstrap/cache.js +++ b/lib/internal/bootstrap/cache.js @@ -7,14 +7,10 @@ const { NativeModule } = require('internal/bootstrap/loaders'); const { - source, getCodeCache, compileFunction + getCodeCache, compileFunction } = internalBinding('native_module'); const { hasTracing, hasInspector } = process.binding('config'); -const depsModule = Object.keys(source).filter( - (key) => NativeModule.isDepsModule(key) || key.startsWith('internal/deps') -); - // Modules with source code compiled in js2c that // cannot be compiled with the code cache. const cannotUseCache = [ @@ -29,7 +25,7 @@ const cannotUseCache = [ // the code cache is also used when compiling these two files. 'internal/bootstrap/loaders', 'internal/bootstrap/node' -].concat(depsModule); +]; // Skip modules that cannot be required when they are not // built into the binary. @@ -67,11 +63,18 @@ if (!process.versions.openssl) { ); } +const cachableBuiltins = []; +for (const id of NativeModule.map.keys()) { + if (id.startsWith('internal/deps')) { + cannotUseCache.push(id); + } + if (!cannotUseCache.includes(id)) { + cachableBuiltins.push(id); + } +} + module.exports = { - cachableBuiltins: Object.keys(source).filter( - (key) => !cannotUseCache.includes(key) - ), - getSource(id) { return source[id]; }, + cachableBuiltins, getCodeCache, compileFunction, cannotUseCache diff --git a/lib/internal/bootstrap/loaders.js b/lib/internal/bootstrap/loaders.js index 879de1bc80..300a362abb 100644 --- a/lib/internal/bootstrap/loaders.js +++ b/lib/internal/bootstrap/loaders.js @@ -158,6 +158,12 @@ let internalBinding; // Create this WeakMap in js-land because V8 has no C++ API for WeakMap. internalBinding('module_wrap').callbackMap = new WeakMap(); +// Think of this as module.exports in this file even though it is not +// written in CommonJS style. +const loaderExports = { internalBinding, NativeModule }; +const loaderId = 'internal/bootstrap/loaders'; +const config = internalBinding('config'); + // Set up NativeModule. function NativeModule(id) { this.filename = `${id}.js`; @@ -167,34 +173,35 @@ function NativeModule(id) { this.exportKeys = undefined; this.loaded = false; this.loading = false; + if (id === loaderId) { + // Do not expose this to user land even with --expose-internals. + this.canBeRequiredByUsers = false; + } else if (id.startsWith('internal/')) { + this.canBeRequiredByUsers = config.exposeInternals; + } else { + this.canBeRequiredByUsers = true; + } } const { - source, + moduleIds, compileFunction } = internalBinding('native_module'); -NativeModule._source = source; -NativeModule._cache = {}; - -const config = internalBinding('config'); - -// Think of this as module.exports in this file even though it is not -// written in CommonJS style. -const loaderExports = { internalBinding, NativeModule }; -const loaderId = 'internal/bootstrap/loaders'; +NativeModule.map = new Map(); +for (var i = 0; i < moduleIds.length; ++i) { + const id = moduleIds[i]; + const mod = new NativeModule(id); + NativeModule.map.set(id, mod); +} NativeModule.require = function(id) { if (id === loaderId) { return loaderExports; } - const cached = NativeModule.getCached(id); - if (cached && (cached.loaded || cached.loading)) { - return cached.exports; - } - - if (!NativeModule.exists(id)) { + const mod = NativeModule.map.get(id); + if (!mod) { // Model the error off the internal/errors.js model, but // do not use that module given that it could actually be // the one causing the error if there's a bug in Node.js. @@ -205,60 +212,31 @@ NativeModule.require = function(id) { throw err; } - moduleLoadList.push(`NativeModule ${id}`); - - const nativeModule = new NativeModule(id); - - nativeModule.cache(); - nativeModule.compile(); - - return nativeModule.exports; -}; - -NativeModule.isDepsModule = function(id) { - return id.startsWith('node-inspect/') || id.startsWith('v8/'); -}; - -NativeModule.requireForDeps = function(id) { - if (!NativeModule.exists(id)) { - id = `internal/deps/${id}`; + if (mod.loaded || mod.loading) { + return mod.exports; } - return NativeModule.require(id); -}; -NativeModule.getCached = function(id) { - return NativeModule._cache[id]; + moduleLoadList.push(`NativeModule ${id}`); + mod.compile(); + return mod.exports; }; NativeModule.exists = function(id) { - return NativeModule._source.hasOwnProperty(id); + return NativeModule.map.has(id); }; -if (config.exposeInternals) { - NativeModule.nonInternalExists = function(id) { - // Do not expose this to user land even with --expose-internals. - if (id === loaderId) { - return false; - } - return NativeModule.exists(id); - }; - - NativeModule.isInternal = function(id) { - // Do not expose this to user land even with --expose-internals. - return id === loaderId; - }; -} else { - NativeModule.nonInternalExists = function(id) { - return NativeModule.exists(id) && !NativeModule.isInternal(id); - }; - - NativeModule.isInternal = function(id) { - return id.startsWith('internal/'); - }; -} +NativeModule.canBeRequiredByUsers = function(id) { + const mod = NativeModule.map.get(id); + return mod && mod.canBeRequiredByUsers; +}; -NativeModule.getSource = function(id) { - return NativeModule._source[id]; +// Allow internal modules from dependencies to require +// other modules from dependencies by providing fallbacks. +NativeModule.requireWithFallbackInDeps = function(request) { + if (!NativeModule.map.has(request)) { + request = `internal/deps/${request}`; + } + return NativeModule.require(request); }; const getOwn = (target, property, receiver) => { @@ -332,13 +310,13 @@ NativeModule.prototype.compile = function() { try { const requireFn = this.id.startsWith('internal/deps/') ? - NativeModule.requireForDeps : + NativeModule.requireWithFallbackInDeps : NativeModule.require; const fn = compileFunction(id); fn(this.exports, requireFn, this, process, internalBinding); - if (config.experimentalModules && !NativeModule.isInternal(this.id)) { + if (config.experimentalModules && this.canBeRequiredByUsers) { this.proxifyExports(); } @@ -348,10 +326,6 @@ NativeModule.prototype.compile = function() { } }; -NativeModule.prototype.cache = function() { - NativeModule._cache[this.id] = this; -}; - // Coverage must be turned on early, so that we can collect // it for Node.js' own internal libraries. if (process.env.NODE_V8_COVERAGE) { diff --git a/lib/internal/modules/cjs/loader.js b/lib/internal/modules/cjs/loader.js index bf6a9d4029..f4551b7d47 100644 --- a/lib/internal/modules/cjs/loader.js +++ b/lib/internal/modules/cjs/loader.js @@ -112,8 +112,12 @@ function Module(id, parent) { this.children = []; } -const builtinModules = Object.keys(NativeModule._source) - .filter(NativeModule.nonInternalExists); +const builtinModules = []; +for (const [id, mod] of NativeModule.map) { + if (mod.canBeRequiredByUsers) { + builtinModules.push(id); + } +} Object.freeze(builtinModules); Module.builtinModules = builtinModules; @@ -417,7 +421,7 @@ if (isWindows) { var indexChars = [ 105, 110, 100, 101, 120, 46 ]; var indexLen = indexChars.length; Module._resolveLookupPaths = function(request, parent, newReturn) { - if (NativeModule.nonInternalExists(request)) { + if (NativeModule.canBeRequiredByUsers(request)) { debug('looking for %j in []', request); return (newReturn ? null : [request, []]); } @@ -534,7 +538,7 @@ Module._load = function(request, parent, isMain) { return cachedModule.exports; } - if (NativeModule.nonInternalExists(filename)) { + if (NativeModule.canBeRequiredByUsers(filename)) { debug('load native module %s', request); return NativeModule.require(filename); } @@ -567,7 +571,7 @@ function tryModuleLoad(module, filename) { } Module._resolveFilename = function(request, parent, isMain, options) { - if (NativeModule.nonInternalExists(request)) { + if (NativeModule.canBeRequiredByUsers(request)) { return request; } diff --git a/lib/internal/modules/esm/default_resolve.js b/lib/internal/modules/esm/default_resolve.js index 9aa54d09a1..2cf9014a67 100644 --- a/lib/internal/modules/esm/default_resolve.js +++ b/lib/internal/modules/esm/default_resolve.js @@ -53,7 +53,7 @@ const extensionFormatMap = { }; function resolve(specifier, parentURL) { - if (NativeModule.nonInternalExists(specifier)) { + if (NativeModule.canBeRequiredByUsers(specifier)) { return { url: specifier, format: 'builtin' diff --git a/lib/internal/modules/esm/translators.js b/lib/internal/modules/esm/translators.js index 6bfc4c8a98..776b8d584d 100644 --- a/lib/internal/modules/esm/translators.js +++ b/lib/internal/modules/esm/translators.js @@ -81,7 +81,7 @@ translators.set('builtin', async (url) => { // slice 'node:' scheme const id = url.slice(5); NativeModule.require(id); - const module = NativeModule.getCached(id); + const module = NativeModule.map.get(id); return createDynamicModule( [...module.exportKeys, 'default'], url, (reflect) => { debug(`Loading BuiltinModule ${url}`); -- cgit v1.2.3