Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Simplify Channel constructors, change kwargs to positional args. #32818

Merged
merged 6 commits into from
Aug 11, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ Multi-threading changes
([#32309], [#32174], [#31981], [#32421]).
* The global random number generator (`GLOBAL_RNG`) is now thread-safe (and thread-local) ([#32407]).
* New experimental `Threads.@spawn` macro that runs a task on any available thread ([#32600]).
* Simplified the `Channel` constructor, which is now easier to read and more idiomatic julia.
The old constructor (which used kwargs) is still available, but use is discouraged ([#30855], [#32818]).

Build system changes
--------------------
Expand Down
45 changes: 22 additions & 23 deletions base/channels.jl
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,9 @@ Representation of a channel passing objects of type `T`.
abstract type AbstractChannel{T} end

"""
Channel{T}(sz::Int=0)
Channel{T=Any}(size::Int=0)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow... maybe we should actually implement this syntax for defining constructors with default type parameter values? Seems quite possible.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow... maybe we should actually implement this syntax for defining constructors with default type parameter values? Seems quite possible.

:D Yeah that makes sense to me! It seems pretty readable, and has an intuitive meaning, i think.


Constructs a `Channel` with an internal buffer that can hold a maximum of `sz` objects
Constructs a `Channel` with an internal buffer that can hold a maximum of `size` objects
of type `T`.
[`put!`](@ref) calls on a full channel block until an object is removed with [`take!`](@ref).

Expand All @@ -24,7 +24,7 @@ Other constructors:
* `Channel(sz)`: equivalent to `Channel{Any}(sz)`

!!! compat "Julia 1.3"
The default constructor `Channel()` and default `sz=0` were added in Julia 1.3.
The default constructor `Channel()` and default `size=0` were added in Julia 1.3.
"""
mutable struct Channel{T} <: AbstractChannel{T}
cond_take::Threads.Condition # waiting for data to become available
Expand All @@ -51,26 +51,27 @@ function Channel{T}(sz::Float64) where T
sz = (sz == Inf ? typemax(Int) : convert(Int, sz))
return Channel{T}(sz)
end
Channel(sz) = Channel{Any}(sz)
Channel() = Channel{Any}(0)
Channel(sz=0) = Channel{Any}(sz)

# special constructors
"""
Channel(func::Function; ctype=Any, csize=0, taskref=nothing)
Channel{T=Any}(func::Function, size=0; taskref=nothing)

Create a new task from `func`, bind it to a new channel of type
`ctype` and size `csize`, and schedule the task, all in a single call.
`T` and size `size`, and schedule the task, all in a single call.

`func` must accept the bound channel as its only argument.

If you need a reference to the created task, pass a `Ref{Task}` object via
keyword argument `taskref`.
the keyword argument `taskref`.

Return a `Channel`.

# Examples
```jldoctest
julia> chnl = Channel(c->foreach(i->put!(c,i), 1:4));
julia> chnl = Channel() do ch
foreach(i -> put!(ch, i), 1:4)
end;

julia> typeof(chnl)
Channel{Any}
Expand All @@ -89,7 +90,9 @@ Referencing the created task:
```jldoctest
julia> taskref = Ref{Task}();

julia> chnl = Channel(c -> println(take!(c)); taskref=taskref);
julia> chnl = Channel(taskref=taskref) do ch
println(take!(ch))
end;

julia> istaskdone(taskref[])
false
Expand All @@ -102,11 +105,8 @@ true
```

!!! compat "Julia 1.3"
The following constructors were added in Julia 1.3.

Other constructors:
* `Channel{T}(func::Function, sz=0)`
* `Channel{T}(func::Function; csize=0, taskref=nothing)`
This constructor was added in Julia 1.3. Earlier versions of Julia used kwargs
to set `size` and `T`, but those constructors are deprecated.

```jldoctest
julia> chnl = Channel{Char}(1) do ch
Expand All @@ -120,22 +120,21 @@ julia> String(collect(chnl))
"hello world"
```
"""
function Channel(func::Function; ctype=Any, csize=0, taskref=nothing)
chnl = Channel{ctype}(csize)
function Channel{T}(func::Function, size=0; taskref=nothing) where T
chnl = Channel{T}(size)
task = Task(() -> func(chnl))
bind(chnl, task)
yield(task) # immediately start it

isa(taskref, Ref{Task}) && (taskref[] = task)
return chnl
end
function Channel{T}(f::Function, sz=0) where T
return Channel(f, csize=sz, ctype=T)
end
function Channel{T}(f::Function; csize=0, taskref=nothing) where T
return Channel(f, csize=csize, ctype=T, taskref=taskref)
end
Channel(func::Function, args...; kwargs...) = Channel{Any}(func, args...; kwargs...)

# This constructor is deprecated as of Julia v1.3, and should not be used.
function Channel(func::Function; ctype=Any, csize=0, taskref=nothing)
return Channel{ctype}(func, csize; taskref=taskref)
end

closed_exception() = InvalidStateException("Channel is closed.", :closed)

Expand Down
20 changes: 12 additions & 8 deletions test/channels.jl
Original file line number Diff line number Diff line change
Expand Up @@ -44,12 +44,6 @@ end
end

@testset "Task constructors" begin
c = Channel(ctype=Float32, csize=2) do c; map(i->put!(c,i), 1:100); end
@test eltype(c) == Float32
@test c.sz_max == 2
@test isopen(c)
@test collect(c) == 1:100

c = Channel{Int}() do c; map(i->put!(c,i), 1:100); end
@test eltype(c) == Int
@test c.sz_max == 0
Expand All @@ -59,18 +53,28 @@ end
@test c.sz_max == 0
@test collect(c) == [1, "hi"]

c = Channel(Inf) do c; put!(c,1); end
@test eltype(c) == Any
@test c.sz_max == typemax(Int)
c = Channel{Int}(Inf) do c; put!(c,1); end
@test eltype(c) == Int
@test c.sz_max == typemax(Int)

taskref = Ref{Task}()
c = Channel{Int}(csize=0, taskref=taskref) do c; put!(c, 0); end
c = Channel{Int}(0, taskref=taskref) do c; put!(c, 0); end
@test eltype(c) == Int
@test c.sz_max == 0
@test istaskstarted(taskref[])
@test !istaskdone(taskref[])
take!(c); wait(taskref[])
@test istaskdone(taskref[])

# Legacy constructor
c = Channel(ctype=Float32, csize=2) do c; map(i->put!(c,i), 1:100); end
@test eltype(c) == Float32
@test c.sz_max == 2
@test isopen(c)
@test collect(c) == 1:100
end

@testset "multiple concurrent put!/take! on a channel for different sizes" begin
Expand Down Expand Up @@ -230,7 +234,7 @@ using Distributed

for T in [Any, Int]
taskref = Ref{Task}()
chnl = Channel(tf6, ctype=T, csize=N, taskref=taskref)
chnl = Channel{T}(tf6, N, taskref=taskref)
put!(chnl, 2)
yield()
@test_throws ErrorException wait(chnl)
Expand Down