[SPARK-12950] [SQL] Improve lookup of BytesToBytesMap in aggregate #11010
Conversation
| Without codegen 7775.53 26.97 1.00 X | ||
| With codegen 342.15 612.94 22.73 X | ||
| Without codegen 5488.16 38.21 1.00 X | ||
| With codegen 531.08 394.88 10.33 X |
There was a problem hiding this comment.
The benchmark is not that stable, this number change from 10 to 20, maybe 200M are still not enough, I will increase it to 500M.
There was a problem hiding this comment.
Probably want to increase it by 10x to amortize the fixed overhead. The runtime was 342 ms, which is too small.
There was a problem hiding this comment.
Then we will wait more than 2 minutes to finish this benchmark, I will send another PR to update these benchmark.
|
Test build #2487 has finished for PR 11010 at commit
|
| * A function that calculates hash value for a group of expressions, which basically XOR all the | ||
| * hash code of children expressions together. | ||
| * | ||
| * Note: This is used for hash map for aggreagte, designed for performance (has worse |
There was a problem hiding this comment.
Does UnsafeRow.hashCode slower than this?
There was a problem hiding this comment.
UnsafeRow will call murmur3, which is still slow
Conflicts: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/misc.scala sql/core/src/main/scala/org/apache/spark/sql/execution/aggregate/TungstenAggregate.scala sql/core/src/test/scala/org/apache/spark/sql/execution/BenchmarkWholeStageCodegen.scala
Conflicts: sql/core/src/test/scala/org/apache/spark/sql/execution/BenchmarkWholeStageCodegen.scala
|
Test build #50705 has finished for PR 11010 at commit
|
Conflicts: sql/core/src/test/scala/org/apache/spark/sql/execution/BenchmarkWholeStageCodegen.scala
|
Test build #50723 has finished for PR 11010 at commit
|
|
Test build #50734 has finished for PR 11010 at commit
|
|
Test build #50736 has finished for PR 11010 at commit
|
|
Do you know how much of this is from the general clean up and how much is from switching to a simpler hash? In my experience, using a very weak hash function can make things really bad if you dont account for it other ways. |
|
@nongli As the benchmark show, the weak hash function could save 10ns per row, others may save 20ns per row. I'm also not sure the weak hash function is enough in this cases. BTW, the hashCode of ing/long in Java are also using this weak hash function, so they may not that bad. |
|
@nongli Had reverted it to Murmur3 (we could figure out a faster hash function later), the improvements become minor. |
|
LGTM |
|
Test build #50999 has finished for PR 11010 at commit
|
|
Merging this into master, thanks! |
This PR improve the lookup of BytesToBytesMap by: