Add CMake option for disabling legacy MR interface #2089
Conversation
|
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
|
/ok to test |
bdice
added
feature request
bdice
changed the title
|
Marked as |
|
I verified that all of RAPIDS builds with this set of changes. ✅ Removing the I'll merge this late in the day to ensure we get a fresh set of nightly builds. |
| std::size_t alignment = rmm::RMM_DEFAULT_HOST_ALIGNMENT) noexcept | ||
| { | ||
| return deallocate_async(ptr, bytes, alignment, stream); | ||
| RMM_FUNC_RANGE(); |
There was a problem hiding this comment.
Is this worthwhile given that deallocate_sync also has RMM_FUNC_RANGE?
There was a problem hiding this comment.
I will be refactoring this MR implementation separately as part of #2090. Punting on this.
| return this->deallocate_sync(ptr, bytes, alignment); | ||
| } | ||
| #endif // RMM_ENABLE_LEGACY_MR_INTERFACE | ||
|
|
There was a problem hiding this comment.
OK so the idea is that the cccl_resource_ref models our old setup where allocate -> synchronous and allocate_async -> stream-ordered.
Whereas async_cccl_resource_ref is allocate_sync -> synchronous and allocate -> stream-ordered.
Since currently we assume allocate is synchronous we need to adapt everyone to that first. And the way to do that is to migrate everyone using allocate to use allocate_sync. Then we can move them onto the async_cccl_resource_ref concept and then we can move sync allocations that could be async back to allocate?
There was a problem hiding this comment.
Generally that's the right direction.
The migration here is actually a little easier than stated above, because the signature for allocate changed too (in addition to changing sync-to-async). Disabling the "legacy" interface will cause a compile error anywhere the old interface was being used, allowing us to migrate to the new API names and new parameter order in each RAPIDS repository. I am starting that migration now. :)
Once that migration is complete, I will deprecate the "legacy" interface (at which point RAPIDS should not be using the legacy interface at all), then remove it in the subsequent release.
|
I forgot that I need to do a bit more internal refactoring before this is ready to merge. I would like for this PR to build with the legacy interface enabled or disabled, and I will need to try rebuilding all of RAPIDS again (only with the legacy interface enabled) once I finish that refactoring. Then I can start working on refactoring downstream libraries to use the new interface. |
4 participants
Description
This is the next step in #2011. This PR makes the current (soon to be legacy) memory resource interface configurable. If the CMake option
RMM_ENABLE_LEGACY_MR_INTERFACEis disabled, then legacy methods will not be exposed. This will allow us to incrementally update RAPIDS until everything builds with the legacy interface disabled.Currently my plan is:
RMM_ENABLE_LEGACY_MR_INTERFACEon or off.This PR will cover step (2) and allow us to move on to step (3) of the proposed plan for CCCL MR adoption. See #2011 (comment).
The only breaking change is that RMM no longer supports CCCL versions before 3.1.0. Everything else in this PR is non-breaking.
Checklist