Skip to content

Commit

Permalink
remove Set.Encoded(Encoder) enconding cache (open-telemetry#1855)
Browse files Browse the repository at this point in the history
* remove Set.Encoded(Encoder) enconding cache

Co-authored-by: Tyler Yahn <[email protected]>
  • Loading branch information
paivagustavo and MrAlias authored Apr 29, 2021
1 parent 7674eeb commit 2bd4840
Show file tree
Hide file tree
Showing 2 changed files with 5 additions and 49 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
- `resource.New()` now creates a Resource without builtin detectors. Previous behavior is now achieved by using `WithBuiltinDetectors` Option. (#1810)
- Move the `Event` type from the `go.opentelemetry.io/otel` package to the `go.opentelemetry.io/otel/sdk/trace` package. (#1846)
- BatchSpanProcessor now report export failures when calling `ForceFlush()` method. (#1860)
- `Set.Encoded(Encoder)` no longer caches the result of an encoding. (#1855)

### Deprecated

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

type (
Expand All @@ -35,10 +34,6 @@ type (
// 3. Correlation map (TODO)
Set struct {
equivalent Distinct

lock sync.Mutex
encoders [maxConcurrentEncoders]EncoderID
encoded [maxConcurrentEncoders]string
}

// Distinct wraps a variable-size array of `KeyValue`,
Expand Down Expand Up @@ -76,8 +71,6 @@ var (
}
)

const maxConcurrentEncoders = 3

// EmptySet returns a reference to a Set with no elements.
//
// This is a convenience provided for optimized calling utility.
Expand Down Expand Up @@ -182,51 +175,13 @@ func (l *Set) Equals(o *Set) bool {
}

// Encoded returns the encoded form of this set, according to
// `encoder`. The result will be cached in this `*Set`.
// `encoder`.
func (l *Set) Encoded(encoder Encoder) string {
if l == nil || encoder == nil {
return ""
}

id := encoder.ID()
if !id.Valid() {
// Invalid IDs are not cached.
return encoder.Encode(l.Iter())
}

var lookup *string
l.lock.Lock()
for idx := 0; idx < maxConcurrentEncoders; idx++ {
if l.encoders[idx] == id {
lookup = &l.encoded[idx]
break
}
}
l.lock.Unlock()

if lookup != nil {
return *lookup
}

r := encoder.Encode(l.Iter())

l.lock.Lock()
defer l.lock.Unlock()

for idx := 0; idx < maxConcurrentEncoders; idx++ {
if l.encoders[idx] == id {
return l.encoded[idx]
}
if !l.encoders[idx].Valid() {
l.encoders[idx] = id
l.encoded[idx] = r
return r
}
}

// TODO: This is a performance cliff. Find a way for this to
// generate a warning.
return r
return encoder.Encode(l.Iter())
}

func empty() Set {
Expand All @@ -246,7 +201,7 @@ func NewSet(kvs ...KeyValue) Set {
return empty()
}
s, _ := NewSetWithSortableFiltered(kvs, new(Sortable), nil)
return s //nolint
return s
}

// NewSetWithSortable returns a new `Set`. See the documentation for
Expand All @@ -259,7 +214,7 @@ func NewSetWithSortable(kvs []KeyValue, tmp *Sortable) Set {
return empty()
}
s, _ := NewSetWithSortableFiltered(kvs, tmp, nil)
return s //nolint
return s
}

// NewSetWithFiltered returns a new `Set`. See the documentation for
Expand Down

0 comments on commit 2bd4840

Please sign in to comment.