-
-
Notifications
You must be signed in to change notification settings - Fork 5.6k
Chained hash pipelining in array hashing #58252
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
base: master
Are you sure you want to change the base?
Conversation
show performance benchmarks and then lgtm. |
#57509 (comment) the graphically: note that this cannot merge before #57509, which is also waiting on #58053 |
CI failure unrelated |
should this method be used for big |
how does the Tuple perf look? if it only helps for 100 or more, I wouldn't bother. if it's useful in the 10-100 range, I think we should |
eh, idt it helps. I guess tuples should mostly be hashing at compile time anyway |
the proposed switch in #57509 from
3h - hash_finalizer(x)
tohash_finalizer(3h -x)
should increase the hash quality of chained hashes, as the expanded expression goes from something likesum((-3)^k * hash(x) for k in ...)
to a non-simplifiable compositionthis does have the unfortunate impact of long chains of hashes getting a bit slower as there is more data dependency and the CPU cannot work on the next element's hash before combining the previous one (I think --- I'm not particularly an expert on this low level stuff). As far as I know this only really impacts
AbstractArray
so, I've implemented a proposal that does some unrolling / pipelining manually to recover
AbstractArray
hashing performance. in fact, it's quite a lot faster now for most lengths. I tuned the thresholds (8 accumulators, certain length breakpoints) by hand on my own machine.