-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
Fix JULIA_EXCLUSIVE setting affinity on non-worker threads #57136
base: master
Are you sure you want to change the base?
Conversation
With JULIA_EXCLUSIVE=1, we would try to give both worker and interactive threads an exclusive CPU, causing --threads=auto to produce a "Too many threads requested" error. Fixes JuliaLang#50702.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This logic is a little odd to me - e.g. if nthreadsi == 1
and nmutator_threads == 2
we no longer end up setting affinity for any thread
Is that intentional?
@@ -794,6 +794,9 @@ void jl_start_threads(void) | |||
{ | |||
int nthreads = jl_atomic_load_relaxed(&jl_n_threads); | |||
int ngcthreads = jl_n_gcthreads; | |||
int nthreadsi = jl_n_threads_per_pool[0]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this ninteractive_threads
? Might be worth the longer name for clarity, or a comment
uv_thread_setaffinity(&uvtid, mask, NULL, cpumasksize); | ||
mask[0] = 0; | ||
|
||
if (nthreadsi == 0) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you leave a comment explaining the guard?
It seems to silently ignore the exclusive
setting, which is worth a call-out (or maybe the affinity is configured elsewhere)
for (i = 1; i < nmutator_threads; ++i) { | ||
jl_threadarg_t *t = (jl_threadarg_t *)malloc_s(sizeof(jl_threadarg_t)); // ownership will be passed to the thread | ||
t->tid = i; | ||
t->barrier = &thread_init_done; | ||
uv_thread_create(&uvtid, jl_threadfun, t); | ||
if (exclusive) { | ||
mask[i] = 1; | ||
if (exclusive && i > nthreadsi) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if (exclusive && i > nthreadsi) { | |
if (exclusive && i > nthreadsi) { | |
// assign any non-interactive threads consecutively, starting at CPU1 | |
assert(i - nthreadsi < cpumasksize); |
With JULIA_EXCLUSIVE=1, we would try to give both worker and interactive threads an exclusive CPU, causing --threads=auto to produce a "Too many threads requested" error.
Fixes #50702.