Skip to content

Add memory resource parameter to cudf::mark_join constructors - #23278

Merged
rapids-bot[bot] merged 2 commits into
NVIDIA:mainfrom
PointKernel:add-mr-mark-join
Jul 20, 2026
Merged

Add memory resource parameter to cudf::mark_join constructors#23278
rapids-bot[bot] merged 2 commits into
NVIDIA:mainfrom
PointKernel:add-mr-mark-join

Conversation

@PointKernel

Copy link
Copy Markdown
Member

Description

Follow-up to #23264 and #23277.

This PR adds an mr parameter to the cudf::mark_join constructors to control allocations for the persistent cuco hash table and optional bloom-filter prefilter, aligning the wrapper with the temporary MR effort (#20780).

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-pr-bot Bot commented Jul 15, 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.

@github-actions github-actions Bot added the libcudf Affects libcudf (C++/CUDA) code. label Jul 15, 2026
@PointKernel PointKernel added non-breaking Non-breaking change feature request New feature or request labels Jul 15, 2026
@PointKernel

Copy link
Copy Markdown
Member Author

/ok to test 0ae3e47

@PointKernel
PointKernel marked this pull request as ready for review July 20, 2026 21:32
@PointKernel
PointKernel requested a review from a team as a code owner July 20, 2026 21:32
@coderabbitai

coderabbitai Bot commented Jul 20, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Summary by CodeRabbit

  • New Features
    • Mark-based joins now support an optional custom device memory resource.
    • Existing behavior remains unchanged by default, using the current device resource automatically.
  • Bug Fixes
    • Improved memory allocation control for join processing and optional bloom-filter prefiltering.
  • Tests
    • Added coverage verifying that custom memory resources are used during mark joins.

Walkthrough

mark_join constructors now accept an optional device memory resource, propagate it through the public and internal implementations, and use it for bucket and bloom-filter allocations. A test verifies allocations through a statistics resource adaptor.

Changes

Mark join memory resource support

Layer / File(s) Summary
Constructor resource plumbing
cpp/include/cudf/join/mark_join.hpp, cpp/src/join/mark_join.cuh, cpp/src/join/mark_join.cu
Public and internal constructors add the device-accessible memory resource parameter, default it to the current device resource, and forward it to the implementation.
Resource-backed allocation validation
cpp/src/join/mark_join.cu, cpp/tests/join/semi_anti_join_tests.cpp
Bucket and bloom-filter allocators use the supplied resource, and a test confirms nonzero peak allocation through a statistics adaptor.

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

Possibly related PRs

  • rapidsai/cudf#23223: Extends a related hash join constructor with a device memory resource and threads it into internal allocation.

Suggested labels: improvement

Suggested reviewers: davidwendt, wence-, mhaseeb123

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: adding a memory resource parameter to mark_join constructors.
Description check ✅ Passed The description directly explains the MR addition and its purpose, and matches the code changes.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.

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

🧹 Nitpick comments (1)
cpp/tests/join/semi_anti_join_tests.cpp (1)

519-531: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Cover the load-factor constructor as well.

This test exercises only the first public constructor; the load-factor overload has an independent mr forwarding path. Add a second case or parameterize the test to cover both.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@cpp/tests/join/semi_anti_join_tests.cpp` around lines 519 - 531, Extend
MarkJoinMemoryResource to also construct cudf::mark_join through the public
load-factor overload, passing the same memory resource and valid load-factor
value, then verify the resource records allocation for that path as well.
Preserve the existing constructor coverage and assertions.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@cpp/tests/join/semi_anti_join_tests.cpp`:
- Around line 519-531: Extend MarkJoinMemoryResource to also construct
cudf::mark_join through the public load-factor overload, passing the same memory
resource and valid load-factor value, then verify the resource records
allocation for that path as well. Preserve the existing constructor coverage and
assertions.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 487ee9a9-6357-424d-8762-0d3310d6b6fd

📥 Commits

Reviewing files that changed from the base of the PR and between 783e214 and 0ae3e47.

📒 Files selected for processing (4)
  • cpp/include/cudf/join/mark_join.hpp
  • cpp/src/join/mark_join.cu
  • cpp/src/join/mark_join.cuh
  • cpp/tests/join/semi_anti_join_tests.cpp

@vyasr

vyasr commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

/merge

@rapids-bot
rapids-bot Bot merged commit 6f05e8b into NVIDIA:main Jul 20, 2026
136 checks passed
@PointKernel
PointKernel deleted the add-mr-mark-join branch July 21, 2026 00:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature request New feature or request 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