Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

src: enable CPU affinity for Node main thread to avoid context switch #11

Open
wants to merge 1,113 commits into
base: anode-v16.x
Choose a base branch
from

Conversation

lucshi
Copy link

@lucshi lucshi commented Mar 3, 2023

No description provided.

theanarkh and others added 30 commits January 3, 2023 12:08
PR-URL: nodejs/node#44345
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Chengzhong Wu <[email protected]>
PR-URL: nodejs/node#44422
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Mohammed Keyvanzadeh <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
PR-URL: nodejs/node#44511
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Daeyeon Jeong <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Mohammed Keyvanzadeh <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: Feng Yu <[email protected]>
PR-URL: nodejs/node#44419
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Mohammed Keyvanzadeh <[email protected]>
PR-URL: nodejs/node#44370
Reviewed-By: Mohammed Keyvanzadeh <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Moshe Atlow <[email protected]>
PR #43954 introduced the use of f-strings to `test.py` making the script
only work with python versions >= 3.6. Remove that use to make it
compatible again with older python versions.

PR-URL: nodejs/node#44407
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: James M Snell <[email protected]>
PR-URL: nodejs/node#44510
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Daeyeon Jeong <[email protected]>
Reviewed-By: Mohammed Keyvanzadeh <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Feng Yu <[email protected]>
By default, the debugger would query the specified inspector
sever port to see if it's available before starting the server,
and it would keep retrying until a timeout (previously 9999 ms)
is reached. This timeout seems to be longer than necessary. This
patch decreases the timeout to 3 seconds.

PR-URL: nodejs/node#44359
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Remove an unnecessary static_cast<char*>().

Use OPENSSL_secure_zalloc() instead of OPENSSL_secure_malloc() +
memset().

Update the comment describing the function which predates support for
OpenSSL's secure heap.

PR-URL: nodejs/node#44302
Reviewed-By: Filip Skokan <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Avoid magic numbers in the code and use an OpenSSL constant instead.

