-
Notifications
You must be signed in to change notification settings - Fork 94
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
ProxifyHostFile: tracking of external objects #527
ProxifyHostFile: tracking of external objects #527
Conversation
196b5b6
to
255a18f
Compare
21c153d
to
d5ea45e
Compare
Codecov Report
@@ Coverage Diff @@
## branch-0.19 #527 +/- ##
================================================
+ Coverage 61.90% 92.41% +30.50%
================================================
Files 22 16 -6
Lines 2452 1595 -857
================================================
- Hits 1518 1474 -44
+ Misses 934 121 -813
Continue to review full report at Codecov.
|
7842f31
to
bd5df7b
Compare
In order to trigger the finalization ASAP
bd5df7b
to
bc67216
Compare
@beckernick @VibhuJawa, it would be great if one of you have time try this PR. You need to set |
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.
I have some questions here, but overall LGTM.
Thanks for the review @pentschev, I think I have addressed all of your concerns. Also added some overall description: c6321bd |
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.
LGTM, thanks @madsbk !
If there's no rush, I would wait another day to see if @beckernick or @VibhuJawa have a chance to try out the PR, otherwise it can be merged any time IMO. |
Since explicit-comms's shuffle is disabled by default, I think it is safe to merge. @gpucibot merge |
@gpucibot merge |
Thanks @madsbk ! |
This PR implements tracking of external objects. The idea is that tasks can register objects that should be exposed to spilling when
ProxifyHostFile
is running out of memory.Also, this PR makes the long running explicit-comms tasks
local_shuffle()
use this new feature, which means it shouldn't require more GPU memory than the regular shuffle implementation in Dask.