From 32fbdcd36c6bee27afc4e97b4c1965d01ffef78c Mon Sep 17 00:00:00 2001 From: Andrew Harding Date: Thu, 17 Mar 2022 12:11:15 -0600 Subject: [PATCH 1/2] Reduce allocations for ID type Going from ID to string causes an allocation. This change avoids this allocation by holding onto the original string and instead tracking where the path begins inside of that string. Note that this does not increase memory usage since the backing array of the original string was already being held by the existing code. This change also reduces the struct size of the ID type. Signed-off-by: Andrew Harding --- v2/go.mod | 1 - v2/go.sum | 5 -- v2/spiffeid/id.go | 99 ++++++++++++++++++++------------------ v2/spiffeid/id_test.go | 37 ++++++++++++++ v2/spiffeid/trustdomain.go | 6 +-- 5 files changed, 91 insertions(+), 57 deletions(-) diff --git a/v2/go.mod b/v2/go.mod index 43b4967d..abfa3494 100644 --- a/v2/go.mod +++ b/v2/go.mod @@ -3,7 +3,6 @@ module github.com/spiffe/go-spiffe/v2 go 1.13 require ( - github.com/golang/protobuf v1.4.2 github.com/stretchr/testify v1.5.1 github.com/zeebo/errs v1.2.2 google.golang.org/grpc v1.33.2 diff --git a/v2/go.sum b/v2/go.sum index 102cda0d..96fe2490 100644 --- a/v2/go.sum +++ b/v2/go.sum @@ -10,11 +10,9 @@ github.com/envoyproxy/go-control-plane v0.9.0/go.mod h1:YTl/9mNaCwkRvm6d1a2C3ymF github.com/envoyproxy/go-control-plane v0.9.1-0.20191026205805-5f8ba28d4473/go.mod h1:YTl/9mNaCwkRvm6d1a2C3ymFceY/DCBVvsKhRF0iEA4= github.com/envoyproxy/go-control-plane v0.9.4/go.mod h1:6rpuAdCZL397s3pYoYcLgu1mIlRU8Am5FuJP05cCM98= github.com/envoyproxy/protoc-gen-validate v0.1.0/go.mod h1:iSmxcyjqTsJpI2R4NaDN7+kN2VEUnK/pcBlmesArF7c= -github.com/golang/glog v0.0.0-20160126235308-23def4e6c14b h1:VKtxabqXZkF25pY9ekfRL6a582T4P37/31XEstQ5p58= github.com/golang/glog v0.0.0-20160126235308-23def4e6c14b/go.mod h1:SBH7ygxi8pfUlaOkMMuAQtPIUF8ecWP5IEl/CR7VP2Q= github.com/golang/mock v1.1.1/go.mod h1:oTYuIxOrZwtPieC+H1uAHpcLFnEyAGVDL/k47Jfbm0A= github.com/golang/protobuf v1.2.0/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U= -github.com/golang/protobuf v1.3.2 h1:6nsPYzhq5kReh6QImI3k5qWzO4PEbvbIW2cwSfR/6xs= github.com/golang/protobuf v1.3.2/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U= github.com/golang/protobuf v1.3.3/go.mod h1:vzj43D7+SQXF/4pzW/hwtAqwc6iTitCiVSaWz5lYuqw= github.com/golang/protobuf v1.4.0-rc.1/go.mod h1:ceaxUfeHdC40wWswd/P6IGgMaK3YpKi5j83Wpe3EHw8= @@ -25,7 +23,6 @@ github.com/golang/protobuf v1.4.0/go.mod h1:jodUvKwWbYaEsadDk5Fwe5c77LiNKVO9IDvq github.com/golang/protobuf v1.4.1/go.mod h1:U8fpvMrcmy5pZrNK1lt4xCsGvpyWQ/VVv6QDs8UjoX8= github.com/golang/protobuf v1.4.2 h1:+Z5KGCizgyZCbGh1KZqA0fcLLkwbsjIzS4aV2v7wJX0= github.com/golang/protobuf v1.4.2/go.mod h1:oDoupMAO8OvCJWAcko0GGGIgR6R6ocIYbsSw735rRwI= -github.com/google/go-cmp v0.2.0 h1:+dTQ8DZQJz0Mb/HjFlkptS1FeQ4cWSnN941F8aEG4SQ= github.com/google/go-cmp v0.2.0/go.mod h1:oXzfMopK8JAjlY9xF4vHSVASa0yLyX7SntLO5aqRK0M= github.com/google/go-cmp v0.3.0/go.mod h1:8QqcDgzrUqlUb/G2PQTWiueGozuR1884gddMywk6iLU= github.com/google/go-cmp v0.3.1/go.mod h1:8QqcDgzrUqlUb/G2PQTWiueGozuR1884gddMywk6iLU= @@ -73,9 +70,7 @@ golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8T google.golang.org/appengine v1.1.0/go.mod h1:EbEs0AVv82hx2wNQdGPgUI5lhzA/G0D9YwlJXL52JkM= google.golang.org/appengine v1.4.0/go.mod h1:xpcJRLb0r/rnEns0DIKYYv+WjYCduHsrkT7/EB5XEv4= google.golang.org/genproto v0.0.0-20180817151627-c66870c02cf8/go.mod h1:JiN7NxoALGmiZfu7CAH4rXhgtRTLTxftemlI0sWmxmc= -google.golang.org/genproto v0.0.0-20190819201941-24fa4b261c55 h1:gSJIx1SDwno+2ElGhA4+qG2zF97qiUzTM+rQ0klBOcE= google.golang.org/genproto v0.0.0-20190819201941-24fa4b261c55/go.mod h1:DMBHOl98Agz4BDEuKkezgsaosCRResVns1a3J2ZsMNc= -google.golang.org/genproto v0.0.0-20200526211855-cb27e3aa2013 h1:+kGHl1aib/qcwaRi1CbqBZ1rk19r85MNUf8HaBghugY= google.golang.org/genproto v0.0.0-20200526211855-cb27e3aa2013/go.mod h1:NbSheEEYHJ7i3ixzK3sjbqSGDJWnxyFXZblF3eUsNvo= google.golang.org/genproto v0.0.0-20200806141610-86f49bd18e98 h1:LCO0fg4kb6WwkXQXRQQgUYsFeFb5taTX5WAx5O/Vt28= google.golang.org/genproto v0.0.0-20200806141610-86f49bd18e98/go.mod h1:FWY/as6DDZQgahTzZj3fqbO1CbirC29ZNUFHwi0/+no= diff --git a/v2/spiffeid/id.go b/v2/spiffeid/id.go index 34fd36ec..35d0a56c 100644 --- a/v2/spiffeid/id.go +++ b/v2/spiffeid/id.go @@ -8,7 +8,8 @@ import ( ) const ( - schemePrefix = "spiffe://" + schemePrefix = "spiffe://" + schemePrefixLen = len(schemePrefix) ) // FromPath returns a new SPIFFE ID in the given trust domain and with the @@ -16,7 +17,10 @@ const ( // SPIFFE specification. // See https://github.com/spiffe/spiffe/blob/main/standards/SPIFFE-ID.md#22-path func FromPath(td TrustDomain, path string) (ID, error) { - return td.ID().ReplacePath(path) + if err := ValidatePath(path); err != nil { + return ID{}, err + } + return makeID(td, path) } // FromPathf returns a new SPIFFE ID from the formatted path in the given trust @@ -24,7 +28,11 @@ func FromPath(td TrustDomain, path string) (ID, error) { // SPIFFE specification. // See https://github.com/spiffe/spiffe/blob/main/standards/SPIFFE-ID.md#22-path func FromPathf(td TrustDomain, format string, args ...interface{}) (ID, error) { - return td.ID().ReplacePathf(format, args...) + path, err := FormatPath(format, args...) + if err != nil { + return ID{}, err + } + return makeID(td, path) } // FromSegments returns a new SPIFFE ID in the given trust domain with joined @@ -32,7 +40,11 @@ func FromPathf(td TrustDomain, format string, args ...interface{}) (ID, error) { // specification and must not contain path separators. // See https://github.com/spiffe/spiffe/blob/main/standards/SPIFFE-ID.md#22-path func FromSegments(td TrustDomain, segments ...string) (ID, error) { - return td.ID().ReplaceSegments(segments...) + path, err := JoinPathSegments(segments...) + if err != nil { + return ID{}, err + } + return makeID(td, path) } // FromString parses a SPIFFE ID from a string. @@ -44,11 +56,9 @@ func FromString(id string) (ID, error) { return ID{}, errWrongScheme } - rest := id[len(schemePrefix):] - - i := 0 - for ; i < len(rest); i++ { - c := rest[i] + pathidx := schemePrefixLen + for ; pathidx < len(id); pathidx++ { + c := id[pathidx] if c == '/' { break } @@ -57,20 +67,17 @@ func FromString(id string) (ID, error) { } } - if i == 0 { + if pathidx == schemePrefixLen { return ID{}, errMissingTrustDomain } - td := rest[:i] - path := rest[i:] - - if err := ValidatePath(path); err != nil { + if err := ValidatePath(id[pathidx:]); err != nil { return ID{}, err } return ID{ - td: TrustDomain{name: td}, - path: path, + id: id, + pathidx: pathidx, }, nil } @@ -86,32 +93,32 @@ func FromURI(uri *url.URL) (ID, error) { // ID is a SPIFFE ID type ID struct { - td TrustDomain - path string + id string + pathidx int } // TrustDomain returns the trust domain of the SPIFFE ID. func (id ID) TrustDomain() TrustDomain { - return id.td + if id.IsZero() { + return TrustDomain{} + } + return TrustDomain{name: id.id[schemePrefixLen:id.pathidx]} } // MemberOf returns true if the SPIFFE ID is a member of the given trust domain. func (id ID) MemberOf(td TrustDomain) bool { - return id.td == td + return id.TrustDomain() == td } // Path returns the path of the SPIFFE ID inside the trust domain. func (id ID) Path() string { - return id.path + return id.id[id.pathidx:] } // String returns the string representation of the SPIFFE ID, e.g., // "spiffe://example.org/foo/bar". func (id ID) String() string { - if id.IsZero() { - return "" - } - return schemePrefix + id.td.String() + id.path + return id.id } // URL returns a URL for SPIFFE ID. @@ -122,14 +129,14 @@ func (id ID) URL() *url.URL { return &url.URL{ Scheme: "spiffe", - Host: id.td.name, - Path: id.path, + Host: id.TrustDomain().String(), + Path: id.Path(), } } // IsZero returns true if the SPIFFE ID is the zero value. func (id ID) IsZero() bool { - return id.td.IsZero() + return id.id == "" } // AppendPath returns an ID with the appended path. It will fail if called on a @@ -143,7 +150,7 @@ func (id ID) AppendPath(path string) (ID, error) { if err := ValidatePath(path); err != nil { return ID{}, err } - id.path += path + id.id += path return id, nil } @@ -159,7 +166,7 @@ func (id ID) AppendPathf(format string, args ...interface{}) (ID, error) { if err != nil { return ID{}, err } - id.path += path + id.id += path return id, nil } @@ -175,7 +182,7 @@ func (id ID) AppendSegments(segments ...string) (ID, error) { if err != nil { return ID{}, err } - id.path += path + id.id += path return id, nil } @@ -187,11 +194,7 @@ func (id ID) ReplacePath(path string) (ID, error) { if id.IsZero() { return ID{}, errors.New("cannot replace path on a zero ID value") } - if err := ValidatePath(path); err != nil { - return ID{}, err - } - id.path = path - return id, nil + return FromPath(id.TrustDomain(), path) } // ReplacePathf returns an ID with the formatted path in the same trust domain. @@ -202,12 +205,7 @@ func (id ID) ReplacePathf(format string, args ...interface{}) (ID, error) { if id.IsZero() { return ID{}, errors.New("cannot replace path on a zero ID value") } - path, err := FormatPath(format, args...) - if err != nil { - return ID{}, err - } - id.path = path - return id, nil + return FromPathf(id.TrustDomain(), format, args...) } // ReplaceSegments returns an ID with the joined path segments in the same @@ -219,12 +217,7 @@ func (id ID) ReplaceSegments(segments ...string) (ID, error) { if id.IsZero() { return ID{}, errors.New("cannot replace path segments on a zero ID value") } - path, err := JoinPathSegments(segments...) - if err != nil { - return ID{}, err - } - id.path = path - return id, nil + return FromSegments(id.TrustDomain(), segments...) } // MarshalText returns a text representation of the ID. If the ID is the zero @@ -250,3 +243,13 @@ func (id *ID) UnmarshalText(text []byte) error { *id = unmarshaled return nil } + +func makeID(td TrustDomain, path string) (ID, error) { + if td.IsZero() { + return ID{}, errors.New("trust domain is empty") + } + return ID{ + id: schemePrefix + td.name + path, + pathidx: schemePrefixLen + len(td.name), + }, nil +} diff --git a/v2/spiffeid/id_test.go b/v2/spiffeid/id_test.go index c9f1ad12..0c528388 100644 --- a/v2/spiffeid/id_test.go +++ b/v2/spiffeid/id_test.go @@ -461,6 +461,31 @@ func TestIDTextUnmarshaler(t *testing.T) { require.Equal(t, "spiffe://trustdomain/path", s.ID.String()) } +func BenchmarkIDFromString(b *testing.B) { + s := "spiffe://trustdomain/path" + for n := 0; n < b.N; n++ { + escapes(spiffeid.RequireFromString(s).String()) + } +} + +func BenchmarkIDFromPath(b *testing.B) { + for n := 0; n < b.N; n++ { + escapes(spiffeid.RequireFromPath(td, "/path").String()) + } +} + +func BenchmarkIDFromPathf(b *testing.B) { + for n := 0; n < b.N; n++ { + escapes(spiffeid.RequireFromPathf(td, "%s", "/path").String()) + } +} + +func BenchmarkIDFromSegments(b *testing.B) { + for n := 0; n < b.N; n++ { + escapes(spiffeid.RequireFromSegments(td, "path").String()) + } +} + func assertIDEqual(t *testing.T, id spiffeid.ID, expectTD spiffeid.TrustDomain, expectPath string) { assert.Equal(t, expectTD, id.TrustDomain(), "unexpected trust domain") assert.Equal(t, expectPath, id.Path(), "unexpected path") @@ -491,3 +516,15 @@ func assertErrorContains(t *testing.T, err error, contains string) { assert.Contains(t, err.Error(), contains) } } + +var dummy struct { + b bool + x string +} + +// escapes prevents a string from being stack allocated due to escape analysis +func escapes(s string) { + if dummy.b { + dummy.x = s + } +} diff --git a/v2/spiffeid/trustdomain.go b/v2/spiffeid/trustdomain.go index 9f6af1e9..4e3157a6 100644 --- a/v2/spiffeid/trustdomain.go +++ b/v2/spiffeid/trustdomain.go @@ -57,10 +57,10 @@ func (td TrustDomain) String() string { // ID returns the SPIFFE ID of the trust domain. func (td TrustDomain) ID() ID { - return ID{ - td: td, - path: "", + if id, err := makeID(td, ""); err == nil { + return id } + return ID{} } // IDString returns a string representation of the the SPIFFE ID of the trust From d61b3ee99842d084917debe9ed17c7b9770cf631 Mon Sep 17 00:00:00 2001 From: Andrew Harding Date: Fri, 18 Mar 2022 14:31:05 -0600 Subject: [PATCH 2/2] add a small comment explaining pathidx Signed-off-by: Andrew Harding --- v2/spiffeid/id.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/v2/spiffeid/id.go b/v2/spiffeid/id.go index 35d0a56c..f4e02eee 100644 --- a/v2/spiffeid/id.go +++ b/v2/spiffeid/id.go @@ -93,7 +93,10 @@ func FromURI(uri *url.URL) (ID, error) { // ID is a SPIFFE ID type ID struct { - id string + id string + + // pathidx tracks the index to the beginning of the path inside of id. This + // is used when extracting the trust domain or path portions of the id. pathidx int }