PR-URL: nodejs/node#44305
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Filip Skokan <[email protected]>
sequential/test-child-process-execsync and
parallel/test-child-process-spawnsync-timeout are both flaky
on azure Windows machines, where it may take longer for Node.js
to launch and receive output from child processes. These tests
work by spawning a child processes that is supposed to sleep
for a long time, but the option is configured so that Node.js
would terminate them early when a shorter timeout is reached.
Then the tests assert that the time taken for the whole thing
is shorter than the specified sleep time (meaning the process
don't actually get to sleep for that long). To make the tests
less brittle on azure Windows, this patch raises the sleep
times in those tests on Windows platform, so that the overhead
can be taken into account there.

PR-URL: nodejs/node#44375
Refs: nodejs/build#3014
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Nitzan Uziely <[email protected]>
This does not fix occurrences in test/parallel/test-dns-* because those
tests contain unrelated pre-existing bugs that would cause the tests to
fail with this fix. This unrelated bug in those tests should be fixed
separately before the use of mustNotCall() can be fixed in those files.

PR-URL: nodejs/node#44022
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Juan José Arboleda <[email protected]>
Reviewed-By: James M Snell <[email protected]>
The original heap prof tests can take too long to complete on
azure Windows machines, resulting in timeouts. Split them into
smaller tests and move them into the parallel directory to
speed up the execution.

PR-URL: nodejs/node#44388
Refs: nodejs/reliability#356
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Previously the tests required that Node.js finish the initialization
of the watchdog thread and fires the timeout within 100ms, which
can be difficult on certain systems under load. This patch
relaxes the requirement to 2000ms. If there is a bug and the
timeout can actually be escaped, raising the timeout to 2000ms
would not make a difference anyway.

PR-URL: nodejs/node#44433
Refs: nodejs/reliability#333
Refs: nodejs/reliability#361
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Chengzhong Wu <[email protected]>
Reviewed-By: Stephen Belanger <[email protected]>
On Windows it might take too long for the parent to start the
communication with a child process, so by the time the parent
starts its own timer, the child process might have already
completed running, and the parent in those tests won't have a
chance to terminate these child processes after the timeout.
To address this issue, raise the time for which the child is
supposed to run to make sure that the parent starts
its own timer before the child terminates in the tests.
Also, split the test into smaller ones to reduce the overhead.

PR-URL: nodejs/node#44390
Refs: nodejs/reliability#356
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Stephen Belanger <[email protected]>
The test previously created two fs.promises.open calls on the
same file with w+ back-to-back, and one of them could fail
when checking the contents of that file if the other happened
to be opening the file for write. Split them into different
tests (with different tmpdir) to avoid the race.

PR-URL: nodejs/node#44380
Refs: nodejs/reliability#354
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Evan Lucas <[email protected]>
md and mgf1_md are internal variable names and should not appear in
JS error messages. Also include the invalid digest name in the error
message.

PR-URL: nodejs/node#44307
Reviewed-By: Filip Skokan <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Mohammed Keyvanzadeh <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
The THROW_ERR_* functions interpret the first argument as a printf-like
format string, which is problematic when it contains unsanitized user
input. This typically happens when a printf-like function is used to
produce the error message, which is then passed to a THROW_ERR_*
function, which again interprets the error message as a format string.

Fix such occurrences by properly formatting error messages using static
format strings only, and in a single step.

PR-URL: nodejs/node#44314
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Zeyu "Alex" Yang <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
symmetric_key_len_ is always equal to symmetric_key_.size(). Storing it
separately is redundant and has no significant benefit.

PR-URL: nodejs/node#44346
Reviewed-By: Zeyu "Alex" Yang <[email protected]>
Reviewed-By: Filip Skokan <[email protected]>
Functions declared in anonymous namespaces are not necessarily to be
marked as static.

PR-URL: nodejs/node#44301
Refs: nodejs/node#44141
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
There is no need to explicitly branch based on num_curves or on the
return value of the second call to EC_get_builtin_curves. Remove
unnecessary branches and replace the loop with a functional transform.

PR-URL: nodejs/node#44309
Reviewed-By: Filip Skokan <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
This will allow distribution packages to select an alternative
location for the unofficial libnode.so. For example, on Fedora it
will install into /usr/lib64 on 64-bit systems.

Signed-off-by: Stephen Gallagher <[email protected]>
PR-URL: nodejs/node#44361
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
The previous implementation was typically compiled to a fair amount of
code even though all inputs are available at compile time.

The fact that GetOpenSSLVersion() returns a std::string and used an
uninitialized buffer with snprintf made it impossible to make
GetOpenSSLVersion() a constexpr, and compilers would typically emit code
to dynamically construct the resulting string.

The simplified implementation usually boils down to a few mov
instructions.

(Ideally, this function could be a constexpr returning a
std::string_view, but that does not have any advantage in the current
design of node::Metadata::Versions which stores versions as
std::string instances.)

Also make the function static since it is not in an anonymous namespace
and change the argument types and the return type of search() to
types that are more appropriate, semantically. (The use of snprintf
previously made this difficult.) Lastly, make the n argument of search()
optional because the simplified implementation always sets it to 0
except during recursive calls within search() itself.

PR-URL: nodejs/node#44395
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: Shelley Vohr <[email protected]>
Reviewed-By: Minwoo Jung <[email protected]>
The "node_http2.h" include reordering enforced by clang-format broke
Electron's Node.js upgrade on Windows. ssize_t is a part of the POSIX
standard and it's not available on Windows, so the fix for this is to
include "node.h" which typedefs it on Windows in
https://github.com/nodejs/node/blob/bb4dff783ddb3b20c67041f7ccef796c335c2407/src/node.h#L212-L220.

Refs: electron/electron#35350 (comment)
Signed-off-by: Darshan Sen <[email protected]>
PR-URL: nodejs/node#44393
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Shelley Vohr <[email protected]>
Reviewed-By: Rafael Gonzaga <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
PR-URL: nodejs/node#44411
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
PR-URL: nodejs/node#44421
Refs: nodejs/node#44401
Reviewed-By: Chengzhong Wu <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Juan José Arboleda <[email protected]>
mhdawson and others added 28 commits January 3, 2023 12:10
Refs: electron/electron#35801
Refs: nodejs/abi-stable-node#441

Electron recently dropped support for external
buffers. Provide a way for addon authors to:
- hide the methods to create external buffers so they can
  avoid using them if they want the broadest compatibility.
- call the methods that create external buffers at runtime
  to check if external buffers are supported and either
  use them or not based on the return code.

Signed-off-by: Michael Dawson <[email protected]>

PR-URL: nodejs/node#45181
Reviewed-By: Chengzhong Wu <[email protected]>
Reviewed-By: Minwoo Jung <[email protected]>
PR-URL: nodejs/node#45191
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Christian Clauss <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: Juan José Arboleda <[email protected]>
PR-URL: nodejs/node#45191
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Christian Clauss <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: Juan José Arboleda <[email protected]>
Users can set a default
value for every expected
input argument

PR-URL: nodejs/node#44631
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Refs: nodejs/node#44631
PR-URL: nodejs/node#45083
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Zeyu "Alex" Yang <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Daijiro Wachi <[email protected]>
PR-URL: nodejs/node#44149
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Refs: nodejs/node#44950 (comment)
Signed-off-by: Darshan Sen <[email protected]>
PR-URL: nodejs/node#45021
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Tierney Cyren <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Update gr2m/create-or-update-pull-request-action to version 1.9.1.

PR-URL: nodejs/node#45022
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: Tierney Cyren <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Refs: nodejs/node#45022 (comment)
PR-URL: nodejs/node#45166
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Jiawen Geng <[email protected]>
Currently, there's no way to know if a timezone upgrade PR is correct
without building and testing the change locally. This change provides a
solution for that.

Tested in RaisinTen/node#4.

Signed-off-by: Darshan Sen <[email protected]>
PR-URL: nodejs/node#45299
Reviewed-By: Antoine du Hamel <[email protected]>
The spawnSync call was previously silently failing with this error:
```sh
icupkg: unable to open input file "icudt*.dat"
```
because spawnSync doesn't support globbing. This change replaces the
spawnSync call with execSync because that supports globbing.

I have tested this workflow with some minor modifications in my fork and
I can confirm that it works as expected now. The bot opened this PR -
RaisinTen/node#2 which updates
deps/icu-small/source/data/in/icudt71l.dat.bz2.

Fixes: nodejs/node#44865
Signed-off-by: Darshan Sen <[email protected]>
PR-URL: nodejs/node#44870
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
We do not build Node.js in the workflow so
https://github.com/nodejs/node/blob/f4815fcd7691364d8139b44c1295dbc46f6ee4a8/tools/update-timezone.mjs#L18
is actually the version of `tzdata` in the Node.js in the runner instead
of what's in `main`.

The script is pretty fast even when the versions differ and there is an
update, so this optimization doesn't seem to be worth having given the
problem.

Signed-off-by: Darshan Sen <[email protected]>
PR-URL: nodejs/node#44870
Fixes: nodejs/node#44865
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
PR-URL: nodejs/node#45613
Refs: nodejs/node#45289
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
PR-URL: nodejs/node#44241
Backport-PR-URL: nodejs/node#44873
Fixes: nodejs/node#44023
Fixes: nodejs/node#43675
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
PR-URL: nodejs/node#44450
Backport-PR-URL: nodejs/node#44873
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Franziska Hinkelmann <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Minwoo Jung <[email protected]>
PR-URL: nodejs/node#44520
Backport-PR-URL: nodejs/node#44873
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
PR-URL: nodejs/node#44620
Backport-PR-URL: nodejs/node#44873
Fixes: nodejs/node#44600
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
PR-URL: nodejs/node#44592
Backport-PR-URL: nodejs/node#44873
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
This was confirmed flaky on those platforms:
- Windows x64
- Windows on Arm
- Linux
- Freebsd

Tests randomly fail because of bad order in messages expected, which
seems related to threads scheduling at execution.

PR-URL: nodejs/node#45049
Backport-PR-URL: nodejs/node#44873
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
This is the certdata.txt[0] from NSS 3.85, released on 2022-11-10.

This is the version of NSS that will ship in Firefox 108 on
2022-12-13.

[0] https://hg.mozilla.org/projects/nss/raw-file/NSS_3_85_RTM/lib/ckfw/builtins/certdata.txt

PR-URL: nodejs/node#45490
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
Update the list of root certificates in src/node_root_certs.h with
tools/mk-ca-bundle.pl.

Certificates added:
- Autoridad de Certificacion Firmaprofesional CIF A62634068
- Certainly Root E1
- Certainly Root R1
- D-TRUST BR Root CA 1 2020
- D-TRUST EV Root CA 1 2020
- DigiCert TLS ECC P384 Root G5
- DigiCert TLS RSA4096 Root G5
- E-Tugra Global Root CA ECC v3
- E-Tugra Global Root CA RSA v3
- HiPKI Root CA - G1
- ISRG Root X2
- Security Communication ECC RootCA1
- Security Communication RootCA3
- Telia Root CA v2
- vTrus ECC Root CA
- vTrus Root CA

Certificates removed:
- Cybertrust Global Root
- DST Root CA X3
- GlobalSign Root CA - R2
- Hellenic Academic and Research Institutions RootCA 2011

PR-URL: nodejs/node#45490
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
Add a new test to check that the changelog files have been correctly
updated for releases.

PR-URL: nodejs/node#45325
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Rafael Gonzaga <[email protected]>
After the stream has been marked as closed by the nghttp2 stack, there
might be still pending data to be sent: trailing headers is an example
of this. In that case, avoid reentering the nghttp2 stack synchronously
to allow the data to be written before actually closing the stream.

Fixes: nodejs/node#42713
PR-URL: nodejs/node#45153
Backport-PR-URL: nodejs/node#45660
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Paolo Insogna <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Rafael Gonzaga <[email protected]>
The term "native module" dates back to some of the oldest code
in the code base. Within the context of Node.js core it usually
refers to modules that are native to Node.js (e.g. fs, http),
but it can cause confusion for people who don't work on this
part of the code base, as "native module" can also refer to
native addons - which is even the case in some of the API
docs and error messages.

This patch tries to make the usage of these terms more consistent.
Now within the context of Node.js core:

- JavaScript scripts that are built-in to Node.js are now referred
  to as "built-in(s)". If they are available as modules,
  they can also be referred to as "built-in module(s)".
- Dynamically-linked shared objects that are loaded into
  the Node.js processes are referred to as "addons".

We will try to avoid using the term "native modules" because it could
be ambiguous.

Changes in this patch:

File names:
- node_native_module.h -> node_builtins.h,
- node_native_module.cc -> node_builtins.cc

C++ binding names:
- `native_module` -> `builtins`

`node::Environment`:
- `native_modules_without_cache` -> `builtins_without_cache`
- `native_modules_with_cache` -> `builtins_with_cache`
- `native_modules_in_snapshot` -> `builtins_in_cache`
- `native_module_require` -> `builtin_module_require`

`node::EnvSerializeInfo`:
- `native_modules` -> `builtins

`node::native_module::NativeModuleLoader`:
- `native_module` namespace -> `builtins` namespace
- `NativeModuleLoader` -> `BuiltinLoader`
- `NativeModuleRecordMap` -> `BuiltinSourceMap`
- `NativeModuleCacheMap` -> `BuiltinCodeCacheMap`
- `ModuleIds` -> `BuiltinIds`
- `ModuleCategories` -> `BuiltinCategories`
- `LoadBuiltinModuleSource` -> `LoadBuiltinSource`

`loader.js`:
- `NativeModule` -> `BuiltinModule` (the `NativeModule` name used in
  `process.moduleLoadList` is kept for compatibility)

And other clarifications in the documentation and comments.

PR-URL: nodejs/node#44135
Backport-PR-URL: nodejs/node#45663
Fixes: nodejs/node#44036
Reviewed-By: Jacob Smith <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: Chengzhong Wu <[email protected]>
Reviewed-By: Mohammed Keyvanzadeh <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Jan Krems <[email protected]>
Notable changes:

- OpenSSL 1.1.1s
- Root certificates updated to NSS 3.85
- Time zone update to 2022f
- add dgram send queue info
- upgrade npm to 8.19.3
- add `--watch`
- add default value option to parsearg

PR-URL: nodejs/node#45791
@oraluben oraluben force-pushed the anode-v16.x branch 2 times, most recently from e62fe8a to 0ad783a Compare August 16, 2023 07:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.