Skip to content

Commit dc41c13

Browse files
joyeecheungtargos
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 <yagiz.nizipli@sentry.io> Reviewed-By: Moshe Atlow <moshe@atlow.co.il> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com>
1 parent fb24c44 commit dc41c13

File tree

6 files changed

+183
-139
lines changed

6 files changed

+183
-139
lines changed

lib/internal/main/print_help.js

+5-2
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

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

33
const {
4-
StringPrototypeSlice,
5-
} = primordials;
6-
7-
const {
8-
getCLIOptions,
4+
getCLIOptionsValues,
5+
getCLIOptionsInfo,
96
getEmbedderOptions: getEmbedderOptionsFromBinding,
107
} = internalBinding('options');
118

129
let warnOnAllowUnauthorized = true;
1310

14-
let optionsMap;
15-
let aliasesMap;
11+
let optionsDict;
12+
let cliInfo;
1613
let embedderOptions;
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
function getCLIOptionsFromBinding() {
23-
if (!optionsMap) {
24-
({ options: optionsMap } = getCLIOptions());
20+
if (!optionsDict) {
21+
optionsDict = getCLIOptionsValues();
2522
}
26-
return optionsMap;
23+
return optionsDict;
2724
}
2825

29-
function getAliasesFromBinding() {
30-
if (!aliasesMap) {
31-
({ aliases: aliasesMap } = getCLIOptions());
26+
function getCLIOptionsInfoFromBinding() {
27+
if (!cliInfo) {
28+
cliInfo = getCLIOptionsInfo();
3229
}
33-
return aliasesMap;
30+
return cliInfo;
3431
}
3532

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

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

4844
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;
45+
const optionsDict = getCLIOptionsFromBinding();
46+
return optionsDict[optionName];
6247
}
6348

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

7863
module.exports = {
79-
get options() {
80-
return getCLIOptionsFromBinding();
81-
},
82-
get aliases() {
83-
return getAliasesFromBinding();
84-
},
64+
getCLIOptionsInfo: getCLIOptionsInfoFromBinding,
8565
getOptionValue,
8666
getAllowUnauthorized,
8767
getEmbedderOptions,

lib/internal/process/per_thread.js

+2-1
Original file line numberDiff line numberDiff line change
@@ -284,7 +284,8 @@ function buildAllowedFlags() {
284284
envSettings: { kAllowedInEnvvar },
285285
types: { kBoolean },
286286
} = internalBinding('options');
287-
const { options, aliases } = require('internal/options');
287+
const { getCLIOptionsInfo } = require('internal/options');
288+
const { options, aliases } = getCLIOptionsInfo();
288289

289290
const allowedNodeEnvironmentFlags = [];
290291
for (const { 0: name, 1: info } of options) {

0 commit comments

Comments
 (0)