src: reduce unnecessary serialization of CLI options in C++ · nodejs/node@dc41c13 · GitHub
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

6 files changed

Lines changed: 183 additions & 139 deletions

File tree

lib/internal/main/print_help.js

Lines changed: 5 additions & 2 deletions

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-
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

Lines changed: 2 additions & 1 deletion

0 commit comments

Comments
 (0)