From c2ef8feafcd8282dbd83857d0f3dbab1d7ba4353 Mon Sep 17 00:00:00 2001 From: Jameson Nash Date: Tue, 27 Nov 2018 12:58:28 -0500 Subject: [PATCH] fixup [ci skip]: switch to runtime branching for MT-safe Condition API Jeff suggested we branch at runtime on whether `Condition` requires the MT-safe API or not. This implements that, to encourage further discussion and experimentation with the API before we finalize the implementation. Thus, in addition to the existing API and usage of `Condition()` (which remain unchanged), this commit adds the option to construct it as a thread-safe variant: `Condition(#=threadsafe=#true)` and `Condition(ReentrantLock())` thereby enabling the same type to be used for both MT and ST usage, and branching the API at runtime based on the additional requirements imposed by MT-safety. --- base/event.jl | 15 ++++++--------- base/lock.jl | 4 ++-- base/sysimg.jl | 6 ++++++ 3 files changed, 14 insertions(+), 11 deletions(-) diff --git a/base/event.jl b/base/event.jl index 29e0d378c179c..9be935942f25d 100644 --- a/base/event.jl +++ b/base/event.jl @@ -94,7 +94,7 @@ this, and can be used for level-triggered events. This object is NOT thread-safe. See [`Threads.ConditionMT`](@ref) for a thread-safe version. """ -mutable struct GenericCondition{L<:AbstractLock} +struct GenericCondition{L<:AbstractLock} waitq::Vector{Any} lock::L @@ -224,12 +224,9 @@ function notify(e::GenericEvent) end -const ConditionST = GenericCondition{CooperativeLock} +const ConditionST = GenericCondition{AlwaysLockedST} const EventST = GenericEvent{CooperativeLock} -# default (Julia v1.0) is currently single-threaded -const Condition = GenericCondition{AlwaysLockedST} - ## scheduler and work queue @@ -433,11 +430,11 @@ Use [`isopen`](@ref) to check whether it is still active. """ mutable struct AsyncCondition handle::Ptr{Cvoid} - cond::Condition + cond::ConditionST isopen::Bool function AsyncCondition() - this = new(Libc.malloc(_sizeof_uv_async), Condition(), true) + this = new(Libc.malloc(_sizeof_uv_async), ConditionST(), true) associate_julia_struct(this.handle, this) finalizer(uvfinalize, this) err = ccall(:uv_async_init, Cint, (Ptr{Cvoid}, Ptr{Cvoid}, Ptr{Cvoid}), @@ -491,14 +488,14 @@ to check whether a timer is still active. """ mutable struct Timer handle::Ptr{Cvoid} - cond::Condition + cond::ConditionST isopen::Bool function Timer(timeout::Real; interval::Real = 0.0) timeout ≥ 0 || throw(ArgumentError("timer cannot have negative timeout of $timeout seconds")) interval ≥ 0 || throw(ArgumentError("timer cannot have negative repeat interval of $interval seconds")) - this = new(Libc.malloc(_sizeof_uv_timer), Condition(), true) + this = new(Libc.malloc(_sizeof_uv_timer), ConditionST(), true) err = ccall(:uv_timer_init, Cint, (Ptr{Cvoid}, Ptr{Cvoid}), eventloop(), this) if err != 0 #TODO: this codepath is currently not tested diff --git a/base/lock.jl b/base/lock.jl index 6e65be67a9015..23fe2114a0070 100644 --- a/base/lock.jl +++ b/base/lock.jl @@ -169,8 +169,8 @@ This construct is NOT threadsafe. mutable struct Semaphore sem_size::Int curr_cnt::Int - cond_wait::Condition - Semaphore(sem_size) = sem_size > 0 ? new(sem_size, 0, Condition()) : throw(ArgumentError("Semaphore size must be > 0")) + cond_wait::ConditionST + Semaphore(sem_size) = sem_size > 0 ? new(sem_size, 0, ConditionST()) : throw(ArgumentError("Semaphore size must be > 0")) end """ diff --git a/base/sysimg.jl b/base/sysimg.jl index 9f28add8b4c76..19635c2ee97f0 100644 --- a/base/sysimg.jl +++ b/base/sysimg.jl @@ -317,6 +317,12 @@ include("task.jl") include("lock.jl") include("threads.jl") include("weakkeydict.jl") + +# default (Julia v1.0) is currently single-threaded +const Condition = GenericCondition{Union{AlwaysLockedST, Threads.ReentrantLockMT}} +Condition() = Condition(AlwaysLockedST()) +Condition(threadsafe::Bool) = Condition(threadsafe ? Threads.ReentrantLockMT() : AlwaysLockedST()) +# but uses MT-safe versions, when possible const ReentrantLock = Threads.ReentrantLockMT const Event = Threads.EventMT