Reduce allocations for ID type (#185)

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 <[email protected]>
spiffe · Mar 19, 2022 · 3d166bc · 3d166bc
1 parent fbdcc18
commit 3d166bc
Show file tree

Hide file tree

Showing 5 changed files with 94 additions and 57 deletions.
diff --git 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
@@ -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
@@ -8,31 +8,43 @@ import (
 )
 
 const (
-	schemePrefix = "spiffe://"
+	schemePrefix    = "spiffe://"
+	schemePrefixLen = len(schemePrefix)
 )
 
 // FromPath returns a new SPIFFE ID in the given trust domain and with the
 // given path. The supplied path must be a valid absolute path according to the
 // 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
 // domain. The formatted path must be a valid absolute path according to the
 // 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
 // path segments. The path segments must be valid according to the SPIFFE
 // 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,35 @@ func FromURI(uri *url.URL) (ID, error) {
 
 // ID is a SPIFFE ID
 type ID struct {
-	td   TrustDomain
-	path 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
 }
 
 // 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 +132,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 +153,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 +169,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 +185,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 +197,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 +208,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 +220,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 +246,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
@@ -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
@@ -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