Skip to content
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

proposal: runtime: replace wyhash with rapidhash #67935

Closed
Nicoshev opened this issue Jun 11, 2024 · 2 comments
Closed

proposal: runtime: replace wyhash with rapidhash #67935

Nicoshev opened this issue Jun 11, 2024 · 2 comments
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. Performance Proposal

Comments

@Nicoshev
Copy link

Proposal Details

hash64.go currently implements wyhash

wyhash owner recognized rapidhash as the official new version.

Several measurements showed rapidhash as faster and of better quality than wyhash.

It is currently the best recomended hash function by both SMHasher and SMHasher3

Integrating rapidhash may be an opportunity to easily improve hash64.go without any drawbacks.

Cheers,
Nicolas

@Nicoshev Nicoshev changed the title Upgrade src/runtime/hash64.go to rapidhash proposal: Upgrade src/runtime/hash64.go to rapidhash Jun 11, 2024
@seankhliao seankhliao changed the title proposal: Upgrade src/runtime/hash64.go to rapidhash proposal: runtime: replace wyhash with rapidhash Jun 11, 2024
@seankhliao
Copy link
Member

I think we'd like to see the performance of a Go implementation first?

@mauri870 mauri870 added Performance compiler/runtime Issues related to the Go compiler and/or runtime. labels Jun 11, 2024
@randall77
Copy link
Contributor

The hash used in hash64.go is inspired by wyhash, it isn't an actual copy of it. So I'm not sure exactly what changes are being proposed here.

Also, this is the fallback hash. Popular architectures don't use this hash, they use assembly implementations that use things like AES instructions. Rapidhash claims better performance on amd64 and arm64, but those are both platforms where hash64.go doesn't matter.

That said, I'm happy to look at CLs. I'm going to close this issue, as it doesn't need to be part of the proposal process. Make sure we see some performance data with the CLs (you can force use of the fallback hash by doing things like GODEBUG=cpu.aes=off).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. Performance Proposal
Projects
None yet
Development

No branches or pull requests

4 participants