Skip to content

Use explicit resources in memory resource tests#2389

Merged
rapids-bot[bot] merged 2 commits into
rapidsai:mainfrom
bdice:update-memory-resource-tests-26.06
May 22, 2026
Merged

Use explicit resources in memory resource tests#2389
rapids-bot[bot] merged 2 commits into
rapidsai:mainfrom
bdice:update-memory-resource-tests-26.06

Conversation

@bdice
Copy link
Copy Markdown
Collaborator

@bdice bdice commented May 17, 2026

Description

Part of #2011.

Updates memory resource ref tests to avoid using get_current_device_resource_ref() as generic upstream setup. The affected factory helpers and CCCL fixture tests now construct resources with explicit rmm::mr::cuda_memory_resource upstreams.

Current-resource tests that specifically exercise get_current_device_resource_ref() / set_current_device_resource() behavior are left unchanged.

Checklist

  • I am familiar with the Contributing Guidelines.
  • New or existing tests cover these changes.
  • The documentation is up to date with these changes.

@copy-pr-bot
Copy link
Copy Markdown

copy-pr-bot Bot commented May 17, 2026

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 bdice moved this to In Progress in RMM Project Board May 17, 2026
@bdice bdice self-assigned this May 17, 2026
@bdice bdice added non-breaking Non-breaking change improvement Improvement / enhancement to an existing function labels May 17, 2026
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 17, 2026

Review Change Stack

📝 Walkthrough

Walkthrough

All memory resource test fixtures in RMM's test suite are refactored to construct resource adaptors from an explicit local rmm::mr::cuda_memory_resource instance instead of using the global get_current_device_resource_ref(), reducing test coupling to global device state.

Changes

Test memory resource fixture refactoring

Layer / File(s) Summary
Test helper base and factory functions
cpp/tests/mr/mr_ref_test.hpp
Test factories make_arena, make_fixed_size, and make_binning are updated to construct wrapped resources with explicit cuda_memory_resource{} instances; base fixtures mr_factory_base and mr_ref_test add default_mr members and initialize resource references from that member.
Individual memory resource test fixtures updated
cpp/tests/mr/mr_ref_aligned_tests.cpp, cpp/tests/mr/mr_ref_arena_tests.cpp, cpp/tests/mr/mr_ref_binning_tests.cpp, cpp/tests/mr/mr_ref_failure_callback_tests.cpp, cpp/tests/mr/mr_ref_fixed_size_tests.cpp, cpp/tests/mr/mr_ref_limiting_tests.cpp, cpp/tests/mr/mr_ref_pool_tests.cpp, cpp/tests/mr/mr_ref_prefetch_tests.cpp, cpp/tests/mr/mr_ref_statistics_tests.cpp, cpp/tests/mr/mr_ref_thread_safe_tests.cpp, cpp/tests/mr/mr_ref_tracking_tests.cpp
Each test fixture consistently adds a local rmm::mr::cuda_memory_resource upstream member, updates includes to add cuda_memory_resource.hpp and remove per_device_resource.hpp, and reinitializes wrapped memory resources to use the local upstream instead of get_current_device_resource_ref().

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

  • rapidsai/rmm#2361: The primary memory resource migration PR that continues with updates to test fixtures, applying the same pattern of replacing global device resource references with locally owned cuda_memory_resource instances across the memory resource test suite.

Suggested labels

non-breaking, improvement

Suggested reviewers

  • harrism
  • wence-
  • ttnghia
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: updating memory resource tests to use explicit resources instead of relying on get_current_device_resource_ref().
Linked Issues check ✅ Passed The PR fully addresses the post-task objective from #2011 to use explicit cuda_mr resources in tests rather than get_current_device_resource_ref(), updating all affected test fixtures and factory helpers consistently.
Out of Scope Changes check ✅ Passed All changes are scoped to memory resource tests: 11 test files and 1 test header file were updated to use explicit cuda_memory_resource, with no unrelated modifications.
Description check ✅ Passed The pull request description accurately relates to the changeset, explaining the migration from using get_current_device_resource_ref() to explicit cuda_memory_resource upstreams across multiple memory resource tests.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands and usage tips.

@bdice bdice changed the base branch from release/26.06 to main May 17, 2026 15:46
@bdice bdice marked this pull request as ready for review May 21, 2026 14:19
@bdice bdice requested a review from a team as a code owner May 21, 2026 14:19
@bdice bdice requested review from PointKernel and ttnghia May 21, 2026 14:19
@bdice
Copy link
Copy Markdown
Collaborator Author

bdice commented May 22, 2026

/merge

@rapids-bot rapids-bot Bot merged commit 38b8581 into rapidsai:main May 22, 2026
85 checks passed
@github-project-automation github-project-automation Bot moved this from In Progress to Done in RMM Project Board May 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

improvement Improvement / enhancement to an existing function non-breaking Non-breaking change

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants