Skip to content

Commit c15afda

Browse files
joyeecheungtargos
authored andcommitted
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. PR-URL: #40357 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Shelley Vohr <[email protected]>
1 parent bfdd32f commit c15afda

File tree

4 files changed

+48
-30
lines changed

4 files changed

+48
-30
lines changed

lib/internal/bootstrap/pre_execution.js

+3-4
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,7 @@ const {
1111

1212
const {
1313
getOptionValue,
14-
noGlobalSearchPaths,
15-
shouldNotRegisterESMLoader,
14+
getEmbedderOptions,
1615
} = require('internal/options');
1716
const { reconnectZeroFillToggle } = require('internal/buffer');
1817

@@ -421,7 +420,7 @@ function initializeWASI() {
421420

422421
function initializeCJSLoader() {
423422
const CJSLoader = require('internal/modules/cjs/loader');
424-
if (!noGlobalSearchPaths) {
423+
if (!getEmbedderOptions().noGlobalSearchPaths) {
425424
CJSLoader.Module._initPaths();
426425
}
427426
// TODO(joyeecheung): deprecate this in favor of a proper hook?
@@ -433,7 +432,7 @@ function initializeESMLoader() {
433432
// Create this WeakMap in js-land because V8 has no C++ API for WeakMap.
434433
internalBinding('module_wrap').callbackMap = new SafeWeakMap();
435434

436-
if (shouldNotRegisterESMLoader) return;
435+
if (getEmbedderOptions().shouldNotRegisterESMLoader) return;
437436

438437
const {
439438
setImportModuleDynamicallyCallback,

lib/internal/options.js

+17-11
Original file line numberDiff line numberDiff line change
@@ -1,36 +1,43 @@
11
'use strict';
22

33
const {
4-
getOptions,
5-
noGlobalSearchPaths,
6-
shouldNotRegisterESMLoader,
4+
getCLIOptions,
5+
getEmbedderOptions: getEmbedderOptionsFromBinding,
76
} = internalBinding('options');
87

98
let warnOnAllowUnauthorized = true;
109

1110
let optionsMap;
1211
let aliasesMap;
12+
let embedderOptions;
1313

14-
// getOptions() would serialize the option values from C++ land.
14+
// getCLIOptions() would serialize the option values from C++ land.
1515
// It would error if the values are queried before bootstrap is
1616
// complete so that we don't accidentally include runtime-dependent
1717
// states into a runtime-independent snapshot.
18-
function getOptionsFromBinding() {
18+
function getCLIOptionsFromBinding() {
1919
if (!optionsMap) {
20-
({ options: optionsMap } = getOptions());
20+
({ options: optionsMap } = getCLIOptions());
2121
}
2222
return optionsMap;
2323
}
2424

2525
function getAliasesFromBinding() {
2626
if (!aliasesMap) {
27-
({ aliases: aliasesMap } = getOptions());
27+
({ aliases: aliasesMap } = getCLIOptions());
2828
}
2929
return aliasesMap;
3030
}
3131

32+
function getEmbedderOptions() {
33+
if (!embedderOptions) {
34+
embedderOptions = getEmbedderOptionsFromBinding();
35+
}
36+
return embedderOptions;
37+
}
38+
3239
function getOptionValue(optionName) {
33-
const options = getOptionsFromBinding();
40+
const options = getCLIOptionsFromBinding();
3441
if (optionName.startsWith('--no-')) {
3542
const option = options.get('--' + optionName.slice(5));
3643
return option && !option.value;
@@ -54,13 +61,12 @@ function getAllowUnauthorized() {
5461

5562
module.exports = {
5663
get options() {
57-
return getOptionsFromBinding();
64+
return getCLIOptionsFromBinding();
5865
},
5966
get aliases() {
6067
return getAliasesFromBinding();
6168
},
6269
getOptionValue,
6370
getAllowUnauthorized,
64-
noGlobalSearchPaths,
65-
shouldNotRegisterESMLoader,
71+
getEmbedderOptions
6672
};

src/node_options.cc

+27-14
Original file line numberDiff line numberDiff line change
@@ -915,7 +915,7 @@ std::string GetBashCompletion() {
915915

916916
// Return a map containing all the options and their metadata as well
917917
// as the aliases
918-
void GetOptions(const FunctionCallbackInfo<Value>& args) {
918+
void GetCLIOptions(const FunctionCallbackInfo<Value>& args) {
919919
Mutex::ScopedLock lock(per_process::cli_options_mutex);
920920
Environment* env = Environment::GetCurrent(args);
921921
if (!env->has_run_bootstrapping_code()) {
@@ -1056,13 +1056,38 @@ void GetOptions(const FunctionCallbackInfo<Value>& args) {
10561056
args.GetReturnValue().Set(ret);
10571057
}
10581058

1059+
void GetEmbedderOptions(const FunctionCallbackInfo<Value>& args) {
1060+
Environment* env = Environment::GetCurrent(args);
1061+
if (!env->has_run_bootstrapping_code()) {
1062+
// No code because this is an assertion.
1063+
return env->ThrowError(
1064+
"Should not query options before bootstrapping is done");
1065+
}
1066+
Isolate* isolate = args.GetIsolate();
1067+
Local<Context> context = env->context();
1068+
Local<Object> ret = Object::New(isolate);
1069+
1070+
if (ret->Set(context,
1071+
FIXED_ONE_BYTE_STRING(env->isolate(), "shouldNotRegisterESMLoader"),
1072+
Boolean::New(isolate, env->should_not_register_esm_loader()))
1073+
.IsNothing()) return;
1074+
1075+
if (ret->Set(context,
1076+
FIXED_ONE_BYTE_STRING(env->isolate(), "noGlobalSearchPaths"),
1077+
Boolean::New(isolate, env->no_global_search_paths()))
1078+
.IsNothing()) return;
1079+
1080+
args.GetReturnValue().Set(ret);
1081+
}
1082+
10591083
void Initialize(Local<Object> target,
10601084
Local<Value> unused,
10611085
Local<Context> context,
10621086
void* priv) {
10631087
Environment* env = Environment::GetCurrent(context);
10641088
Isolate* isolate = env->isolate();
1065-
env->SetMethodNoSideEffect(target, "getOptions", GetOptions);
1089+
env->SetMethodNoSideEffect(target, "getCLIOptions", GetCLIOptions);
1090+
env->SetMethodNoSideEffect(target, "getEmbedderOptions", GetEmbedderOptions);
10661091

10671092
Local<Object> env_settings = Object::New(isolate);
10681093
NODE_DEFINE_CONSTANT(env_settings, kAllowedInEnvironment);
@@ -1072,18 +1097,6 @@ void Initialize(Local<Object> target,
10721097
context, FIXED_ONE_BYTE_STRING(isolate, "envSettings"), env_settings)
10731098
.Check();
10741099

1075-
target
1076-
->Set(context,
1077-
FIXED_ONE_BYTE_STRING(env->isolate(), "shouldNotRegisterESMLoader"),
1078-
Boolean::New(isolate, env->should_not_register_esm_loader()))
1079-
.Check();
1080-
1081-
target
1082-
->Set(context,
1083-
FIXED_ONE_BYTE_STRING(env->isolate(), "noGlobalSearchPaths"),
1084-
Boolean::New(isolate, env->no_global_search_paths()))
1085-
.Check();
1086-
10871100
Local<Object> types = Object::New(isolate);
10881101
NODE_DEFINE_CONSTANT(types, kNoOp);
10891102
NODE_DEFINE_CONSTANT(types, kV8Option);

src/node_options.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -461,7 +461,7 @@ class OptionsParser {
461461
template <typename OtherOptions>
462462
friend class OptionsParser;
463463

464-
friend void GetOptions(const v8::FunctionCallbackInfo<v8::Value>& args);
464+
friend void GetCLIOptions(const v8::FunctionCallbackInfo<v8::Value>& args);
465465
friend std::string GetBashCompletion();
466466
};
467467

0 commit comments

Comments
 (0)