Skip to content

Default libcudf tests to CUDA async allocation - #24595

Open
vyasr wants to merge 3 commits into
NVIDIA:mainfrom
vyasr:codex/async-cpp-test-reservations
Open

vyasr wants to merge 3 commits into
NVIDIA:mainfrom
vyasr:codex/async-cpp-test-reservations

Conversation

@vyasr

@vyasr vyasr commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

Description

Default libcudf's test harness to CUDA async allocation instead of the pool allocator, avoiding each test process's up-front reservation of 10% of device memory. Explicit allocator overrides, CTest GPU reservations, and test coverage are unchanged.

Local allocator A/B measurements on a 32 GiB V100 reduced peak device memory from approximately 29.2 GiB to 19.0 GiB (35%). Runtime measurements did not establish a clear allocator-only speedup; lower memory usage is the benefit of this change.

Checklist

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

@vyasr vyasr added tests Unit testing for project libcudf Affects libcudf (C++/CUDA) code. CMake CMake build issue improvement Improvement / enhancement to an existing function non-breaking Non-breaking change ci labels Oct 14, 2026
@copy-pr-bot

copy-pr-bot Bot commented Oct 14, 2026

Copy link
Copy Markdown

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.

@vyasr

vyasr commented Oct 14, 2026

Copy link
Copy Markdown
Contributor Author

/ok to test

@vyasr vyasr changed the title Use async allocation and tune libcudf test GPU reservations Use async allocation by default in libcudf tests Oct 16, 2026
@vyasr

vyasr commented Oct 16, 2026

Copy link
Copy Markdown
Contributor Author

/ok to test

@vyasr
vyasr force-pushed the codex/async-cpp-test-reservations branch from 50bd130 to 4f00edb Compare October 16, 2026 18:29
@vyasr

vyasr commented Oct 16, 2026

Copy link
Copy Markdown
Contributor Author

/ok to test

@vyasr
vyasr force-pushed the codex/async-cpp-test-reservations branch from 4f00edb to 4a8c4c7 Compare November 2, 2026 20:03
@vyasr vyasr changed the title Use async allocation by default in libcudf tests Default libcudf tests to CUDA async allocation Nov 2, 2026
@vyasr

vyasr commented Nov 2, 2026

Copy link
Copy Markdown
Contributor Author

/ok to test

@vyasr
vyasr marked this pull request as ready for review November 2, 2026 20:03
@vyasr
vyasr requested a review from a team as a code owner November 2, 2026 20:03
@vyasr
vyasr requested review from PointKernel and wence- November 2, 2026 20:03
@coderabbitai

coderabbitai Bot commented Nov 2, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Important

Review skipped

Review was skipped as selected files did not have any reviewable changes.

Run configuration
  • Source mode: Auto
  • Repository: NVIDIA/cudf
  • Branch: main
  • File: .coderabbit.yaml
  • Commit: ff6c4ad
  • Layers: Repository YAML
  • Review profile: CHILL
  • Plan: Enterprise
  • Run ID: 4c60ef3e-efbe-457e-9391-787ed84b72cf
Commits

Reviewing files that changed from the base of the PR and between 26e9c83 and 4c89dbe.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • Trigger review

No actionable comments were generated in the recent review.

Summary

Summary by CodeRabbit

  • Chores
    • The default memory allocation mode is now asynchronous when no mode is specified. An explicitly configured mode continues to take precedence. This change affects the default behavior of test runs that do not set a memory allocation mode.

The testing header now documents and uses "async" as the default RMM allocation mode when GTEST_CUDF_RMM_MODE is unset. An environment-provided mode still takes precedence.

Changes: RMM default mode

Layer / File(s) Summary
Document and set the default mode
cpp/include/cudf_test/testing_main.hpp
The comment and fallback value now use "async" instead of "pool". The environment-provided mode remains unchanged.

Estimated code review effort: 1 (Trivial) | ~2 minutes

Change: Other

Suggested reviewers: bdice, pointkernel

Merge Risk: Minimal · previous review 4a8c4

No actionable merge-blocking risk is evident in the reviewed change.

Pre-merge checks: Passed 5
Check name Status Explanation
Title check Passed The title "Default libcudf tests to CUDA async allocation" matches the main change. The raw summary says the default RMM allocation mode for libcudf tests changed from pool to async in cpp/include/cud…
Description check Passed The description states that the libcudf test harness now defaults to CUDA async allocation instead of the pool allocator. It explains the reason (avoiding up-front reservation of 10% of device memory)…
Docstring Coverage Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Title check

Explanation

The title "Default libcudf tests to CUDA async allocation" matches the main change. The raw summary says the default RMM allocation mode for libcudf tests changed from pool to async in cpp/include/cudf_test/testing_main.hpp, and an environment-provided mode still takes precedence. The title is short, specific, and states the primary change.

Full details: Description check

Explanation

The description states that the libcudf test harness now defaults to CUDA async allocation instead of the pool allocator. It explains the reason (avoiding up-front reservation of 10% of device memory), notes that explicit overrides and CTest GPU reservations are unchanged, and reports local memory measurements. This matches the raw summary's change to testing_main.hpp.

Full details: Docstring Coverage

Explanation

Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 1 functions across 1 files.

Finishing Touches
Generate unit tests (beta)
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Tip

🎯 Customize your risk assessments

Guide how CodeRabbit assesses merge risk and security architecture risk. Highlight sensitive changes and explain your security boundaries using Merge Risk Instructions and Security Risk Instructions in your review settings. Security architecture risk assessments require the Advanced plan or above.

✅ Automatic approvals, with your rules

Let CodeRabbit approve pull requests once required threads are resolved, pre-merge checks pass, and your risk limits are met. Enable Automatic approvals in review settings, with overrides for teams or file paths.

CodeRabbit Verify

  • Run CodeRabbit Verify to test the changes in this pull request.
Recent review info
Run configuration
  • Source mode: Auto
  • Repository: NVIDIA/cudf
  • Branch: main
  • File: .coderabbit.yaml
  • Commit: 4abb25f
  • Layers: Repository YAML
  • Review profile: CHILL
  • Plan: Enterprise
  • Run ID: 8d8a71d4-1d67-4e0a-8450-6fa86d7b4a84
Commits

Reviewing files that changed from the base of the PR and between 4abb25f and 4a8c4c7.

Files selected for processing: 1
  • cpp/include/cudf_test/testing_main.hpp

Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 11 remain after this review.

@vyasr
vyasr force-pushed the codex/async-cpp-test-reservations branch from 4a8c4c7 to 26e9c83 Compare November 3, 2026 00:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci CMake CMake build issue improvement Improvement / enhancement to an existing function libcudf Affects libcudf (C++/CUDA) code. non-breaking Non-breaking change tests Unit testing for project

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants