Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sdk/trace: use slices.Grow() to avoid excessive runtime.growslice() #4818

Merged
merged 18 commits into from
Jan 12, 2024
Merged

sdk/trace: use slices.Grow() to avoid excessive runtime.growslice() #4818

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
50e4fa1
sdk/trace: use slices.Grow() to avoid excessive runtime.growslice()
lizthegrey Jan 9, 2024
9fb618c
go1.20 compat
lizthegrey Jan 9, 2024
1e4fcfa
update go.mod/sum
lizthegrey Jan 9, 2024
5409f3a
Update CHANGELOG.md
lizthegrey Jan 9, 2024
b4e888e
Revert "update go.mod/sum"
lizthegrey Jan 9, 2024
bc42f63
inline slices.Grow as ensureAttributesCapacity
lizthegrey Jan 9, 2024
05f5a8d
fix comment
lizthegrey Jan 9, 2024
da77f25
add bench
lizthegrey Jan 9, 2024
af0d2a6
fix inlining
lizthegrey Jan 9, 2024
2a08289
Apply suggestions from code review
lizthegrey Jan 10, 2024
1cf32e2
add ReportAllocs
lizthegrey Jan 10, 2024
f4ba48d
Merge branch 'main' into lizf.trace-slices-grow
lizthegrey Jan 10, 2024
1dcc51f
add benchmark variations with lower limit
lizthegrey Jan 11, 2024
e954fef
bugfix: we always want to set cap, just the value is a min()
lizthegrey Jan 11, 2024
910f7cc
lint
lizthegrey Jan 11, 2024
8f974a1
Merge branch 'main' into lizf.trace-slices-grow
lizthegrey Jan 11, 2024
df9d38d
Merge branch 'main' into lizf.trace-slices-grow
lizthegrey Jan 12, 2024
867aeb8
Merge branch 'main' into lizf.trace-slices-grow
lizthegrey Jan 12, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
inline slices.Grow as ensureAttributesCapacity
  • Loading branch information
@lizthegrey
lizthegrey committed Jan 9, 2024
commit bc42f637b57018ea47674988677d7d52218236f0
1 change: 0 additions & 1 deletion sdk/go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ require (
github.com/stretchr/testify v1.8.4
go.opentelemetry.io/otel v1.21.0
go.opentelemetry.io/otel/trace v1.21.0
golang.org/x/exp v0.0.0-20240103183307-be819d1f06fc
golang.org/x/sys v0.16.0
)

Expand Down
2 changes: 0 additions & 2 deletions sdk/go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,6 @@ github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZb
github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
github.com/stretchr/testify v1.8.4 h1:CcVxjf3Q8PM0mHUKJCdn+eZZtm5yQwehR5yeSVQQcUk=
github.com/stretchr/testify v1.8.4/go.mod h1:sz/lmYIOXD/1dqDmKjjqLyZ2RngseejIcXlSw2iwfAo=
golang.org/x/exp v0.0.0-20240103183307-be819d1f06fc h1:ao2WRsKSzW6KuUY9IWPwWahcHCgR0s52IfwutMfEbdM=
golang.org/x/exp v0.0.0-20240103183307-be819d1f06fc/go.mod h1:iRJReGqOEeBhDZGkGbynYwcHlctCvnjTYIamk7uXpHI=
golang.org/x/sys v0.16.0 h1:xWw16ngr6ZMtmxDyKyIgsE93KNKz5HKmMa3b8ALHidU=
golang.org/x/sys v0.16.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA=
gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM=
Expand Down
14 changes: 10 additions & 4 deletions sdk/trace/span.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,6 @@ import (
"time"
"unicode/utf8"

"golang.org/x/exp/slices"

"go.opentelemetry.io/otel/attribute"
"go.opentelemetry.io/otel/codes"
"go.opentelemetry.io/otel/sdk/instrumentation"
Expand Down Expand Up @@ -210,6 +208,14 @@ func (s *recordingSpan) SetStatus(code codes.Code, description string) {
s.status = status
}

// ensureAttributesCapacity inlines functionality from golang.org/x/exp/slices.Grow
lizthegrey marked this conversation as resolved.
Show resolved Hide resolved
// so that we can avoid needing to import golang.org/x/exp
func (s *recordingSpan) ensureAttributesCapacity(minCapacity int) {
if n := cap(s.attributes) - minCapacity; n > 0 {
s.attributes = append(s.attributes[:cap(s.attributes)], make([]attribute.KeyValue, n)...)[:len(s.attributes)]
}
}

// SetAttributes sets attributes of this span.
//
// If a key from attributes already exists the value associated with that key
Expand Down Expand Up @@ -244,7 +250,7 @@ func (s *recordingSpan) SetAttributes(attributes ...attribute.KeyValue) {

// Otherwise, add without deduplication. When attributes are read they
// will be deduplicated, optimizing the operation.
s.attributes = slices.Grow(s.attributes, len(attributes))
s.ensureAttributesCapacity(len(s.attributes) + len(attributes))
for _, a := range attributes {
if !a.Valid() {
// Drop all invalid attributes.
Expand Down Expand Up @@ -281,7 +287,7 @@ func (s *recordingSpan) addOverCapAttrs(limit int, attrs []attribute.KeyValue) {
// Now that s.attributes is deduplicated, adding unique attributes up to
// the capacity of s will not over allocate s.attributes.
if limit-len(s.attributes) > 0 {
lizthegrey marked this conversation as resolved.
Show resolved Hide resolved
s.attributes = slices.Grow(s.attributes, limit-len(s.attributes))
s.ensureAttributesCapacity(limit)
}
for _, a := range attrs {
if !a.Valid() {
Expand Down
Loading