-
Notifications
You must be signed in to change notification settings - Fork 137
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
[Dynamic Instrumentation] Fixed SymDB upload when PDB is absent #5789
Conversation
5ab91c6
to
3c1843e
Compare
Datadog ReportBranch report: ✅ 0 Failed, 354051 Passed, 2251 Skipped, 22h 46m 51.16s Total Time |
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 (5789) - mean (76ms) : 67, 85
. : milestone, 76,
master - mean (75ms) : 64, 87
. : milestone, 75,
section CallTarget+Inlining+NGEN
This PR (5789) - mean (1,045ms) : 1027, 1063
. : milestone, 1045,
master - mean (1,051ms) : 1026, 1077
. : milestone, 1051,
gantt
title Execution time (ms) FakeDbCommand (.NET Core 3.1)
dateFormat X
axisFormat %s
todayMarker off
section Baseline
This PR (5789) - mean (111ms) : 107, 115
. : milestone, 111,
master - mean (110ms) : 107, 114
. : milestone, 110,
section CallTarget+Inlining+NGEN
This PR (5789) - mean (728ms) : 709, 747
. : milestone, 728,
master - mean (739ms) : 715, 764
. : milestone, 739,
gantt
title Execution time (ms) FakeDbCommand (.NET 6)
dateFormat X
axisFormat %s
todayMarker off
section Baseline
This PR (5789) - mean (95ms) : 92, 99
. : milestone, 95,
master - mean (94ms) : 90, 98
. : milestone, 94,
section CallTarget+Inlining+NGEN
This PR (5789) - mean (684ms) : 662, 706
. : milestone, 684,
master - mean (689ms) : 666, 711
. : milestone, 689,
gantt
title Execution time (ms) HttpMessageHandler (.NET Framework 4.6.2)
dateFormat X
axisFormat %s
todayMarker off
section Baseline
This PR (5789) - mean (193ms) : 189, 197
. : milestone, 193,
master - mean (192ms) : 188, 195
. : milestone, 192,
section CallTarget+Inlining+NGEN
This PR (5789) - mean (1,135ms) : 1113, 1157
. : milestone, 1135,
master - mean (1,158ms) : 1134, 1182
. : milestone, 1158,
gantt
title Execution time (ms) HttpMessageHandler (.NET Core 3.1)
dateFormat X
axisFormat %s
todayMarker off
section Baseline
This PR (5789) - mean (278ms) : 271, 286
. : milestone, 278,
master - mean (276ms) : 272, 280
. : milestone, 276,
section CallTarget+Inlining+NGEN
This PR (5789) - mean (911ms) : 882, 940
. : milestone, 911,
master - mean (937ms) : 920, 955
. : milestone, 937,
gantt
title Execution time (ms) HttpMessageHandler (.NET 6)
dateFormat X
axisFormat %s
todayMarker off
section Baseline
This PR (5789) - mean (265ms) : 261, 269
. : milestone, 265,
master - mean (265ms) : 261, 268
. : milestone, 265,
section CallTarget+Inlining+NGEN
This PR (5789) - mean (888ms) : 864, 912
. : milestone, 888,
master - mean (913ms) : 895, 931
. : milestone, 913,
|
Benchmarks Report for tracer 🐌Benchmarks for #5789 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 - Slower
|
Benchmark | diff/base | Base Median (ns) | Diff Median (ns) | Modality |
---|---|---|---|---|
Benchmarks.Trace.SpanBenchmark.StartFinishSpan‑net472 | 1.250 | 577.90 | 722.28 |
Raw results
Branch | Method | Toolchain | Mean | StdError | StdDev | Gen 0 | Gen 1 | Gen 2 | Allocated |
---|---|---|---|---|---|---|---|---|---|
master | StartFinishSpan |
net6.0 | 405ns | 0.182ns | 0.704ns | 0.00809 | 0 | 0 | 576 B |
master | StartFinishSpan |
netcoreapp3.1 | 568ns | 0.503ns | 1.95ns | 0.00768 | 0 | 0 | 576 B |
master | StartFinishSpan |
net472 | 579ns | 0.732ns | 2.84ns | 0.0918 | 0 | 0 | 578 B |
master | StartFinishScope |
net6.0 | 532ns | 0.209ns | 0.811ns | 0.00983 | 0 | 0 | 696 B |
master | StartFinishScope |
netcoreapp3.1 | 766ns | 0.379ns | 1.47ns | 0.0095 | 0 | 0 | 696 B |
master | StartFinishScope |
net472 | 900ns | 0.62ns | 2.4ns | 0.104 | 0 | 0 | 658 B |
#5789 | StartFinishSpan |
net6.0 | 394ns | 0.144ns | 0.558ns | 0.00811 | 0 | 0 | 576 B |
#5789 | StartFinishSpan |
netcoreapp3.1 | 630ns | 0.409ns | 1.59ns | 0.00786 | 0 | 0 | 576 B |
#5789 | StartFinishSpan |
net472 | 722ns | 0.561ns | 2.02ns | 0.0915 | 0 | 0 | 578 B |
#5789 | StartFinishScope |
net6.0 | 520ns | 0.139ns | 0.538ns | 0.00987 | 0 | 0 | 696 B |
#5789 | StartFinishScope |
netcoreapp3.1 | 711ns | 0.211ns | 0.788ns | 0.00951 | 0 | 0 | 696 B |
#5789 | StartFinishScope |
net472 | 864ns | 0.455ns | 1.7ns | 0.104 | 0 | 0 | 658 B |
Benchmarks.Trace.TraceAnnotationsBenchmark - Same speed ✔️ Same allocations ✔️
Raw results
Branch | Method | Toolchain | Mean | StdError | StdDev | Gen 0 | Gen 1 | Gen 2 | Allocated |
---|---|---|---|---|---|---|---|---|---|
master | RunOnMethodBegin |
net6.0 | 676ns | 0.312ns | 1.21ns | 0.00988 | 0 | 0 | 696 B |
master | RunOnMethodBegin |
netcoreapp3.1 | 895ns | 0.243ns | 0.909ns | 0.00941 | 0 | 0 | 696 B |
master | RunOnMethodBegin |
net472 | 1.14μs | 1.46ns | 5.64ns | 0.104 | 0 | 0 | 658 B |
#5789 | RunOnMethodBegin |
net6.0 | 653ns | 0.288ns | 1.11ns | 0.00969 | 0 | 0 | 696 B |
#5789 | RunOnMethodBegin |
netcoreapp3.1 | 912ns | 0.404ns | 1.51ns | 0.00918 | 0 | 0 | 696 B |
#5789 | RunOnMethodBegin |
net472 | 1.07μs | 2.2ns | 8.23ns | 0.104 | 0 | 0 | 658 B |
3c1843e
to
3f0daea
Compare
Summary of changes
We had some issues in building up the symbols when PDB is absent which lead to failure in uploading them.
Reason for change
We are entering into the Open Beta of "Autocomplete and Search" feature of the Dynamic Instrumentation product and want to make sure things are working before shipping it. The absent of PDBs is a common scenario and we still want to display Methods and Classes information that exist in the assembly itself. Failing to build the symbols may yield bad user experience and possibly hurt the adoption of DI for .NET.
Implementation details
Mainly fixed a miscalculation of an index that lead to
IndexOutOfBoundsException
.