sha256: drop minio in favor of crypto/sha256 for go1.21 and above #173
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
Jorropo
force-pushed
the
no-minio
branch
4 times, most recently
from
335109a
to
75fa368
Compare
In go1.21 the go std has a new SHANI accelerated rountine (just like minio's sha256). See golang/go#50543. Reduce the code surface by dropping what is now a redondent librairy. Benchmarks using go1.21 and go1.20 shows no significant difference between minio and the std for sha256: ``` name old time/op new time/op delta SumAllLarge/sha2-256-12 8.63ms ± 3% 8.63ms ± 1% ~ (p=0.220 n=20+17) SumAllLarge/sha2-512-12 23.1ms ± 3% 23.0ms ± 3% ~ (p=0.361 n=18+20) SumAllLarge/dbl-sha2-256-12 36.0ms ± 4% 8.6ms ± 4% -76.16% (p=0.000 n=20+20) name old speed new speed delta SumAllLarge/sha2-256-12 1.94GB/s ± 4% 1.95GB/s ± 1% ~ (p=0.220 n=20+17) SumAllLarge/sha2-512-12 725MB/s ± 3% 729MB/s ± 3% ~ (p=0.350 n=18+20) SumAllLarge/dbl-sha2-256-12 467MB/s ± 4% 1958MB/s ± 4% +319.53% (p=0.000 n=20+20) ``` `dlb-sha2-256` quadrupled in speed because it was never wired to use minio's sha256 implementation and thus was brought back up to speed by `crypto/sha256`'s improvement. I think we should remove `github.com/multiformats/go-multihash/register/miniosha256` one go1.22 is released (~1 year) since it would be purposeless, we could also stub it to github.com/multiformats/go-multihash/register/sha256 for all versions but [no one imports it directly](https://pkg.go.dev/github.com/multiformats/go-multihash/register/miniosha256?tab=importedby).
marten-seemann
approved these changes
May 24, 2023
Jorropo
added a commit
to Jorropo/go-libp2p
that referenced
this pull request
May 24, 2023
See rational and benchmarks in multiformats/go-multihash#173. Fixes: libp2p#2308
This was referenced May 24, 2023
Jorropo
added a commit
to Jorropo/go-libp2p
that referenced
this pull request
Aug 16, 2023
See rational and benchmarks in multiformats/go-multihash#173. Fixes: libp2p#2308
Jorropo
added a commit
to Jorropo/go-libp2p
that referenced
this pull request
Aug 16, 2023
See rational and benchmarks in multiformats/go-multihash#173. Fixes: libp2p#2308
marten-seemann
pushed a commit
to libp2p/go-libp2p
that referenced
this pull request
Aug 17, 2023
See rational and benchmarks in multiformats/go-multihash#173. Fixes: #2308
lunny
pushed a commit
to go-gitea/gitea
that referenced
this pull request
Feb 25, 2024
Go 1.21 improved the performance of `crypto/sha256`. It's now similar to `minio/sha256-simd`, so we should just use the standard libs. https://go.dev/doc/go1.21#crypto/sha256 https://go-review.googlesource.com/c/go/+/408795 multiformats/go-multihash#173
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.
In go1.21 the go std has a new SHANI accelerated rountine (just like minio's sha256). See golang/go#50543.
Reduce the code surface by dropping what is now a redondent librairy.
Benchmarks using go1.21 and go1.20 shows no significant difference between minio and the std for sha256:
dlb-sha2-256quadrupled in speed because it was never wired to use minio's sha256 implementation and thuswas brought back up to speed by
crypto/sha256's improvement.I think we should remove
github.com/multiformats/go-multihash/register/miniosha256one go1.22 is released (~1 year) since it would be purposeless, we could also stub it to github.com/multiformats/go-multihash/register/sha256 for all versions but no one imports it directly.