Skip to content

Commit b16bbd3

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 b16bbd3

File tree

6 files changed

+183
-135
lines changed

6 files changed

+183
-135
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
const typeLookup = [];
2830
for (const key of ObjectKeys(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
} of sortedOptions) {
139141
if (!helpText) continue;
142+
const value = getOptionValue(name);
140143

141144
let displayName = name;
142145

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

200203
function print(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
const width = MathMax(70, (stream.columns || 0) * 0.75);

lib/internal/options.js

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

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

@@ -11,26 +12,26 @@ const {
1112

1213
let warnOnAllowUnauthorized = true;
1314

14-
let optionsMap;
15-
let aliasesMap;
15+
let optionsDict;
16+
let cliInfo;
1617
let embedderOptions;
1718

18-
// getCLIOptions() would serialize the option values from C++ land.
19+
// getCLIOptionsValues() would serialize the option values from C++ land.
1920
// It would error if the values are queried before bootstrap is
2021
// complete so that we don't accidentally include runtime-dependent
2122
// states into a runtime-independent snapshot.
2223
function getCLIOptionsFromBinding() {
23-
if (!optionsMap) {
24-
({ options: optionsMap } = getCLIOptions());
24+
if (!optionsDict) {
25+
optionsDict = getCLIOptionsValues();
2526
}
26-
return optionsMap;
27+
return optionsDict;
2728
}
2829

29-
function getAliasesFromBinding() {
30-
if (!aliasesMap) {
31-
({ aliases: aliasesMap } = getCLIOptions());
30+
function getCLIOptionsInfoFromBinding() {
31+
if (!cliInfo) {
32+
cliInfo = getCLIOptionsInfo();
3233
}
33-
return aliasesMap;
34+
return cliInfo;
3435
}
3536

3637
function getEmbedderOptions() {
@@ -41,24 +42,12 @@ function getEmbedderOptions() {
4142
}
4243

4344
function refreshOptions() {
44-
optionsMap = undefined;
45-
aliasesMap = undefined;
45+
optionsDict = undefined;
4646
}
4747

4848
function getOptionValue(optionName) {
49-
const options = 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-
const option = options.get('--' + StringPrototypeSlice(optionName, 5));
59-
return option && !option.value;
60-
}
61-
return options.get(optionName)?.value;
49+
const optionsDict = getCLIOptionsFromBinding();
50+
return optionsDict[optionName];
6251
}
6352

6453
function getAllowUnauthorized() {
@@ -76,12 +65,7 @@ function getAllowUnauthorized() {
7665
}
7766

7867
module.exports = {
79-
get options() {
80-
return getCLIOptionsFromBinding();
81-
},
82-
get aliases() {
83-
return getAliasesFromBinding();
84-
},
68+
getCLIOptionsInfo: getCLIOptionsInfoFromBinding,
8569
getOptionValue,
8670
getAllowUnauthorized,
8771
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
const allowedNodeEnvironmentFlags = [];
293294
for (const { 0: name, 1: info } of options) {

0 commit comments

Comments
 (0)