Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(swordfish): Enable left/right joins to build probe table on either side #3548

Merged
merged 4 commits into from
Dec 12, 2024

Conversation

colin-ho
Copy link
Contributor

@colin-ho colin-ho commented Dec 11, 2024

Enable left/right joins to build on either side. E.g. if left join, we can build on left and save used values in a bitmap (same as outer join), then emit the unmatched rows from the finalize.

Total tpch time after this pr: 65s, down from 88s

@github-actions github-actions bot added the feat label Dec 11, 2024
Copy link

codspeed-hq bot commented Dec 11, 2024

CodSpeed Performance Report

Merging #3548 will degrade performances by 12.43%

Comparing colin/swordfish-left-right (8e1cdc8) with main (da6f499)

Summary

❌ 1 regressions
✅ 26 untouched benchmarks

⚠️ Please fix the performance issues or acknowledge them on CodSpeed.

Benchmarks breakdown

Benchmark main colin/swordfish-left-right Change
test_iter_rows_first_row[100 Small Files] 187.7 ms 214.4 ms -12.43%

Copy link

codecov bot commented Dec 12, 2024

Codecov Report

Attention: Patch coverage is 82.35294% with 39 lines in your changes missing coverage. Please review.

Project coverage is 77.80%. Comparing base (f23ee37) to head (8e1cdc8).
Report is 10 commits behind head on main.

Files with missing lines Patch % Lines
src/daft-local-execution/src/pipeline.rs 53.19% 22 Missing ⚠️
...local-execution/src/sinks/outer_hash_join_probe.rs 90.22% 17 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #3548      +/-   ##
==========================================
+ Coverage   77.69%   77.80%   +0.10%     
==========================================
  Files         710      716       +6     
  Lines       86941    87985    +1044     
==========================================
+ Hits        67552    68457     +905     
- Misses      19389    19528     +139     
Files with missing lines Coverage Δ
...local-execution/src/sinks/outer_hash_join_probe.rs 94.73% <90.22%> (-2.24%) ⬇️
src/daft-local-execution/src/pipeline.rs 90.57% <53.19%> (-3.19%) ⬇️

... and 45 files with indirect coverage changes

@colin-ho colin-ho marked this pull request as ready for review December 12, 2024 08:12
@colin-ho
Copy link
Contributor Author

TPCH Sf10 stats:

Query Before After % Difference
Q1 492.0893 ms 487.3327 ms -0.97%
Q2 129.7754 ms 128.9266 ms -0.65%
Q3 460.6002 ms 465.1448 ms +0.99%
Q4 2.7178 s 2.5625 s -5.71%
Q5 748.5831 ms 738.6838 ms -1.32%
Q6 167.5320 ms 156.0695 ms -6.84%
Q7 446.4988 ms 424.4770 ms -4.93%
Q8 528.1977 ms 521.5541 ms -1.26%
Q9 3.7559 s 3.6353 s -3.21%
Q10 2.4192 s 2.4236 s +0.18%
Q11 259.1618 ms 270.5231 ms +4.38%
Q12 310.5514 ms 311.4046 ms +0.27%
Q13 4.6433 s 2.4069 s -48.16%
Q14 497.7973 ms 491.8836 ms -1.19%
Q15 506.1940 ms 513.2820 ms +1.40%
Q16 463.8566 ms 459.9157 ms -0.85%
Q17 19.4504 s 365.0166 ms -98.12%
Q18 5.5117 s 5.4426 s -1.25%
Q19 718.0324 ms 728.8470 ms +1.51%
Q20 3.3136 s 3.3422 s +0.86%
Q21 39.2219 s 38.4184 s -2.05%
Q22 2.5562 s 2.5282 s -1.10%

The biggest win here is Q13 with 48% and Q17 with 98%

@colin-ho colin-ho merged commit 35ed63c into main Dec 12, 2024
43 of 44 checks passed
@colin-ho colin-ho deleted the colin/swordfish-left-right branch December 12, 2024 20:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants