Skip to content
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

[EXPERIMENT] Use resource_ref for get/set_per_device_resource #1634

Draft
wants to merge 9 commits into
base: branch-24.10
Choose a base branch
from

Conversation

harrism
Copy link
Member

@harrism harrism commented Aug 1, 2024

Description

Components of this PR.

  1. C++: Use rmm::device_async_resource_ref in get/set_per_device_resource(). (per_device_resource.hpp)
  2. C++: Use the new API above in tests.
  3. C++: Modify resource adaptor classes to store the upstream resource as a resource_ref rather than an MR pointer. Add a constructor that takes a resource_ref. (MR pointer constructor to be deprecated)
  4. Python: Add minimal Cython bindings for device_async_resource_ref and cuda::stream_ref. Since we can't stack allocate device_async_resource_ref, add a get_ref() method on all Python DeviceMemoryResource classes that returns a shared_ptr[device_async_resource_ref]. This is dereferenced internally when passed to the bindings to the new cpp_set_per_device_resource. Refactor Cython for external resource classes.

This is currently an experiment.

Checklist

  • I am familiar with the Contributing Guidelines.
  • New or existing tests cover these changes.
  • The documentation is up to date with these changes.

@github-actions github-actions bot added CMake Python Related to RMM Python API cpp Pertains to C++ code labels Aug 1, 2024
@wence-
Copy link
Contributor

wence- commented Aug 1, 2024

I think the cython problems are not possible to work around right now :(, see cython/cython#6317

@harrism
Copy link
Member Author

harrism commented Aug 14, 2024

I think the cython problems are not possible to work around right now :(, see cython/cython#6317

I worked around them by using a shared_ptr[device_async_resource_ref] as discussed. I don't think the Python user ever needs to interact with this, so it's not too bad to deref it as needed.

@bdice
Copy link
Contributor

bdice commented Dec 17, 2024

I met with some folks from CCCL and cuda-python earlier this week to discuss memory resources. The new CCCL features in NVIDIA/cccl#2824 will change our strategy for owning memory resources. I will study this PR and will probably resurrect some parts of this in a new PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking Breaking change CMake cpp Pertains to C++ code feature request New feature or request Python Related to RMM Python API
Projects
Status: Blocked
Development

Successfully merging this pull request may close these issues.

3 participants