Skip to content

Convert headers' scratch rmm::device_uvector<T> to cuda::device_buffer<T> - #24975

Merged
rapids-bot[bot] merged 2 commits into
NVIDIA:mainfrom
KyleFromNVIDIA:remove-headers-internal-device_uvector
Nov 2, 2026
Merged

rapids-bot[bot] merged 2 commits into
NVIDIA:mainfrom
KyleFromNVIDIA:remove-headers-internal-device_uvector

Conversation

@KyleFromNVIDIA

Copy link
Copy Markdown
Member

Description

Use CCCL buffers instead of RMM vectors where possible in inline implementations in the headers.

Issue: rapidsai/build-planning#321

Checklist

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

…ffer<T>`

Use CCCL buffers instead of RMM vectors where possible in inline
implementations in the headers.

Issue: rapidsai/build-planning#321
@KyleFromNVIDIA KyleFromNVIDIA added the improvement Improvement / enhancement to an existing function label Nov 2, 2026
@KyleFromNVIDIA
KyleFromNVIDIA requested a review from a team as a code owner November 2, 2026 14:07
@KyleFromNVIDIA KyleFromNVIDIA added the non-breaking Non-breaking change label Nov 2, 2026
@github-actions github-actions Bot added the libcudf Affects libcudf (C++/CUDA) code. label Nov 2, 2026
@coderabbitai

coderabbitai Bot commented Nov 2, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review.

Recent review info
Run configuration
  • Source mode: Auto
  • Repository: NVIDIA/cudf
  • Branch: main
  • File: .coderabbit.yaml
  • Commit: f5cb11b
  • Layers: Repository YAML
  • Review profile: CHILL
  • Plan: Enterprise
  • Run ID: 623f521f-4cdb-4e2f-825d-8d37ee19b0fb
Commits

Reviewing files that changed from the base of the PR and between 6fea1f9 and 87da2ca.

Files selected for processing: 2
  • cpp/include/cudf/reduction/detail/approx_distinct_count.cuh
  • cpp/src/reductions/approx_distinct_count.cu

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

Summary

Summary by CodeRabbit

  • Refactor
    • Updated internal temporary storage across data gathering, reductions, string operations, and related workflows. Existing calculation, comparison, and output behavior remains unchanged.
    • No user-facing functionality changes are included in this update.

Internal storage and temporary allocations across detail, reduction, and strings code now use cuda::device_buffer instead of RMM device vectors. Approximate distinct-count sketch storage and its span-backed constructor were also updated.

Changes: Device buffer migration

Layer / File(s) Summary
Detail storage and temporary allocations
cpp/include/cudf/detail/device_scalar.hpp, cpp/include/cudf/detail/gather.cuh, cpp/include/cudf/detail/labeling/label_segments.cuh, cpp/include/cudf/detail/null_mask.cuh, cpp/include/cudf/detail/row_operator/primitive_lexicographic.cuh, cpp/include/cudf/detail/utilities/batched_memset.hpp
Device scalar storage and temporary allocations for gather, labeling, null masks, lexicographic comparison, and batched memset now use CUDA device buffers.
Reduction storage
cpp/include/cudf/reduction/detail/approx_distinct_count.cuh, cpp/include/cudf/reduction/detail/segmented_reduction.cuh, cpp/src/reductions/approx_distinct_count.cu
Reduction-owned storage uses CUDA device buffers. The approximate distinct-count sketch visitors recognize the new owning storage type, and the span-backed constructor no longer accepts or stores a memory resource.
Strings temporary allocations
cpp/include/cudf/strings/detail/copy_if_else.cuh, cpp/include/cudf/strings/detail/strings_children.cuh, cpp/include/cudf/strings/detail/strings_column_factories.cuh
Temporary indices, sizes, character sizes, and valid counts use CUDA device buffers.

Estimated code review effort: 2 (Simple) | ~12 minutes

Change: Refactor

Suggested reviewers: pointkernel

Merge Risk: Minimal · up to 87da2

The migration introduces no confirmed merge-blocking risk and is suitable to merge with normal checks.

Pre-merge checks: Passed 4 · Failed 1

Failed 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%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat… Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Title check Passed The title clearly and concisely describes the main change: replacing scratch rmm::device_uvector storage with cuda::device_buffer in headers.
Description check Passed The description directly relates to the changeset and states the objective of using CCCL buffers instead of RMM vectors in inline header implementations.
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: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. 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 2 functions across 3 files. (1 skipped: 1 unsupported.)

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

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

@coderabbitai coderabbitai Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

  • Fix comments: Fix CodeRabbit comments on this PR

@KyleFromNVIDIA KyleFromNVIDIA added breaking Breaking change and removed non-breaking Non-breaking change labels Nov 2, 2026

@bdice bdice left a comment

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.

Approving but there's an opportunity for cleanup, I think.

@KyleFromNVIDIA

Copy link
Copy Markdown
Member Author

/merge

@rapids-bot
rapids-bot Bot merged commit 5ebf834 into NVIDIA:main Nov 2, 2026
205 of 208 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

breaking Breaking change improvement Improvement / enhancement to an existing function libcudf Affects libcudf (C++/CUDA) code.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants