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

[FEA] Add libcudf cudf::get_current_device_resource() wrapper for rmm::get_current_device_resource() #16676

Closed
harrism opened this issue Aug 28, 2024 · 1 comment · Fixed by #16679
Assignees
Labels
feature request New feature or request libcudf Affects libcudf (C++/CUDA) code.

Comments

@harrism
Copy link
Member

harrism commented Aug 28, 2024

Is your feature request related to a problem? Please describe.
I'm about to undertake yet another change to every function in libcudf that takes an MR parameter. To prevent doing this too many times in the future, I would like to provide a wrapper in libcudf to centralize calls to rmm::get_current_device_resource[_ref]() and an alias for RMM resource_ref type. hopefully this can reduce the number of files that need to change in the future when this functionality changes (e.g. if it moves out of RMM to CCCL).

Describe the solution you'd like
A header, say cpp/include/cudf/utilities/memory_resource.hpp. Please provide suggestions on the best place for this. Contents something like this:

namespace cudf {

using device_async_resource_ref = rmm::device_async_resource_ref;

inline device_async_resource_ref get_current_device_resource_ref() {
  return rmm::get_current_device_resource_ref();
}

inline device_async_resource_ref set_current_device_resource_ref(device_async_resource_ref mr) {
  return rmm::set_current_device_resource_ref(mr);
}

inline device_async_resource_ref reset_current_device_resource_ref() {
  return rmm::reset_current_device_resource_ref();
}

Describe alternatives you've considered

Additional context
rapidsai/rmm#1598

@harrism harrism added feature request New feature or request libcudf Affects libcudf (C++/CUDA) code. labels Aug 28, 2024
@harrism harrism self-assigned this Aug 28, 2024
@harrism harrism moved this from Todo to Review in RMM Project Board Aug 28, 2024
@vyasr
Copy link
Contributor

vyasr commented Aug 28, 2024

I would be in favor of this as being something analogous to our wrappers for default stream handling.

rapids-bot bot pushed a commit that referenced this issue Sep 10, 2024
Merge after rapidsai/rmm#1661

Creates and uses CUDF internal wrappers around RMM `current_device_resource` functions.

I've marked this PR as breaking because it breaks the ABI, however the API is compatible.

For reviewers, the most substantial additions are in the new file `<cudf/utilities/memory_resource.hpp>`, and in the `DEVELOPER_GUIDE.md` and `*.rst` docs. The rest are all replacements of an include and all calls to `rmm::get_current_device_resource()` with `cudf::get_current_device_resource_ref()`.

Closes #16676

Authors:
  - Mark Harris (https://github.com/harrism)

Approvers:
  - Nghia Truong (https://github.com/ttnghia)
  - GALI PREM SAGAR (https://github.com/galipremsagar)
  - https://github.com/nvdbaranec
  - David Wendt (https://github.com/davidwendt)

URL: #16679
@github-project-automation github-project-automation bot moved this from Review to Done in RMM Project Board Sep 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request New feature or request libcudf Affects libcudf (C++/CUDA) code.
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

2 participants