Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Commit dc41c13

Browse filesBrowse 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
Copy full SHA for dc41c13

File tree

Expand file treeCollapse file tree

6 files changed

+183
-139
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

6 files changed

+183
-139
lines changed
Open diff view settings
Collapse file

‎lib/internal/main/print_help.js‎

Copy file name to clipboardExpand all lines: lib/internal/main/print_help.js
+5-2Lines 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);
Collapse file

‎lib/internal/options.js‎

Copy file name to clipboardExpand all lines: lib/internal/options.js
+16-36Lines 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,
Collapse file

‎lib/internal/process/per_thread.js‎

Copy file name to clipboardExpand all lines: lib/internal/process/per_thread.js
+2-1Lines changed: 2 additions & 1 deletion
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 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.