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.

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

Avoid creating new references on WithDeferredSetup for every span #3833

Merged
merged 2 commits into from
Mar 7, 2023

Conversation

alanprot
Copy link
Contributor

@alanprot alanprot commented Mar 3, 2023

We are creating new doDeferredContextSetupType references for every span on this function making the GC busier for no reason (specially on a very high tps service).

This change is just using a global reference instead of creating one for every span.

@alanprot alanprot changed the title Avoid creating new references on WithDeferredSetup call Avoid creating new references on WithDeferredSetup for every span Mar 3, 2023
@codecov
Copy link

codecov bot commented Mar 3, 2023

Codecov Report

Merging #3833 (da65e9f) into main (3df561e) will not change coverage.
The diff coverage is 100.0%.

Additional details and impacted files

Impacted file tree graph

@@          Coverage Diff          @@
##            main   #3833   +/-   ##
=====================================
  Coverage   81.6%   81.6%           
=====================================
  Files        170     170           
  Lines      12474   12474           
=====================================
  Hits       10190   10190           
  Misses      2069    2069           
  Partials     215     215           
Impacted Files Coverage Δ
bridge/opentracing/migration/defer.go 100.0% <100.0%> (ø)
exporters/jaeger/jaeger.go 90.3% <0.0%> (-0.9%) ⬇️
sdk/trace/batch_span_processor.go 81.9% <0.0%> (+0.8%) ⬆️

@MadVikingGod MadVikingGod merged commit 3015c86 into open-telemetry:main Mar 7, 2023
@XSAM XSAM added this to the Old Untracked PRs milestone Nov 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants