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.

Sign up for GitHub

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

Jump to bottom

Change device_scalar to take async_resource_ref #1447

Merged
merged 6 commits into from
Feb 2, 2024
Merged

Change device_scalar to take async_resource_ref #1447

merged 6 commits into from
Feb 2, 2024

Conversation

miscco
Copy link
Contributor

@miscco miscco commented Jan 30, 2024

This rewrite device_scalar to take async_resource_ref instead of a plain device_memory_resource*. This is completely opaque to the user as the underlying device_uvector already takes a async_resource_ref

Checklist

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

closes #1446

@miscco miscco requested a review from a team as a code owner January 30, 2024 13:45
@miscco miscco requested review from rongou and jrhemstad January 30, 2024 13:45
@github-actions github-actions bot added the cpp Pertains to C++ code label Jan 30, 2024
@miscco miscco force-pushed the refactor_device_scalar branch from 6288b8c to 1db2eae Compare January 30, 2024 13:53
@miscco miscco added the feature request New feature or request label Jan 30, 2024
@miscco
Move rmm::device_scalar over to cuda::mr::async_resource_ref
c7288cb 
This rewrites `rmm::device_scalar` to take a `cuda::mr::async_resource_ref` instead of a plain `rmm::device_memory_resource*`.

This is completely opaque to the user as the underlying `rmm::device_uvector` already takes a `cuda::mr::async_resource_ref`

Addresses rapidsai#1446
@miscco miscco force-pushed the refactor_device_scalar branch from 1db2eae to c7288cb Compare January 30, 2024 13:57
@miscco miscco requested a review from harrism January 30, 2024 14:24
@miscco miscco added the non-breaking Non-breaking change label Jan 30, 2024
harrism
harrism approved these changes Jan 30, 2024
View reviewed changes
Copy link
Member

@harrism harrism left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Thanks for finishing this low-hanging fruit!

If we merge this before #1441 we should update that PR to update this file.

include/rmm/device_scalar.hpp Outdated Show resolved Hide resolved
@harrism harrism changed the title Move device_scalar over to async_resource_ref Changedevice_scalar to take async_resource_ref Jan 30, 2024
@harrism harrism changed the title Changedevice_scalar to take async_resource_ref Change device_scalar to take async_resource_ref Jan 30, 2024
@miscco
Copy link
Contributor Author

miscco commented Feb 2, 2024

/merge

@rapids-bot rapids-bot bot merged commit b85f482 into rapidsai:branch-24.04 Feb 2, 2024
47 checks passed
@miscco miscco deleted the refactor_device_scalar branch February 2, 2024 08:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@harrism harrism harrism approved these changes

@rongou rongou rongou approved these changes

@jrhemstad jrhemstad Awaiting requested review from jrhemstad jrhemstad is a code owner automatically assigned from rapidsai/rmm-cpp-codeowners

Assignees

@miscco miscco

Labels
cpp Pertains to C++ code feature request New feature or request non-breaking Non-breaking change
Projects
RMM Project Board
Status: Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Rewrite rmm::device_scalar to take a cuda::mr::async_resource_ref instead of a device_memory_resource*
3 participants
@miscco @rongou @harrism