Likely erroneous use of `Threads.nthreads` and `Threads.threadid`

[KristofferC]

Hello,

From the following PkgEval log: https://s3.amazonaws.com/julialang-reports/nanosoldier/pkgeval/by_hash/5da257d_vs_d63aded/Ephemerides.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> 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).

[MicheleCeresoli]

Hello @KristofferC,

Thanks a lot for pointing this out. We will definitely have a look at your suggestions and fix this wrong usage in the coming weeks.