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

Fix JULIA_EXCLUSIVE setting affinity on non-worker threads #57136

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

Conversation

xal-0
Copy link

@xal-0 xal-0 commented Jan 22, 2025

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.

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.
Copy link
Member

@topolarity topolarity left a 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];
Copy link
Member

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) {
Copy link
Member

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) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (exclusive && i > nthreadsi) {
if (exclusive && i > nthreadsi) {
// assign any non-interactive threads consecutively, starting at CPU1
assert(i - nthreadsi < cpumasksize);

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.

JULIA_EXCLUSIVE with auto threads errors on 1.10, mac/arm64
3 participants