Skip to content

Commit

Permalink
fixup [ci skip]: switch to runtime branching for MT-safe Condition API
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
vtjnash committed Nov 27, 2018
1 parent f8da204 commit c2ef8fe
Show file tree
Hide file tree
Showing 3 changed files with 14 additions and 11 deletions.
15 changes: 6 additions & 9 deletions base/event.jl
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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}),
Expand Down Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions base/lock.jl
Original file line number Diff line number Diff line change
Expand Up @@ -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

"""
Expand Down
6 changes: 6 additions & 0 deletions base/sysimg.jl
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down

0 comments on commit c2ef8fe

Please sign in to comment.