Skip to content

Add primitive row dispatch support for semi/anti join and cudf::contains - #19361

Merged
rapids-bot[bot] merged 14 commits into
NVIDIA:branch-25.08from
tgujar:semi-join-occupancy
Jul 22, 2025
Merged

Add primitive row dispatch support for semi/anti join and cudf::contains#19361
rapids-bot[bot] merged 14 commits into
NVIDIA:branch-25.08from
tgujar:semi-join-occupancy

Conversation

@tgujar

@tgujar tgujar commented Jul 12, 2025

Copy link
Copy Markdown
Contributor

Description

Add primitive row operator for left semi/anti joins. This improves occupancy for join operations as detailed in #15700

Checklist

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

@tgujar
tgujar requested a review from a team as a code owner July 12, 2025 01:09
@tgujar
tgujar requested review from mythrocks and nvdbaranec July 12, 2025 01:09
@copy-pr-bot

copy-pr-bot Bot commented Jul 12, 2025

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@github-actions github-actions Bot added the libcudf Affects libcudf (C++/CUDA) code. label Jul 12, 2025
@tgujar

tgujar commented Jul 12, 2025

Copy link
Copy Markdown
Contributor Author

Benchmark numbers

❯ python nvbench_compare.py ~/data_remote/repos/forks/cudf/main.json ~/data_remote/repos/forks/cudf/semi_test.json
['/home/tgujar/data_remote/repos/forks/cudf/main.json', '/home/tgujar/data_remote/repos/forks/cudf/semi_test.json']
# left_anti_join

## [0] NVIDIA H100 NVL

|  Key  |  Nullable  |  left_size  |  right_size  |   Ref Time |   Ref Noise |   Cmp Time |   Cmp Noise |        Diff |   %Diff |  Status  |
|-------|------------|-------------|--------------|------------|-------------|------------|-------------|-------------|---------|----------|
|  I32  |     0      |    1000     |     1000     |  64.963 us |       2.41% |  63.065 us |       1.68% |   -1.898 us |  -2.92% |   FAST   |
|  I32  |     0      |   100000    |     1000     |  71.572 us |       2.31% |  67.233 us |       2.60% |   -4.339 us |  -6.06% |   FAST   |
|  I32  |     0      |  10000000   |     1000     | 441.637 us |       0.33% | 308.310 us |       0.49% | -133.326 us | -30.19% |   FAST   |
|  I32  |     0      |   100000    |    100000    |  84.289 us |       1.24% |  81.270 us |       1.89% |   -3.019 us |  -3.58% |   FAST   |
|  I32  |     0      |  10000000   |    100000    | 535.629 us |       0.31% | 347.484 us |       0.58% | -188.145 us | -35.13% |   FAST   |
|  I32  |     0      |  10000000   |   10000000   |   1.352 ms |       0.12% |   1.001 ms |       0.18% | -351.054 us | -25.97% |   FAST   |
|  I32  |     1      |    1000     |     1000     |  64.966 us |       1.51% |  61.240 us |       2.00% |   -3.725 us |  -5.73% |   FAST   |
|  I32  |     1      |   100000    |     1000     |  69.565 us |       1.35% |  65.451 us |       1.46% |   -4.114 us |  -5.91% |   FAST   |
|  I32  |     1      |  10000000   |     1000     | 305.228 us |       0.85% | 221.829 us |       0.73% |  -83.399 us | -27.32% |   FAST   |
|  I32  |     1      |   100000    |    100000    |  73.267 us |       1.21% |  69.644 us |       2.78% |   -3.623 us |  -4.94% |   FAST   |
|  I32  |     1      |  10000000   |    100000    | 330.069 us |       0.42% | 235.873 us |       0.65% |  -94.195 us | -28.54% |   FAST   |
|  I32  |     1      |  10000000   |   10000000   | 709.609 us |       0.21% | 468.307 us |       0.31% | -241.301 us | -34.00% |   FAST   |
|  I64  |     0      |    1000     |     1000     |  70.909 us |       1.43% |  66.962 us |       1.40% |   -3.946 us |  -5.57% |   FAST   |
|  I64  |     0      |   100000    |     1000     |  76.788 us |       1.33% |  69.789 us |       1.29% |   -6.999 us |  -9.11% |   FAST   |
|  I64  |     0      |  10000000   |     1000     | 490.858 us |       0.38% | 336.470 us |       0.44% | -154.388 us | -31.45% |   FAST   |
|  I64  |     0      |   100000    |    100000    |  86.662 us |       1.26% |  79.595 us |       1.21% |   -7.067 us |  -8.15% |   FAST   |
|  I64  |     0      |  10000000   |    100000    | 544.787 us |       0.34% | 355.204 us |       0.44% | -189.583 us | -34.80% |   FAST   |
|  I64  |     0      |  10000000   |   10000000   |   1.421 ms |       0.11% |   1.075 ms |       0.13% | -346.447 us | -24.38% |   FAST   |
|  I64  |     1      |    1000     |     1000     |  64.618 us |       1.34% |  63.974 us |       1.44% |   -0.644 us |  -1.00% |   SAME   |
|  I64  |     1      |   100000    |     1000     |  69.150 us |       1.50% |  67.527 us |       1.23% |   -1.623 us |  -2.35% |   FAST   |
|  I64  |     1      |  10000000   |     1000     | 305.949 us |       0.51% | 229.717 us |       0.60% |  -76.232 us | -24.92% |   FAST   |
|  I64  |     1      |   100000    |    100000    |  73.244 us |       1.25% |  69.844 us |       1.42% |   -3.400 us |  -4.64% |   FAST   |
|  I64  |     1      |  10000000   |    100000    | 332.717 us |       0.44% | 239.780 us |       0.58% |  -92.937 us | -27.93% |   FAST   |
|  I64  |     1      |  10000000   |   10000000   | 723.276 us |       0.23% | 479.048 us |       0.33% | -244.228 us | -33.77% |   FAST   |

