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

Fix crash in profiler when span stack switch causes GC run #2885

Merged
merged 1 commit into from
Oct 9, 2024

Conversation

bwoebi
Copy link
Collaborator

@bwoebi bwoebi commented Oct 9, 2024

gc_collect_cycles may be intercepted by profiler and when happening precisely within ddtrace_switch_span_stack, the DDTRACE_G(active_span) points to just freed memory.

gc_collect_cycles may be intercepted by profiler and when happening precisely within ddtrace_switch_span_stack, the DDTRACE_G(active_span) points to just freed memory.

Signed-off-by: Bob Weinand <bob.weinand@datadoghq.com>
@bwoebi bwoebi requested a review from a team as a code owner October 9, 2024 17:11
@codecov-commenter
Copy link

codecov-commenter commented Oct 9, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 78.24%. Comparing base (dcc880f) to head (c2a7eaf).
Report is 1 commits behind head on master.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #2885      +/-   ##
============================================
- Coverage     80.95%   78.24%   -2.72%     
  Complexity     2526     2526              
============================================
  Files           146      173      +27     
  Lines         14711    18742    +4031     
  Branches          0      981     +981     
============================================
+ Hits          11909    14664    +2755     
- Misses         2802     3538     +736     
- Partials          0      540     +540     
Flag Coverage Δ
appsec-extension 68.49% <ø> (?)
tracer-extension 78.10% <ø> (ø)
tracer-php 82.09% <ø> (-0.06%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

see 28 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update dcc880f...c2a7eaf. Read the comment docs.

@pr-commenter
Copy link

pr-commenter bot commented Oct 9, 2024

Benchmarks [ tracer ]

Benchmark execution time: 2024-10-09 17:39:05

Comparing candidate commit c2a7eaf in PR branch bob/span_stack_switch-profiler with baseline commit dcc880f in branch master.

Found 2 performance improvements and 1 performance regressions! Performance is the same for 175 metrics, 0 unstable metrics.

scenario:MessagePackSerializationBench/benchMessagePackSerialization

  • 🟩 execution_time [-6.217µs; -3.863µs] or [-3.599%; -2.236%]

scenario:TraceFlushBench/benchFlushTrace

  • 🟥 execution_time [+1000.000ns; +1000.000ns] or [+100.000%; +100.000%]

scenario:TraceSerializationBench/benchSerializeTrace

  • 🟩 execution_time [-10.556µs; -4.644µs] or [-5.459%; -2.402%]

@bwoebi bwoebi merged commit 0430ed8 into master Oct 9, 2024
477 of 493 checks passed
@bwoebi bwoebi deleted the bob/span_stack_switch-profiler branch October 9, 2024 19:40
@github-actions github-actions bot added this to the 1.5.0 milestone Oct 9, 2024
@bwoebi bwoebi modified the milestones: 1.5.0, 1.4.1 Oct 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants