Enhanced support for memory-tracking raft resources#3004
Conversation
achirkin
added
improvement
achirkin
changed the title
| // NVCC injects __host__ __device__ on std::shared_ptr special members, | ||
| // which makes the *implicit* or *defaulted* special members __host__ | ||
| // __device__ too. That conflicts with Upstream types whose special | ||
| // members are __host__ only (e.g. rmm::device_async_resource_ref). | ||
| // User-defined bodies (not = default) force plain __host__ execution space. |
There was a problem hiding this comment.
Good find. Are there changes you would suggest for RMM or CCCL?
There was a problem hiding this comment.
Thanks! I think, CCCL/rrm are fine, because the cuda::mr::shared_resource defines the copy/move constructors explicitly.
tfeher
left a comment
tfeher
left a comment
There was a problem hiding this comment.
Thanks Artem for the PR! Overall looks good, I have only one question regarding Thrust policy handling.
| } | ||
|
|
||
| // --- Device (global) --- | ||
| // Invalidate the cached thrust policy (the resource_ref it captured |
There was a problem hiding this comment.
Do we need to invalidate this again during destruction of the memory_stats_resources?
There was a problem hiding this comment.
No, unlike the device memory resource, the thrust policy is a local resource object, so it dies out with its owning raft::resources handle.
achirkin
force-pushed
the
enh-memory-resources
branch
from
aa09273 to
053b0c5
Compare
achirkin
added
breaking
|
Moving this to 26.08 as 26.06 deadline is missed |
4 participants
statistics_adaptor.hppto compile via nvcc (see code comment)memory_stats_resources. In constrast tomemory_tracking_resourcesthis doesn't stream the memory usage online, but only reports the overall usage metrics.breaking change: moved raft/util/memory_tracking_resources.hpp -> raft/core/memory_tracking_resources.hpp