Skip to content

Commit

Permalink
src: get embedder options on-demand
Browse files Browse the repository at this point in the history
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.

PR-URL: #40357
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Shelley Vohr <[email protected]>
  • Loading branch information
joyeecheung committed Oct 16, 2021
1 parent 6baea14 commit 38aa7cc
Show file tree
Hide file tree
Showing 4 changed files with 48 additions and 30 deletions.
7 changes: 3 additions & 4 deletions lib/internal/bootstrap/pre_execution.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,7 @@ const {

const {
getOptionValue,
noGlobalSearchPaths,
shouldNotRegisterESMLoader,
getEmbedderOptions,
} = require('internal/options');
const { reconnectZeroFillToggle } = require('internal/buffer');

Expand Down Expand Up @@ -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?
Expand All @@ -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,
Expand Down
28 changes: 17 additions & 11 deletions lib/internal/options.js
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -54,13 +61,12 @@ function getAllowUnauthorized() {

module.exports = {
get options() {
return getOptionsFromBinding();
return getCLIOptionsFromBinding();
},
get aliases() {
return getAliasesFromBinding();
},
getOptionValue,
getAllowUnauthorized,
noGlobalSearchPaths,
shouldNotRegisterESMLoader,
getEmbedderOptions
};
41 changes: 27 additions & 14 deletions src/node_options.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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<Value>& args) {
void GetCLIOptions(const FunctionCallbackInfo<Value>& args) {
Mutex::ScopedLock lock(per_process::cli_options_mutex);
Environment* env = Environment::GetCurrent(args);
if (!env->has_run_bootstrapping_code()) {
Expand Down Expand Up @@ -1061,13 +1061,38 @@ void GetOptions(const FunctionCallbackInfo<Value>& args) {
args.GetReturnValue().Set(ret);
}

void GetEmbedderOptions(const FunctionCallbackInfo<Value>& 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> context = env->context();
Local<Object> ret = Object::New(isolate);

if (ret->Set(context,
FIXED_ONE_BYTE_STRING(env->isolate(), "shouldNotRegisterESMLoader"),
Boolean::New(isolate, env->should_not_register_esm_loader()))
.IsNothing()) return;

if (ret->Set(context,
FIXED_ONE_BYTE_STRING(env->isolate(), "noGlobalSearchPaths"),
Boolean::New(isolate, env->no_global_search_paths()))
.IsNothing()) return;

args.GetReturnValue().Set(ret);
}

void Initialize(Local<Object> target,
Local<Value> unused,
Local<Context> 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<Object> env_settings = Object::New(isolate);
NODE_DEFINE_CONSTANT(env_settings, kAllowedInEnvironment);
Expand All @@ -1077,18 +1102,6 @@ void Initialize(Local<Object> 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<Object> types = Object::New(isolate);
NODE_DEFINE_CONSTANT(types, kNoOp);
NODE_DEFINE_CONSTANT(types, kV8Option);
Expand Down
2 changes: 1 addition & 1 deletion src/node_options.h
Original file line number Diff line number Diff line change
Expand Up @@ -462,7 +462,7 @@ class OptionsParser {
template <typename OtherOptions>
friend class OptionsParser;

friend void GetOptions(const v8::FunctionCallbackInfo<v8::Value>& args);
friend void GetCLIOptions(const v8::FunctionCallbackInfo<v8::Value>& args);
friend std::string GetBashCompletion();
};

Expand Down

0 comments on commit 38aa7cc

Please sign in to comment.