JuliaParallel / JuliaParallel/UCX.jl
Likely erroneous use of `Threads.nthreads` and `Threads.threadid`
- Dominant language
- Julia
- Stars
- 21
- Forks
- 7
- PR merge metrics
- No merged PRs in 30d
Description
Hello,
From the following PkgEval log: https://s3.amazonaws.com/julialang-reports/nanosoldier/pkgeval/by_hash/5da257d_vs_d63aded/UCX.primary.log it seems likely that this package is using `Threads.nthreads` and `Threads.threadid` incorrectly as described in https://julialang.org/blog/2023/07/PSA-dont-use-threadid/ and https://juliafolds2.github.io/OhMyThreads.jl/stable/literate/tls/tls/#The-naive-(and-incorrect)-approach.
In the upcoming Julia 1.12, julia runs with an interactive thread by default which means that the number of worker threads `nthreads()` defaults to 1 but the thread id that things will run on in a threaded context will typically be 2 (the interactive thread having the id 1):
```julia
julia> Threads.nthreads()
1
julia> Threads.@threads for i = 1:3
@show Threads.threadid()
end
Threads.threadid() = 2
Threads.threadid() = 2
Threads.threadid() = 2
```
Trying to index a vector of length `Threads.nthreads()` with `Threads.threadid()` will thus error.
Note that even though this code only started directly erroring in 1.12 the code was likely already incorrect on earlier Julia versions unless it made sure that the spawned tasks did not migrate between threads (which can happen at any yield point).
Some remedies to this are:
- Use the new `OncePerThread` (or `OncePerTask`) functionality added in 1.12.
- Use task local values instead of thread local values as described in https://juliafolds2.github.io/OhMyThreads.jl/stable/literate/tls/tls/#TLV
- Use a `Channel` as described in https://juliafolds2.github.io/OhMyThreads.jl/stable/literate/tls/tls/#The-safe-way:-Channel.
- Use `Threads.maxthreadid()` instead of `Threads.nthreads()`. This can waste some memory since `maxthreadid()` can give a higher number than the number of worker threads. It also doesn't help against the task migration issue described earlier (but the previous code did not do that either).
Contributor guide
No contributing guide indexed for this repository
Research direction
Search UCX.jl for uses of Threads.nthreads and Threads.threadid, then read the Julia 1.12 threading guidance and the linked task-local and channel approaches. Confirm each affected use handles interactive-thread IDs and task migration safely, and run the package's relevant tests to verify the reported failure is addressed.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- julia
- Domain
- networking
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Needs clarification
- Newbie friendliness
- 30/100