Merge pull request #1158 from bloomberg/improveSetTags

Improve performance of SetTags

Author: Dieterbe

api/models/series.go

@@ -26,19 +26,46 @@ type Series struct {
 }
 
 func (s *Series) SetTags() {
-	tagSplits := strings.Split(s.Target, ";")
+	numTags := strings.Count(s.Target, ";")
+
+	if s.Tags == nil {
+		// +1 for the name tag
+		s.Tags = make(map[string]string, numTags+1)
+	} else {
+		for k := range s.Tags {
+			delete(s.Tags, k)
+		}
+	}
+
+	if numTags == 0 {
+		s.Tags["name"] = s.Target
+		return
+	}
+
+	index := strings.IndexByte(s.Target, ';')
+	name := s.Target[:index]
 
-	s.Tags = make(map[string]string, len(tagSplits))
+	remainder := s.Target
+	for index > 0 {
+		remainder = remainder[index+1:]
+		index = strings.IndexByte(remainder, ';')
 
-	for _, tagPair := range tagSplits[1:] {
-		parts := strings.SplitN(tagPair, "=", 2)
-		if len(parts) != 2 {
+		tagPair := remainder
+		if index > 0 {
+			tagPair = remainder[:index]
+		}
+
+		equalsPos := strings.IndexByte(tagPair, '=')
+		if equalsPos < 1 {
 			// Shouldn't happen
 			continue
 		}
-		s.Tags[parts[0]] = parts[1]
+
+		s.Tags[tagPair[:equalsPos]] = tagPair[equalsPos+1:]
 	}
-	s.Tags["name"] = tagSplits[0]
+
+	// Do this last to overwrite any "name" tag that might have been specified in the series tags.
+	s.Tags["name"] = name
 }
 
 func (s Series) Copy(emptyDatapoints []schema.Point) Series {

api/models/series_test.go

@@ -2,6 +2,8 @@ package models
 
 import (
 	"encoding/json"
+	"math/rand"
+	"reflect"
 	"testing"
 
 	"github.com/raintank/schema"
@@ -90,3 +92,106 @@ func TestJsonMarshal(t *testing.T) {
 		}
 	}
 }
+
+func TestSetTags(t *testing.T) {
+	cases := []struct {
+		in  Series
+		out map[string]string
+	}{
+		{
+			in: Series{},
+			out: map[string]string{
+				"name": "",
+			},
+		},
+		{
+			in: Series{
+				Target: "a",
+			},
+			out: map[string]string{
+				"name": "a",
+			},
+		},
+		{
+			in: Series{
+				Target: `a\b`,
+			},
+			out: map[string]string{
+				"name": `a\b`,
+			},
+		},
+		{
+			in: Series{
+				Target: "a;b=c;c=d",
+			},
+			out: map[string]string{
+				"name": "a",
+				"b":    "c",
+				"c":    "d",
+			},
+		},
+		{
+			in: Series{
+				Target: "a;biglongtagkeyhere=andithasabiglongtagvaluetoo;c=d",
+			},
+			out: map[string]string{
+				"name":              "a",
+				"biglongtagkeyhere": "andithasabiglongtagvaluetoo",
+				"c":                 "d",
+			},
+		},
+	}
+
+	for _, c := range cases {
+		c.in.SetTags()
+		if !reflect.DeepEqual(c.out, c.in.Tags) {
+			t.Fatalf("SetTags incorrect\nexpected:%v\ngot:     %v\n", c.out, c.in.Tags)
+		}
+	}
+}
+
+func BenchmarkSetTags_00tags_00chars(b *testing.B) {
+	benchmarkSetTags(b, 0, 0, 0, true)
+}
+
+func BenchmarkSetTags_20tags_32chars(b *testing.B) {
+	benchmarkSetTags(b, 20, 32, 32, true)
+}
+
+func BenchmarkSetTags_20tags_32chars_reused(b *testing.B) {
+	benchmarkSetTags(b, 20, 32, 32, false)
+}
+
+func benchmarkSetTags(b *testing.B, numTags, tagKeyLength, tagValueLength int, resetTags bool) {
+	in := Series{
+		Target: "my.metric.name",
+	}
+
+	for i := 0; i < numTags; i++ {
+		in.Target += ";" + randString(tagKeyLength) + "=" + randString(tagValueLength)
+	}
+
+	b.ResetTimer()
+
+	for i := 0; i < b.N; i++ {
+		in.SetTags()
+		if len(in.Tags) != numTags+1 {
+			b.Fatalf("Expected %d tags, got %d, target = %s, tags = %v", numTags+1, len(in.Tags), in.Target, in.Tags)
+		}
+		if resetTags {
+			// Reset so as to not game the allocations
+			in.Tags = nil
+		}
+	}
+	b.SetBytes(int64(len(in.Target)))
+}
+
+func randString(n int) string {
+	const letterBytes = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ"
+
+	b := make([]byte, n)
+	for i := range b {
+		b[i] = letterBytes[rand.Int63()%int64(len(letterBytes))]
+	}
+	return string(b)
+}