Skip to content

Commit

Permalink
subScope: use a slice of tags instead of a map (#97)
Browse files Browse the repository at this point in the history 
* subScope: use a slice of tags instead of a map

This commit changes the subScope to keep a slice of tags instead of a
map. This means that we no longer modify or "own" the map we're passed,
which has been a source of bugs recently (that gostats owns any map
passed to it was poorly, if at all, documented).

This change also improves performance by 2x and reduces memory usage and
GC pressure as each subScope will no longer be holding a map (which is
sparse and the GC must scan).

This change also moves us closer to passing tags via a slice (instead of
a map) which is a change that I intend to make part of the V1 release
(assuming I ever get around to that).

```
benchmark                                old ns/op     new ns/op     delta
BenchmarkStore_ScopeWithTags-12          1321          646           -51.10%
BenchmarkStore_ScopeNoTags-12            583           179           -69.30%

benchmark                                old allocs     new allocs     delta
BenchmarkStore_ScopeWithTags-12          4              3              -25.00%
BenchmarkStore_ScopeNoTags-12            4              2              -50.00%

benchmark                                old bytes     new bytes     delta
BenchmarkStore_ScopeWithTags-12          544           512           -5.88%
BenchmarkStore_ScopeNoTags-12            304           112           -63.16%
```

* Update tags.go

Co-authored-by: Max R <[email protected]>

* stats_test: code review comments

* tags_test: benchmark tagSet.Search

* tags: rename CompareAndSwap => cas and document network sort

* tags: save alloc in replaceChars

Co-authored-by: Max R <[email protected]>
  • Loading branch information
@charlievieth @mxr
charlievieth and mxr authored May 27, 2020
1 parent cf86465 commit d95cbc0
Show file tree
Hide file tree
Showing 5 changed files with 1,014 additions and 95 deletions.
4 changes: 2 additions & 2 deletions net_sink_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -318,7 +318,7 @@ func TestScopesWithTags(t *testing.T) {

expected := "a.b.d.t.__x=b.__y=a.__z=b:1.000000|ms\na.b.d.c.__x=b.__y=a.__z=b:1|c\na.b.d.g.__x=b.__y=a.__z=b:1|g\n"
if expected != sink.record {
t.Errorf("Expected: '%s' Got: '%s'", expected, sink.record)
t.Errorf("\n# Expected:\n%s\n# Got:\n%s\n", expected, sink.record)
}
}

Expand All @@ -338,7 +338,7 @@ func TestScopesAndMetricsWithTags(t *testing.T) {

expected := "a.b.t.__x=m.__y=a.__z=m:1.000000|ms\na.b.c.__x=m.__y=a.__z=m:1|c\na.b.g.__x=m.__y=a.__z=m:1|g\n"
if expected != sink.record {
t.Errorf("Expected: '%s' Got: '%s'", expected, sink.record)
t.Errorf("\n# Expected:\n%s\n# Got:\n%s\n", expected, sink.record)
}
}

Expand Down
245 changes: 187 additions & 58 deletions stats.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -354,29 +354,36 @@ func (s *statStore) Store() Store {
}

func (s *statStore) Scope(name string) Scope {
return s.ScopeWithTags(name, nil)
return newSubScope(s, name, nil)
}

func (s *statStore) ScopeWithTags(name string, tags map[string]string) Scope {
return subScope{registry: s, name: name, tags: tags}
}

func (s *statStore) NewCounter(name string) Counter {
return s.NewCounterWithTags(name, nil)
return newSubScope(s, name, tags)
}

func (s *statStore) NewCounterWithTags(name string, tags map[string]string) Counter {
name = serializeTags(name, tags)
if v, ok := s.counters.Load(name); ok {
func (s *statStore) newCounter(serializedName string) *counter {
if v, ok := s.counters.Load(serializedName); ok {
return v.(*counter)
}
c := new(counter)
if v, loaded := s.counters.LoadOrStore(name, c); loaded {
c = v.(*counter)
if v, loaded := s.counters.LoadOrStore(serializedName, c); loaded {
return v.(*counter)
}
return c
}

func (s *statStore) NewCounter(name string) Counter {
return s.newCounter(name)
}

func (s *statStore) NewCounterWithTags(name string, tags map[string]string) Counter {
return s.newCounter(serializeTags(name, tags))
}

func (s *statStore) newCounterWithTagSet(name string, tags tagSet) Counter {
return s.newCounter(serializeTagSet(name, tags))
}

var emptyPerInstanceTags = map[string]string{"_f": "i"}

func (s *statStore) NewPerInstanceCounter(name string, tags map[string]string) Counter {
Expand All @@ -391,22 +398,42 @@ func (s *statStore) NewPerInstanceCounter(name string, tags map[string]string) C
return s.NewCounterWithTags(name, tags)
}

func (s *statStore) NewGauge(name string) Gauge {
return s.NewGaugeWithTags(name, nil)
type statCache struct {
cache sync.Map
new func() interface{}
}

func (s *statStore) NewGaugeWithTags(name string, tags map[string]string) Gauge {
name = serializeTags(name, tags)
if v, ok := s.gauges.Load(name); ok {
func (s *statCache) LoadOrCreate(key string) interface{} {
if v, ok := s.cache.Load(key); ok {
return v
}
v, _ := s.cache.LoadOrStore(key, s.new())
return v
}

func (s *statStore) newGauge(serializedName string) *gauge {
if v, ok := s.gauges.Load(serializedName); ok {
return v.(*gauge)
}
g := new(gauge)
if v, loaded := s.gauges.LoadOrStore(name, g); loaded {
g = v.(*gauge)
if v, loaded := s.gauges.LoadOrStore(serializedName, g); loaded {
return v.(*gauge)
}
return g
}

func (s *statStore) NewGauge(name string) Gauge {
return s.newGauge(name)
}

func (s *statStore) NewGaugeWithTags(name string, tags map[string]string) Gauge {
return s.newGauge(serializeTags(name, tags))
}

func (s *statStore) newGaugeWithTagSet(name string, tags tagSet) Gauge {
return s.newGauge(serializeTagSet(name, tags))
}

func (s *statStore) NewPerInstanceGauge(name string, tags map[string]string) Gauge {
if len(tags) == 0 {
return s.NewGaugeWithTags(name, emptyPerInstanceTags)
Expand All @@ -419,22 +446,29 @@ func (s *statStore) NewPerInstanceGauge(name string, tags map[string]string) Gau
return s.NewGaugeWithTags(name, tags)
}

func (s *statStore) NewTimer(name string) Timer {
return s.NewTimerWithTags(name, nil)
}

func (s *statStore) NewTimerWithTags(name string, tags map[string]string) Timer {
name = serializeTags(name, tags)
if v, ok := s.timers.Load(name); ok {
func (s *statStore) newTimer(serializedName string) *timer {
if v, ok := s.timers.Load(serializedName); ok {
return v.(*timer)
}
t := &timer{name: name, sink: s.sink}
if v, loaded := s.timers.LoadOrStore(name, t); loaded {
t = v.(*timer)
t := &timer{name: serializedName, sink: s.sink}
if v, loaded := s.timers.LoadOrStore(serializedName, t); loaded {
return v.(*timer)
}
return t
}

func (s *statStore) NewTimer(name string) Timer {
return s.newTimer(name)
}

func (s *statStore) NewTimerWithTags(name string, tags map[string]string) Timer {
return s.newTimer(serializeTags(name, tags))
}

func (s *statStore) newTimerWithTagSet(name string, tags tagSet) Timer {
return s.newTimer(serializeTagSet(name, tags))
}

func (s *statStore) NewPerInstanceTimer(name string, tags map[string]string) Timer {
if len(tags) == 0 {
return s.NewTimerWithTags(name, emptyPerInstanceTags)
Expand All @@ -450,74 +484,169 @@ func (s *statStore) NewPerInstanceTimer(name string, tags map[string]string) Tim
type subScope struct {
registry *statStore
name string
tags map[string]string
tags tagSet // read-only and may be shared by multiple subScopes
}

func newSubScope(registry *statStore, name string, tags map[string]string) *subScope {
a := make(tagSet, 0, len(tags))
for k, v := range tags {
if k != "" && v != "" {
a = append(a, tagPair{key: k, value: replaceChars(v)})
}
}
a.Sort()
return &subScope{registry: registry, name: name, tags: a}
}

func (s subScope) Scope(name string) Scope {
func (s *subScope) Scope(name string) Scope {
return s.ScopeWithTags(name, nil)
}

func (s subScope) ScopeWithTags(name string, tags map[string]string) Scope {
return &subScope{registry: s.registry, name: joinScopes(s.name, name), tags: s.mergeTags(tags)}
func (s *subScope) ScopeWithTags(name string, tags map[string]string) Scope {
return &subScope{
registry: s.registry,
name: joinScopes(s.name, name),
tags: s.mergeTags(tags),
}
}

func (s subScope) Store() Store {
func (s *subScope) Store() Store {
return s.registry
}

func (s subScope) NewCounter(name string) Counter {
func (s *subScope) NewCounter(name string) Counter {
return s.NewCounterWithTags(name, nil)
}

func (s subScope) NewCounterWithTags(name string, tags map[string]string) Counter {
return s.registry.NewCounterWithTags(joinScopes(s.name, name), s.mergeTags(tags))
func (s *subScope) NewCounterWithTags(name string, tags map[string]string) Counter {
return s.registry.newCounterWithTagSet(joinScopes(s.name, name), s.mergeTags(tags))
}

func (s subScope) NewPerInstanceCounter(name string, tags map[string]string) Counter {
return s.registry.NewPerInstanceCounter(joinScopes(s.name, name), s.mergeTags(tags))
func (s *subScope) NewPerInstanceCounter(name string, tags map[string]string) Counter {
return s.registry.newCounterWithTagSet(joinScopes(s.name, name),
s.mergePerInstanceTags(tags))
}

func (s subScope) NewGauge(name string) Gauge {
func (s *subScope) NewGauge(name string) Gauge {
return s.NewGaugeWithTags(name, nil)
}

func (s subScope) NewGaugeWithTags(name string, tags map[string]string) Gauge {
return s.registry.NewGaugeWithTags(joinScopes(s.name, name), s.mergeTags(tags))
func (s *subScope) NewGaugeWithTags(name string, tags map[string]string) Gauge {
return s.registry.newGaugeWithTagSet(joinScopes(s.name, name), s.mergeTags(tags))
}

func (s subScope) NewPerInstanceGauge(name string, tags map[string]string) Gauge {
return s.registry.NewPerInstanceGauge(joinScopes(s.name, name), s.mergeTags(tags))
func (s *subScope) NewPerInstanceGauge(name string, tags map[string]string) Gauge {
return s.registry.newGaugeWithTagSet(joinScopes(s.name, name),
s.mergePerInstanceTags(tags))
}

func (s subScope) NewTimer(name string) Timer {
func (s *subScope) NewTimer(name string) Timer {
return s.NewTimerWithTags(name, nil)
}

func (s subScope) NewTimerWithTags(name string, tags map[string]string) Timer {
return s.registry.NewTimerWithTags(joinScopes(s.name, name), s.mergeTags(tags))
func (s *subScope) NewTimerWithTags(name string, tags map[string]string) Timer {
return s.registry.newTimerWithTagSet(joinScopes(s.name, name), s.mergeTags(tags))
}

func (s subScope) NewPerInstanceTimer(name string, tags map[string]string) Timer {
return s.registry.NewPerInstanceTimer(joinScopes(s.name, name), s.mergeTags(tags))
func (s *subScope) NewPerInstanceTimer(name string, tags map[string]string) Timer {
return s.registry.newTimerWithTagSet(joinScopes(s.name, name),
s.mergePerInstanceTags(tags))
}

func joinScopes(parent, child string) string {
return parent + "." + child
}

// mergeTags augments tags with all scope-level tags that are not already present.
// Modifies and returns tags directly.
func (s subScope) mergeTags(tags map[string]string) map[string]string {
if len(s.tags) == 0 {
return tags
// mergeOneTag is an optimized for inserting 1 tag and will panic otherwise.
func (s *subScope) mergeOneTag(tags map[string]string) tagSet {
if len(tags) != 1 {
panic("invalid usage")
}
if len(tags) == 0 {
var p tagPair
for k, v := range tags {
p = tagPair{key: k, value: replaceChars(v)}
break
}
if p.key == "" || p.value == "" {
return s.tags
}
for k, v := range s.tags {
if _, ok := tags[k]; !ok {
tags[k] = v
a := make(tagSet, len(s.tags), len(s.tags)+1)
copy(a, s.tags)
return a.Insert(p)
}

// mergeTags returns a tagSet that is the union of subScope's tags and the
// provided tags map. If any keys overlap the values from the provided map
// are used.
func (s *subScope) mergeTags(tags map[string]string) tagSet {
switch len(tags) {
case 0:
return s.tags
case 1:
// optimize for the common case of there only being one tag
return s.mergeOneTag(tags)
default:
// write tags to the end of the scratch slice
scratch := make(tagSet, len(s.tags)+len(tags))
a := scratch[len(s.tags):]
i := 0
for k, v := range tags {
if k != "" && v != "" {
a[i] = tagPair{key: k, value: replaceChars(v)}
i++
}
}
a = a[:i]
a.Sort()

if len(s.tags) == 0 {
return a
}
return mergeTagSets(s.tags, a, scratch)
}
}

// mergePerInstanceTags returns a tagSet that is the union of subScope's
// tags and the provided tags map with. If any keys overlap the values from
// the provided map are used.
//
// The returned tagSet will have a per-instance key ("_f") and if neither the
// subScope or tags have this key it's value will be the default per-instance
// value ("i").
//
// The method does not optimize for the case where there is only one tag
// because it is used less frequently.
func (s *subScope) mergePerInstanceTags(tags map[string]string) tagSet {
if len(tags) == 0 {
if s.tags.Contains("_f") {
return s.tags
}
// create copy with the per-instance tag
a := make(tagSet, len(s.tags), len(s.tags)+1)
copy(a, s.tags)
return a.Insert(tagPair{key: "_f", value: "i"})
}

// write tags to the end of scratch slice
scratch := make(tagSet, len(s.tags)+len(tags)+1)
a := scratch[len(s.tags):]
i := 0
for k, v := range tags {
if k != "" && v != "" {
a[i] = tagPair{key: k, value: replaceChars(v)}
i++
}
}
// add the default per-instance tag if not present
if tags["_f"] == "" && !s.tags.Contains("_f") {
a[i] = tagPair{key: "_f", value: "i"}
i++
}
a = a[:i]
a.Sort()

if len(s.tags) == 0 {
return a
}
return tags
return mergeTagSets(s.tags, a, scratch)
}
Loading

0 comments on commit d95cbc0

Please sign in to comment.