jayzhan211 commented on PR #25434:
URL: https://github.com/apache/datafusion/pull/25434#issuecomment-5738046900
Speculate-to-`num_rows` then `shrink_to_fit` regresses mid-cardinality
builds; `bench_hash_join_build` has no such shape. 1M Utf8 rows, base
`62f039f0d` vs this branch, separate target dirs, interleaved:
| distinct | base | PR | Δ | peak `build_mem_used` |
| --- | ---: | ---: | ---: | ---: |
| 20k | 4.61 ms | 4.83 ms | +4.8% | 54.5 → 59.1 MB |
| 100k | 5.63 ms | 6.42 ms | +14–16% | 55.4 → 61.6 MB |
Disabling this block restores base time (5.85 ms vs 6.03 ms), so the cost is
the compaction rehash. Acceptable as a trade for ~33 MB less retained during
probe, but please cover it in the bench and note it in the description. Fine to
tune in a follow-up: "hash join: avoid speculate-then-compact double work for
mid-cardinality builds".
```diff
for distribution in [
"duplicates",
+ "mid_20k",
+ "mid_100k",
"mostly_unique",
```
```diff
"duplicates" => row % 64,
+ "mid_20k" => row % 20_000,
+ "mid_100k" => row % 100_000,
"mostly_unique" => row % 900_000,
```
@sunchao I think we should try to improve mid_20k and mid 100k case 🤔
```
┌──────────────────────┬─────────┬─────────┬─────────┬─────────────────────┬──────────┐
│ distribution │ base │ PR │ Δ time │ base build_mem_used │
PR │
├──────────────────────┼─────────┼─────────┼─────────┼─────────────────────┼──────────┤
│ duplicates (64 keys) │ 3.12 ms │ 3.03 ms │ −3% │ 47.66 MB │
16.35 MB │
├──────────────────────┼─────────┼─────────┼─────────┼─────────────────────┼──────────┤
│ mid_20k │ 4.61 ms │ 4.83 ms │ +4.8% │ 54.54 MB │
59.10 MB │
├──────────────────────┼─────────┼─────────┼─────────┼─────────────────────┼──────────┤
│ mid_100k │ 5.63 ms │ 6.42 ms │ +14–16% │ 55.40 MB │
61.62 MB │
├──────────────────────┼─────────┼─────────┼─────────┼─────────────────────┼──────────┤
│ mostly_unique │ 6.07 ms │ 5.73 ms │ −5% │ 55.53 MB │
59.87 MB │
├──────────────────────┼─────────┼─────────┼─────────┼─────────────────────┼──────────┤
│ unique │ 6.01 ms │ 5.62 ms │ −6% │ 55.59 MB │
59.94 MB │
├──────────────────────┼─────────┼─────────┼─────────┼─────────────────────┼──────────┤
│ unique_prefix │ 4.72 ms │ 4.52 ms │ −4% │ 51.59 MB │
55.94 MB │
├──────────────────────┼─────────┼─────────┼─────────┼─────────────────────┼──────────┤
│ duplicate_prefix │ 4.74 ms │ 4.54 ms │ −4% │ 51.59 MB │
55.94 MB │
└──────────────────────┴─────────┴─────────┴─────────┴─────────────────────┴──────────┘
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
