digest: use github.com/minio/sha256-simd#7732
Conversation
38efacc to
f71f1f0
Compare
f71f1f0 to
a7a2b39
Compare
|
Hopefully this is could be a temporary change and Go can find an upstream solution. The performance improvement is significant enough to pull in though. |
Looks like it was merged in Go master; golang/go#53084 - see golang/go@f1b1557 (not sure if more is needed after that>), which would mean this will be in go1.20? If so, should we add a |
|
a7a2b39 to
493b586
Compare
|
Added a code comment func NewSHA256() hash.Hash {
// An equivalent of sha256-simd is expected to be merged in Go 1.20 (1.21?): https://go-review.googlesource.com/c/go/+/408795/1
return sha256simd.New()
} |
493b586 to
b4bf3d2
Compare
|
Strange that github claims there are conflicts but doesn't list any file conflicts? |
b4bf3d2 to
92a57af
Compare
|
@AkihiroSuda Would it be relevant to have this new computation in BuildKit too? |
Yes, I guess we can safely have this in BuildKit (and Moby, etc.) |
|
This is pretty cool, but I wonder if this should be opt-in given that the performance advantage is fairly negligible unless you are doing a lot digesting vs the breadth of code that relies on sha256. |
Any drawback to enable SIMD by default?
I'm not sure. The plugin will be NOP when https://go-review.googlesource.com/c/go/+/408795/1 is merged. |
Are you concerned mostly with bugs in this implementation or reliability of the CPU instructions or something else?
Another alternative would be to make this a build tag. Then there wouldn't be an extra plugin hanging around if this gets folded into Go. |
|
Does the Go upstream have aarch64 support already? If not, sha256-simd would cover more architectures than the upstream change. |
Yes https://github.com/golang/go/blob/master/src/crypto/sha256/sha256block_arm64.s |
92a57af to
4199fc2
Compare
Signed-off-by: Akihiro Suda <[email protected]>
4199fc2 to
cde9490
Compare
There was a problem hiding this comment.
LGTM
Edit for correction: Given it appears the Go core library will be adopting this a hardware-accelerated implementation of sha256, seems reasonable for us to move here as we're only doing it for the 1.7 line, which will end up on Go 1.20/1.21+ not that long into the support cycle.
https://github.com/minio/sha256-simd simdizes sha256 computation on amd64 and arm64.
Micro benchmark:
ctr images pull docker.io/library/fedora:latestonIntel(R) Core(TM) i7-1068NG7 CPU @ 2.30GHz(withsha_ni), 4 coresBefore:
After: