-
Notifications
You must be signed in to change notification settings - Fork 118
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
Basic chore cleanups of tests and benchmarks #66
Conversation
After the latest renames I am also noticing that the arm go-preamble does a lot of assignment work: compared to the intel one pushing everything into asm-land: Sadly I do not know enough assembly yet to properly adjust the preamble of https://github.com/minio/sha256-simd/blob/9235fbaea/sha256block_arm64.s#L28-L36, but I am pretty sure this will speed up things even more. |
@ribasushi Thanks. The preamble work should be pretty harmless since it doesn't escape. I had to double check the logic for the Xeon case, but it seems that the library correctly falls back to using the stdlib. I guess now we could remove the arm64 version as well. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Yeah. Although if you look closely the ARM version on sub-block messages is noticeably faster than the stdlib one. I didn't manage to find the source of the difference
|
Virtually all the time is spend in the (main) hashing loop, so optimizing calling into the assembly will have a negligible performance effect (even for very short messages that are hashed, but those are super fast regardless). |
No functional changes whatsoever, only renaming of some internal functions and shoring up the benchmark executor.
Interesting results on go 1.19 /cc @klauspost
AWS Graviton Neoverse-N1
Intel(R) Xeon(R) Silver 4214 CPU @ 2.20GHz
AMD Ryzen 7 3700X 8-Core Processor
Macbook 2015 Intel(R) Core(TM) i7-4770HQ CPU @ 2.20GHz
Macbook M1 Pro 2022