E544 trace: Use non-generic to replace newEvictedQueue in trace.start to reduce memory usage. by tttoad · Pull Request #5497 · open-telemetry/opentelemetry-go · GitHub
[go: up one dir, main page]
Skip to content

trace: Use non-generic to replace newEvictedQueue in trace.start to reduce memory usage. #5497

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

Jump to bottom
Merged
merged 6 commits into from
Jun 17, 2024
Merged

trace: Use non-generic to replace newEvictedQueue in trace.start to reduce memory usage. #5497

merged 6 commits into from
Jun 17, 2024

Conversation

tttoad
Copy link
Contributor
@tttoad tttoad commented Jun 6, 2024

benchstat:

goos: darwin
goarch: arm64
pkg: go.opentelemetry.io/otel/sdk/trace
              │     old     │                 new                 │
              │   sec/op    │   sec/op     vs base                │
TraceStart-10   950.6n ± 1%   641.0n ± 0%  -32.57% (p=0.000 n=10)

              │     old     │                new                 │
              │    B/op     │    B/op     vs base                │
TraceStart-10   1040.0 ± 0%   704.0 ± 0%  -32.31% (p=0.000 n=10)

              │    old     │                new                 │
              │ allocs/op  │ allocs/op   vs base                │
TraceStart-10   20.00 ± 0%   14.00 ± 0%  -30.00% (p=0.000 n=10)

@codecov Codecov
Copy link
codecov bot commented Jun 6, 2024 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 84.8%. Comparing base (d3f3ae1) to head (2919062).

Additional details and impacted files

Impacted file tree graph

@@          Coverage Diff          @@
##            main   #5497   +/-   ##
=====================================
  Coverage   84.7%   84.8%           
=====================================
  Files        269     269           
  Lines      18077   18079    +2     
=====================================
+ Hits       15329   15332    +3     
+ Misses      2422    2421    -1     
  Partials     326     326
Files Coverage Δ
sdk/trace/evictedqueue.go 100.0% <100.0%> (ø)
sdk/trace/tracer.go 100.0% <100.0%> (ø)

... and 2 files with indirect coverage changes

dmathieu
dmathieu reviewed Jun 7, 2024
View reviewed changes
@tttoad tttoad force-pushed the perf-trace-start branch 2 times, most recently from 1549d10 to 29512b5 Compare June 10, 2024 20:14
@tttoad tttoad force-pushed the perf-trace-start branch from 29512b5 to 59e5a08 Compare June 10, 2024 20:17
MadVikingGod
MadVikingGod reviewed Jun 13, 2024
View reviewed changes
@MadVikingGod MadVikingGod mentioned this pull request Jun 13, 2024
Open
MrAlias
MrAlias approved these changes Jun 17, 2024
View reviewed changes
@MrAlias MrAlias merged commit 30e82e0 into open-telemetry:main Jun 17, 2024
@MrAlias MrAlias added this to the v1.28.0 milestone Jun 20, 2024
@MrAlias MrAlias mentioned this pull request Jul 2, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dmathieu dmathieu dmathieu approved these changes

@MrAlias MrAlias MrAlias approved these changes

@evantorrie evantorrie Awaiting requested review from evantorrie

@XSAM XSAM Awaiting requested review from XSAM XSAM is a code owner

@dashpole dashpole Awaiting requested review from dashpole dashpole is a code owner

@pellared pellared Awaiting requested review from pellared pellared is a code owner

@hanyuancheung hanyuancheung Awaiting requested review from hanyuancheung

+1 more reviewer

@MadVikingGod MadVikingGod MadVikingGod approved these changes

Reviewers whose approvals may not affect merge requirements
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v1.28.0
Development

Successfully merging this pull request may close these issues.
4 participants
@tttoad @dmathieu @MadVikingGod @MrAlias
0