# left_semi_join

## [0] NVIDIA H100 NVL

|  Key  |  Nullable  |  left_size  |  right_size  |   Ref Time |   Ref Noise |   Cmp Time |   Cmp Noise |        Diff |   %Diff |  Status  |
|-------|------------|-------------|--------------|------------|-------------|------------|-------------|-------------|---------|----------|
|  I32  |     0      |    1000     |     1000     |  69.111 us |       1.72% |  66.564 us |       1.29% |   -2.547 us |  -3.69% |   FAST   |
|  I32  |     0      |   100000    |     1000     |  72.361 us |       1.55% |  67.765 us |       1.53% |   -4.595 us |  -6.35% |   FAST   |
|  I32  |     0      |  10000000   |     1000     | 436.516 us |       0.33% | 303.324 us |       0.44% | -133.191 us | -30.51% |   FAST   |
|  I32  |     0      |   100000    |    100000    |  84.024 us |       1.40% |  80.836 us |       1.25% |   -3.188 us |  -3.79% |   FAST   |
|  I32  |     0      |  10000000   |    100000    | 530.761 us |       0.26% | 342.551 us |       0.40% | -188.210 us | -35.46% |   FAST   |
|  I32  |     0      |  10000000   |   10000000   |   1.348 ms |       0.11% | 996.248 us |       0.14% | -351.450 us | -26.08% |   FAST   |
|  I32  |     1      |    1000     |     1000     |  65.360 us |       1.52% |  61.543 us |       1.37% |   -3.817 us |  -5.84% |   FAST   |
|  I32  |     1      |   100000    |     1000     |  69.502 us |       1.29% |  65.209 us |       1.35% |   -4.293 us |  -6.18% |   FAST   |
|  I32  |     1      |  10000000   |     1000     | 297.837 us |       0.48% | 215.431 us |       0.55% |  -82.406 us | -27.67% |   FAST   |
|  I32  |     1      |   100000    |    100000    |  72.982 us |       1.29% |  69.377 us |       1.63% |   -3.606 us |  -4.94% |   FAST   |
|  I32  |     1      |  10000000   |    100000    | 321.932 us |       0.54% | 229.511 us |       0.48% |  -92.421 us | -28.71% |   FAST   |
|  I32  |     1      |  10000000   |   10000000   | 702.780 us |       0.20% | 462.232 us |       0.30% | -240.548 us | -34.23% |   FAST   |
|  I64  |     0      |    1000     |     1000     |  71.306 us |       1.33% |  67.360 us |       1.42% |   -3.946 us |  -5.53% |   FAST   |
|  I64  |     0      |   100000    |     1000     |  76.351 us |       1.31% |  69.307 us |       1.35% |   -7.044 us |  -9.23% |   FAST   |
|  I64  |     0      |  10000000   |     1000     | 485.017 us |       0.28% | 331.038 us |       0.37% | -153.980 us | -31.75% |   FAST   |
|  I64  |     0      |   100000    |    100000    |  86.361 us |       1.30% |  79.207 us |       1.30% |   -7.154 us |  -8.28% |   FAST   |
|  I64  |     0      |  10000000   |    100000    | 540.600 us |       0.32% | 351.188 us |       0.45% | -189.412 us | -35.04% |   FAST   |
|  I64  |     0      |  10000000   |   10000000   |   1.417 ms |       0.10% |   1.070 ms |       0.13% | -347.190 us | -24.50% |   FAST   |
|  I64  |     1      |    1000     |     1000     |  65.254 us |       1.48% |  64.269 us |       1.41% |   -0.985 us |  -1.51% |   FAST   |
|  I64  |     1      |   100000    |     1000     |  68.653 us |       1.38% |  67.223 us |       2.98% |   -1.430 us |  -2.08% |   FAST   |
|  I64  |     1      |  10000000   |     1000     | 299.539 us |       0.38% | 222.513 us |       0.52% |  -77.026 us | -25.71% |   FAST   |
|  I64  |     1      |   100000    |    100000    |  72.967 us |       1.09% |  69.653 us |       1.50% |   -3.314 us |  -4.54% |   FAST   |
|  I64  |     1      |  10000000   |    100000    | 326.441 us |       0.39% | 232.851 us |       0.49% |  -93.589 us | -28.67% |   FAST   |
|  I64  |     1      |  10000000   |   10000000   | 717.637 us |       0.19% | 472.761 us |       0.26% | -244.876 us | -34.12% |   FAST   |

# Summary

- Total Matches: 48
  - Pass    (diff <= min_noise): 1
  - Unknown (infinite noise):    0
  - Failure (diff > min_noise):  **47**

@PointKernel

Copy link
Copy Markdown
Member

/ok to test d041d8b

@PointKernel PointKernel added non-breaking Non-breaking change improvement Improvement / enhancement to an existing function Performance Performance related issue labels Jul 15, 2025
@PointKernel

Copy link
Copy Markdown
Member

/ok to test dfe4413

@PointKernel PointKernel left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

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

One optional minor suggestion but LGTM otherwise

Comment thread cpp/include/cudf/table/primitive_row_operators.cuh
Comment thread cpp/include/cudf/table/primitive_row_operators.cuh Outdated
@davidwendt

Copy link
Copy Markdown
Contributor

Doesn't this double the compile-time for contains_table.cu?
Is the title of this PR correct? Seems like this is changes for cudf::contains and not join specifically.

@PointKernel

Copy link
Copy Markdown
Member

Is the title of this PR correct? Seems like this is changes for cudf::contains and not join specifically.

Ha, makes sense. The goal was to enhance all join operations using primitive row operators, but since semi/anti joins rely on contains internally, this PR specifically updates contains. @tgujar, could you please gather benchmark results for contains as well and update the PR title to reflect that?

@PointKernel

PointKernel commented Jul 17, 2025

Copy link
Copy Markdown
Member

Doesn't this double the compile-time for contains_table.cu?

Yes, it does. I’ll fix it now. Putting this on hold in the meantime.

@PointKernel PointKernel added the DO NOT MERGE Hold off on merging; see PR for details label Jul 17, 2025
@tgujar tgujar changed the title Add primitive row dispatch support for semi/anti join Add primitive row dispatch support for semi/anti join and cudf::contains Jul 17, 2025
@tgujar

tgujar commented Jul 18, 2025

Copy link
Copy Markdown
Contributor Author

Benchmark results for contains

['/home/tgujar/data_remote/repos/forks/cudf/cpp/build/benchmarks/main.json', '/home/tgujar/data_remote/repos/forks/cudf/cpp/build/benchmarks/test.json']
# contains_table

## [0] NVIDIA H100 PCIe

|      type       |  null_probability  |  table_size  |   Ref Time |   Ref Noise |   Cmp Time |   Cmp Noise |       Diff |   %Diff |  Status  |
|-----------------|--------------------|--------------|------------|-------------|------------|-------------|------------|---------|----------|
|       I32       |         0          |    10000     |  47.373 us |       2.25% |  45.562 us |       3.90% |  -1.811 us |  -3.82% |   FAST   |
|       I32       |        0.1         |    10000     |  49.000 us |       1.96% |  48.055 us |       1.72% |  -0.945 us |  -1.93% |   FAST   |
|       I32       |         0          |    100000    |  46.193 us |       1.75% |  44.717 us |       1.87% |  -1.476 us |  -3.20% |   FAST   |
|       I32       |        0.1         |    100000    |  48.462 us |       1.76% |  47.849 us |       1.91% |  -0.612 us |  -1.26% |   SAME   |
|       I32       |         0          |   1000000    |  52.373 us |       1.66% |  48.181 us |       1.73% |  -4.192 us |  -8.00% |   FAST   |
|       I32       |        0.1         |   1000000    |  64.982 us |       1.83% |  57.261 us |       1.89% |  -7.721 us | -11.88% |   FAST   |
|       I32       |         0          |   10000000   | 122.930 us |       0.65% |  87.830 us |       0.87% | -35.100 us | -28.55% |   FAST   |
|       I32       |        0.1         |   10000000   | 159.430 us |       0.72% | 110.844 us |       0.99% | -48.586 us | -30.47% |   FAST   |
| cudf::list_view |         0          |    10000     | 752.466 us |       0.19% | 751.824 us |       0.16% |  -0.643 us |  -0.09% |   SAME   |
| cudf::list_view |        0.1         |    10000     | 953.991 us |       0.09% | 953.573 us |       0.10% |  -0.418 us |  -0.04% |   SAME   |
| cudf::list_view |         0          |    100000    | 663.034 us |       0.22% | 664.170 us |       0.27% |   1.136 us |   0.17% |   SAME   |
| cudf::list_view |        0.1         |    100000    | 723.545 us |       0.36% | 724.876 us |       0.21% |   1.331 us |   0.18% |   SAME   |
| cudf::list_view |         0          |   1000000    |   3.322 ms |       0.14% |   3.325 ms |       0.15% |   2.433 us |   0.07% |   SAME   |
| cudf::list_view |        0.1         |   1000000    |   4.488 ms |       0.15% |   4.489 ms |       0.14% |   0.547 us |   0.01% |   SAME   |
| cudf::list_view |         0          |   10000000   |   1.927 ms |       0.32% |   1.925 ms |       0.28% |  -1.222 us |  -0.06% |   SAME   |
| cudf::list_view |        0.1         |   10000000   |   1.599 ms |       0.25% |   1.600 ms |       0.25% |   0.768 us |   0.05% |   SAME   |

# Summary

- Total Matches: 16
  - Pass    (diff <= min_noise): 9
  - Unknown (infinite noise):    0
  - Failure (diff > min_noise):  7

PointKernel and others added 2 commits July 18, 2025 09:39
Co-authored-by: Muhammad Haseeb <14217455+mhaseeb123@users.noreply.github.com>
@PointKernel
PointKernel requested a review from a team as a code owner July 18, 2025 18:04
@github-actions github-actions Bot added the CMake CMake build issue label Jul 18, 2025
@PointKernel

Copy link
Copy Markdown
Member

/ok to test a301142

@PointKernel PointKernel added 5 - Ready to Merge Testing and reviews complete, ready to merge and removed DO NOT MERGE Hold off on merging; see PR for details labels Jul 18, 2025

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

Cmake approval

Comment thread cpp/src/search/contains_table_impl.cu Outdated
Comment thread cpp/src/search/contains_table_impl.cu Outdated
Comment thread cpp/src/search/contains_table_impl_nested.cu Outdated
@PointKernel

Copy link
Copy Markdown
Member

/ok to test fb3f147

@PointKernel PointKernel moved this to Burndown in libcudf Jul 21, 2025
@PointKernel

Copy link
Copy Markdown
Member

/ok to test 2c16dd9

@PointKernel

Copy link
Copy Markdown
Member

As discussed offline, we’ve decided to merge this PR into 25.08 since the risk of performance regression is minimal.

@PointKernel

Copy link
Copy Markdown
Member

/merge

@rapids-bot
rapids-bot Bot merged commit 421d9ac into NVIDIA:branch-25.08 Jul 22, 2025
@GregoryKimball GregoryKimball moved this from Slip to Landed in libcudf Jul 22, 2025
rapids-bot Bot pushed a commit that referenced this pull request Jul 29, 2025
…ins (#19518)

This PR is a follow-up to #19361, which was reverted due to a NaN handling bug and incorrect CG size used in explicit instantiations. This revised PR addresses those issues and retargets the work for the 25.10 release.

Authors:
  - Yunsong Wang (https://github.com/PointKernel)

Approvers:
  - Bradley Dice (https://github.com/bdice)
  - Nghia Truong (https://github.com/ttnghia)
  - Shruti Shivakumar (https://github.com/shrshi)

URL: #19518
@GregoryKimball GregoryKimball removed this from libcudf Sep 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

5 - Ready to Merge Testing and reviews complete, ready to merge 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

Development

Successfully merging this pull request may close these issues.

7 participants