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

Cannot spawn task on interactive thread after starting Julia 1.9.0-beta4 with -t1,1 #48644

Closed
simsurace opened this issue Feb 10, 2023 · 8 comments · Fixed by #48702
Closed

Comments

@simsurace
Copy link
Contributor

simsurace commented Feb 10, 2023

The CLI help for -t states:

-t, --threads {auto|N[,auto|M]}
       Enable N[+M] threads; N threads are assigned to the `default`
       threadpool, and if M is specified, M threads are assigned to the
       `interactive` threadpool...

So I would expect to be able to spawn a task on the interactive thread when starting Julia with -t1,1 (one default and one interactive thread). But this fails on Julia 1.9.0-beta4:

$ julia +beta -q -t1,1

julia> fetch(Threads.@spawn :interactive Threads.threadpool())
:default

It works when starting Julia with -t2,1 (two default and one interactive thread):

$ julia +beta -q -t2,1

julia> fetch(Threads.@spawn :interactive Threads.threadpool())
:interactive

I think this is a bug.

julia> versioninfo()
Julia Version 1.9.0-beta4
Commit b75ddb787ff (2023-02-07 21:53 UTC)
Platform Info:
  OS: Linux (x86_64-linux-gnu)
  CPU: 8 × Intel(R) Core(TM) i7-9700K CPU @ 3.60GHz
  WORD_SIZE: 64
  LIBM: libopenlibm
  LLVM: libLLVM-14.0.6 (ORCJIT, skylake)
  Threads: 2 on 8 virtual cores
@simsurace
Copy link
Contributor Author

simsurace commented Feb 15, 2023

I did some digging and I believe there is a bug in

julia/src/partr.c

Lines 71 to 77 in 45b7e7a

JL_DLLEXPORT int jl_set_task_threadpoolid(jl_task_t *task, int8_t tpid) JL_NOTSAFEPOINT
{
if (tpid < 0 || tpid >= jl_n_threadpools)
return 0;
task->threadpoolid = tpid;
return 1;
}

Setting the threadpool does not work, but setting the thread id works:

$ julia +beta -q -t1,1

julia> test(set_threadpoolid!)
:default

julia> test(set_threadid!)
:interactive

where

set_threadpoolid!(task) = ccall(:jl_set_task_threadpoolid, Cint, (Any, Int8), task, Int8(1))
set_threadid!(task) = ccall(:jl_set_task_tid, Cint, (Any, Int16), task, Int16(1))

function test(f)
    task = @task Threads.threadpool()
    task.sticky = false
    f(task)
    schedule(task)
    return fetch(task)
end

This is despite the fact that

julia> ccall(:jl_threadpoolid, Int8, (Int16,), 0)
0

julia> ccall(:jl_threadpoolid, Int8, (Int16,), 1)
1

@simsurace
Copy link
Contributor Author

@jpsamaroo You might be able to help.

@jpsamaroo
Copy link
Member

@kpamnany

@kpamnany
Copy link
Contributor

kpamnany commented Feb 16, 2023

This is a bug introduced by #46609, here. Previously, nthreads() returned the total number of threads in all the threadpools, so the if check would fail. Now, threadpoolsize() returns the number of threads in the default thread pool only, which is 1 for the breaking case.

Looks like nthreads() was deprecated (and changed to mean nthreads(:default) which seems like a bad change) which is presumably why it was replaced with threadpoolsize(), but it probably should have been replaced with maxthreadid()?

Cc: @vtjnash and @vchuravy

@simsurace
Copy link
Contributor Author

See related discussions #48589 and #48643

@simsurace
Copy link
Contributor Author

I do not know how best to fix this, but I would think it is important to

  • Put this on the 1.9 milestone
  • Add a test workflow to the CI to detect this type of bug

I could contribute the second item, but I'm unfamiliar with this project's CI pipelines. Where are they defined? Where would such a workflow go (e.g. starting Julia with different thread configurations and checking for correct behavior)?

@kpamnany
Copy link
Contributor

Thanks for the good report @simsurace.

@simsurace
Copy link
Contributor Author

My pleasure

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 a pull request may close this issue.

3 participants