Skip to content

Removing cudax components#856

Merged
rapids-bot[bot] merged 3 commits intorapidsai:mainfrom
nirandaperera:removing_cudax
Feb 12, 2026
Merged

Removing cudax components#856
rapids-bot[bot] merged 3 commits intorapidsai:mainfrom
nirandaperera:removing_cudax

Conversation

@nirandaperera
Copy link
Copy Markdown
Contributor

@nirandaperera nirandaperera commented Feb 11, 2026

This PR removes the cudax components from the build because from CCCL 3.2+ the memory pools have been moved to libcudaxx. Also removes the pimpl idiom from PinnedMemoryResouce.

cmake/thirdparty/get_cccl_cudax.cmake is still kept for future use.

@nirandaperera nirandaperera requested review from a team as code owners February 11, 2026 22:35
@nirandaperera nirandaperera added breaking Introduces a breaking change improvement Improves an existing functionality labels Feb 11, 2026
Copy link
Copy Markdown
Member

@pentschev pentschev left a comment

Choose a reason for hiding this comment

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

Generally looks good to me. I left a question, but it doesn't need to be addressed in this PR.

Comment on lines +20 to +33
cuda::memory_pool_properties get_memory_pool_properties() {
return cuda::memory_pool_properties{
// It was observed that priming async pools have little effect for performance.
// See <https://github.com/rapidsai/rmm/issues/1931>.
.initial_pool_size = 0,
// Before <https://github.com/NVIDIA/cccl/pull/6718>, the default
// `release_threshold` was 0, which defeats the purpose of having a pool. We
// now set it so the pool never releases unused pinned memory.
.release_threshold = std::numeric_limits<size_t>::max(),
// This defines how the allocations can be exported (IPC). See the docs of
// `cudaMemPoolCreate` in <https://docs.nvidia.com/cuda/cuda-runtime-api>.
.allocation_handle_type = ::cudaMemAllocationHandleType::cudaMemHandleTypeNone
};
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Do we want to allow the user to configure those?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is coming as part of #851

@nirandaperera
Copy link
Copy Markdown
Contributor Author

/merge

@rapids-bot rapids-bot bot merged commit 6ef1a80 into rapidsai:main Feb 12, 2026
169 of 175 checks passed
Comment on lines +172 to +178
// We cannot assign cuda::pinned_memory_pool directly to device_async_resource_ref /
// host_async_resource_ref: the ref only stores a pointer, but its constructor
// requires the referenced type to be copyable and movable (CCCL __basic_any_ref
// constraint). pinned_memory_pool is neither, so we wrap it in PinnedMemoryResource,
// which holds the pool in a shared_ptr and is copyable and movable. Copies share
// the same pool (is_equal compares pool_ pointers).
std::shared_ptr<cuda::pinned_memory_pool> pool_;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Aha, the magic you were looking for was storing a

cuda::mr::any_resource<cuda::mr::device_accessible, cuda::mr::host_accessible>

And then you should make the pool with:

cuda::mr::make_shared_resource<cuda::pinned_memory_pool>(...);

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It seems any_resource requires the Resource to be copyable 😞
https://nvidia.slack.com/archives/CCP05T27R/p1771366256232879

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@wence- Based on the comments I received for the above slack thread, I changed this to use a shared_resource<pinned_memory_pool> here 67fa83c

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

breaking Introduces a breaking change improvement Improves an existing functionality

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants