Improve hash join detail functions #10273
Conversation
PointKernel
added
bug
Codecov Report
@@ Coverage Diff @@
## branch-22.04 #10273 +/- ##
================================================
+ Coverage 10.42% 10.68% +0.25%
================================================
Files 119 122 +3
Lines 20603 20857 +254
================================================
+ Hits 2148 2228 +80
- Misses 18455 18629 +174
Continue to review full report at Codecov.
|
cpp/src/join/join.cu
Outdated
| @@ -134,23 +135,24 @@ std::unique_ptr<table> left_join(table_view const& left_input, | |||
| table_view const left = scatter_columns(matched.second.front(), left_on, left_input); | |||
| table_view const right = scatter_columns(matched.second.back(), right_on, right_input); | |||
|
|
|||
| auto join_indices = left_join(left.select(left_on), right.select(right_on), compare_nulls); | |||
| auto const [left_join_indices, right_join_indices] = cudf::detail::left_join( | |||
| left.select(left_on), right.select(right_on), compare_nulls, stream, mr); | |||
|
|
|||
| if ((left_on.empty() || right_on.empty()) || | |||
There was a problem hiding this comment.
It looks like this if statement is for an early exit with an empty result. However, we're calling cudf::detail::left_join before the early-exit. None of the early exit conditions left_on.empty(), right_on.empty(), or is_trivial_join(left, right, ...) appear to depend on the result of the call to cudf::detail::left_join.
It seems like we could move these lines up, so that the if statement and early exit could be evaluated earlier in the function. Similarly for full_join below. (Am I missing something important?)
Apologies for expanding the scope of the PR review twice -- this just seemed important for performance.
#7454 is where this section of code was last changed.
There was a problem hiding this comment.
Good catch! And tbh no need to apologize at all. I'm working on improving the hash join performance #10248 thus found small issues here and there. That's also why this PR exists 😄. The goal is to improve hash join!
PointKernel
added
improvement
PointKernel
changed the title
PointKernel
added
improvement
|
@gpucibot merge |
This PR includes several changes in the hash join
detailfunctions:detailjoin functions. External invocations are replaced with the correspondingdetailones.