Discussion: switch minio/blake2b-simd to golang.org/x/crypto

[mroth]

Intentionally opening as a Draft for discussion, rather than a PR.

This explores a migration away from minio/blake2b-simd module for hashing, in favor of the golang.org/x/crypto implementation. The minio version appears to have been archived in 2018 and thus no longer receives updates/audits.

While the impetus for suggesting this change was code maintenance and supply-chain security rather than performance, the performance characteristics of the standard golang.org/x/crypto blake2b hash does appear to now be better optimized for both Intel and ARM intrinsics than the original minio/blake2b-simd. I created a benchmark suite to test hashing performance specifically at https://github.com/mroth/blakebench, which on the hardware I had access to appears to perform roughly ~25% better.

arm64:

$ go test -bench=. -benchmem -cpu=1
goos: darwin
goarch: arm64
pkg: github.com/mroth/blakebench
cpu: Apple M2 Pro
BenchmarkXCryptoBlake/fcaddr-payload             6282751           180.0 ns/op         0 B/op          0 allocs/op
BenchmarkXCryptoBlake/fcaddr-chksum              6738614           176.3 ns/op         0 B/op          0 allocs/op
BenchmarkMinioBlakeSIMD/fcaddr-payload           5156140           231.2 ns/op         0 B/op          0 allocs/op
BenchmarkMinioBlakeSIMD/fcaddr-chksum            4713930           255.0 ns/op         0 B/op          0 allocs/op
PASS
ok      github.com/mroth/blakebench     5.689s

amd64:

$ GOAMD64=v3 go test -bench=. -benchmem -cpu=1
goos: linux
goarch: amd64
pkg: github.com/mroth/blakebench
cpu: Intel(R) Xeon(R) Platinum 8272CL CPU @ 2.60GHz
BenchmarkXCryptoBlake/fcaddr-payload         	 5997702	       205.4 ns/op	       0 B/op	       0 allocs/op
BenchmarkXCryptoBlake/fcaddr-chksum          	 6164827	       194.6 ns/op	       0 B/op	       0 allocs/op
BenchmarkMinioBlakeSIMD/fcaddr-payload       	 4708561	       254.9 ns/op	       0 B/op	       0 allocs/op
BenchmarkMinioBlakeSIMD/fcaddr-chksum        	 4252591	       281.9 ns/op	       0 B/op	       0 allocs/op
PASS
ok  	github.com/mroth/blakebench	5.779s

Note that for the existing benchmarks in this module, the difference appears to be negligible, since the current implementation (which I did not adjust) initializes a new hash.Hash for every operation, which I suspect likely consumes nontrivial time compared to the actual hashing itself. Before and after comparison on my laptop of go test -bench=. -count=5 -cpu=1.

$ benchstat before.txt after.txt
name                         old time/op    new time/op    delta
CborMarshal                    18.2ns ± 0%    18.2ns ± 0%    ~     (p=0.357 n=5+5)
CborUnmarshal                  89.4ns ± 1%    89.0ns ± 1%    ~     (p=0.095 n=5+5)
ParseActorAddress/actor        28.5ns ± 1%    27.9ns ± 1%  -2.04%  (p=0.008 n=5+5)
ParseActorAddress/bls          33.8ns ± 1%    33.0ns ± 1%  -2.35%  (p=0.008 n=5+5)
ParseActorAddress/secp256k1    28.5ns ± 1%    27.8ns ± 0%  -2.19%  (p=0.008 n=5+5)
ParseActorAddress/id           24.3ns ± 0%    22.1ns ± 0%  -9.00%  (p=0.008 n=5+5)

name                         old alloc/op   new alloc/op   delta
CborMarshal                     1.00B ± 0%     1.00B ± 0%    ~     (all equal)
CborUnmarshal                    120B ± 0%      120B ± 0%    ~     (all equal)
ParseActorAddress/actor         48.0B ± 0%     48.0B ± 0%    ~     (all equal)
ParseActorAddress/bls            128B ± 0%      128B ± 0%    ~     (all equal)
ParseActorAddress/secp256k1     48.0B ± 0%     48.0B ± 0%    ~     (all equal)
ParseActorAddress/id            4.00B ± 0%     4.00B ± 0%    ~     (all equal)

name                         old allocs/op  new allocs/op  delta
CborMarshal                      1.00 ± 0%      1.00 ± 0%    ~     (all equal)
CborUnmarshal                    4.00 ± 0%      4.00 ± 0%    ~     (all equal)
ParseActorAddress/actor          2.00 ± 0%      2.00 ± 0%    ~     (all equal)
ParseActorAddress/bls            2.00 ± 0%      2.00 ± 0%    ~     (all equal)
ParseActorAddress/secp256k1      2.00 ± 0%      2.00 ± 0%    ~     (all equal)
ParseActorAddress/id             2.00 ± 0%      2.00 ± 0%    ~     (all equal)

Note that looking at the dependency chain, this module does still indirectly import minio/blake2b-simd via its dependency on the (now deprecated, it appears) ipfs/go-ipld-cbor module, so if going this route one may also wish to explore a migration to ipfs/go-ipld-prime which is apparently the stated replacement.