Skip to content

Commit

Permalink
Pool sortables used to create attribute sets (open-telemetry#3832)
Browse files Browse the repository at this point in the history
* Pool sortables used to create attribute sets

* Move sync pool to attribute pkg

* Add change to changelog

* Fix comment

* Apply suggestions from code review

Co-authored-by: Peter Liu <[email protected]>

* Update sdk/metric/instrument.go

Co-authored-by: Robert Pająk <[email protected]>

* Update comment based on feedback

* Apply feedback

---------

Co-authored-by: Peter Liu <[email protected]>
Co-authored-by: Robert Pająk <[email protected]>
  • Loading branch information
3 people authored Mar 13, 2023
1 parent 4af35a9 commit b62eb2c
Show file tree
Hide file tree
Showing 4 changed files with 88 additions and 4 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm

### Changed

- Optimize memory allocation when creation a new `Set` using `NewSet` or `NewSetWithFiltered` in `go.opentelemetry.io/otel/attribute`. (#3832)
- Optimize memory allocation when creation new metric instruments in `go.opentelemetry.io/otel/sdk/metric`. (#3832)
- Avoid creating new objects on all calls to `WithDeferredSetup` and `SkipContextSetup` in OpenTracing bridge. (#3833)
- The `New` and `Detect` functions from `go.opentelemetry.io/otel/sdk/resource` return errors that wrap underlying errors instead of just containing the underlying error strings. (#3844)

Expand Down
16 changes: 14 additions & 2 deletions attribute/set.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
"encoding/json"
"reflect"
"sort"
"sync"
)

type (
Expand Down Expand Up @@ -62,6 +63,12 @@ var (
iface: [0]KeyValue{},
},
}

// sortables is a pool of Sortables used to create Sets with a user does
// not provide one.
sortables = sync.Pool{
New: func() interface{} { return new(Sortable) },
}
)

// EmptySet returns a reference to a Set with no elements.
Expand Down Expand Up @@ -191,7 +198,9 @@ func NewSet(kvs ...KeyValue) Set {
if len(kvs) == 0 {
return empty()
}
s, _ := NewSetWithSortableFiltered(kvs, new(Sortable), nil)
srt := sortables.Get().(*Sortable)
s, _ := NewSetWithSortableFiltered(kvs, srt, nil)
sortables.Put(srt)
return s
}

Expand All @@ -218,7 +227,10 @@ func NewSetWithFiltered(kvs []KeyValue, filter Filter) (Set, []KeyValue) {
if len(kvs) == 0 {
return empty(), nil
}
return NewSetWithSortableFiltered(kvs, new(Sortable), filter)
srt := sortables.Get().(*Sortable)
s, filtered := NewSetWithSortableFiltered(kvs, srt, filter)
sortables.Put(srt)
return s, filtered
}

// NewSetWithSortableFiltered returns a new Set.
Expand Down
12 changes: 10 additions & 2 deletions sdk/metric/instrument.go
Original file line number Diff line number Diff line change
Expand Up @@ -194,8 +194,12 @@ func (i *instrumentImpl[N]) aggregate(ctx context.Context, val N, attrs []attrib
if err := ctx.Err(); err != nil {
return
}
// Do not use single attribute.Sortable and attribute.NewSetWithSortable,
// this method needs to be concurrent safe. Let the sync.Pool in the
// attribute package handle allocations of the Sortable.
s := attribute.NewSet(attrs...)
for _, agg := range i.aggregators {
agg.Aggregate(val, attribute.NewSet(attrs...))
agg.Aggregate(val, s)
}
}

Expand Down Expand Up @@ -260,8 +264,12 @@ func newObservable[N int64 | float64](scope instrumentation.Scope, kind Instrume

// observe records the val for the set of attrs.
func (o *observable[N]) observe(val N, attrs []attribute.KeyValue) {
// Do not use single attribute.Sortable and attribute.NewSetWithSortable,
// this method needs to be concurrent safe. Let the sync.Pool in the
// attribute package handle allocations of the Sortable.
s := attribute.NewSet(attrs...)
for _, agg := range o.aggregators {
agg.Aggregate(val, attribute.NewSet(attrs...))
agg.Aggregate(val, s)
}
}

Expand Down
62 changes: 62 additions & 0 deletions sdk/metric/instrument_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
// Copyright The OpenTelemetry Authors
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package metric

import (
"context"
"testing"

"go.opentelemetry.io/otel/attribute"
"go.opentelemetry.io/otel/sdk/metric/internal"
)

func BenchmarkInstrument(b *testing.B) {
attr := func(id int) []attribute.KeyValue {
return []attribute.KeyValue{
attribute.String("user", "Alice"),
attribute.Bool("admin", true),
attribute.Int("id", id),
}
}

b.Run("instrumentImpl/aggregate", func(b *testing.B) {
inst := instrumentImpl[int64]{aggregators: []internal.Aggregator[int64]{
internal.NewLastValue[int64](),
internal.NewCumulativeSum[int64](true),
internal.NewDeltaSum[int64](true),
}}
ctx := context.Background()

b.ReportAllocs()
b.ResetTimer()
for i := 0; i < b.N; i++ {
inst.aggregate(ctx, int64(i), attr(i))
}
})

b.Run("observable/observe", func(b *testing.B) {
o := observable[int64]{aggregators: []internal.Aggregator[int64]{
internal.NewLastValue[int64](),
internal.NewCumulativeSum[int64](true),
internal.NewDeltaSum[int64](true),
}}

b.ReportAllocs()
b.ResetTimer()
for i := 0; i < b.N; i++ {
o.observe(int64(i), attr(i))
}
})
}

0 comments on commit b62eb2c

Please sign in to comment.