Skip to content

Commit a6dd864

Browse files
joyeecheungmarco-ippolito
authored andcommitted
src: reduce unnecessary serialization of CLI options in C++
In this patch we split the serialization routine into two different routines: `getCLIOptionsValues()` for only serializing the key-value pairs and `getCLIOptionsInfo()` for getting additional information such as help text etc. The former is used a lot more frequently than the latter, which is only used for generating `--help` and building `process.allowedNodeEnvironmentFlags`. `getCLIOptionsValues()` also adds `--no-` entries for boolean options so there is no need to special case in the JS land. This patch also refactors the option serialization routines to use v8::Object constructor that takes key-value pairs in one go to avoid calling Map::Set or Object::Set repeatedly which can go up to a patched prototype. PR-URL: #52451 Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Moshe Atlow <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: James M Snell <[email protected]>
1 parent 9b82b15 commit a6dd864

File tree

6 files changed

+183
-139
lines changed

6 files changed

+183
-139
lines changed

‎lib/internal/main/print_help.js‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,8 @@ const{
2424
markBootstrapComplete,
2525
}=require('internal/process/pre_execution');
2626

27+
const{ getCLIOptionsInfo, getOptionValue }=require('internal/options');
28+
2729
consttypeLookup=[];
2830
for(constkeyofObjectKeys(types))
2931
typeLookup[types[key]]=key;
@@ -134,9 +136,10 @@ function format(
134136
);
135137

136138
for(const{
137-
0: name,1: { helpText, type,value,defaultIsTrue },
139+
0: name,1: { helpText, type, defaultIsTrue },
138140
}ofsortedOptions){
139141
if(!helpText)continue;
142+
constvalue=getOptionValue(name);
140143

141144
letdisplayName=name;
142145

@@ -198,7 +201,7 @@ function format(
198201
}
199202

200203
functionprint(stream){
201-
const{ options, aliases }=require('internal/options');
204+
const{ options, aliases }=getCLIOptionsInfo();
202205

203206
// Use 75 % of the available width, and at least 70 characters.
204207
constwidth=MathMax(70,(stream.columns||0)*0.75);

‎lib/internal/options.js‎

Lines changed: 16 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -1,36 +1,33 @@
11
'use strict';
22

33
const{
4-
getCLIOptions,
4+
getCLIOptionsValues,
5+
getCLIOptionsInfo,
56
getEmbedderOptions: getEmbedderOptionsFromBinding,
67
}=internalBinding('options');
78

8-
const{
9-
StringPrototypeSlice,
10-
}=primordials;
11-
129
letwarnOnAllowUnauthorized=true;
1310

14-
letoptionsMap;
15-
letaliasesMap;
11+
letoptionsDict;
12+
letcliInfo;
1613
letembedderOptions;
1714

18-
// getCLIOptions() would serialize the option values from C++ land.
15+
// getCLIOptionsValues() would serialize the option values from C++ land.
1916
// It would error if the values are queried before bootstrap is
2017
// complete so that we don't accidentally include runtime-dependent
2118
// states into a runtime-independent snapshot.
2219
functiongetCLIOptionsFromBinding(){
23-
if(!optionsMap){
24-
({options: optionsMap}=getCLIOptions());
20+
if(!optionsDict){
21+
optionsDict=getCLIOptionsValues();
2522
}
26-
returnoptionsMap;
23+
returnoptionsDict;
2724
}
2825

29-
functiongetAliasesFromBinding(){
30-
if(!aliasesMap){
31-
({aliases: aliasesMap}=getCLIOptions());
26+
functiongetCLIOptionsInfoFromBinding(){
27+
if(!cliInfo){
28+
cliInfo=getCLIOptionsInfo();
3229
}
33-
returnaliasesMap;
30+
returncliInfo;
3431
}
3532

3633
functiongetEmbedderOptions(){
@@ -41,24 +38,12 @@ function getEmbedderOptions(){
4138
}
4239

4340
functionrefreshOptions(){
44-
optionsMap=undefined;
45-
aliasesMap=undefined;
41+
optionsDict=undefined;
4642
}
4743

4844
functiongetOptionValue(optionName){
49-
constoptions=getCLIOptionsFromBinding();
50-
if(
51-
optionName.length>5&&
52-
optionName[0]==='-'&&
53-
optionName[1]==='-'&&
54-
optionName[2]==='n'&&
55-
optionName[3]==='o'&&
56-
optionName[4]==='-'
57-
){
58-
constoption=options.get('--'+StringPrototypeSlice(optionName,5));
59-
returnoption&&!option.value;
60-
}
61-
returnoptions.get(optionName)?.value;
45+
constoptionsDict=getCLIOptionsFromBinding();
46+
returnoptionsDict[optionName];
6247
}
6348

6449
functiongetAllowUnauthorized(){
@@ -76,12 +61,7 @@ function getAllowUnauthorized(){
7661
}
7762

7863
module.exports={
79-
getoptions(){
80-
returngetCLIOptionsFromBinding();
81-
},
82-
getaliases(){
83-
returngetAliasesFromBinding();
84-
},
64+
getCLIOptionsInfo: getCLIOptionsInfoFromBinding,
8565
getOptionValue,
8666
getAllowUnauthorized,
8767
getEmbedderOptions,

‎lib/internal/process/per_thread.js‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -287,7 +287,8 @@ function buildAllowedFlags(){
287287
envSettings: { kAllowedInEnvvar },
288288
types: { kBoolean },
289289
}=internalBinding('options');
290-
const{ options, aliases }=require('internal/options');
290+
const{ getCLIOptionsInfo }=require('internal/options');
291+
const{ options, aliases }=getCLIOptionsInfo();
291292

292293
constallowedNodeEnvironmentFlags=[];
293294
for(const{0: name,1: info}ofoptions){

0 commit comments

Comments
(0)