From 2bd4840c306dbfae2bb92f6ae30db8fac08ee40c Mon Sep 17 00:00:00 2001 From: Gustavo Silva Paiva Date: Thu, 29 Apr 2021 15:28:04 -0300 Subject: [PATCH] remove Set.Encoded(Encoder) enconding cache (#1855) * remove Set.Encoded(Encoder) enconding cache Co-authored-by: Tyler Yahn --- CHANGELOG.md | 1 + attribute/set.go | 53 ++++-------------------------------------------- 2 files changed, 5 insertions(+), 49 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b87369ddd..337a585fc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/attribute/set.go b/attribute/set.go index 882dc112f..bd591894d 100644 --- a/attribute/set.go +++ b/attribute/set.go @@ -18,7 +18,6 @@ import ( "encoding/json" "reflect" "sort" - "sync" ) type ( @@ -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`, @@ -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. @@ -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 { @@ -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 @@ -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