Refactor fixed_size_memory_resource and binning_memory_resource to shared CCCL MR design#2264
Merged
bdice merged 3 commits intorapidsai:stagingfrom Feb 26, 2026
Merged
Conversation
…ared CCCL MR design Converts both resources from header-only class templates to non-template classes backed by detail::*_impl held via cuda::mr::shared_resource, following the pattern established by logging_resource_adaptor (rapidsai#2246) and pool_memory_resource (rapidsai#2258). Part of rapidsai#2011.
|
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. |
bdice
commented
Feb 26, 2026
| auto cuda_async_mr = make_cuda_async(); | ||
| // Add a binning_memory_resource with fixed-size bins of sizes 256, 512, 1024, 2048 and 4096KiB | ||
| // Larger allocations will use the CUDA async resource | ||
| // Larger allocations will use the current device resource |
Collaborator
Author
There was a problem hiding this comment.
We should avoid current device resource usage in these tests. Maybe we can use rmm::mr::cuda_memory_resource.
Collaborator
Author
There was a problem hiding this comment.
This isn't going to work cleanly until we finish the CCCL migration. We can create a ref via device_memory_resource_view but that has issues with lifetimes until we use CCCL refs properly. Marking this as a TODO in #2011.
bdice
commented
Feb 26, 2026
bdice
added
breaking
bdice
requested review from
PointKernel and
miscco
and removed request for
a team
Collaborator
Author
|
I self-reviewed this PR which was all generated code. I'm going to merge this soon, all seems aligned with my expectations from the past two migration PRs. |
bdice
added a commit
that referenced
this pull request
Mar 6, 2026
## Summary - De-template `arena_memory_resource` by removing the `Upstream` template parameter - Split implementation into `detail::arena_memory_resource_impl` held via `cuda::mr::shared_resource` for reference-counted, copyable ownership - Retain the `device_memory_resource` legacy compatibility layer (`do_allocate`, `do_deallocate`, `do_is_equal`) - Update benchmarks, C++ tests, and Python (Cython) bindings to use the non-template `arena_memory_resource` This follows the same pattern established in #2246, #2258, #2264, and #2265 for the other memory resources. ### New files | File | Contents | |------|----------| | `cpp/include/rmm/mr/detail/arena_memory_resource_impl.hpp` | `detail::arena_memory_resource_impl` class declaration | | `cpp/src/mr/detail/arena_memory_resource_impl.cpp` | Impl member function definitions | | `cpp/src/mr/arena_memory_resource.cpp` | Outer class constructor + delegating method definitions | ### Modified files | File | Change | |------|--------| | `cpp/include/rmm/mr/arena_memory_resource.hpp` | De-template, `shared_resource` wrapping | | `cpp/CMakeLists.txt` | Add new `.cpp` source files | | `cpp/tests/mr/arena_mr_tests.cpp` | `arena_memory_resource<device_memory_resource>` → `arena_memory_resource` | | `cpp/tests/mr/mr_ref_arena_tests.cpp` | Add `ArenaMRFixture` + `CcclMrRefTest`/`CcclMrRefAllocationTest`/`CcclMrRefTestMT` instantiations | | `cpp/tests/mr/mr_ref_test.hpp` | Update `make_arena()` and `arena_mr` type alias | | `cpp/tests/mr/cccl_adaptor_tests.cpp` | Add arena `static_assert` + `ArenaMRAdaptorTest` | | `cpp/benchmarks/multi_stream_allocations/multi_stream_allocations_bench.cu` | Update `make_arena()` | | `cpp/benchmarks/random_allocations/random_allocations.cpp` | Update `make_arena()` | | `cpp/benchmarks/replay/replay.cpp` | Update `make_arena()` | | `python/rmm/rmm/librmm/memory_resource.pxd` | Remove `[Upstream]` template from `arena_memory_resource` | | `python/rmm/rmm/pylibrmm/memory_resource/_memory_resource.pyx` | Remove template instantiation syntax for arena |
44 tasks
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
1 participant
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
Converts
fixed_size_memory_resourceandbinning_memory_resourcefrom header-only class templates to non-template classes backed by adetail::*_implheld viacuda::mr::shared_resource. Follows the pattern established bylogging_resource_adaptor(#2246) andpool_memory_resource(#2258). Part of #2011.Changes
New files (6)
cpp/include/rmm/mr/detail/fixed_size_memory_resource_impl.hpp— impl class declaration; inheritsstream_ordered_memory_resource<impl, fixed_size_free_list>(same CRTP pattern as pool)cpp/src/mr/detail/fixed_size_memory_resource_impl.cpp— impl member definitionscpp/src/mr/fixed_size_memory_resource.cpp— outer class constructor and delegating methodscpp/include/rmm/mr/detail/binning_memory_resource_impl.hpp— impl class declaration; includesfixed_size_memory_resource.hppforunique_ptrmembercpp/src/mr/detail/binning_memory_resource_impl.cpp— impl member definitionscpp/src/mr/binning_memory_resource.cpp— outer class constructor and delegating methodsModified files (11)
cpp/include/rmm/mr/fixed_size_memory_resource.hpp— de-templated;shared_resourceinheritance;device_async_resource_refconstructor only;static_assertfor conceptcpp/include/rmm/mr/binning_memory_resource.hpp— de-templated; same pattern;Upstream*constructors removedcpp/CMakeLists.txt— four new.cppfiles added to library sourcespython/rmm/rmm/librmm/memory_resource.pxd— template parameters removed from both declarationspython/rmm/rmm/pylibrmm/memory_resource/_memory_resource.pyx— template instantiation syntax removed fromnewexpressionscpp/tests/mr/mr_ref_fixed_size_tests.cpp— replaced string-factory suite with typedFixedSizeMRFixture+CcclMrRefTestcpp/tests/mr/mr_ref_binning_tests.cpp— replaced string-factory suites with typedBinningMRFixture+ all threeCcclMrRef*suitescpp/tests/mr/mr_ref_test.hpp—make_fixed_size/make_binningfactory helpers rewritten withoutowning_wrapper; type aliases de-templatedcpp/tests/mr/binning_mr_tests.cpp— removed explicit template instantiation andThrowOnNullUpstream(null pointer constructor no longer exists); updatedExplicitBinMRto usedevice_async_resource_refcpp/tests/mr/cccl_adaptor_tests.cpp— addedfixed_size_memory_resourceandbinning_memory_resourceto the shared-ownership typed test suitecpp/tests/mr/thrust_allocator_tests.cu— removed"Binning"from the string-dispatch parameterization (coverage moved toBINNING_MR_REF_*typed suites; the old dispatch path caused a dangling ref crash — the exact bug this PR fixes)Breaking changes
fixed_size_memory_resource<Upstream>→fixed_size_memory_resource(template parameter removed)binning_memory_resource<Upstream>→binning_memory_resource(template parameter removed)Upstream*constructor overloads removed; usedevice_async_resource_refTesting
build-rmm-cpp -j0 && test-rmm-cpp: 89/89 tests pass.