-
-
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
RFC/WIP : cache mapped function remotely for pmap. [ci skip] #16695
Conversation
end | ||
|
||
function exec_from_cache(f, rr::RemoteChannel, args...) | ||
if (f==nothing) |
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 parens needed around if
condition when it's this short, usually a bit better to compare to nothing
using ===
I think we need a more comprehensive cache. The slow part is sending a |
end | ||
end | ||
|
||
cached_remote(cwp::CachedWorkerPool, f) = (args...) -> remotecall_fetch(f, cwp, args...) |
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.
Why not instead use multiple dispatch and have
remote(cwp::CachedWorkerPool, f) = (args...) -> remotecall_fetch(f, cwp, args...)
If CachedWorkerPool<:AbstractWorkerPool
, and WorkerPool<:AbstractWorkerPool
,
then in general there could just be
remote(p::AbstractWorkerPool, f) = (args...; kwargs...)->remotecall_fetch(f, p, args...; kwargs...)
(that would also mean adding support for kwargs
, but it think that would be a good thing for making things transparent.
I think in general maybe if CachedWorkerPool acted just like a workerpool, but caching all suitable data passed through remote_*
with it, might be good. So a CachingWorkerPool
(I'm not entirely satisfied with that either)
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.
Yeah, I have thought about this, i.e., defining an AbstractWorkerPool
and the expected methods for any implementation of the same.
I think it should be done irrespective of how this PR unfolds - will be useful to allow users to extend the WorkerPool concept as per their specific needs.
Will open another PR with a generic version of this functionality |
This is a WIP to address #16345 and the initial results are encouraging.
It caches the mapping function remotely for the duration of the pmap call.
Some timings with
julia -p4
:I'll hold off working on this till #16508 is addressed as both the code and interface may change. For now feedback on the caching method used here will be appreciated.