Performance improvements for formatPath

rules/callback_listener.go

@@ -19,7 +19,7 @@ type httpCallbackListener struct {
 func (htcbl httpCallbackListener) callbackDone(ruleID string, attributes extendedAttributes) {
 	attributeMap := make(map[string]string)
 	for _, k := range attributes.names() {
-		attributeMap[k] = *attributes.GetAttribute(k)
+		attributeMap[k], _ = attributes.FindAttribute(k)
 	}
 	postObj := callbackEvent{
 		RuleID:     ruleID,

rules/dynamic_rule.go

@@ -211,6 +211,15 @@ func (na *nestingAttributes) GetAttribute(key string) *string {
 	return na.nested.GetAttribute(key)
 }
 
+func (na *nestingAttributes) FindAttribute(key string) (string, bool) {
+	for _, attribute := range na.attrs {
+		if attribute.key == key {
+			return *attribute.value, true
+		}
+	}
+	return na.nested.FindAttribute(key)
+}
+
 func (na *nestingAttributes) Format(pattern string) string {
 	return FormatWithAttributes(pattern, na)
 }

rules/matcher.go

@@ -19,7 +19,9 @@ type regexKeyMatcher struct {
 }
 
 type keyMatch interface {
+	// GetAttribute usage should be replaced with FindAttribute
 	GetAttribute(name string) *string
+	FindAttribute(name string) (string, bool)
 	Format(pattern string) string
 	names() []string
 }
@@ -103,6 +105,15 @@ func (m *regexKeyMatch) GetAttribute(name string) *string {
 	return &result
 }
 
+func (m *regexKeyMatch) FindAttribute(name string) (string, bool) {
+	index, ok := m.fieldMap[name]
+	if !ok {
+		return "", false
+	}
+	result := m.matchStrings[index]
+	return result, true
+}
+
 func (m *regexKeyMatch) names() []string {
 	names := make([]string, 0, len(m.fieldMap))
 	for name := range m.fieldMap {
@@ -122,28 +133,51 @@ func FormatWithAttributes(pattern string, m Attributes) string {
 	return result
 }
 
+type finderWrapper struct{ Attributes }
+
+func (f finderWrapper) FindAttribute(s string) (string, bool) {
+	if ptr := f.GetAttribute(s); ptr != nil {
+		return *ptr, true
+	}
+
+	return "", false
+}
+
 func formatPath(pattern string, m Attributes) (string, bool) {
+	sb := new(strings.Builder)
+	// If the formatted string can fit into 2.5x the length of the pattern
+	// (and mapAttributes is the attribute implementation used)
+	// this will be the only allocation
+	sb.Grow(2*len(pattern) + (len(pattern) / 2))
+
+	var finder AttributeFinder
+	if f, ok := m.(AttributeFinder); ok {
+		finder = f
+	} else {
+		finder = finderWrapper{m}
+	}
+
 	allFound := true
-	paths := strings.Split(pattern, "/")
-	result := strings.Builder{}
-	for _, path := range paths {
-		if len(path) == 0 {
-			continue
-		}
-		result.WriteString("/")
-		if strings.HasPrefix(path, ":") {
-			attr := m.GetAttribute(path[1:])
-			if attr == nil {
-				s := path
-				attr = &s
+	var segment string
+	for found := true; found; {
+		segment, pattern, found = strings.Cut(pattern, "/")
+		switch {
+		case segment == "":
+		case strings.HasPrefix(segment, ":"):
+			sb.WriteByte('/')
+			if attr, ok := finder.FindAttribute(segment[1:]); ok {
+				sb.WriteString(attr)
+			} else {
 				allFound = false
+				sb.WriteString(segment)
 			}
-			result.WriteString(*attr)
-		} else {
-			result.WriteString(path)
+
+		default:
+			sb.WriteByte('/')
+			sb.WriteString(segment)
 		}
 	}
-	return result.String(), allFound
+	return sb.String(), allFound
 }
 
 // Keep the bool return value, because it's tricky to check for null
@@ -187,6 +221,11 @@ func (ma *mapAttributes) GetAttribute(key string) *string {
 	return &value
 }
 
+func (ma *mapAttributes) FindAttribute(key string) (string, bool) {
+	value, ok := ma.values[key]
+	return value, ok
+}
+
 func (ma *mapAttributes) Format(path string) string {
 	return FormatWithAttributes(path, ma)
 }

rules/matcher_test.go

@@ -1,9 +1,11 @@
 package rules
 
 import (
+	"strings"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 func TestBasic(t *testing.T) {
@@ -72,3 +74,152 @@ func TestNoRegex(t *testing.T) {
 		t.Fail()
 	}
 }
+
+func BenchmarkFormatPath(b *testing.B) {
+	cases := []struct {
+		name    string
+		attr    Attributes
+		pattern string
+	}{
+		{
+			name:    "03 matches",
+			attr:    NewAttributes(map[string]string{"first": "abcdefghijklomnopqrstuvwxyz", "second": "some_region_name", "third": "acde070d-8c4c-4f0d-9d8a-162843c10333"}),
+			pattern: "/first/:first/second/:second/third/:third",
+		},
+		{
+			name:    "05 matches",
+			attr:    NewAttributes(map[string]string{"a": "abcdefghijklomnopqrstuvwxyz", "b": "abcdefghijklomnopqrstuvwxyz", "c": "acde070d-8c4c-4f0d-9d8a-162843c10333", "d": "acde070d-8c4c-4f0d-9d8a-162843c10333", "e": "acde070d-8c4c-4f0d-9d8a-162843c10333"}),
+			pattern: "first/:a/second/:b/third/:c/fourth/:d/fifth/:e/sixth",
+		},
+		{
+			name:    "10 matches",
+			attr:    NewAttributes(map[string]string{"a": "abcdefghijklomnopqrstuvwxyz", "b": "abcdefghijklomnopqrstuvwxyz", "c": "acde070d-8c4c-4f0d-9d8a-162843c10333", "d": "acde070d-8c4c-4f0d-9d8a-162843c10333", "e": "acde070d-8c4c-4f0d-9d8a-162843c10333"}),
+			pattern: "first/:a/second/:b/third/:c/fourth/:d/fifth/:e/first/:a/second/:b/third/:c/fourth/:d/fifth/:e/sixth",
+		},
+		{
+			name:    "50 matches",
+			attr:    NewAttributes(map[string]string{"param": "acde070d-8c4c-4f0d-9d8a-162843c10333"}),
+			pattern: strings.Repeat("/:param", 50),
+		},
+		{
+			name:    "03 missing",
+			attr:    NewAttributes(map[string]string{"x": "one", "y": "two", "z": "three"}),
+			pattern: "/first/:first/second/:second/third/:third",
+		},
+		{
+			name:    "05 missing",
+			attr:    NewAttributes(map[string]string{"1": "aaaaaaaaaa", "2": "aaaaaaaaaa", "3": "aaaaaaaaaa", "4": "aaaaaaaaaa", "5": "eeeeeeeeee"}),
+			pattern: "first/:a/second/:b/third/:c/fourth/:d/fifth/:e/sixth",
+		},
+		{
+			name:    "10 missing",
+			attr:    NewAttributes(map[string]string{"1": "aaaaaaaaaa", "2": "aaaaaaaaaa", "3": "aaaaaaaaaa", "4": "aaaaaaaaaa", "5": "eeeeeeeeee"}),
+			pattern: "first/:a/second/:b/third/:c/fourth/:d/fifth/:e/first/:a/second/:b/third/:c/fourth/:d/fifth/:e/sixth",
+		},
+		{
+			name:    "50 missing",
+			attr:    NewAttributes(map[string]string{"x": ""}),
+			pattern: strings.Repeat("/:param", 50),
+		},
+		{
+			name:    "all slashes",
+			attr:    NewAttributes(map[string]string{}),
+			pattern: "////////////////////",
+		},
+		{
+			name:    "all patterns",
+			attr:    NewAttributes(map[string]string{}),
+			pattern: ":/:/:/:/:/:/:/:/:/:/:/:/:/:/:/:/:/:/:/:",
+		},
+	}
+
+	for _, tc := range cases {
+		b.Run("curr_"+tc.name, func(b *testing.B) {
+			for n := 0; n < b.N; n++ {
+				formatPath(tc.pattern, tc.attr)
+			}
+		})
+	}
+}
+
+func TestFormatPath(t *testing.T) {
+	cases := []struct {
+		name       string
+		attr       Attributes
+		pattern    string
+		expectstr  string
+		expectbool bool
+	}{
+		{
+			name:       "3 matching",
+			attr:       NewAttributes(map[string]string{"first": "one", "second": "two", "third": "three"}),
+			pattern:    "/first/:first/second/:second/third/:third",
+			expectstr:  "/first/one/second/two/third/three",
+			expectbool: true,
+		},
+		{
+			name:       "5 matching",
+			attr:       NewAttributes(map[string]string{"a": "aaaaaaaaaa", "b": "aaaaaaaaaa", "c": "aaaaaaaaaa", "d": "aaaaaaaaaa", "e": "eeeeeeeeee"}),
+			pattern:    "first/:a/second/:b/third/:c/fourth/:d/fifth/:e/sixth",
+			expectstr:  "/first/aaaaaaaaaa/second/aaaaaaaaaa/third/aaaaaaaaaa/fourth/aaaaaaaaaa/fifth/eeeeeeeeee/sixth",
+			expectbool: true,
+		},
+		{
+			name:       "empty segment",
+			attr:       NewAttributes(map[string]string{}),
+			pattern:    "a///b",
+			expectstr:  "/a/b",
+			expectbool: true,
+		},
+		{
+			name:       "empty",
+			attr:       NewAttributes(map[string]string{}),
+			pattern:    "",
+			expectstr:  "",
+			expectbool: true,
+		},
+		{
+			name:       "single slash",
+			attr:       NewAttributes(map[string]string{}),
+			pattern:    "/",
+			expectstr:  "",
+			expectbool: true,
+		},
+		{
+			name:       "empty paths",
+			attr:       NewAttributes(map[string]string{}),
+			pattern:    "///",
+			expectstr:  "",
+			expectbool: true,
+		},
+		{
+			name:       "empty pattern",
+			attr:       NewAttributes(map[string]string{}),
+			pattern:    "/:",
+			expectstr:  "/:",
+			expectbool: false,
+		},
+		{
+			name:       "empty pattern",
+			attr:       NewAttributes(map[string]string{"": "test"}),
+			pattern:    "/:",
+			expectstr:  "/test",
+			expectbool: true,
+		},
+		{
+			name:       "empty value",
+			attr:       NewAttributes(map[string]string{"x": ""}),
+			pattern:    ":x/:x/:x",
+			expectstr:  "///",
+			expectbool: true,
+		},
+	}
+
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			actualstr, actualbool := formatPath(tc.pattern, tc.attr)
+			require.Equal(t, tc.expectstr, actualstr)
+			require.Equal(t, tc.expectbool, actualbool)
+		})
+	}
+}

rules/task.go

@@ -11,13 +11,23 @@ import (
 // with dynamic keys.  For instance, a dynamic key "/static/:dynamic"
 // that is matched against "/static/value1" would contain an yield
 // an attribute with the key "dynamic" and the value "value1".
+// Attributes implementers should also implement AttributeFinder so that
+// the more performant/explicit implementation can be used in internal functions
 type Attributes interface {
 	GetAttribute(string) *string
 	Format(string) string
 }
 
+// AttributeFinder is a more performant replacement for the GetAttribute
+// method of Attributes. Internal functions use the FindAttribute method
+// if passed an implementation of Attributes that also implements AttributeFinder
+type AttributeFinder interface {
+	FindAttribute(string) (string, bool)
+}
+
 type extendedAttributes interface {
 	Attributes
+	AttributeFinder
 	names() []string
 }
 

v3enginetest/main.go

@@ -128,7 +128,8 @@ func main() {
 		// value is set that will prevent further polling even after the polling key TTL has expired.
 		task.Logger.Info("Callback called")
 		// This is thread safe, because the map is only being read and not written to.
-		p := ps[*task.Attr.GetAttribute("id")]
+		id := *task.Attr.GetAttribute("id")
+		p := ps[id]
 		pPollCount := atomic.LoadInt32(&p.pollCount)
 		// Retrieve a value from etcd.
 		path := task.Attr.Format(dataPath)