nodejs
src: use v8::Isolate::GetDefaultLocale() to compute navigator.language
src: use v8::Isolate::GetDefaultLocale() to compute navigator.language
Using the Intl API to get the default locale slows down the startup significantly. This patch uses a new v8 API to get the default locale directly.
Refs: https://github.com/nodejs/node/pull/53765#issuecomment-2276802345
First commit comes from https://chromium-review.googlesource.com/c/v8/v8/+/5772938
From benchmark CI:
https://ci.nodejs.org/view/Node.js%20benchmark/job/benchmark-node-micro-benchmarks/1594/console
14:54:15 confidence improvement accuracy (*) (**) (***)
14:54:15 misc/startup-core.js count=30 mode='process' script='benchmark/fixtures/require-builtins' *** 18.60 % ±0.30% ±0.41% ±0.54%
14:54:15 misc/startup-core.js count=30 mode='process' script='test/fixtures/semicolon' *** 25.02 % ±2.92% ±3.93% ±5.19%
14:54:15 misc/startup-core.js count=30 mode='process' script='test/fixtures/snapshot/typescript' *** 4.32 % ±0.04% ±0.06% ±0.07%
14:54:15 misc/startup-core.js count=30 mode='worker' script='benchmark/fixtures/require-builtins' -0.05 % ±0.18% ±0.24% ±0.31%
14:54:15 misc/startup-core.js count=30 mode='worker' script='test/fixtures/semicolon' 0.02 % ±0.36% ±0.47% ±0.62%
14:54:15 misc/startup-core.js count=30 mode='worker' script='test/fixtures/snapshot/typescript' ** -1.48 % ±0.87% ±1.16%
Review requested:
- [ ] @nodejs/security-wg
- [ ] @nodejs/v8-update
- [ ] @nodejs/web-standards
From benchmark CI:
https://ci.nodejs.org/view/Node.js%20benchmark/job/benchmark-node-micro-benchmarks/1594/console
14:54:15 confidence improvement accuracy (*) (**) (***)
14:54:15 misc/startup-core.js count=30 mode='process' script='benchmark/fixtures/require-builtins' *** 18.60 % ±0.30% ±0.41% ±0.54%
14:54:15 misc/startup-core.js count=30 mode='process' script='test/fixtures/semicolon' *** 25.02 % ±2.92% ±3.93% ±5.19%
14:54:15 misc/startup-core.js count=30 mode='process' script='test/fixtures/snapshot/typescript' *** 4.32 % ±0.04% ±0.06% ±0.07%
14:54:15 misc/startup-core.js count=30 mode='worker' script='benchmark/fixtures/require-builtins' -0.05 % ±0.18% ±0.24% ±0.31%
14:54:15 misc/startup-core.js count=30 mode='worker' script='test/fixtures/semicolon' 0.02 % ±0.36% ±0.47% ±0.62%
14:54:15 misc/startup-core.js count=30 mode='worker' script='test/fixtures/snapshot/typescript' ** -1.48 % ±0.87% ±1.16%
Upstream CL has landed, backported and this should be ready for review now.
cc @nodejs/startup
V8 CI: https://ci.nodejs.org/job/node-test-commit-v8-linux/nodes=benchmark-ubuntu2204-intel-64,v8test=v8test/6142/
V8 CI: https://ci.nodejs.org/job/node-test-commit-v8-linux/nodes=rhel8-s390x,v8test=v8test/6142/
V8 CI: https://ci.nodejs.org/job/node-test-commit-v8-linux/nodes=rhel8-ppc64le,v8test=v8test/6142/
V8 CI: https://ci.nodejs.org/job/node-test-commit-v8-linux/nodes=benchmark-ubuntu2204-intel-64,v8test=v8test/6143/
V8 CI: https://ci.nodejs.org/job/node-test-commit-v8-linux/nodes=rhel8-s390x,v8test=v8test/6143/
V8 CI: https://ci.nodejs.org/job/node-test-commit-v8-linux/nodes=rhel8-ppc64le,v8test=v8test/6143/
Codecov Report
Attention: Patch coverage is 95.00000% with 1 line in your changes missing coverage. Please review.
Project coverage is 87.31%. Comparing base (
2c14615) to head (c330418). Report is 403 commits behind head on main.
| Files with missing lines | Patch % | Lines |
|---|---|---|
| src/node_config.cc | 93.75% | 0 Missing and 1 partial :warning: |
Additional details and impacted files
@@ Coverage Diff @@
## main #54279 +/- ##
==========================================
+ Coverage 87.08% 87.31% +0.23%
==========================================
Files 648 648
Lines 182341 182376 +35
Branches 34982 34982
==========================================
+ Hits 158783 159240 +457
+ Misses 16831 16401 -430
- Partials 6727 6735 +8
| Files with missing lines | Coverage Δ | |
|---|---|---|
| lib/internal/navigator.js | 99.32% <100.00%> (+<0.01%) |
:arrow_up: |
| lib/internal/process/pre_execution.js | 88.46% <ø> (-2.46%) |
:arrow_down: |
| src/node_external_reference.h | 100.00% <ø> (ø) |
|
| src/node_config.cc | 96.42% <93.75%> (-3.58%) |
:arrow_down: |
![codecov[bot] avatar](https://avatars.githubusercontent.com/in/254?v=4 "Published by a pub.dev verified publisher"){kind=small}
V8 CI: https://ci.nodejs.org/job/node-test-commit-v8-linux/nodes=benchmark-ubuntu2204-intel-64,v8test=v8test/6146/
V8 CI: https://ci.nodejs.org/job/node-test-commit-v8-linux/nodes=rhel8-s390x,v8test=v8test/6146/
V8 CI: https://ci.nodejs.org/job/node-test-commit-v8-linux/nodes=rhel8-ppc64le,v8test=v8test/6146/
V8 CI: https://ci.nodejs.org/job/node-test-commit-v8-linux/nodes=benchmark-ubuntu2204-intel-64,v8test=v8test/6148/
V8 CI: https://ci.nodejs.org/job/node-test-commit-v8-linux/nodes=rhel8-s390x,v8test=v8test/6148/
V8 CI: https://ci.nodejs.org/job/node-test-commit-v8-linux/nodes=rhel8-ppc64le,v8test=v8test/6148/
This has been failing on something that should've been fixed by https://github.com/nodejs/node/pull/54556
V8 CI: https://ci.nodejs.org/job/node-test-commit-v8-linux/nodes=benchmark-ubuntu2204-intel-64,v8test=v8test/6155/
V8 CI: https://ci.nodejs.org/job/node-test-commit-v8-linux/nodes=rhel8-s390x,v8test=v8test/6155/
V8 CI: https://ci.nodejs.org/job/node-test-commit-v8-linux/nodes=rhel8-ppc64le,v8test=v8test/6155/