chore(core): FastMap to use Robin Hood hashing #4054
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
puzpuzpuz
added
SQL
|
|
puzpuzpuz
reviewed
Dec 12, 2023
| @@ -333,22 +333,63 @@ private FastMapValue asNew(BaseKey keyWriter, int index, int hashCode, FastMapVa | |||
| kPos = (kPos + 7) & ~0x7; | |||
| setOffset(offsets, index, keyWriter.startAddress - heapStart); | |||
| setHashCode(offsets, index, hashCode); | |||
| if (--free == 0) { | |||
There was a problem hiding this comment.
Shouldn't we change the default load factor for FastMap to 0.8 or even 0.9? To me, the main benefit of using Robin Hood hashing is being able to fit more entries into less memory and that should improve performance in certain scenarios.
There was a problem hiding this comment.
good idea, I can test it.
[PR Coverage check]😍 pass : 50 / 50 (100.00%) file detail
|
|
closing as new results do not show any improvements with default load and actually it performs worse with higher loads. chances are this PR shifted bottlenecks: #4032 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
FastMap to use Robin Hood hashing.
Testing setup: Clickbench running on c6a.16xlarge
Query:
SELECT UserID, COUNT(*) AS c FROM hits GROUP BY UserID ORDER BY c DESC LIMIT 10;7.3.8-snapshot vanilla
timings: 10992530610
timings: 10956769125
timings: 10934153712
timings: 10952430208
timings: 10920044785
timings: 10868601623
timings: 10865651603
timings: 10885177297
7.3.8-snapshot + Robin Hood FastMap
timings: 10353366441
timings: 10266570821
timings: 10262674985
timings: 10261647727
timings: 10245524141
timings: 10273126124
timings: 10252424759
timings: 10245550197