Update RMM memory resource APIs to ref-based equivalents#2920
Closed
bdice wants to merge 5 commits intorapidsai:mainfrom
Closed
Update RMM memory resource APIs to ref-based equivalents#2920bdice wants to merge 5 commits intorapidsai:mainfrom
bdice wants to merge 5 commits intorapidsai:mainfrom
Conversation
Replace pointer-based RMM APIs (which will soon be deprecated) with ref-based alternatives as part of the RMM CCCL 3.2 memory resource migration: - device_resources_manager.hpp: set_current_device_resource() → set_current_device_resource_ref() - benchmark.hpp: Update using_pool_memory_res to use ref-based APIs - gather.cu: Update to use ref-based APIs for memory resource management - subsample.cu: Update to use ref-based APIs for memory resource management - buffer.cpp: set_current_device_resource() → set_current_device_resource_ref()
44 tasks
Change mem_resource_ from device_async_resource_ref to cuda::mr::any_resource to ensure the memory resource lifetime is properly managed. This is part of the RMM CCCL 3.2 migration.
Contributor
|
This seems like a change with potential for unexpected side-effects. I'd urge to merge this asap if it cannot be delayed to 26.04. |
Member
|
The changes look good to me, there is one CI failure that looks fairly minor: [ RUN ] DeviceResourcesManager.ObeysSetters
/tmp/conda-bld-output/bld/rattler-build_libraft-headers-only/work/cpp/tests/core/device_resources_manager.cpp:111: Failure
Expected: (mr) != (nullptr), actual: NULL vs (nullptr)
/tmp/conda-bld-output/bld/rattler-build_libraft-headers-only/work/cpp/tests/core/device_resources_manager.cpp:111: Failure
Expected: (mr) != (nullptr), actual: NULL vs (nullptr)but I agree with @csadorf that I'd rather be conservative and aim this to |
Contributor
Author
|
This should target 26.04, I’ll update you after I’ve had time to figure out the errors in this and other repositories. |
achirkin
added
3 - Ready for Review
non-breaking
Contributor
Author
|
Closing — superseded by #2996, which includes these changes as part of the full RMM CCCL memory resource migration (rapidsai/rmm#2011). |
github-project-automation
bot
moved this from In Progress
to Done
in Unstructured Data Processing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
device_resources_manager.hpp:set_current_device_resource()→set_current_device_resource_ref()benchmark.hpp: Updateusing_pool_memory_resto use ref-based APIsgather.cu: Use ref-based APIs for memory resource managementsubsample.cu: Use ref-based APIs for memory resource managementbuffer.cpp:set_current_device_resource()→set_current_device_resource_ref()