pmap speedup #17331
pmap speedup #17331
Conversation
| elseif isa(ntasks, Function) | ||
| max_tasks = ntasks | ||
| else | ||
| throw(ArgumentError("ntasks must be an Integer or a zero-arg function returning the maximum number of tasks allowed.")) |
There was a problem hiding this comment.
why is a zero-arg function desirable for this?
There was a problem hiding this comment.
Allows for the number of concurrent tasks in AsyncCollector to be dynamic. One of the requested features was that pmap must be able to leverage workers as and when they come online. This allows for a user of AsyncCollector, like pmap to increase count of parallel tasks depending on current number of workers. Note that it is not a major issue if the number of tasks is more than the number of workers, but it will be quite inefficient the other way around.
There was a problem hiding this comment.
so this would be accomplished via a zero-arg function that reads from some global runtime state?
There was a problem hiding this comment.
A WorkerPool allows for workers to be dynamically added. pmap now passes this keyword arg as ntasks=()->nworkers(p) where p is the worker pool.
amitmurthy
force-pushed
the
amitm/pmap_perf
branch
from
674434b to
0d385f5
Compare
| @@ -51,7 +51,6 @@ function fetch(c::Channel) | |||
| end | |||
|
|
|||
| function take!(c::Channel) | |||
| !isopen(c) && !isready(c) && throw(closed_exception()) | |||
There was a problem hiding this comment.
does the added test in the next commit exercise this?
There was a problem hiding this comment.
Well yes, because that is how I found the bug. But it ought to have an explicit test anyway. Will add one.
| @@ -19,44 +19,97 @@ type AsyncCollector | |||
| f | |||
| results | |||
| enumerator::Enumerate | |||
| ntasks::Int | |||
| max_tasks::Function | |||
| c::Channel{Tuple{Int, Any}} # to communicate with the tasks | |||
There was a problem hiding this comment.
a slightly more descriptive field name than c might be good for clarity's sake
AsyncCollector: Limit numer of tasks and communicate with channels
amitmurthy
force-pushed
the
amitm/pmap_perf
branch
from
0d385f5 to
191796e
Compare
Partially addresses #17301
On my machine:
this PR takes 8.8 seconds vs 6.6 seconds on 0.4