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: add option to set cpu affinity of mainthread #44459

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

qdaoming
Copy link
Contributor

Add runtime option '--set-mainthread-cpu-affinity' to pin the
mainthread to the current running cpu before entering event loop.
This feature is implemented on Linux OS currently.

Enabling this option will benefit node.js applications running on
heavy-workload system and can increase performance for several
node.js benchmarks, ex., several cases in benchmark/vm/ improved
by 40%.

Add runtime option '--set-mainthread-cpu-affinity' to pin the
mainthread to the current running cpu before entering event loop.
This feature is implemented on Linux OS currently.

Enabling this option will benefit node.js applications running on
heavy-workload system and can increase performance for several
node.js benchmarks, ex., several cases in benchmark/vm/ improved
by 40%.
@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run. process Issues and PRs related to the process subsystem. labels Aug 30, 2022
@mscdex
Copy link
Contributor

mscdex commented Aug 30, 2022

I feel like something like this should be implemented by libuv instead and then node can call out to the libuv API for setting this sort of thing.

@qdaoming
Copy link
Contributor Author

I feel like something like this should be implemented by libuv instead and then node can call out to the libuv API for setting this sort of thing.

@mscdex The main thread is not created by libuv API, and it might be better to keep this simple feature in node instead of libuv.

@mscdex
Copy link
Contributor

mscdex commented Aug 31, 2022

The main thread is not created by libuv API

libuv is generally where the platform-specific bits get abstracted away, allowing node to escape ifdef hell. Changing the affinity should work cross-platform whether we're changing it for a uv_thread_t or the current thread. For this particular scenario, we'd be doing the latter.

@addaleax
Copy link
Member

addaleax commented Aug 31, 2022

Since CPU affinities are inherited across threads, and this affects the main thread on startup, this seems like it's just adding the functionality of taskset to the Node.js binary? That doesn't seem like a meaningful addition, unless I'm missing something.

@qdaoming
Copy link
Contributor Author

qdaoming commented Sep 1, 2022

Since CPU affinities are inherited across threads, and this affects the main thread on startup, this seems like it's just adding the functionality of taskset to the Node.js binary? That doesn't seem like a meaningful addition, unless I'm missing something.
@addaleax Your comments are what I considered before this PR.
The user might not have opportunity or permission to call taskset for node.js process, ex., in some microservice scenario. It will be convenient for node.js to provide this option. We should pin the main thread only, and should not pin other helper threads to the same cpu to avoid decreasing concurrency.
Is it reasonable?

@addaleax
Copy link
Member

addaleax commented Sep 1, 2022

@qdaoming-intel If that’s the concern, then I’d also recommend putting a cross-platform API for this into libuv, and then maybe even just exposing it as a regular JS API in Node.js?

@qdaoming
Copy link
Contributor Author

qdaoming commented Sep 1, 2022

@addaleax @mscdex
OK, I will add one libuv API like: set_cpu_affinity_to_current_cpu(). It will not be generic to set cpu affinity to sets of cpu, as that makes things complicated and there is no such requirement now.
I would suggest not to expose it to JS api in current stage.

One question about modifying libuv code:
Should I directly modify code in deps/uv directory, OR,
modify the code in libuv github repo (https://github.com/libuv/libuv), then after (several days later) node.js syncing the latest libuv code, I modify node.js code to call the new libuv API?

@mscdex
Copy link
Contributor

mscdex commented Sep 1, 2022

@qdaoming-intel The process starts by submitting a PR to the libuv repo.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run. process Issues and PRs related to the process subsystem.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants