🚀 Speed up toMetricTagsID (#130)

metrics/benchmark_test.go

@@ -0,0 +1,37 @@
+// Copyright (c) 2018 Palantir Technologies. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package metrics
+
+import (
+	"context"
+	"fmt"
+	"testing"
+)
+
+func BenchmarkRegisterMetric(b *testing.B) {
+	b.Run("1 tag", func(b *testing.B) {
+		doBench(b, 1)
+	})
+	b.Run("10 tag", func(b *testing.B) {
+		doBench(b, 10)
+	})
+	b.Run("100 tag", func(b *testing.B) {
+		doBench(b, 100)
+	})
+}
+
+func doBench(b *testing.B, n int) {
+	var tags Tags
+	for i := 0; i < n; i++ {
+		tags = append(tags, MustNewTag(fmt.Sprintf("key%d", i), fmt.Sprintf("val%d", i)))
+	}
+	ctx := AddTags(WithRegistry(context.Background(), NewRootMetricsRegistry()), tags...)
+	b.ResetTimer()
+	b.ReportAllocs()
+	reg := FromContext(ctx)
+	for i := 0; i < b.N; i++ {
+		reg.Counter("metricName").Inc(1)
+	}
+}

metrics/registry.go

@@ -5,8 +5,8 @@
 package metrics
 
 import (
+	"bytes"
 	"context"
-	"fmt"
 	"sort"
 	"strings"
 	"sync"
@@ -260,35 +260,37 @@ func (r *rootRegistry) Subregistry(prefix string, tags ...Tag) Registry {
 
 func (r *rootRegistry) Each(f MetricVisitor) {
 	// sort names so that iteration order is consistent
-	var sortedNames []string
+	var sortedMetricIDs []string
 	allMetrics := make(map[string]interface{})
 	r.registry.Each(func(name string, metric interface{}) {
 		// filter out the runtime metrics that are defined in the exclude list
 		if _, ok := goRuntimeMetricsToExclude[name]; ok {
 			return
 		}
-		sortedNames = append(sortedNames, name)
+		sortedMetricIDs = append(sortedMetricIDs, name)
 		allMetrics[name] = metric
 	})
-	sort.Strings(sortedNames)
+	sort.Strings(sortedMetricIDs)
 
-	for _, name := range sortedNames {
-		metric := allMetrics[name]
-
-		var tags Tags
+	for _, id := range sortedMetricIDs {
 		r.idToMetricMutex.RLock()
-		metricWithTags, ok := r.idToMetricWithTags[metricTagsID(name)]
+		metricWithTags, ok := r.idToMetricWithTags[metricTagsID(id)]
 		r.idToMetricMutex.RUnlock()
+
+		var name string
+		var tags Tags
 		if ok {
 			name = metricWithTags.name
-			for t := range metricWithTags.tags {
-				tags = append(tags, t)
-			}
-			sort.Slice(tags, func(i, j int) bool {
-				return tags[i].String() < tags[j].String()
-			})
+			tags = make(Tags, len(metricWithTags.tags))
+			copy(tags, metricWithTags.tags)
+			sort.Sort(tags)
+		} else {
+			// Metric was added to rcrowley registry outside of our registry.
+			// This is likely a go runtime metric (nothing else is exposed).
+			name = id
 		}
-		val := ToMetricVal(metric)
+
+		val := ToMetricVal(allMetrics[id])
 		if val == nil {
 			// this should never happen as all the things we put inside the registry can be turned into MetricVal
 			panic("could not convert metric to MetricVal")
@@ -343,7 +345,7 @@ func (r *rootRegistry) registerMetric(name string, tags Tags) string {
 	r.idToMetricMutex.Lock()
 	r.idToMetricWithTags[metricID] = metricWithTags{
 		name: name,
-		tags: tags.ToSet(),
+		tags: tags,
 	}
 	r.idToMetricMutex.Unlock()
 	return string(metricID)
@@ -352,29 +354,31 @@ func (r *rootRegistry) registerMetric(name string, tags Tags) string {
 // metricWithTags stores a specific metric with its set of tags.
 type metricWithTags struct {
 	name string
-	tags map[Tag]struct{}
+	tags Tags
 }
 
 // metricTagsID is the unique identifier for a given metric. Each {metricName, set<Tag>} pair is considered to be a
-// unique metric. A metricTagsID is a string of the following form: "<name>|tags:|<tag1>|<tag2>|". The tags appear in
-// ascending alphanumeric order. If a metric does not have any tags, its metricsTagsID is of the form: "<name>|tags:||".
+// unique metric. A metricTagsID is a string of the following form: "<name>|<tag1>|<tag2>". The tags appear in
+// ascending alphanumeric order. If a metric does not have any tags, its metricsTagsID is of the form: "<name>".
 type metricTagsID string
 
-// toID generates the metricTagsID identifier for the metricWithTags. A unique {metricName, set<Tag>} input will
-// generate a unique output.
-func (m *metricWithTags) toID() metricTagsID {
-	var sortedTags []string
-	for t := range m.tags {
-		sortedTags = append(sortedTags, t.String())
-	}
-	sort.Strings(sortedTags)
+// toMetricTagsID generates the metricTagsID identifier for the metricWithTags. A unique {metricName, set<Tag>} input will
+// generate a unique output. This implementation tries to minimize memory allocation and runtime.
+func toMetricTagsID(name string, tags Tags) metricTagsID {
+	// TODO(maybe): Ensure tags is already sorted when it comes in so we can remove this.
+	sort.Sort(tags)
 
-	return metricTagsID(fmt.Sprintf("%s|tags:|%s|", m.name, strings.Join(sortedTags, "|")))
-}
+	// calculate how large to make our byte buffer below
+	bufSize := len(name)
+	for _, t := range tags {
+		bufSize += len(t.keyValue) + 1 // 1 for separator
+	}
 
-func toMetricTagsID(name string, tags Tags) metricTagsID {
-	return (&metricWithTags{
-		name: name,
-		tags: tags.ToSet(),
-	}).toID()
+	buf := bytes.NewBuffer(make([]byte, 0, bufSize))
+	_, _ = buf.WriteString(name)
+	for _, tag := range tags {
+		_, _ = buf.WriteRune('|')
+		_, _ = buf.WriteString(tag.keyValue)
+	}
+	return metricTagsID(buf.Bytes())
 }

metrics/tag.go

@@ -16,6 +16,8 @@ import (
 type Tag struct {
 	key   string
 	value string
+	// Store the concatenated key and value so we don't need to reconstruct it in String() (used in toMetricTagID)
+	keyValue string
 }
 
 func (t Tag) Key() string {
@@ -28,7 +30,7 @@ func (t Tag) Value() string {
 
 // The full representation of the tag, which is "key:value".
 func (t Tag) String() string {
-	return t.key + ":" + t.value
+	return t.keyValue
 }
 
 type Tags []Tag
@@ -52,6 +54,18 @@ func (t Tags) ToMap() map[string]string {
 	return tags
 }
 
+func (t Tags) Len() int {
+	return len(t)
+}
+
+func (t Tags) Less(i, j int) bool {
+	return t[i].keyValue < t[j].keyValue
+}
+
+func (t Tags) Swap(i, j int) {
+	t[i], t[j] = t[j], t[i]
+}
+
 // MustNewTag returns the result of calling NewTag, but panics if NewTag returns an error. Should only be used in
 // instances where the inputs are statically defined and known to be valid.
 func MustNewTag(k, v string) Tag {
@@ -89,10 +103,17 @@ func NewTag(k, v string) (Tag, error) {
 		return Tag{}, errors.New(`full tag ("key:value") must be <= 200 characters`)
 	}
 
+	return newTag(k, v), nil
+}
+
+func newTag(k, v string) Tag {
+	normalizedKey := normalizeTag(k)
+	normalizedValue := normalizeTag(v)
 	return Tag{
-		key:   normalizeTag(k),
-		value: normalizeTag(v),
-	}, nil
+		key:      normalizedKey,
+		value:    normalizedValue,
+		keyValue: normalizedKey + ":" + normalizedValue,
+	}
 }
 
 // MustNewTags returns the result of calling NewTags, but panics if NewTags returns an error. Should only be used in