From 4dae68ced4a0be4234fc16f2226f0d52bf0a9152 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 Backport-PR-URL: https://github.com/nodejs/node/pull/53500 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 bdcd6028dacb39..d39bd37d801d42 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) { 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 d7867864bba714..150816057129c1 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)) { @@ -288,6 +325,29 @@ async function initializeHooks() { return { __proto__: null, hooks, preloadScripts }; } +/** + * 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, @@ -296,4 +356,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 5db3d9535f3974..74c1870337fc30 100644 --- a/lib/internal/vm/module.js +++ b/lib/internal/vm/module.js @@ -131,6 +131,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, @@ -138,12 +143,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 c7eae579c4ec7d..70f49314eebfa5 100644 --- a/src/env_properties.h +++ b/src/env_properties.h @@ -45,6 +45,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 c2e2aa37ddefc5..2379cdbd27b45b 100644 --- a/src/module_wrap.cc +++ b/src/module_wrap.cc @@ -102,8 +102,10 @@ ModuleWrap* ModuleWrap::GetFromModule(Environment* env, return nullptr; } -// 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); @@ -132,22 +134,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()); @@ -173,8 +189,7 @@ void ModuleWrap::New(const FunctionCallbackInfo& args) { 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()); @@ -237,13 +252,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. @@ -837,7 +845,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));