Reduce filtered join build time - #23320
Conversation
…n-build-time # Conflicts: # cpp/src/join/filtered_join.cu
…n-build-time # Conflicts: # cpp/src/join/filtered_join.cu
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughSummary by CodeRabbit
WalkthroughFiltered joins are reorganized into split primitive, flat, and nested CUDA paths. The implementation adds preprocessed-table MurmurHash3 support, mode-specific CUCO probing, nested-null validity masks, semi/anti join result handling, and build integration. ChangesFiltered Join Refactor
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
cpp/src/hash/murmurhash3_x86_32.cu (1)
45-48: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDeclare the device lambda’s return type.
Line 47 passes an extended device lambda to a device algorithm without an explicit return type. As per coding guidelines, “Declare explicit return types for extended device lambdas passed to device algorithms.”
Proposed fix
- [output_begin, hasher] __device__(size_type i) mutable { output_begin[i] = hasher(i); }, + [output_begin, hasher] __device__(size_type i) mutable -> void { output_begin[i] = hasher(i); },🤖 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/src/hash/murmurhash3_x86_32.cu` around lines 45 - 48, Update the extended device lambda passed to cub::DeviceFor::Bulk in the MurmurHash3 processing call to declare its return type explicitly, while preserving its existing mutable capture and output assignment behavior.Source: Coding guidelines
🤖 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/src/hash/murmurhash3_x86_32.cu`:
- Around line 45-48: Update the extended device lambda passed to
cub::DeviceFor::Bulk in the MurmurHash3 processing call to declare its return
type explicitly, while preserving its existing mutable capture and output
assignment behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 33fb40a4-83b6-46d4-bbeb-50718e2e0f95
📒 Files selected for processing (12)
cpp/CMakeLists.txtcpp/include/cudf/detail/join/distinct_filtered_join.cuhcpp/include/cudf/detail/join/filtered_join.cuhcpp/src/hash/murmurhash3_x86_32.cucpp/src/hash/murmurhash3_x86_32.cuhcpp/src/join/filtered_join.cucpp/src/join/filtered_join/filtered_join.cucpp/src/join/filtered_join/filtered_join_common.cuhcpp/src/join/filtered_join/filtered_join_flat.cucpp/src/join/filtered_join/filtered_join_nested.cucpp/src/join/filtered_join/filtered_join_nested_query.cucpp/src/join/filtered_join/filtered_join_primitive.cu
💤 Files with no reviewable changes (1)
- cpp/src/join/filtered_join.cu
|
/merge |
Description
Related to #21973
join/filtered_join.cuwas the second most expensive libcudf TU in a cold build. This PR reduces its build time by splitting primitive, flat, and nested row operators into separate TUs.For nested rows, this PR materializes MurmurHash values in a separate pass and uses lightweight hash lookups during CUCO insertion and probing. This trades an extra hash pass and a temporary 32-bit hash per row for keeping recursive nested hashing out of the template-heavy cuco kernels, reducing compile complexity and register pressure. The existing preprocessed row metadata is reused, while primitive and flat paths continue hashing inline.
The longest CUDA compile drops from 432s to 91s. The combined affected object size drops by 15%. Nested LIST and STRUCT benchmarks improve by up to 44%, with no regressions across 32 cases.
Checklist