Skip to content

Replace remaining rmm::cuda_stream_view with cuda::stream_ref - #24152

Closed
davidwendt wants to merge 1 commit into
NVIDIA:mainfrom
davidwendt:cuda-stream-ref
Closed

davidwendt wants to merge 1 commit into
NVIDIA:mainfrom
davidwendt:cuda-stream-ref

Conversation

@davidwendt

Copy link
Copy Markdown
Contributor

Description

One rmm::cuda_stream_view leaked through via crossed merge with #23929 work.

Checklist

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

@davidwendt davidwendt self-assigned this Sep 14, 2026
@davidwendt davidwendt added the bug Something isn't working label Sep 14, 2026
@davidwendt
davidwendt requested a review from a team as a code owner September 14, 2026 13:38
@davidwendt davidwendt added 3 - Ready for Review Ready for review by team libcudf Affects libcudf (C++/CUDA) code. labels Sep 14, 2026
@davidwendt
davidwendt requested review from bdice and vyasr September 14, 2026 13:38
@davidwendt davidwendt added the non-breaking Non-breaking change label Sep 14, 2026
@coderabbitai

coderabbitai Bot commented Sep 14, 2026

Copy link
Copy Markdown

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: a774e97f-6111-40e5-a75a-32d84c24a5de

📥 Commits

Reviewing files that changed from the base of the PR and between 4868806 and a964431.

📒 Files selected for processing (1)
  • cpp/src/io/parquet/reader_impl_dict_transcode.cu

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


📝 Summary

Summary by CodeRabbit

  • Refactor
    • Updated internal stream handling for Parquet dictionary index remapping without changing its behavior.

Walkthrough

The Parquet dictionary remapping function now accepts cuda::stream_ref instead of the deprecated rmm::cuda_stream_view. Its remapping behavior is unchanged.

Changes

Parquet reader update

Layer / File(s) Summary
CUDA stream parameter update
cpp/src/io/parquet/reader_impl_dict_transcode.cu
The function signature uses cuda::stream_ref instead of rmm::cuda_stream_view.

Priority: ⬇️ Low

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

Change: Refactor

Merge Risk: ⚪ Minimal · up to a9644

This localized type migration does not change remapping behavior or introduce a merge-blocking risk.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the main change: replacing the remaining rmm::cuda_stream_view usage with cuda::stream_ref.
Description check ✅ Passed The description directly explains the remaining rmm::cuda_stream_view usage and relates it to the pull request changes.
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. Analyzed 1 functions across 1 files.
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.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

@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.

Please retarget to 26.10 if the other PR landed there.

@davidwendt

Copy link
Copy Markdown
Contributor Author

The code is correct in 26.10 so looks like we are blocked by the forward-merger conflict somewhere.

@davidwendt davidwendt closed this Sep 14, 2026
@davidwendt

Copy link
Copy Markdown
Contributor Author

The change is in blocked in here #24133

@davidwendt
davidwendt deleted the cuda-stream-ref branch September 14, 2026 16:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3 - Ready for Review Ready for review by team bug Something isn't working libcudf Affects libcudf (C++/CUDA) code. non-breaking Non-breaking change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants