Skip to content

Commit d5df4c5

Browse files
authored
Compatibility with CCCL 3.2 (#755)
This PR introduces some fixes needed for CCCL 3.2. The noteworthy changes are: - Pinned memory resource must be copyable (required by CCCL 3.2's memory resource concepts), which means the "PIMPL" idiom must use a `shared_ptr` to make the resource copyable - Header/namespace updates (pinned MR is no longer experimental) - Resource adaptor comparison does not work with `optional` types due to constraints in the CCCL resource type machinery. This requires a workaround where optionals are not compared directly, but instead checked for `has_value()` before comparing their underlying objects. It may be possible to improve this as I continue refactoring RMM's bindings around CCCL memory resources. Authors: - Bradley Dice (https://github.com/bdice) Approvers: - Niranda Perera (https://github.com/nirandaperera) URL: #755
1 parent ec84d83 commit d5df4c5

3 files changed

Lines changed: 41 additions & 6 deletions

File tree

cpp/include/rapidsmpf/memory/pinned_memory_resource.hpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -149,7 +149,7 @@ class PinnedMemoryResource final : public HostMemoryResource {
149149
// using PImpl idiom to hide cudax .cuh headers from rapidsmpf. cudax cuh headers will
150150
// only be used by the impl in .cu file.
151151
struct PinnedMemoryResourceImpl;
152-
std::unique_ptr<PinnedMemoryResourceImpl> impl_;
152+
std::shared_ptr<PinnedMemoryResourceImpl> impl_;
153153
};
154154

155155
static_assert(cuda::mr::resource<PinnedMemoryResource>);

cpp/src/memory/pinned_memory_resource.cu

Lines changed: 27 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,23 +7,41 @@
77

88
#include <cuda_runtime_api.h>
99

10-
#include <cuda/memory_resource>
11-
1210
#include <rmm/resource_ref.hpp>
1311

1412
#include <rapidsmpf/error.hpp>
1513
#include <rapidsmpf/memory/pinned_memory_resource.hpp>
1614
#include <rapidsmpf/utils.hpp>
1715

1816
#if RAPIDSMPF_CUDA_VERSION_AT_LEAST(RAPIDSMPF_PINNED_MEM_RES_MIN_CUDA_VERSION)
17+
#if CCCL_MAJOR_VERSION > 3 || (CCCL_MAJOR_VERSION == 3 && CCCL_MINOR_VERSION >= 2)
18+
#include <cuda/memory_resource>
19+
#else
1920
#include <cuda/experimental/memory_resource.cuh>
2021
#endif
22+
#endif
2123

2224
namespace rapidsmpf {
2325

2426
#if RAPIDSMPF_CUDA_VERSION_AT_LEAST(RAPIDSMPF_PINNED_MEM_RES_MIN_CUDA_VERSION)
2527

2628
namespace {
29+
#if CCCL_MAJOR_VERSION > 3 || (CCCL_MAJOR_VERSION == 3 && CCCL_MINOR_VERSION >= 2)
30+
cuda::memory_pool_properties get_memory_pool_properties() {
31+
return cuda::memory_pool_properties{
32+
// It was observed that priming async pools have little effect for performance.
33+
// See <https://github.com/rapidsai/rmm/issues/1931>.
34+
.initial_pool_size = 0,
35+
// Before <https://github.com/NVIDIA/cccl/pull/6718>, the default
36+
// `release_threshold` was 0, which defeats the purpose of having a pool. We
37+
// now set it so the pool never releases unused pinned memory.
38+
.release_threshold = std::numeric_limits<size_t>::max(),
39+
// This defines how the allocations can be exported (IPC). See the docs of
40+
// `cudaMemPoolCreate` in <https://docs.nvidia.com/cuda/cuda-runtime-api>.
41+
.allocation_handle_type = ::cudaMemAllocationHandleType::cudaMemHandleTypeNone
42+
};
43+
}
44+
#else
2745
cuda::experimental::memory_pool_properties get_memory_pool_properties() {
2846
return cuda::experimental::memory_pool_properties{
2947
// It was observed that priming async pools have little effect for performance.
@@ -39,6 +57,7 @@ cuda::experimental::memory_pool_properties get_memory_pool_properties() {
3957
cuda::experimental::cudaMemAllocationHandleType::cudaMemHandleTypeNone
4058
};
4159
}
60+
#endif
4261
} // namespace
4362

4463
struct PinnedMemoryResource::PinnedMemoryResourceImpl {
@@ -55,8 +74,13 @@ struct PinnedMemoryResource::PinnedMemoryResourceImpl {
5574
resource.deallocate(stream, ptr, bytes, alignment);
5675
}
5776

77+
#if CCCL_MAJOR_VERSION > 3 || (CCCL_MAJOR_VERSION == 3 && CCCL_MINOR_VERSION >= 2)
78+
cuda::pinned_memory_pool pool;
79+
cuda::pinned_memory_pool_ref resource;
80+
#else
5881
cuda::experimental::pinned_memory_pool pool;
5982
cuda::experimental::pinned_memory_resource resource;
83+
#endif
6084
};
6185
#else // CUDA_VERSION < RAPIDSMPF_PINNED_MEM_RES_MIN_CUDA_VERSION
6286

@@ -77,7 +101,7 @@ struct PinnedMemoryResource::PinnedMemoryResourceImpl {
77101
#endif
78102

79103
PinnedMemoryResource::PinnedMemoryResource(int numa_id)
80-
: impl_(std::make_unique<PinnedMemoryResourceImpl>(numa_id)) {
104+
: impl_(std::make_shared<PinnedMemoryResourceImpl>(numa_id)) {
81105
RAPIDSMPF_EXPECTS(
82106
is_pinned_memory_resources_supported(),
83107
"PinnedMemoryResource is not supported for CUDA versions "

cpp/src/rmm_resource_adaptor.cpp

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -126,8 +126,19 @@ bool RmmResourceAdaptor::do_is_equal(
126126
if (cast == nullptr) {
127127
return false;
128128
}
129-
return get_upstream_resource() == cast->get_upstream_resource()
130-
&& get_fallback_resource() == cast->get_fallback_resource();
129+
// Manual comparison of optionals to avoid recursive constraint satisfaction in
130+
// CCCL 3.2. std::optional::operator== triggers infinite concept checking when the
131+
// wrapped type (rmm::device_async_resource_ref) inherits from CCCL's concept-based
132+
// resource_ref.
133+
// TODO: Revert this after the RMM resource ref types are replaced with
134+
// plain cuda::mr ref types. This depends on
135+
// https://github.com/rapidsai/rmm/issues/2011.
136+
auto this_fallback = get_fallback_resource();
137+
auto other_fallback = cast->get_fallback_resource();
138+
bool fallbacks_equal =
139+
(this_fallback.has_value() == other_fallback.has_value())
140+
&& (!this_fallback.has_value() || (*this_fallback == *other_fallback));
141+
return get_upstream_resource() == cast->get_upstream_resource() && fallbacks_equal;
131142
}
132143

133144
} // namespace rapidsmpf

0 commit comments

Comments
 (0)