Skip to content

Replace hash join internals with HashCSR - #23640

Draft
PointKernel wants to merge 15 commits into
NVIDIA:mainfrom
PointKernel:hashcsr_hash_join
Draft

Replace hash join internals with HashCSR#23640
PointKernel wants to merge 15 commits into
NVIDIA:mainfrom
PointKernel:hashcsr_hash_join

Conversation

@PointKernel

@PointKernel PointKernel commented Aug 12, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Description

TBD

Following the design proposed by @rootjalex

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 Aug 12, 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 libcudf Affects libcudf (C++/CUDA) code. CMake CMake build issue labels Aug 12, 2026
@PointKernel

PointKernel commented Aug 12, 2026

Copy link
Copy Markdown
Member Author

Performance results:

Benchmark     high_multiplicity_inner_join
GPU           NVIDIA GH200 480GB
Left input    100,000 rows
Right input   100,000 rows

Multiplicity      OLD HASH      HASH CSR    ORIGINAL SMJ    LATEST SMJ
1               169.364 us    166.843 us      772.107 us    614.171 us
10              251.927 us    165.929 us      780.373 us    596.207 us
100             876.574 us    183.180 us      876.098 us    640.105 us
1,000             6.816 ms      0.272 ms        1.138 ms      0.999 ms
10,000           56.742 ms      0.625 ms        3.292 ms      1.845 ms
50,000          286.811 ms      3.507 ms       14.756 ms      4.264 ms

@PointKernel PointKernel added non-breaking Non-breaking change Performance Performance related issue improvement Improvement / enhancement to an existing function 2 - In Progress Currently a work in progress labels Aug 12, 2026
rootjalex
rootjalex reviewed Aug 12, 2026
View reviewed changes
Comment thread cpp/src/join/hash_join/hash_csr_kernels.cuh
Comment on lines +52 to +53
auto const rank = slot_count_ref.fetch_add(size_type{1}, cuda::memory_order_relaxed);
build_positions[index] = {slot, rank};

@rootjalex rootjalex Aug 12, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This, storing rank AOT, is a nice optimization the original implementation did not have! :)

rootjalex
rootjalex reviewed Aug 12, 2026
View reviewed changes
Comment thread cpp/src/join/hash_join/hash_csr_kernels.cuh
probe_slots[index] = found ? static_cast<size_type>(slot) : CUDF_SIZE_TYPE_SENTINEL;
}
if (match_counts != nullptr) {
match_counts[index] = IsOuter ? cuda::std::max(count, size_type{1}) : count;

@rootjalex rootjalex Aug 12, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I think this should be IsOuterOrLeft ?

@PointKernel PointKernel Aug 13, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

An outer join is any LEFT, RIGHT, or FULL OUTER JOIN, as opposed to an INNER JOIN. As we don't support right join in cudf. IsOuter is accurate. Make sense to you?

rootjalex
rootjalex reviewed Aug 12, 2026
View reviewed changes
Comment thread cpp/src/join/hash_join/hash_csr_kernels.cuh
Comment on lines +103 to +112
if (found && matched_slots != nullptr) {
auto matched_slot_ref =
cuda::atomic_ref<std::uint32_t, cuda::thread_scope_device>{matched_slots[slot]};
auto expected = std::uint32_t{0};
if (matched_slot_ref.compare_exchange_strong(
expected, std::uint32_t{1}, cuda::memory_order_relaxed)) {
cuda::atomic_ref<unsigned long long, cuda::thread_scope_device>{*matched_build_rows}
.fetch_add(static_cast<unsigned long long>(count), cuda::memory_order_relaxed);
}
}

@rootjalex rootjalex Aug 12, 2026 •
edited
Loading

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I'm a bit confused by this. Some comments could help with it. Is this implementing the right/outer join matching array we discussed?

rootjalex
rootjalex reviewed Aug 12, 2026
View reviewed changes
Comment thread cpp/src/join/hash_join/hash_csr_kernels.cuh
}

template <bool IsOuter>
CUDF_KERNEL void hash_csr_retrieve_kernel(std::int64_t output_size,

@rootjalex rootjalex Aug 12, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I don't see the logic for handling right/outer joins emission here. Is the IsOuter flag only for left joins for now?

@PointKernel PointKernel Aug 13, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

It's for both left and full join.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer
@rootjalex rootjalex rootjalex left review comments
Reviewers whose approvals may not affect merge requirements

At least 1 approving review is required to merge this pull request.

Assignees

@PointKernel PointKernel

Labels

2 - In Progress Currently a work in progress CMake CMake build issue improvement Improvement / enhancement to an existing function libcudf Affects libcudf (C++/CUDA) code. non-breaking Non-breaking change Performance Performance related issue

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@PointKernel @rootjalex