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

[lizthegrey]

Profiling of our otel load generator shows excessive time in growslice. This indicates that we need to expand cap(s.attributes) all at once, rather than one element at a time in the loop.

$ ~/hny/go/bin/benchstat /tmp/old /tmp/new
goos: linux
goarch: amd64
pkg: go.opentelemetry.io/otel/sdk/trace
cpu: AMD Ryzen 5 5600X 6-Core Processor             
                                              │  /tmp/old   │              /tmp/new               │
                                              │   sec/op    │   sec/op     vs base                │
RecordingSpanSetAttributes/WithLimit/false-12   3.850µ ± 0%   2.721µ ± 2%  -29.32% (p=0.000 n=20)
RecordingSpanSetAttributes/WithLimit/true-12    8.040µ ± 1%   7.362µ ± 0%   -8.44% (p=0.000 n=20)
geomean                                         5.563µ        4.476µ       -19.55%

                                              │   /tmp/old    │               /tmp/new               │
                                              │     B/op      │     B/op      vs base                │
RecordingSpanSetAttributes/WithLimit/false-12   16.406Ki ± 0%   6.844Ki ± 0%  -58.29% (p=0.000 n=20)
RecordingSpanSetAttributes/WithLimit/true-12     15.81Ki ± 0%   11.00Ki ± 0%  -30.43% (p=0.000 n=20)
geomean                                          16.11Ki        8.677Ki       -46.13%

                                              │  /tmp/old   │              /tmp/new              │
                                              │  allocs/op  │ allocs/op   vs base                │
RecordingSpanSetAttributes/WithLimit/false-12   13.000 ± 0%   6.000 ± 0%  -53.85% (p=0.000 n=20)
RecordingSpanSetAttributes/WithLimit/true-12     18.00 ± 0%   12.00 ± 0%  -33.33% (p=0.000 n=20)
geomean                                          15.30        8.485       -44.53%

[codecov]

Codecov Report

Attention: 4 lines in your changes are missing coverage. Please review.

Comparison is base (19622d3) 82.3% compared to head (867aeb8) 82.3%.

Additional details and impacted files

@@           Coverage Diff           @@
##            main   #4818     +/-   ##
=======================================
- Coverage   82.3%   82.3%   -0.1%     
=======================================
  Files        226     226             
  Lines      18410   18421     +11     
=======================================
+ Hits       15157   15164      +7     
- Misses      2969    2971      +2     
- Partials     284     286      +2     

Files	Coverage Δ
sdk/trace/span.go	87.2% <63.6%> (-0.7%)	⬇️

[pellared]

Do we have some benchmarks that would show the reduced number of allocations? Can you add a benchstat result in the description?

[lizthegrey]

Interesting, adding the bench shows no change in allocs, which is very suspicious. I looked at my bench and it looks like I am calling SetAttributes() one at a time, which isn't otel's fault, it's my fault, I'll fix my loadgen binary.

[lizthegrey]

Interesting, adding the bench shows no change in allocs, which is very suspicious. I looked at my bench and it looks like I am calling SetAttributes() one at a time, which isn't otel's fault, it's my fault, I'll fix my loadgen binary.

That's because I introduced a bug while inlining. It's fixed now. Description updated.

[dmathieu]

Out of curiosity, do you have access to profiling data after the change?

[lizthegrey]

Thanks for the feedback, I'll get that sorted out.

And yes, I have profiles before/after.

[lizthegrey]

Profiles after: it's much better, and now the problem has shifted to dedupeAttributes which is a separate map allocation problem that the comment says not to pre-allocate and yet is causing us problems in our bench

[lizthegrey]

Benchmark updated and bug in the pre-sizing fixed!!

[lizthegrey]

@MadVikingGod any chance of squeezing this into v1.22? It's ready to go otherwise (2 approvals)