From d6b57f6629da5616362be60ae0f3e9510a7f700e Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Thu, 4 Apr 2024 00:31:48 +0100 Subject: [PATCH] module: centralize SourceTextModule compilation for builtin loader This refactors the code that compiles SourceTextModule for the built-in ESM loader to use a common routine so that it's easier to customize cache handling for the ESM loader. In addition this introduces a common symbol for import.meta and import() so that we don't need to create additional closures as handlers, since we can get all the information we need from the V8 callback already. This should reduce the memory footprint of ESM as well. PR-URL: https://github.com/nodejs/node/pull/52291 Refs: https://github.com/nodejs/node/issues/47472 Reviewed-By: Geoffrey Booth Reviewed-By: Stephen Belanger --- .../modules/esm/create_dynamic_module.js | 5 +- lib/internal/modules/esm/loader.js | 34 +------- lib/internal/modules/esm/translators.js | 26 +----- lib/internal/modules/esm/utils.js | 81 ++++++++++++++++--- lib/internal/vm/module.js | 11 ++- src/env_properties.h | 1 + src/module_wrap.cc | 42 ++++++---- 7 files changed, 109 insertions(+), 91 deletions(-) diff --git a/lib/internal/modules/esm/create_dynamic_module.js b/lib/internal/modules/esm/create_dynamic_module.js index d4f5a85db95f77..26e21d8407c729 100644 --- a/lib/internal/modules/esm/create_dynamic_module.js +++ b/lib/internal/modules/esm/create_dynamic_module.js @@ -55,8 +55,8 @@ ${ArrayPrototypeJoin(ArrayPrototypeMap(imports, createImport), '\n')} ${ArrayPrototypeJoin(ArrayPrototypeMap(exports, createExport), '\n')} import.meta.done(); `; - const { ModuleWrap } = internalBinding('module_wrap'); - const m = new ModuleWrap(`${url}`, undefined, source, 0, 0); + const { registerModule, compileSourceTextModule } = require('internal/modules/esm/utils'); + const m = compileSourceTextModule(`${url}`, source); const readyfns = new SafeSet(); /** @type {DynamicModuleReflect} */ @@ -68,7 +68,6 @@ import.meta.done(); if (imports.length) { reflect.imports = { __proto__: null }; } - const { registerModule } = require('internal/modules/esm/utils'); registerModule(m, { __proto__: null, initializeImportMeta: (meta, wrap) => { diff --git a/lib/internal/modules/esm/loader.js b/lib/internal/modules/esm/loader.js index 85c4960c8e3b11..8b1cceb44046f5 100644 --- a/lib/internal/modules/esm/loader.js +++ b/lib/internal/modules/esm/loader.js @@ -25,13 +25,10 @@ const { getOptionValue } = require('internal/options'); const { isURL, pathToFileURL, URL } = require('internal/url'); const { emitExperimentalWarning, kEmptyObject } = require('internal/util'); const { - registerModule, + compileSourceTextModule, getDefaultConditions, } = require('internal/modules/esm/utils'); const { kImplicitAssertType } = require('internal/modules/esm/assert'); -const { - maybeCacheSourceMap, -} = require('internal/source_map/source_map_cache'); const { canParse } = internalBinding('url'); const { ModuleWrap } = internalBinding('module_wrap'); let defaultResolve, defaultLoad, defaultLoadSync, importMetaInitializer; @@ -193,16 +190,7 @@ class ModuleLoader { async eval(source, url, isEntryPoint = false) { const evalInstance = (url) => { - const module = new ModuleWrap(url, undefined, source, 0, 0); - registerModule(module, { - __proto__: null, - initializeImportMeta: (meta, wrap) => this.importMetaInitialize(meta, { url }), - importModuleDynamically: (specifier, { url }, importAttributes) => { - return this.import(specifier, url, importAttributes); - }, - }); - - return module; + return compileSourceTextModule(url, source, this); }; const { ModuleJob } = require('internal/modules/esm/module_job'); const job = new ModuleJob( @@ -273,26 +261,10 @@ class ModuleLoader { if (job !== undefined) { return job.module.getNamespaceSync(); } - // TODO(joyeecheung): refactor this so that we pre-parse in C++ and hit the // cache here, or use a carrier object to carry the compiled module script // into the constructor to ensure cache hit. - const wrap = new ModuleWrap(url, undefined, source, 0, 0); - // Cache the source map for the module if present. - if (wrap.sourceMapURL) { - maybeCacheSourceMap(url, source, null, false, undefined, wrap.sourceMapURL); - } - const { registerModule } = require('internal/modules/esm/utils'); - // TODO(joyeecheung): refactor so that the default options are shared across - // the built-in loaders. - registerModule(wrap, { - __proto__: null, - initializeImportMeta: (meta, wrap) => this.importMetaInitialize(meta, { url }), - importModuleDynamically: (specifier, wrap, importAttributes) => { - return this.import(specifier, url, importAttributes); - }, - }); - + const wrap = compileSourceTextModule(url, source, this); const inspectBrk = (isMain && getOptionValue('--inspect-brk')); const { ModuleJobSync } = require('internal/modules/esm/module_job'); diff --git a/lib/internal/modules/esm/translators.js b/lib/internal/modules/esm/translators.js index 7312bd0b09f41a..4614b37570c65b 100644 --- a/lib/internal/modules/esm/translators.js +++ b/lib/internal/modules/esm/translators.js @@ -156,35 +156,13 @@ function errPath(url) { return url; } -/** - * Dynamically imports a module using the ESM loader. - * @param {string} specifier - The module specifier to import. - * @param {object} options - An object containing options for the import. - * @param {string} options.url - The URL of the module requesting the import. - * @param {Record} [attributes] - An object containing attributes for the import. - * @returns {Promise} The imported module. - */ -async function importModuleDynamically(specifier, { url }, attributes) { - const cascadedLoader = require('internal/modules/esm/loader').getOrInitializeCascadedLoader(); - return cascadedLoader.import(specifier, url, attributes); -} - // Strategy for loading a standard JavaScript module. translators.set('module', function moduleStrategy(url, source, isMain) { assertBufferSource(source, true, 'load'); source = stringify(source); debug(`Translating StandardModule ${url}`); - const module = new ModuleWrap(url, undefined, source, 0, 0); - // Cache the source map for the module if present. - if (module.sourceMapURL) { - maybeCacheSourceMap(url, source, null, false, undefined, module.sourceMapURL); - } - const { registerModule } = require('internal/modules/esm/utils'); - registerModule(module, { - __proto__: null, - initializeImportMeta: (meta, wrap) => this.importMetaInitialize(meta, { url }), - importModuleDynamically, - }); + const { compileSourceTextModule } = require('internal/modules/esm/utils'); + const module = compileSourceTextModule(url, source, this); return module; }); diff --git a/lib/internal/modules/esm/utils.js b/lib/internal/modules/esm/utils.js index 7c1fb2a5745b5b..622f9462ac2931 100644 --- a/lib/internal/modules/esm/utils.js +++ b/lib/internal/modules/esm/utils.js @@ -13,12 +13,18 @@ const { }, } = internalBinding('util'); const { + source_text_module_default_hdo, vm_dynamic_import_default_internal, vm_dynamic_import_main_context_default, vm_dynamic_import_missing_flag, vm_dynamic_import_no_callback, } = internalBinding('symbols'); +const { ModuleWrap } = internalBinding('module_wrap'); +const { + maybeCacheSourceMap, +} = require('internal/source_map/source_map_cache'); + const { ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING_FLAG, ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING, @@ -167,28 +173,55 @@ function registerModule(referrer, registry) { moduleRegistries.set(idSymbol, registry); } +/** + * Proxy the import meta handling to the default loader for source text modules. + * @param {Record} meta - The import.meta object to initialize. + * @param {ModuleWrap} wrap - The ModuleWrap of the SourceTextModule where `import.meta` is referenced. + */ +function defaultInitializeImportMetaForModule(meta, wrap) { + const cascadedLoader = require('internal/modules/esm/loader').getOrInitializeCascadedLoader(); + return cascadedLoader.importMetaInitialize(meta, { url: wrap.url }); +} + /** * Defines the `import.meta` object for a given module. * @param {symbol} symbol - Reference to the module. * @param {Record} meta - The import.meta object to initialize. + * @param {ModuleWrap} wrap - The ModuleWrap of the SourceTextModule where `import.meta` is referenced. */ -function initializeImportMetaObject(symbol, meta) { - if (moduleRegistries.has(symbol)) { - const { initializeImportMeta, callbackReferrer } = moduleRegistries.get(symbol); - if (initializeImportMeta !== undefined) { - meta = initializeImportMeta(meta, callbackReferrer); - } +function initializeImportMetaObject(symbol, meta, wrap) { + if (symbol === source_text_module_default_hdo) { + defaultInitializeImportMetaForModule(meta, wrap); + return; + } + const data = moduleRegistries.get(symbol); + assert(data, `import.meta registry not found for ${wrap.url}`); + const { initializeImportMeta, callbackReferrer } = data; + if (initializeImportMeta !== undefined) { + meta = initializeImportMeta(meta, callbackReferrer); } } /** - * Proxy the dynamic import to the default loader. + * Proxy the dynamic import handling to the default loader for source text modules. + * @param {string} specifier - The module specifier string. + * @param {Record} attributes - The import attributes object. + * @param {string|null|undefined} referrerName - name of the referrer. + * @returns {Promise} - The imported module object. + */ +function defaultImportModuleDynamicallyForModule(specifier, attributes, referrerName) { + const cascadedLoader = require('internal/modules/esm/loader').getOrInitializeCascadedLoader(); + return cascadedLoader.import(specifier, referrerName, attributes); +} + +/** + * Proxy the dynamic import to the default loader for classic scripts. * @param {string} specifier - The module specifier string. * @param {Record} attributes - The import attributes object. * @param {string|null|undefined} referrerName - name of the referrer. * @returns {Promise} - The imported module object. */ -function defaultImportModuleDynamically(specifier, attributes, referrerName) { +function defaultImportModuleDynamicallyForScript(specifier, attributes, referrerName) { const parentURL = normalizeReferrerURL(referrerName); const cascadedLoader = require('internal/modules/esm/loader').getOrInitializeCascadedLoader(); return cascadedLoader.import(specifier, parentURL, attributes); @@ -208,12 +241,16 @@ async function importModuleDynamicallyCallback(referrerSymbol, specifier, attrib // and fall back to the default loader. if (referrerSymbol === vm_dynamic_import_main_context_default) { emitExperimentalWarning('vm.USE_MAIN_CONTEXT_DEFAULT_LOADER'); - return defaultImportModuleDynamically(specifier, attributes, referrerName); + return defaultImportModuleDynamicallyForScript(specifier, attributes, referrerName); } // For script compiled internally that should use the default loader to handle dynamic // import, proxy the request to the default loader without the warning. if (referrerSymbol === vm_dynamic_import_default_internal) { - return defaultImportModuleDynamically(specifier, attributes, referrerName); + return defaultImportModuleDynamicallyForScript(specifier, attributes, referrerName); + } + // For SourceTextModules compiled internally, proxy the request to the default loader. + if (referrerSymbol === source_text_module_default_hdo) { + return defaultImportModuleDynamicallyForModule(specifier, attributes, referrerName); } if (moduleRegistries.has(referrerSymbol)) { @@ -286,6 +323,29 @@ async function initializeHooks() { return hooks; } +/** + * Compile a SourceTextModule for the built-in ESM loader. Register it for default + * source map and import.meta and dynamic import() handling if cascadedLoader is provided. + * @param {string} url URL of the module. + * @param {string} source Source code of the module. + * @param {typeof import('./loader.js').ModuleLoader|undefined} cascadedLoader If provided, + * register the module for default handling. + * @returns {ModuleWrap} + */ +function compileSourceTextModule(url, source, cascadedLoader) { + const hostDefinedOption = cascadedLoader ? source_text_module_default_hdo : undefined; + const wrap = new ModuleWrap(url, undefined, source, 0, 0, hostDefinedOption); + + if (!cascadedLoader) { + return wrap; + } + // Cache the source map for the module if present. + if (wrap.sourceMapURL) { + maybeCacheSourceMap(url, source, null, false, undefined, wrap.sourceMapURL); + } + return wrap; +} + module.exports = { registerModule, initializeESM, @@ -294,4 +354,5 @@ module.exports = { getConditionsSet, loaderWorkerId: 'internal/modules/esm/worker', forceDefaultLoader, + compileSourceTextModule, }; diff --git a/lib/internal/vm/module.js b/lib/internal/vm/module.js index cd4f3a65f8466c..fe504a99f22e69 100644 --- a/lib/internal/vm/module.js +++ b/lib/internal/vm/module.js @@ -141,6 +141,11 @@ class Module { importModuleDynamicallyWrap(options.importModuleDynamically) : undefined, }; + // This will take precedence over the referrer as the object being + // passed into the callbacks. + registry.callbackReferrer = this; + const { registerModule } = require('internal/modules/esm/utils'); + registerModule(this[kWrap], registry); } else { assert(syntheticEvaluationSteps); this[kWrap] = new ModuleWrap(identifier, context, @@ -148,12 +153,6 @@ class Module { syntheticEvaluationSteps); } - // This will take precedence over the referrer as the object being - // passed into the callbacks. - registry.callbackReferrer = this; - const { registerModule } = require('internal/modules/esm/utils'); - registerModule(this[kWrap], registry); - this[kContext] = context; } diff --git a/src/env_properties.h b/src/env_properties.h index 05d8fcf576c51a..9c9c27c3d6ade0 100644 --- a/src/env_properties.h +++ b/src/env_properties.h @@ -49,6 +49,7 @@ V(onpskexchange_symbol, "onpskexchange") \ V(resource_symbol, "resource_symbol") \ V(trigger_async_id_symbol, "trigger_async_id_symbol") \ + V(source_text_module_default_hdo, "source_text_module_default_hdo") \ V(vm_dynamic_import_default_internal, "vm_dynamic_import_default_internal") \ V(vm_dynamic_import_main_context_default, \ "vm_dynamic_import_main_context_default") \ diff --git a/src/module_wrap.cc b/src/module_wrap.cc index 136502d6461355..4201ebfafbe973 100644 --- a/src/module_wrap.cc +++ b/src/module_wrap.cc @@ -142,8 +142,10 @@ v8::Maybe ModuleWrap::CheckUnsettledTopLevelAwait() { return v8::Just(false); } -// new ModuleWrap(url, context, source, lineOffset, columnOffset) -// new ModuleWrap(url, context, exportNames, syntheticExecutionFunction) +// new ModuleWrap(url, context, source, lineOffset, columnOffset, cachedData) +// new ModuleWrap(url, context, source, lineOffset, columOffset, +// hostDefinedOption) new ModuleWrap(url, context, exportNames, +// syntheticExecutionFunction) void ModuleWrap::New(const FunctionCallbackInfo& args) { CHECK(args.IsConstructCall()); CHECK_GE(args.Length(), 3); @@ -172,22 +174,36 @@ void ModuleWrap::New(const FunctionCallbackInfo& args) { int column_offset = 0; bool synthetic = args[2]->IsArray(); + + Local host_defined_options = + PrimitiveArray::New(isolate, HostDefinedOptions::kLength); + Local id_symbol; if (synthetic) { // new ModuleWrap(url, context, exportNames, syntheticExecutionFunction) CHECK(args[3]->IsFunction()); } else { // new ModuleWrap(url, context, source, lineOffset, columOffset, cachedData) + // new ModuleWrap(url, context, source, lineOffset, columOffset, + // hostDefinedOption) CHECK(args[2]->IsString()); CHECK(args[3]->IsNumber()); line_offset = args[3].As()->Value(); CHECK(args[4]->IsNumber()); column_offset = args[4].As()->Value(); - } + if (args[5]->IsSymbol()) { + id_symbol = args[5].As(); + } else { + id_symbol = Symbol::New(isolate, url); + } + host_defined_options->Set(isolate, HostDefinedOptions::kID, id_symbol); - Local host_defined_options = - PrimitiveArray::New(isolate, HostDefinedOptions::kLength); - Local id_symbol = Symbol::New(isolate, url); - host_defined_options->Set(isolate, HostDefinedOptions::kID, id_symbol); + if (that->SetPrivate(context, + realm->isolate_data()->host_defined_option_symbol(), + id_symbol) + .IsNothing()) { + return; + } + } ShouldNotAbortOnUncaughtScope no_abort_scope(realm->env()); TryCatchScope try_catch(realm->env()); @@ -215,8 +231,7 @@ void ModuleWrap::New(const FunctionCallbackInfo& args) { isolate, url, span, SyntheticModuleEvaluationStepsCallback); } else { ScriptCompiler::CachedData* cached_data = nullptr; - if (!args[5]->IsUndefined()) { - CHECK(args[5]->IsArrayBufferView()); + if (args[5]->IsArrayBufferView()) { Local cached_data_buf = args[5].As(); uint8_t* data = static_cast(cached_data_buf->Buffer()->Data()); @@ -279,13 +294,6 @@ void ModuleWrap::New(const FunctionCallbackInfo& args) { return; } - if (that->SetPrivate(context, - realm->isolate_data()->host_defined_option_symbol(), - id_symbol) - .IsNothing()) { - return; - } - // Use the extras object as an object whose GetCreationContext() will be the // original `context`, since the `Context` itself strictly speaking cannot // be stored in an internal field. @@ -878,7 +886,7 @@ void ModuleWrap::HostInitializeImportMetaObjectCallback( return; } DCHECK(id->IsSymbol()); - Local args[] = {id, meta}; + Local args[] = {id, meta, wrap}; TryCatchScope try_catch(env); USE(callback->Call( context, Undefined(realm->isolate()), arraysize(args), args));