Skip to content

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

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

adienes
Copy link
Member

@adienes adienes commented Apr 28, 2025

the proposed switch in #57509 from 3h - hash_finalizer(x) to hash_finalizer(3h -x) should increase the hash quality of chained hashes, as the expanded expression goes from something like sum((-3)^k * hash(x) for k in ...) to a non-simplifiable composition

this 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.

@adienes adienes added performance Must go faster hashing labels Apr 28, 2025
@adienes adienes mentioned this pull request Apr 28, 2025
@oscardssmith
Copy link
Member

show performance benchmarks and then lgtm.

@adienes
Copy link
Member Author

adienes commented Apr 28, 2025

#57509 (comment) the vec column contains timing; the data for this PR is under :commit == "pipeline" sorry I should have been more clear in that comment

graphically:

image
image

note that this cannot merge before #57509, which is also waiting on #58053

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants