-
Notifications
You must be signed in to change notification settings - Fork 145
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
[Profiler] Make timer_create-based CPU profiler default #6606
base: master
Are you sure you want to change the base?
Conversation
ace076b
to
c47cbdd
Compare
Datadog ReportBranch report: ❌ 734 Failed (2 Known Flaky), 243094 Passed, 2558 Skipped, 18h 47m 5.61s Total Time ❌ Failed Tests (734)
|
Execution-Time Benchmarks Report ⏱️Execution-time results for samples comparing the following branches/commits: Execution-time benchmarks measure the whole time it takes to execute a program. And are intended to measure the one-off costs. Cases where the execution time results for the PR are worse than latest master results are shown in red. The following thresholds were used for comparing the execution times:
Note that these results are based on a single point-in-time result for each branch. For full results, see the dashboard. Graphs show the p99 interval based on the mean and StdDev of the test run, as well as the mean value of the run (shown as a diamond below the graph). gantt
title Execution time (ms) FakeDbCommand (.NET Framework 4.6.2)
dateFormat X
axisFormat %s
todayMarker off
section Baseline
This PR (6606) - mean (69ms) : 64, 75
. : milestone, 69,
master - mean (69ms) : 66, 71
. : milestone, 69,
section CallTarget+Inlining+NGEN
This PR (6606) - mean (996ms) : 976, 1016
. : milestone, 996,
master - mean (996ms) : 972, 1019
. : milestone, 996,
gantt
title Execution time (ms) FakeDbCommand (.NET Core 3.1)
dateFormat X
axisFormat %s
todayMarker off
section Baseline
This PR (6606) - mean (102ms) : 99, 104
. : milestone, 102,
master - mean (101ms) : 99, 103
. : milestone, 101,
section CallTarget+Inlining+NGEN
This PR (6606) - mean (675ms) : 653, 696
. : milestone, 675,
master - mean (675ms) : 660, 691
. : milestone, 675,
gantt
title Execution time (ms) FakeDbCommand (.NET 6)
dateFormat X
axisFormat %s
todayMarker off
section Baseline
This PR (6606) - mean (89ms) : 87, 91
. : milestone, 89,
master - mean (89ms) : 87, 91
. : milestone, 89,
section CallTarget+Inlining+NGEN
This PR (6606) - mean (634ms) : 614, 654
. : milestone, 634,
master - mean (634ms) : 614, 654
. : milestone, 634,
gantt
title Execution time (ms) HttpMessageHandler (.NET Framework 4.6.2)
dateFormat X
axisFormat %s
todayMarker off
section Baseline
This PR (6606) - mean (192ms) : 188, 195
. : milestone, 192,
master - mean (191ms) : 187, 195
. : milestone, 191,
section CallTarget+Inlining+NGEN
This PR (6606) - mean (1,112ms) : 1083, 1141
. : milestone, 1112,
master - mean (1,113ms) : 1085, 1140
. : milestone, 1113,
gantt
title Execution time (ms) HttpMessageHandler (.NET Core 3.1)
dateFormat X
axisFormat %s
todayMarker off
section Baseline
This PR (6606) - mean (272ms) : 268, 276
. : milestone, 272,
master - mean (271ms) : 266, 276
. : milestone, 271,
section CallTarget+Inlining+NGEN
This PR (6606) - mean (868ms) : 834, 902
. : milestone, 868,
master - mean (868ms) : 837, 898
. : milestone, 868,
gantt
title Execution time (ms) HttpMessageHandler (.NET 6)
dateFormat X
axisFormat %s
todayMarker off
section Baseline
This PR (6606) - mean (264ms) : 260, 268
. : milestone, 264,
master - mean (262ms) : 258, 267
. : milestone, 262,
section CallTarget+Inlining+NGEN
This PR (6606) - mean (850ms) : 814, 885
. : milestone, 850,
master - mean (848ms) : 818, 878
. : milestone, 848,
|
Benchmarks Report for tracer 🐌Benchmarks for #6606 compared to master:
The following thresholds were used for comparing the benchmark speeds:
Allocation changes below 0.5% are ignored. Benchmark detailsBenchmarks.Trace.ActivityBenchmark - Same speed ✔️ Same allocations ✔️Raw results
Benchmarks.Trace.AgentWriterBenchmark - Same speed ✔️ Same allocations ✔️Raw results
Benchmarks.Trace.AspNetCoreBenchmark - Same speed ✔️ Same allocations ✔️Raw results
Benchmarks.Trace.CIVisibilityProtocolWriterBenchmark - Same speed ✔️ Same allocations ✔️Raw results
Benchmarks.Trace.DbCommandBenchmark - Same speed ✔️ Same allocations ✔️Raw results
Benchmarks.Trace.ElasticsearchBenchmark - Same speed ✔️ Same allocations ✔️Raw results
Benchmarks.Trace.GraphQLBenchmark - Same speed ✔️ Same allocations ✔️Raw results
Benchmarks.Trace.HttpClientBenchmark - Same speed ✔️ Same allocations ✔️Raw results
Benchmarks.Trace.ILoggerBenchmark - Same speed ✔️ Same allocations ✔️Raw results
Benchmarks.Trace.Log4netBenchmark - Same speed ✔️ Same allocations ✔️Raw results
Benchmarks.Trace.NLogBenchmark - Same speed ✔️ Same allocations ✔️Raw results
Benchmarks.Trace.RedisBenchmark - Same speed ✔️ Same allocations ✔️Raw results
Benchmarks.Trace.SerilogBenchmark - Same speed ✔️ Same allocations ✔️Raw results
Benchmarks.Trace.SpanBenchmark - Same speed ✔️ Same allocations ✔️Raw results
Benchmarks.Trace.TraceAnnotationsBenchmark - Same speed ✔️ Same allocations ✔️Raw results
|
0bd7fdb
to
524002e
Compare
524002e
to
44c8009
Compare
Snapshots difference summaryThe following differences have been observed in committed snapshots. It is meant to help the reviewer. |
8d48fad
to
916eaf5
Compare
916eaf5
to
c6e5636
Compare
Summary of changes
This is another try at making the
timer_create
-based CPU profiler the default one. (previous PR #6315)Reason for change
We had to revert the previous PR because
pthread_exit
is explicitly called by the CLR, which starts cleaning up the thread thread_locals. But at the same time, the CPU timer_create-base kicks in and tries using, indirectly, those thread_locals.This situation lead to crashes.
In this PR, we wrap
pthread_create
to remove the cpu timer on the thread before continuing withpthread_exit
Implementation details
pthread_exit
to call__dd_on_thread_routine_finished
before calling the realpthread_exit
timer_create
as default cpu profilerTest coverage
Updated the tests
Other details