Conversation
|
🤖 |
| mod topk_stream; | ||
|
|
||
| /// Hard-coded seed for aggregations to ensure hash values differ from `RepartitionExec`, avoiding collisions. | ||
| const AGGREGATION_HASH_SEED: ahash::RandomState = |
|
🤖: Benchmark completed Details
|
|
I am surprised this shows any performance difference. I will rerun and see if I can reproduce |
|
🤖 |
|
🤖: Benchmark completed Details
|
|
🤖 |
|
🤖: Benchmark completed Details
|
|
Second performance run looks as good / better so let's merge this in! |
|
Thanks again @ctsk |
|
Got an interesting link https://morestina.net/1843/the-stable-hashmap-trap from this reddit discussion. Cross-referencing it in this thread. (and it looks like we're not in that trap) |
This PR hard-codes the seed for the hash aggregation. The main benefit compared to the previously runtime-determined seed is that after applying this PR, partial aggregation and final aggregation will share the same hash function.
I haven't measured it, but in theory, this should make the final aggregation step more efficient, because the partial aggregation will emit the group values in a way that will be clustered in the final aggregation hash table - thus causing a benefitial memory access pattern when building the final aggregation.
I expect it speeds up large-cardinality aggregations that don't trigger the skipping of the partial aggregation step a tiny bit.