From dc394bcdaff95286488e3de00cd52b463f5ea48e Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Thu, 7 Oct 2021 12:03:10 +0800 Subject: [PATCH 1/2] src: get embedder options on-demand Only query embedder options when they are needed so that the bootstrap remains as stateless as possible so that the bootstrap snapshot is controlled solely by configure options, and subsequent runtime changes should be done in pre-execution. --- lib/internal/bootstrap/pre_execution.js | 7 ++--- lib/internal/options.js | 28 ++++++++++------- src/node_options.cc | 41 ++++++++++++++++--------- src/node_options.h | 2 +- 4 files changed, 48 insertions(+), 30 deletions(-) diff --git a/lib/internal/bootstrap/pre_execution.js b/lib/internal/bootstrap/pre_execution.js index a4c73cba5481b3..f2a10641906e31 100644 --- a/lib/internal/bootstrap/pre_execution.js +++ b/lib/internal/bootstrap/pre_execution.js @@ -11,8 +11,7 @@ const { const { getOptionValue, - noGlobalSearchPaths, - shouldNotRegisterESMLoader, + getEmbedderOptions, } = require('internal/options'); const { reconnectZeroFillToggle } = require('internal/buffer'); @@ -421,7 +420,7 @@ function initializeWASI() { function initializeCJSLoader() { const CJSLoader = require('internal/modules/cjs/loader'); - if (!noGlobalSearchPaths) { + if (!getEmbedderOptions().noGlobalSearchPaths) { CJSLoader.Module._initPaths(); } // TODO(joyeecheung): deprecate this in favor of a proper hook? @@ -433,7 +432,7 @@ function initializeESMLoader() { // Create this WeakMap in js-land because V8 has no C++ API for WeakMap. internalBinding('module_wrap').callbackMap = new SafeWeakMap(); - if (shouldNotRegisterESMLoader) return; + if (getEmbedderOptions().shouldNotRegisterESMLoader) return; const { setImportModuleDynamicallyCallback, diff --git a/lib/internal/options.js b/lib/internal/options.js index cb694c7dfdd576..01b334d4ec5614 100644 --- a/lib/internal/options.js +++ b/lib/internal/options.js @@ -1,36 +1,43 @@ 'use strict'; const { - getOptions, - noGlobalSearchPaths, - shouldNotRegisterESMLoader, + getCLIOptions, + getEmbedderOptions: getEmbedderOptionsFromBinding, } = internalBinding('options'); let warnOnAllowUnauthorized = true; let optionsMap; let aliasesMap; +let embedderOptions; -// getOptions() would serialize the option values from C++ land. +// getCLIOptions() would serialize the option values from C++ land. // It would error if the values are queried before bootstrap is // complete so that we don't accidentally include runtime-dependent // states into a runtime-independent snapshot. -function getOptionsFromBinding() { +function getCLIOptionsFromBinding() { if (!optionsMap) { - ({ options: optionsMap } = getOptions()); + ({ options: optionsMap } = getCLIOptions()); } return optionsMap; } function getAliasesFromBinding() { if (!aliasesMap) { - ({ aliases: aliasesMap } = getOptions()); + ({ aliases: aliasesMap } = getCLIOptions()); } return aliasesMap; } +function getEmbedderOptions() { + if (!embedderOptions) { + embedderOptions = getEmbedderOptionsFromBinding(); + } + return embedderOptions; +} + function getOptionValue(optionName) { - const options = getOptionsFromBinding(); + const options = getCLIOptionsFromBinding(); if (optionName.startsWith('--no-')) { const option = options.get('--' + optionName.slice(5)); return option && !option.value; @@ -54,13 +61,12 @@ function getAllowUnauthorized() { module.exports = { get options() { - return getOptionsFromBinding(); + return getCLIOptionsFromBinding(); }, get aliases() { return getAliasesFromBinding(); }, getOptionValue, getAllowUnauthorized, - noGlobalSearchPaths, - shouldNotRegisterESMLoader, + getEmbedderOptions }; diff --git a/src/node_options.cc b/src/node_options.cc index c778de58dc8fa8..eac774969f2697 100644 --- a/src/node_options.cc +++ b/src/node_options.cc @@ -920,7 +920,7 @@ std::string GetBashCompletion() { // Return a map containing all the options and their metadata as well // as the aliases -void GetOptions(const FunctionCallbackInfo& args) { +void GetCLIOptions(const FunctionCallbackInfo& args) { Mutex::ScopedLock lock(per_process::cli_options_mutex); Environment* env = Environment::GetCurrent(args); if (!env->has_run_bootstrapping_code()) { @@ -1061,13 +1061,38 @@ void GetOptions(const FunctionCallbackInfo& args) { args.GetReturnValue().Set(ret); } +void GetEmbedderOptions(const FunctionCallbackInfo& args) { + Environment* env = Environment::GetCurrent(args); + if (!env->has_run_bootstrapping_code()) { + // No code because this is an assertion. + return env->ThrowError( + "Should not query options before bootstrapping is done"); + } + Isolate* isolate = args.GetIsolate(); + Local context = env->context(); + Local ret = Object::New(isolate); + + ret->Set(context, + FIXED_ONE_BYTE_STRING(env->isolate(), "shouldNotRegisterESMLoader"), + Boolean::New(isolate, env->should_not_register_esm_loader())) + .Check(); + + ret->Set(context, + FIXED_ONE_BYTE_STRING(env->isolate(), "noGlobalSearchPaths"), + Boolean::New(isolate, env->no_global_search_paths())) + .Check(); + + args.GetReturnValue().Set(ret); +} + void Initialize(Local target, Local unused, Local context, void* priv) { Environment* env = Environment::GetCurrent(context); Isolate* isolate = env->isolate(); - env->SetMethodNoSideEffect(target, "getOptions", GetOptions); + env->SetMethodNoSideEffect(target, "getCLIOptions", GetCLIOptions); + env->SetMethodNoSideEffect(target, "getEmbedderOptions", GetEmbedderOptions); Local env_settings = Object::New(isolate); NODE_DEFINE_CONSTANT(env_settings, kAllowedInEnvironment); @@ -1077,18 +1102,6 @@ void Initialize(Local target, context, FIXED_ONE_BYTE_STRING(isolate, "envSettings"), env_settings) .Check(); - target - ->Set(context, - FIXED_ONE_BYTE_STRING(env->isolate(), "shouldNotRegisterESMLoader"), - Boolean::New(isolate, env->should_not_register_esm_loader())) - .Check(); - - target - ->Set(context, - FIXED_ONE_BYTE_STRING(env->isolate(), "noGlobalSearchPaths"), - Boolean::New(isolate, env->no_global_search_paths())) - .Check(); - Local types = Object::New(isolate); NODE_DEFINE_CONSTANT(types, kNoOp); NODE_DEFINE_CONSTANT(types, kV8Option); diff --git a/src/node_options.h b/src/node_options.h index 02511d520802fc..4ef6921a3a1517 100644 --- a/src/node_options.h +++ b/src/node_options.h @@ -462,7 +462,7 @@ class OptionsParser { template friend class OptionsParser; - friend void GetOptions(const v8::FunctionCallbackInfo& args); + friend void GetCLIOptions(const v8::FunctionCallbackInfo& args); friend std::string GetBashCompletion(); }; From c9504e1dc51f814f7eb487ce0ab21e431fbfb832 Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Mon, 11 Oct 2021 10:28:17 +0800 Subject: [PATCH 2/2] Update src/node_options.cc Co-authored-by: Anna Henningsen --- src/node_options.cc | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/node_options.cc b/src/node_options.cc index eac774969f2697..00bdc6688a4c4f 100644 --- a/src/node_options.cc +++ b/src/node_options.cc @@ -1072,15 +1072,15 @@ void GetEmbedderOptions(const FunctionCallbackInfo& args) { Local context = env->context(); Local ret = Object::New(isolate); - ret->Set(context, + if (ret->Set(context, FIXED_ONE_BYTE_STRING(env->isolate(), "shouldNotRegisterESMLoader"), Boolean::New(isolate, env->should_not_register_esm_loader())) - .Check(); + .IsNothing()) return; - ret->Set(context, + if (ret->Set(context, FIXED_ONE_BYTE_STRING(env->isolate(), "noGlobalSearchPaths"), Boolean::New(isolate, env->no_global_search_paths())) - .Check(); + .IsNothing()) return; args.GetReturnValue().Set(ret); }