Threading improvements (#493)

* better safety in nested threading

* SpinLock brings back the speeeed

* NEWS entry

(cherry picked from commit 624805c)

NEWS.md

@@ -3,6 +3,11 @@ MixedModels v3.5.0 Release Notes
 * The Progressbar for `parametricbootstrap` and `replicate` is not displayed
   when in a non-interactive (i.e. logging) context. The progressbar can also
   be manually disabled with `hide_progress=true`.[#495]
+* Threading in `parametricbootstrap` now uses a `SpinLock` instead of a `ReentrantLock`.
+  This improves performance, but care should be taken when nesting spin locks. [#493]
+* Single-threaded use of `paramatricbootstrap` now works when nested within a larger
+  multi-threaded context (e.g. `Threads.@threads for`). (Multi-threaded `parametricbootstrap`
+  worked and continues to work within a nested threading context.) [#493]
 
 MixedModels v3.4.1 Release Notes
 ========================
@@ -166,4 +171,5 @@ Package dependencies
 [#486]: https://github.com/JuliaStats/MixedModels.jl/issues/486
 [#489]: https://github.com/JuliaStats/MixedModels.jl/issues/489
 [#490]: https://github.com/JuliaStats/MixedModels.jl/issues/490
+[#493]: https://github.com/JuliaStats/MixedModels.jl/issues/493
 [#495]: https://github.com/JuliaStats/MixedModels.jl/issues/495

src/bootstrap.jl

@@ -50,11 +50,16 @@ families with a dispersion parameter.
 `hide_progress` can be used to disable the progress bar. Note that the progress
 bar is automatically disabled for non-interactive (i.e. logging) contexts.
 
-Note that `use_threads=true` may not offer a performance boost and may even
-decrease peformance if multithreaded linear algebra (BLAS) routines are available.
-In this case, threads at the level of the linear algebra may already occupy all
-processors/processor cores. There are plans to provide better support in coordinating
-Julia- and BLAS-level threads in the future.
+!!! note
+    Note that `use_threads=true` may not offer a performance boost and may even
+    decrease peformance if multithreaded linear algebra (BLAS) routines are available.
+    In this case, threads at the level of the linear algebra may already occupy all
+    processors/processor cores. There are plans to provide better support in coordinating
+    Julia- and BLAS-level threads in the future.
+
+!!! warning
+    The PRNG shared between threads is locked using [`Threads.SpinLock`](@ref), which
+    should not be used recursively. Do not wrap `parametricbootstrap` in an outer `SpinLock`.
 """
 function parametricbootstrap(
     rng::AbstractRNG,
@@ -88,11 +93,12 @@ function parametricbootstrap(
     # we use locks to guarantee thread-safety, but there might be better ways to do this for some RNGs
     # see https://docs.julialang.org/en/v1.3/manual/parallel-computing/#Side-effects-and-mutable-function-arguments-1
     # see https://docs.julialang.org/en/v1/stdlib/Future/index.html
-    rnglock = ReentrantLock()
+    rnglock = Threads.SpinLock()
     samp = replicate(n; use_threads=use_threads, hide_progress=hide_progress) do
-        mod = m_threads[Threads.threadid()]
-        local βsc = βsc_threads[Threads.threadid()]
-        local θsc = θsc_threads[Threads.threadid()]
+        tidx = use_threads ? Threads.threadid() : 1
+        mod = m_threads[tidx]
+        local βsc = βsc_threads[tidx]
+        local θsc = θsc_threads[tidx]
         lock(rnglock)
         mod = simulate!(rng, mod, β = β, σ = σ, θ = θ)
         unlock(rnglock)

test/utilities.jl

@@ -70,4 +70,4 @@ end
 	str = String(take!(io))
 	@test !isempty(findall("PC1", str))
 	@test !isempty(findall("load: yes", str))
-end
+end