-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
@catch, retry, partition, asyncmap and pmap #15409
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,187 @@ | ||
# This file is a part of Julia. License is MIT: http://julialang.org/license | ||
|
||
|
||
""" | ||
AsyncCollector(f, results, c...; ntasks=100) -> iterator | ||
|
||
Apply f to each element of c using at most 100 asynchronous tasks. | ||
For multiple collection arguments, apply f elementwise. | ||
Output is collected into "results". | ||
|
||
Note: `next(::AsyncCollector, state) -> (nothing, state)` | ||
|
||
Note: `for task in AsyncCollector(f, results, c...) end` is equivalent to | ||
`map!(f, results, c...)`. | ||
""" | ||
type AsyncCollector | ||
f | ||
results | ||
enumerator::Enumerate | ||
ntasks::Int | ||
end | ||
|
||
function AsyncCollector(f, results, c...; ntasks=0) | ||
if ntasks == 0 | ||
ntasks = 100 | ||
end | ||
AsyncCollector(f, results, enumerate(zip(c...)), ntasks) | ||
end | ||
|
||
|
||
type AsyncCollectorState | ||
enum_state | ||
active_count::Int | ||
task_done::Condition | ||
done::Bool | ||
end | ||
|
||
|
||
# Busy if the maximum number of concurrent tasks is running. | ||
function isbusy(itr::AsyncCollector, state::AsyncCollectorState) | ||
state.active_count == itr.ntasks | ||
end | ||
|
||
|
||
# Wait for @async task to end. | ||
wait(state::AsyncCollectorState) = wait(state.task_done) | ||
|
||
|
||
# Open a @sync block and initialise iterator state. | ||
function start(itr::AsyncCollector) | ||
sync_begin() | ||
AsyncCollectorState(start(itr.enumerator), 0, Condition(), false) | ||
end | ||
|
||
# Close @sync block when iterator is done. | ||
function done(itr::AsyncCollector, state::AsyncCollectorState) | ||
if !state.done && done(itr.enumerator, state.enum_state) | ||
state.done = true | ||
sync_end() | ||
end | ||
return state.done | ||
end | ||
|
||
function next(itr::AsyncCollector, state::AsyncCollectorState) | ||
|
||
# Wait if the maximum number of concurrent tasks are already running... | ||
while isbusy(itr, state) | ||
wait(state) | ||
end | ||
|
||
# Get index and mapped function arguments from enumeration iterator... | ||
(i, args), state.enum_state = next(itr.enumerator, state.enum_state) | ||
|
||
# Execute function call and save result asynchronously... | ||
@async begin | ||
itr.results[i] = itr.f(args...) | ||
state.active_count -= 1 | ||
notify(state.task_done, nothing) | ||
end | ||
|
||
# Count number of concurrent tasks... | ||
state.active_count += 1 | ||
|
||
return (nothing, state) | ||
end | ||
|
||
|
||
|
||
""" | ||
AsyncGenerator(f, c...; ntasks=100) -> iterator | ||
|
||
Apply f to each element of c using at most 100 asynchronous tasks. | ||
For multiple collection arguments, apply f elementwise. | ||
Results are returned by the iterator as they become available. | ||
Note: `collect(AsyncGenerator(f, c...; ntasks=1))` is equivalent to | ||
`map(f, c...)`. | ||
""" | ||
type AsyncGenerator | ||
collector::AsyncCollector | ||
end | ||
|
||
function AsyncGenerator(f, c...; ntasks=0) | ||
AsyncGenerator(AsyncCollector(f, Dict{Int,Any}(), c...; ntasks=ntasks)) | ||
end | ||
|
||
|
||
type AsyncGeneratorState | ||
i::Int | ||
async_state::AsyncCollectorState | ||
end | ||
|
||
|
||
start(itr::AsyncGenerator) = AsyncGeneratorState(0, start(itr.collector)) | ||
|
||
# Done when source async collector is done and all results have been consumed. | ||
function done(itr::AsyncGenerator, state::AsyncGeneratorState) | ||
done(itr.collector, state.async_state) && isempty(itr.collector.results) | ||
end | ||
|
||
# Pump the source async collector if it is not already busy... | ||
function pump_source(itr::AsyncGenerator, state::AsyncGeneratorState) | ||
if !isbusy(itr.collector, state.async_state) && | ||
!done(itr.collector, state.async_state) | ||
ignored, state.async_state = next(itr.collector, state.async_state) | ||
return true | ||
else | ||
return false | ||
end | ||
end | ||
|
||
function next(itr::AsyncGenerator, state::AsyncGeneratorState) | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think a line break is in order here, even if we allowed the exception of blocks starting with a comment. |
||
state.i += 1 | ||
|
||
results = itr.collector.results | ||
while !haskey(results, state.i) | ||
|
||
# Wait for results to become available... | ||
if !pump_source(itr,state) && !haskey(results, state.i) | ||
wait(state.async_state) | ||
end | ||
end | ||
r = results[state.i] | ||
delete!(results, state.i) | ||
|
||
return (r, state) | ||
end | ||
|
||
iteratorsize(::Type{AsyncGenerator}) = SizeUnknown() | ||
|
||
|
||
""" | ||
asyncgenerate(f, c...) -> iterator | ||
|
||
Apply `@async f` to each element of `c`. | ||
|
||
For multiple collection arguments, apply f elementwise. | ||
|
||
Results are returned in order as they become available. | ||
""" | ||
asyncgenerate(f, c...) = AsyncGenerator(f, c...) | ||
|
||
|
||
""" | ||
asyncmap(f, c...) -> collection | ||
|
||
Transform collection `c` by applying `@async f` to each element. | ||
|
||
For multiple collection arguments, apply f elementwise. | ||
""" | ||
asyncmap(f, c...) = collect(asyncgenerate(f, c...)) | ||
|
||
|
||
""" | ||
asyncmap!(f, c) | ||
|
||
In-place version of `asyncmap()`. | ||
""" | ||
asyncmap!(f, c) = (for x in AsyncCollector(f, c, c) end; c) | ||
|
||
|
||
""" | ||
asyncmap!(f, results, c...) | ||
|
||
Like `asyncmap()`, but stores output in `results` rather returning a collection. | ||
""" | ||
asyncmap!(f, r, c1, c...) = (for x in AsyncCollector(f, r, c1, c...) end; r) |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1003,5 +1003,33 @@ export call | |
# 1933 | ||
@deprecate_binding SingleAsyncWork AsyncCondition | ||
|
||
|
||
# #12872 | ||
@deprecate istext istextmime | ||
|
||
#15409 | ||
function pmap(f, c...; err_retry=nothing, err_stop=nothing, pids=nothing) | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same here (and likely elsewhere). |
||
if err_retry != nothing | ||
depwarn("`err_retry` is deprecated, use `pmap(retry(f), c...)`.", :pmap) | ||
if err_retry == true | ||
f = retry(f) | ||
end | ||
end | ||
|
||
if err_stop != nothing | ||
depwarn("`err_stop` is deprecated, use `pmap(@catch(f), c...).", :pmap) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Missing closing backquote. BTW, are you sure backquotes in error messages are the usual way of printing exceptions? I think I made a PR about that ages ago which was rejected. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've removed the backquotes. |
||
if err_stop == false | ||
f = @catch(f) | ||
end | ||
end | ||
|
||
if pids == nothing | ||
p = default_worker_pool() | ||
else | ||
depwarn("`pids` is deprecated, use `pmap(::WorkerPool, f, c...).", :pmap) | ||
p = WorkerPool(pids) | ||
end | ||
|
||
return pmap(p, f, c...) | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -22,6 +22,16 @@ function next(g::Generator, s) | |
g.f(v), s2 | ||
end | ||
|
||
|
||
""" | ||
generate(f, c...) -> iterator | ||
|
||
Return an iterator applying `f` to each element of `c`. | ||
For multiple collection arguments, apply f elementwise. | ||
""" | ||
generate(f, c...) = Generator(f, c...) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. please don't export this. It's a very small wrapper and unfortunately close in syntax to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why not call There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I guess, because it feels to me like it should not be necessary to think about types to use a Julia API. It seems like there is a greater burden of understanding if I have to call a type constructor. If I'm making something explicitly this type, what else do I have to understand about this type? If I just call a function, the type will be whatever clever type the method dispatch system figures out is best for the particular situation. Maybe Note, that this is not actually called anywhere. It is included mostly for completeness to establish the pattern followed by Also, there are precedents for having a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I see your point. Clearly, we don't know what policy to follow as regards the naming of iterators and functions that create them... Anyway, the docs should explicitly say "Return an iterator applying There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Isn't that what There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. fair enough. done There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is just #10452 - though "not having to think about types" sounds like you're using the wrong language. You have to think about types all the time with Julia, and unless there are actual examples of a lowercase function API returning different types for different inputs, exporting a new function that's just a trivial wrapper around an existing type is not necessary. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why wouldn't you want to allow julia> typeof(zip([1]))
Base.Zip1{Array{Int64,1}}
julia> typeof(zip([1],[2]))
Base.Zip2{Array{Int64,1},Array{Int64,1}}
julia> typeof(zip([1],[2],[3]))
Zip{Array{Int64,1},Base.Zip2{Array{Int64,1},Array{Int64,1}}} There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If we had any of the latter, I might agree with you. At the moment we don't, so it's redundant to start creating and using API's that don't need to exist yet. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This definition serves no purpose. As long as this is the only method of |
||
|
||
|
||
## iterator traits | ||
|
||
abstract IteratorSize | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"retries nor returns"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No strong opinion on this. My feeling is that if it was "neither retries", then it would be "nor returns". I believe that in the "does not verb or verb" case, there is no hard rule about "or" or "nor". When I read it out aloud as written, it sounds better to me with "or" rather than "nor". However, I'm Australian, so what sounds normal to me would make an Englishman cringe! Happy to change it if you feel strongly ...