From a1f6e2eda3b7a539ac014e92017fc4dd890ed934 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Tue, 2 Jul 2024 12:08:01 +0200 Subject: [PATCH 1/7] address some linting issues - rename "digester" vars that shadowed package-level digester type - rename "hexdigestbytes" to be properly camelCase - use "errors.Is()" instead of straight comparing errors Signed-off-by: Sebastiaan van Stijn --- algorithm.go | 18 ++++++++---------- algorithm_test.go | 3 ++- digestset/set.go | 2 +- testdigest/testdigest.go | 3 ++- 4 files changed, 13 insertions(+), 13 deletions(-) diff --git a/algorithm.go b/algorithm.go index 22656e4..6fdd987 100644 --- a/algorithm.go +++ b/algorithm.go @@ -156,8 +156,8 @@ func RegisterAlgorithm(algorithm Algorithm, implementation CryptoHash) bool { // hexDigestRegex can be used to generate a regex for RegisterAlgorithm. func hexDigestRegex(cryptoHash CryptoHash) *regexp.Regexp { - hexdigestbytes := cryptoHash.Size() * 2 - return regexp.MustCompile(fmt.Sprintf("^[a-f0-9]{%d}$", hexdigestbytes)) + hexDigestBytes := cryptoHash.Size() * 2 + return regexp.MustCompile(fmt.Sprintf("^[a-f0-9]{%d}$", hexDigestBytes)) } // Available returns true if the digest type is available for use. If this @@ -254,20 +254,18 @@ func (a Algorithm) Encode(d []byte) string { // FromReader returns the digest of the reader using the algorithm. func (a Algorithm) FromReader(rd io.Reader) (Digest, error) { - digester := a.Digester() - - if _, err := io.Copy(digester.Hash(), rd); err != nil { + d := a.Digester() + if _, err := io.Copy(d.Hash(), rd); err != nil { return "", err } - return digester.Digest(), nil + return d.Digest(), nil } // FromBytes digests the input and returns a Digest. func (a Algorithm) FromBytes(p []byte) Digest { - digester := a.Digester() - - if _, err := digester.Hash().Write(p); err != nil { + d := a.Digester() + if _, err := d.Hash().Write(p); err != nil { // Writes to a Hash should never fail. None of the existing // hash implementations in the stdlib or hashes vendored // here can return errors from Write. Having a panic in this @@ -276,7 +274,7 @@ func (a Algorithm) FromBytes(p []byte) Digest { panic("write to hash function returned error: " + err.Error()) } - return digester.Digest() + return d.Digest() } // FromString digests the string input and returns a Digest. diff --git a/algorithm_test.go b/algorithm_test.go index 76b47d1..6ca9a58 100644 --- a/algorithm_test.go +++ b/algorithm_test.go @@ -19,6 +19,7 @@ import ( "bytes" "crypto" "crypto/rand" + "errors" "flag" "fmt" "strings" @@ -56,7 +57,7 @@ func TestFlagInterface(t *testing.T) { } { t.Run(testcase.Name, func(t *testing.T) { alg = Canonical - if err := flagSet.Parse(testcase.Args); err != testcase.Err { + if err := flagSet.Parse(testcase.Args); !errors.Is(err, testcase.Err) { if testcase.Err == nil { t.Fatal("unexpected error", err) } diff --git a/digestset/set.go b/digestset/set.go index 991a5f7..7c72c6e 100644 --- a/digestset/set.go +++ b/digestset/set.go @@ -93,7 +93,7 @@ func (dst *Set) Lookup(d string) (digest.Digest, error) { hex string ) dgst, err := digest.Parse(d) - if err == digest.ErrDigestInvalidFormat { + if errors.Is(err, digest.ErrDigestInvalidFormat) { hex = d searchFunc = func(i int) bool { return dst.entries[i].val >= d diff --git a/testdigest/testdigest.go b/testdigest/testdigest.go index bdb3db5..d6ae0bf 100644 --- a/testdigest/testdigest.go +++ b/testdigest/testdigest.go @@ -20,6 +20,7 @@ package testdigest import ( + "errors" "testing" pkgdigest "github.com/opencontainers/go-digest" @@ -38,7 +39,7 @@ type TestCase struct { func RunTestCase(t *testing.T, testcase TestCase) { digest, err := pkgdigest.Parse(testcase.Input) - if err != testcase.Err { + if errors.Is(err, testcase.Err) { t.Fatalf("error differed from expected while parsing %q: %v != %v", testcase.Input, err, testcase.Err) } From f5b4cdae29dc0b73a94fdedf05fbb5e6d4cd8bd7 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Tue, 2 Jul 2024 13:23:25 +0200 Subject: [PATCH 2/7] testdigest: fix package GoDoc to be in the right format Signed-off-by: Sebastiaan van Stijn --- testdigest/testdigest.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/testdigest/testdigest.go b/testdigest/testdigest.go index d6ae0bf..520be13 100644 --- a/testdigest/testdigest.go +++ b/testdigest/testdigest.go @@ -12,11 +12,11 @@ // See the License for the specific language governing permissions and // limitations under the License. -// testdigest is a separate package, because it has some testing utilities in it that may be useful -// to other internal Algorithm implementors. +// Package testdigest is a separate package, because it has some testing +// utilities that may be useful to other internal Algorithm implementors. // -// It is not a stable interface and not meant for consumption outside of digest developers. - +// It is not a stable interface and not meant for consumption outside of +// digest developers. package testdigest import ( From 3d866abde690b1c6a707298130044f458538efcf Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Tue, 2 Jul 2024 13:24:49 +0200 Subject: [PATCH 3/7] remove import aliases Signed-off-by: Sebastiaan van Stijn --- digestset/set.go | 2 +- digestset/set_test.go | 2 +- testdigest/testdigest.go | 28 ++++++++++++++-------------- 3 files changed, 16 insertions(+), 16 deletions(-) diff --git a/digestset/set.go b/digestset/set.go index 7c72c6e..ea45e4c 100644 --- a/digestset/set.go +++ b/digestset/set.go @@ -21,7 +21,7 @@ import ( "strings" "sync" - digest "github.com/opencontainers/go-digest" + "github.com/opencontainers/go-digest" ) var ( diff --git a/digestset/set_test.go b/digestset/set_test.go index 05ad1d4..b260d99 100644 --- a/digestset/set_test.go +++ b/digestset/set_test.go @@ -21,7 +21,7 @@ import ( "math/rand" "testing" - digest "github.com/opencontainers/go-digest" + "github.com/opencontainers/go-digest" ) func assertEqualDigests(t *testing.T, d1, d2 digest.Digest) { diff --git a/testdigest/testdigest.go b/testdigest/testdigest.go index 520be13..79f4a51 100644 --- a/testdigest/testdigest.go +++ b/testdigest/testdigest.go @@ -23,7 +23,7 @@ import ( "errors" "testing" - pkgdigest "github.com/opencontainers/go-digest" + "github.com/opencontainers/go-digest" ) type TestCase struct { @@ -32,14 +32,14 @@ type TestCase struct { // If err is non-nil, then the parsing of Input is expected to return this error Err error // Algorithm should be an available or registered algorithm - Algorithm pkgdigest.Algorithm + Algorithm digest.Algorithm // Encoded is the the encoded portion of the digest to expect, for example e58fcf7418d4390dec8e8fb69d88c06ec07039d651fedd3aa72af9972e7d046b Encoded string } func RunTestCase(t *testing.T, testcase TestCase) { - digest, err := pkgdigest.Parse(testcase.Input) - if errors.Is(err, testcase.Err) { + dgst, err := digest.Parse(testcase.Input) + if !errors.Is(err, testcase.Err) { t.Fatalf("error differed from expected while parsing %q: %v != %v", testcase.Input, err, testcase.Err) } @@ -47,26 +47,26 @@ func RunTestCase(t *testing.T, testcase TestCase) { return } - if digest.Algorithm() != testcase.Algorithm { - t.Fatalf("incorrect Algorithm for parsed digest: %q != %q", digest.Algorithm(), testcase.Algorithm) + if dgst.Algorithm() != testcase.Algorithm { + t.Fatalf("incorrect Algorithm for parsed digest: %q != %q", dgst.Algorithm(), testcase.Algorithm) } - if digest.Encoded() != testcase.Encoded { - t.Fatalf("incorrect hex for parsed digest: %q != %q", digest.Encoded(), testcase.Encoded) + if dgst.Encoded() != testcase.Encoded { + t.Fatalf("incorrect hex for parsed digest: %q != %q", dgst.Encoded(), testcase.Encoded) } // Parse string return value and check equality - newParsed, err := pkgdigest.Parse(digest.String()) + newParsed, err := digest.Parse(dgst.String()) if err != nil { t.Fatalf("unexpected error parsing Input %q: %v", testcase.Input, err) } - if newParsed != digest { - t.Fatalf("expected equal: %q != %q", newParsed, digest) + if newParsed != dgst { + t.Fatalf("expected equal: %q != %q", newParsed, dgst) } - newFromHex := pkgdigest.NewDigestFromEncoded(newParsed.Algorithm(), newParsed.Encoded()) - if newFromHex != digest { - t.Fatalf("%v != %v", newFromHex, digest) + newFromHex := digest.NewDigestFromEncoded(newParsed.Algorithm(), newParsed.Encoded()) + if newFromHex != dgst { + t.Fatalf("%v != %v", newFromHex, dgst) } } From 38caa322a4cb7d10a2f5433e0ad99ebcf887a087 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Tue, 2 Jul 2024 13:37:00 +0200 Subject: [PATCH 4/7] RegisterAlgorithm: skip regex if not needed No need to validate the format if we previously accepted it to be registered. Signed-off-by: Sebastiaan van Stijn --- algorithm.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/algorithm.go b/algorithm.go index 6fdd987..0bc71a1 100644 --- a/algorithm.go +++ b/algorithm.go @@ -138,14 +138,14 @@ func RegisterAlgorithm(algorithm Algorithm, implementation CryptoHash) bool { algorithmsLock.Lock() defer algorithmsLock.Unlock() - if !algorithmRegexp.MatchString(string(algorithm)) { - panic(fmt.Sprintf("Algorithm %s has a name which does not fit within the allowed grammar", algorithm)) - } - if _, ok := algorithms[algorithm]; ok { return false } + if !algorithmRegexp.MatchString(string(algorithm)) { + panic(fmt.Sprintf("Algorithm %s has a name which does not fit within the allowed grammar", algorithm)) + } + algorithms[algorithm] = implementation // We can do this since the Digest function below only implements a hex digest. If we open this in the future // we need to allow for alternative digest algorithms to be implemented and for the user to pass their own From 5d624a8c56b20d478c86d962e6d29683033273cc Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Tue, 2 Jul 2024 13:37:43 +0200 Subject: [PATCH 5/7] use errors.New() for errors that don't need formatting Signed-off-by: Sebastiaan van Stijn --- digest.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/digest.go b/digest.go index 3c473ac..857bcdf 100644 --- a/digest.go +++ b/digest.go @@ -16,6 +16,7 @@ package digest import ( + "errors" "fmt" "hash" "io" @@ -69,13 +70,13 @@ var DigestRegexpAnchored = regexp.MustCompile(`^` + DigestRegexp.String() + `$`) var ( // ErrDigestInvalidFormat returned when digest format invalid. - ErrDigestInvalidFormat = fmt.Errorf("invalid checksum digest format") + ErrDigestInvalidFormat = errors.New("invalid checksum digest format") // ErrDigestInvalidLength returned when digest has invalid length. - ErrDigestInvalidLength = fmt.Errorf("invalid checksum digest length") + ErrDigestInvalidLength = errors.New("invalid checksum digest length") // ErrDigestUnsupported returned when the digest algorithm is unsupported. - ErrDigestUnsupported = fmt.Errorf("unsupported digest algorithm") + ErrDigestUnsupported = errors.New("unsupported digest algorithm") ) // Parse parses s and returns the validated digest object. An error will From e3a8687c795c1d21f2c9f26791480d0684a3ff37 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Tue, 2 Jul 2024 13:47:50 +0200 Subject: [PATCH 6/7] digestset: report allocations in benchmarks Just for convenience ':-) Before: pkg: github.com/opencontainers/go-digest/digestset BenchmarkAdd10-10 183613 6368 ns/op BenchmarkAdd100-10 15739 75741 ns/op BenchmarkAdd1000-10 1430 831884 ns/op BenchmarkRemove10-10 169999 6984 ns/op BenchmarkRemove100-10 15312 78299 ns/op BenchmarkRemove1000-10 1470 815144 ns/op BenchmarkLookup10-10 27067897 43.69 ns/op BenchmarkLookup100-10 17203653 69.17 ns/op BenchmarkLookup1000-10 13043708 96.32 ns/op BenchmarkShortCode10-10 3574094 334.5 ns/op BenchmarkShortCode100-10 320438 3782 ns/op BenchmarkShortCode1000-10 29110 41460 ns/op After: pkg: github.com/opencontainers/go-digest/digestset BenchmarkAdd10 BenchmarkAdd10-10 176776 6400 ns/op 608 B/op 12 allocs/op BenchmarkAdd100-10 15267 76016 ns/op 5746 B/op 102 allocs/op BenchmarkAdd1000-10 1426 833977 ns/op 56267 B/op 1002 allocs/op BenchmarkRemove10-10 169448 7043 ns/op 128 B/op 2 allocs/op BenchmarkRemove100-10 15249 78730 ns/op 944 B/op 2 allocs/op BenchmarkRemove1000-10 1444 819247 ns/op 8240 B/op 2 allocs/op BenchmarkLookup10-10 27042379 45.68 ns/op 0 B/op 0 allocs/op BenchmarkLookup100-10 17098725 69.72 ns/op 0 B/op 0 allocs/op BenchmarkLookup1000-10 12769602 92.97 ns/op 0 B/op 0 allocs/op BenchmarkShortCode10-10 3551202 332.1 ns/op 630 B/op 2 allocs/op BenchmarkShortCode100-10 313929 3805 ns/op 5449 B/op 4 allocs/op BenchmarkShortCode1000-10 28963 41373 ns/op 81992 B/op 3 allocs/op Signed-off-by: Sebastiaan van Stijn --- digestset/set_test.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/digestset/set_test.go b/digestset/set_test.go index b260d99..26a60dc 100644 --- a/digestset/set_test.go +++ b/digestset/set_test.go @@ -257,6 +257,7 @@ func createDigests(count int) ([]digest.Digest, error) { } func benchAddNTable(b *testing.B, n int) { + b.ReportAllocs() digests, err := createDigests(n) if err != nil { b.Fatal(err) @@ -273,6 +274,7 @@ func benchAddNTable(b *testing.B, n int) { } func benchLookupNTable(b *testing.B, n int, shortLen int) { + b.ReportAllocs() digests, err := createDigests(n) if err != nil { b.Fatal(err) @@ -297,6 +299,7 @@ func benchLookupNTable(b *testing.B, n int, shortLen int) { } func benchRemoveNTable(b *testing.B, n int) { + b.ReportAllocs() digests, err := createDigests(n) if err != nil { b.Fatal(err) @@ -320,6 +323,7 @@ func benchRemoveNTable(b *testing.B, n int) { } func benchShortCodeNTable(b *testing.B, n int, shortLen int) { + b.ReportAllocs() digests, err := createDigests(n) if err != nil { b.Fatal(err) From 247c259e4d5ec81efa2c36591df889ccc10e922e Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Tue, 2 Jul 2024 14:08:05 +0200 Subject: [PATCH 7/7] remove unneeded fmt.Sprintf and indirections Remove fmt.Sprintf and use string-concatenation instead to reduce some allocations. Before / After: BenchmarkNewDigestFromEncoded-10 8474174 128.4 ns/op 112 B/op 3 allocs/op BenchmarkNewDigestFromEncoded-10 37912695 31.55 ns/op 80 B/op 1 allocs/op BenchmarkNewDigestFromBytes-10 5087299 237.2 ns/op 200 B/op 5 allocs/op BenchmarkNewDigestFromBytes-10 8416543 146.8 ns/op 168 B/op 3 allocs/op Signed-off-by: Sebastiaan van Stijn --- digest.go | 6 +++--- digest_test.go | 19 +++++++++++++++++++ 2 files changed, 22 insertions(+), 3 deletions(-) diff --git a/digest.go b/digest.go index 857bcdf..5868140 100644 --- a/digest.go +++ b/digest.go @@ -47,19 +47,19 @@ func NewDigest(alg Algorithm, h hash.Hash) Digest { // functions. This is also useful for rebuilding digests from binary // serializations. func NewDigestFromBytes(alg Algorithm, p []byte) Digest { - return NewDigestFromEncoded(alg, alg.Encode(p)) + return Digest(string(alg) + ":" + alg.Encode(p)) } // NewDigestFromHex returns a Digest from alg and the hex encoded digest. // // Deprecated: use [NewDigestFromEncoded] instead. func NewDigestFromHex(alg, hex string) Digest { - return NewDigestFromEncoded(Algorithm(alg), hex) + return Digest(alg + ":" + hex) } // NewDigestFromEncoded returns a Digest from alg and the encoded digest. func NewDigestFromEncoded(alg Algorithm, encoded string) Digest { - return Digest(fmt.Sprintf("%s:%s", alg, encoded)) + return Digest(string(alg) + ":" + encoded) } // DigestRegexp matches valid digest types. diff --git a/digest_test.go b/digest_test.go index 1c8ffc8..2dadbe4 100644 --- a/digest_test.go +++ b/digest_test.go @@ -15,6 +15,7 @@ package digest_test import ( + "crypto/sha256" "testing" "github.com/opencontainers/go-digest" @@ -118,3 +119,21 @@ func TestParseDigest(t *testing.T) { }) } } + +func BenchmarkNewDigestFromEncoded(b *testing.B) { + b.ReportAllocs() + + for i := 0; i < b.N; i++ { + _ = digest.NewDigestFromEncoded("sha256", "e58fcf7418d4390dec8e8fb69d88c06ec07039d651fedd3aa72af9972e7d046b") + } +} + +func BenchmarkNewDigestFromBytes(b *testing.B) { + s := sha256.Sum256([]byte("hello world")) + + b.ReportAllocs() + + for i := 0; i < b.N; i++ { + _ = digest.NewDigestFromBytes("sha256", s[:]) + } +}