From 799bca97f2dd7421faaaf9d0b5842264359a87ea Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Thu, 27 Jun 2024 11:41:43 +0200 Subject: [PATCH 1/4] pkg/reference: Spec.String(): use string-concatenation instead of sprintf These were straight concatenations of strings; reduce some allocations by removing fmt.Sprintf for this. Signed-off-by: Sebastiaan van Stijn --- pkg/reference/reference.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/pkg/reference/reference.go b/pkg/reference/reference.go index a4bf6da60..23b88214e 100644 --- a/pkg/reference/reference.go +++ b/pkg/reference/reference.go @@ -18,7 +18,6 @@ package reference import ( "errors" - "fmt" "net/url" "path" "regexp" @@ -146,10 +145,10 @@ func (r Spec) String() string { return r.Locator } if r.Object[:1] == "@" { - return fmt.Sprintf("%v%v", r.Locator, r.Object) + return r.Locator + r.Object } - return fmt.Sprintf("%v:%v", r.Locator, r.Object) + return r.Locator + ":" + r.Object } // SplitObject provides two parts of the object spec, delimited by an `@` From 74a6156ac2fa53928f8c40b92ca096f12948bd0d Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Thu, 27 Jun 2024 11:44:36 +0200 Subject: [PATCH 2/4] pkg/reference: SplitObject: zero allocations Before / After: BenchmarkSplitObject-10 2785656 428.1 ns/op 416 B/op 13 allocs/op BenchmarkSplitObjectNew-10 13510520 88.2 ns/op 0 B/op 0 allocs/op Signed-off-by: Sebastiaan van Stijn --- pkg/reference/reference.go | 8 ++++---- pkg/reference/reference_test.go | 30 ++++++++++++++++++++++++++++++ 2 files changed, 34 insertions(+), 4 deletions(-) diff --git a/pkg/reference/reference.go b/pkg/reference/reference.go index 23b88214e..0bbcdab29 100644 --- a/pkg/reference/reference.go +++ b/pkg/reference/reference.go @@ -157,9 +157,9 @@ func (r Spec) String() string { // Either may be empty and it is the callers job to validate them // appropriately. func SplitObject(obj string) (tag string, dgst digest.Digest) { - parts := strings.SplitAfterN(obj, "@", 2) - if len(parts) < 2 { - return parts[0], "" + if i := strings.Index(obj, "@"); i >= 0 { + // Offset by one so preserve the "@" in the tag returned. + return obj[:i+1], digest.Digest(obj[i+1:]) } - return parts[0], digest.Digest(parts[1]) + return obj, "" } diff --git a/pkg/reference/reference_test.go b/pkg/reference/reference_test.go index 91f479ba3..b8929a5c8 100644 --- a/pkg/reference/reference_test.go +++ b/pkg/reference/reference_test.go @@ -192,3 +192,33 @@ func TestReferenceParser(t *testing.T) { }) } } + +func BenchmarkSplitObject(b *testing.B) { + inputs := []string{ + "", + "@", + "docker.io@", + "@digest", + "docker.io/library/redis:foo?fooo=asdf", + "docker.io/library/redis:foo@sha256:abcdef?fooo=asdf", + "docker.io/library/redis@sha256:abcdef?fooo=asdf", + "docker.io/library/redis:obj@abcdef?fooo=asdf", + "localhost:5000/library/redis:obj@abcdef?fooo=asdf", + "/docker.io/library/redis:obj@abcdef?fooo=asdf", + "docker.io/library/redis?fooo=asdf", + "sub-dom1.foo.com/bar/baz/quux:latest", + "sub-dom1.foo.com/bar/baz/quux:some-long-tag", + "b.gcr.io/test.example.com/my-app:test.example.com", + "xn--n3h.com/myimage:xn--n3h.com", // ☃.com in punycode + "xn--7o8h.com/myimage:xn--7o8h.com@sha512:fffffff", + "http://xn--7o8h.com/myimage:xn--7o8h.com@sha512:fffffff", + } + + b.ReportAllocs() + + for i := 0; i < b.N; i++ { + for _, input := range inputs { + _, _ = SplitObject(input) + } + } +} From 42145950bbcd0e0e310397354c97408431102c71 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Thu, 27 Jun 2024 11:53:24 +0200 Subject: [PATCH 3/4] pkg/reference: SplitObject: add proper GoDoc The behavior of this function is quite counter-intuitive, as it preserves the delimiter in the result. This function should probably have been an internal function, as its use for external consumers would be very limited, but let's at least document the (surprising) behavior for those that are considering to use it. It appears that BuildKit is currently the only (publicly visible) external consumer of this function; I am planning to inline its functionality in Spec.Digest() and to deprecate this function so that it can be removed. Signed-off-by: Sebastiaan van Stijn --- pkg/reference/reference.go | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/pkg/reference/reference.go b/pkg/reference/reference.go index 0bbcdab29..5d1b40772 100644 --- a/pkg/reference/reference.go +++ b/pkg/reference/reference.go @@ -151,11 +151,19 @@ func (r Spec) String() string { return r.Locator + ":" + r.Object } -// SplitObject provides two parts of the object spec, delimited by an `@` -// symbol. +// SplitObject provides two parts of the object spec, delimited by an "@" +// symbol. It does not perform any validation on correctness of the values +// returned, and it's the callers' responsibility to validate the result. // -// Either may be empty and it is the callers job to validate them -// appropriately. +// If an "@" delimiter is found, it returns the part *including* the "@" +// delimiter as "tag", and the part after the "@" as digest. +// +// The example below produces "docker.io/library/ubuntu:latest@" and +// "sha256:deadbeef"; +// +// t, d := SplitObject("docker.io/library/ubuntu:latest@sha256:deadbeef") +// fmt.Println(t) // docker.io/library/ubuntu:latest@ +// fmt.Println(d) // sha256:deadbeef func SplitObject(obj string) (tag string, dgst digest.Digest) { if i := strings.Index(obj, "@"); i >= 0 { // Offset by one so preserve the "@" in the tag returned. From a5fce38f31bd514ffa82bb4590752dd3b70d4f06 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Thu, 27 Jun 2024 11:57:42 +0200 Subject: [PATCH 4/4] pkg/reference: Spec.Digest(): inline SplitObject code Inline the relevant code from SplitObject, as we're only interested in the digest portion. Signed-off-by: Sebastiaan van Stijn --- pkg/reference/reference.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/pkg/reference/reference.go b/pkg/reference/reference.go index 5d1b40772..1f3a47063 100644 --- a/pkg/reference/reference.go +++ b/pkg/reference/reference.go @@ -135,8 +135,12 @@ func (r Spec) Hostname() string { // Digest returns the digest portion of the reference spec. This may be a // partial or invalid digest, which may be used to lookup a complete digest. func (r Spec) Digest() digest.Digest { - _, dgst := SplitObject(r.Object) - return dgst + i := strings.Index(r.Object, "@") + + if i < 0 { + return "" + } + return digest.Digest(r.Object[i+1:]) } // String returns the normalized string for the ref.