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

[Dynamic Instrumentation] Fix type of local in async method #5414

Merged
merged 1 commit into from
Apr 8, 2024

Conversation

dudikeleti
Copy link
Contributor

Summary of changes

Fix incorrect type handling of locals in async methods

Test coverage

BaseLocalWithConcreteTypeInAsyncMethod

@dudikeleti dudikeleti requested a review from a team as a code owner April 8, 2024 10:06
@datadog-ddstaging
Copy link

datadog-ddstaging bot commented Apr 8, 2024

Datadog Report

Branch report: dudik/fix-type-detection-in-async-method
Commit report: dbe39cc
Test service: dd-trace-dotnet

✅ 0 Failed, 328100 Passed, 2047 Skipped, 41m 5.55s Wall Time

@andrewlock
Copy link
Member

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:

  • Welch test with statistical test for significance of 5%
  • Only results indicating a difference greater than 5% and 5 ms are considered.

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 (5414) - mean (73ms)  : 66, 81
     .   : milestone, 73,
    master - mean (74ms)  : 61, 87
     .   : milestone, 74,

    section CallTarget+Inlining+NGEN
    This PR (5414) - mean (1,000ms)  : 976, 1023
     .   : milestone, 1000,
    master - mean (995ms)  : 979, 1010
     .   : milestone, 995,

Loading
gantt
    title Execution time (ms) FakeDbCommand (.NET Core 3.1) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (5414) - mean (110ms)  : 105, 115
     .   : milestone, 110,
    master - mean (109ms)  : 105, 113
     .   : milestone, 109,

    section CallTarget+Inlining+NGEN
    This PR (5414) - mean (713ms)  : 688, 737
     .   : milestone, 713,
    master - mean (715ms)  : 689, 741
     .   : milestone, 715,

Loading
gantt
    title Execution time (ms) FakeDbCommand (.NET 6) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (5414) - mean (94ms)  : 90, 98
     .   : milestone, 94,
    master - mean (93ms)  : 90, 96
     .   : milestone, 93,

    section CallTarget+Inlining+NGEN
    This PR (5414) - mean (665ms)  : 641, 688
     .   : milestone, 665,
    master - mean (665ms)  : 642, 689
     .   : milestone, 665,

Loading
gantt
    title Execution time (ms) HttpMessageHandler (.NET Framework 4.6.2) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (5414) - mean (185ms)  : 179, 191
     .   : milestone, 185,
    master - mean (187ms)  : 185, 190
     .   : milestone, 187,

    section CallTarget+Inlining+NGEN
    This PR (5414) - mean (1,068ms)  : 1041, 1096
     .   : milestone, 1068,
    master - mean (1,077ms)  : 1049, 1105
     .   : milestone, 1077,

Loading
gantt
    title Execution time (ms) HttpMessageHandler (.NET Core 3.1) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (5414) - mean (270ms)  : 264, 275
     .   : milestone, 270,
    master - mean (271ms)  : 268, 275
     .   : milestone, 271,

    section CallTarget+Inlining+NGEN
    This PR (5414) - mean (867ms)  : 842, 891
     .   : milestone, 867,
    master - mean (877ms)  : 852, 902
     .   : milestone, 877,

Loading
gantt
    title Execution time (ms) HttpMessageHandler (.NET 6) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (5414) - mean (260ms)  : 255, 266
     .   : milestone, 260,
    master - mean (259ms)  : 254, 264
     .   : milestone, 259,

    section CallTarget+Inlining+NGEN
    This PR (5414) - mean (858ms)  : 833, 883
     .   : milestone, 858,
    master - mean (851ms)  : 830, 872
     .   : milestone, 851,

Loading

@andrewlock
Copy link
Member

Benchmarks Report for tracer 🐌

Benchmarks for #5414 compared to master:

  • 1 benchmarks are faster, with geometric mean 1.148
  • 1 benchmarks are slower, with geometric mean 1.140
  • All benchmarks have the same allocations

The following thresholds were used for comparing the benchmark speeds:

  • Mann–Whitney U test with statistical test for significance of 5%
  • Only results indicating a difference greater than 10% and 0.3 ns are considered.

Allocation changes below 0.5% are ignored.

Benchmark details

Benchmarks.Trace.ActivityBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master StartStopWithChild net6.0 8.58μs 42.5ns 199ns 0.0219 0.00874 0 7.51 KB
master StartStopWithChild netcoreapp3.1 10.8μs 54.8ns 257ns 0.0345 0.0173 0.00575 7.6 KB
master StartStopWithChild net472 17μs 51.1ns 191ns 1.35 0.334 0.106 8.05 KB
#5414 StartStopWithChild net6.0 8.8μs 48.3ns 277ns 0.036 0.018 0 7.5 KB
#5414 StartStopWithChild netcoreapp3.1 10.6μs 56.3ns 282ns 0.0259 0.0104 0 7.6 KB
#5414 StartStopWithChild net472 17μs 47.7ns 185ns 1.35 0.359 0.12 8.05 KB
Benchmarks.Trace.AgentWriterBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master WriteAndFlushEnrichedTraces net6.0 488μs 220ns 853ns 0 0 0 2.7 KB
master WriteAndFlushEnrichedTraces netcoreapp3.1 605μs 154ns 595ns 0 0 0 2.7 KB
master WriteAndFlushEnrichedTraces net472 818μs 494ns 1.91μs 0.403 0 0 3.3 KB
#5414 WriteAndFlushEnrichedTraces net6.0 484μs 277ns 1.07μs 0 0 0 2.7 KB
#5414 WriteAndFlushEnrichedTraces netcoreapp3.1 599μs 376ns 1.46μs 0 0 0 2.7 KB
#5414 WriteAndFlushEnrichedTraces net472 820μs 350ns 1.31μs 0.408 0 0 3.3 KB
Benchmarks.Trace.AspNetCoreBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master SendRequest net6.0 171μs 169ns 656ns 0.17 0 0 18.33 KB
master SendRequest netcoreapp3.1 191μs 244ns 946ns 0.19 0 0 20.49 KB
master SendRequest net472 0.000966ns 0.000316ns 0.00118ns 0 0 0 0 b
#5414 SendRequest net6.0 170μs 229ns 792ns 0.169 0 0 18.33 KB
#5414 SendRequest netcoreapp3.1 189μs 251ns 973ns 0.187 0 0 20.49 KB
#5414 SendRequest net472 0.00114ns 0.000515ns 0.00193ns 0 0 0 0 b
Benchmarks.Trace.CIVisibilityProtocolWriterBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master WriteAndFlushEnrichedTraces net6.0 544μs 341ns 1.28μs 0.563 0 0 41.52 KB
master WriteAndFlushEnrichedTraces netcoreapp3.1 644μs 1.58μs 6.11μs 0.324 0 0 41.65 KB
master WriteAndFlushEnrichedTraces net472 842μs 3.54μs 13.7μs 8.39 2.52 0.419 53.24 KB
#5414 WriteAndFlushEnrichedTraces net6.0 535μs 598ns 2.24μs 0.548 0 0 41.51 KB
#5414 WriteAndFlushEnrichedTraces netcoreapp3.1 652μs 785ns 3.04μs 0.326 0 0 41.67 KB
#5414 WriteAndFlushEnrichedTraces net472 868μs 4.1μs 15.9μs 8.04 2.23 0.446 53.26 KB
Benchmarks.Trace.DbCommandBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master ExecuteNonQuery net6.0 1.08μs 0.249ns 0.964ns 0.0109 0 0 784 B
master ExecuteNonQuery netcoreapp3.1 1.51μs 2.37ns 9.17ns 0.0105 0 0 784 B
master ExecuteNonQuery net472 1.83μs 0.359ns 1.34ns 0.118 0 0 746 B
#5414 ExecuteNonQuery net6.0 1.13μs 0.194ns 0.726ns 0.0108 0 0 784 B
#5414 ExecuteNonQuery netcoreapp3.1 1.57μs 1.01ns 3.8ns 0.0101 0 0 784 B
#5414 ExecuteNonQuery net472 1.71μs 0.681ns 2.55ns 0.118 0 0 746 B
Benchmarks.Trace.ElasticsearchBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master CallElasticsearch net6.0 1.25μs 0.342ns 1.28ns 0.0145 0 0 1.01 KB
master CallElasticsearch netcoreapp3.1 1.71μs 1.86ns 7.19ns 0.0135 0 0 1.01 KB
master CallElasticsearch net472 2.59μs 1.77ns 6.6ns 0.162 0 0 1.02 KB
master CallElasticsearchAsync net6.0 1.27μs 0.52ns 2.02ns 0.0135 0 0 984 B
master CallElasticsearchAsync netcoreapp3.1 1.76μs 0.811ns 2.92ns 0.0141 0 0 1.06 KB
master CallElasticsearchAsync net472 2.58μs 1.65ns 6.41ns 0.17 0 0 1.08 KB
#5414 CallElasticsearch net6.0 1.33μs 0.704ns 2.73ns 0.0141 0 0 1.01 KB
#5414 CallElasticsearch netcoreapp3.1 1.68μs 0.623ns 2.33ns 0.0134 0 0 1.01 KB
#5414 CallElasticsearch net472 2.65μs 0.767ns 2.87ns 0.162 0 0 1.02 KB
#5414 CallElasticsearchAsync net6.0 1.4μs 1.21ns 4.69ns 0.014 0 0 984 B
#5414 CallElasticsearchAsync netcoreapp3.1 1.69μs 1.87ns 7ns 0.0142 0 0 1.06 KB
#5414 CallElasticsearchAsync net472 2.7μs 1.57ns 6.08ns 0.17 0.00134 0 1.08 KB
Benchmarks.Trace.GraphQLBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master ExecuteAsync net6.0 1.34μs 0.97ns 3.76ns 0.0127 0 0 928 B
master ExecuteAsync netcoreapp3.1 1.69μs 0.918ns 3.43ns 0.0129 0 0 928 B
master ExecuteAsync net472 1.92μs 4.05ns 14.6ns 0.141 0 0 891 B
#5414 ExecuteAsync net6.0 1.27μs 0.917ns 3.55ns 0.0127 0 0 928 B
#5414 ExecuteAsync netcoreapp3.1 1.65μs 1.13ns 4.38ns 0.0123 0 0 928 B
#5414 ExecuteAsync net472 1.9μs 0.883ns 3.3ns 0.141 0 0 891 B
Benchmarks.Trace.HttpClientBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master SendAsync net6.0 4.05μs 1.03ns 3.72ns 0.0303 0 0 2.16 KB
master SendAsync netcoreapp3.1 4.97μs 2.48ns 9.61ns 0.0346 0 0 2.7 KB
master SendAsync net472 7.72μs 8.52ns 31.9ns 0.484 0 0 3.05 KB
#5414 SendAsync net6.0 4.17μs 3.39ns 12.2ns 0.0294 0 0 2.16 KB
#5414 SendAsync netcoreapp3.1 5.08μs 1.97ns 7.09ns 0.0356 0 0 2.7 KB
#5414 SendAsync net472 7.65μs 5.27ns 19.7ns 0.481 0 0 3.05 KB
Benchmarks.Trace.ILoggerBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master EnrichedLog net6.0 1.53μs 2.96ns 11.5ns 0.0233 0 0 1.65 KB
master EnrichedLog netcoreapp3.1 2.16μs 0.907ns 3.39ns 0.0219 0 0 1.65 KB
master EnrichedLog net472 2.83μs 1.16ns 4.35ns 0.249 0 0 1.57 KB
#5414 EnrichedLog net6.0 1.55μs 0.711ns 2.75ns 0.0228 0 0 1.65 KB
#5414 EnrichedLog netcoreapp3.1 2.21μs 1.25ns 4.68ns 0.0222 0 0 1.65 KB
#5414 EnrichedLog net472 2.56μs 1.11ns 4.29ns 0.249 0 0 1.57 KB
Benchmarks.Trace.Log4netBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master EnrichedLog net6.0 113μs 218ns 845ns 0.0565 0 0 4.23 KB
master EnrichedLog netcoreapp3.1 118μs 139ns 519ns 0 0 0 4.23 KB
master EnrichedLog net472 147μs 131ns 507ns 0.659 0.22 0 4.41 KB
#5414 EnrichedLog net6.0 111μs 92.8ns 347ns 0.0552 0 0 4.23 KB
#5414 EnrichedLog netcoreapp3.1 117μs 213ns 824ns 0 0 0 4.23 KB
#5414 EnrichedLog net472 146μs 91.1ns 353ns 0.655 0.218 0 4.41 KB
Benchmarks.Trace.NLogBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master EnrichedLog net6.0 3.02μs 1.87ns 7.25ns 0.0303 0 0 2.21 KB
master EnrichedLog netcoreapp3.1 4.2μs 2.12ns 7.93ns 0.0297 0 0 2.21 KB
master EnrichedLog net472 4.81μs 5.36ns 20.8ns 0.321 0 0 2.02 KB
#5414 EnrichedLog net6.0 2.98μs 2.53ns 9.48ns 0.03 0 0 2.21 KB
#5414 EnrichedLog netcoreapp3.1 4.14μs 3.53ns 13.2ns 0.0289 0 0 2.21 KB
#5414 EnrichedLog net472 4.84μs 3.53ns 13.7ns 0.321 0 0 2.02 KB
Benchmarks.Trace.RedisBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master SendReceive net6.0 1.54μs 1.05ns 3.92ns 0.0162 0 0 1.18 KB
master SendReceive netcoreapp3.1 1.91μs 0.803ns 2.9ns 0.0161 0 0 1.18 KB
master SendReceive net472 2.37μs 1.09ns 4.21ns 0.186 0 0 1.18 KB
#5414 SendReceive net6.0 1.47μs 2.38ns 8.59ns 0.0167 0 0 1.18 KB
#5414 SendReceive netcoreapp3.1 1.89μs 1.13ns 4.38ns 0.0162 0 0 1.18 KB
#5414 SendReceive net472 2.2μs 1.05ns 3.93ns 0.187 0.00111 0 1.18 KB
Benchmarks.Trace.SerilogBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master EnrichedLog net6.0 2.78μs 0.912ns 3.53ns 0.0222 0 0 1.55 KB
master EnrichedLog netcoreapp3.1 3.88μs 2.19ns 8.19ns 0.0213 0 0 1.6 KB
master EnrichedLog net472 4.39μs 1.81ns 7.01ns 0.314 0 0 1.99 KB
#5414 EnrichedLog net6.0 2.82μs 0.847ns 3.17ns 0.0211 0 0 1.55 KB
#5414 EnrichedLog netcoreapp3.1 4.06μs 1.58ns 6.12ns 0.0203 0 0 1.6 KB
#5414 EnrichedLog net472 4.46μs 2.89ns 11.2ns 0.314 0 0 1.99 KB
Benchmarks.Trace.SpanBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master StartFinishSpan net6.0 526ns 1.29ns 5ns 0.00771 0 0 552 B
master StartFinishSpan netcoreapp3.1 728ns 1.4ns 5.43ns 0.0074 0 0 552 B
master StartFinishSpan net472 680ns 0.921ns 3.57ns 0.0877 0 0 554 B
master StartFinishScope net6.0 534ns 0.99ns 3.83ns 0.00946 0 0 672 B
master StartFinishScope netcoreapp3.1 925ns 1.46ns 5.64ns 0.00875 0 0 672 B
master StartFinishScope net472 977ns 1.67ns 6.25ns 0.101 0 0 634 B
#5414 StartFinishSpan net6.0 514ns 0.901ns 3.49ns 0.00766 0 0 552 B
#5414 StartFinishSpan netcoreapp3.1 707ns 0.676ns 2.62ns 0.00738 0 0 552 B
#5414 StartFinishSpan net472 752ns 1.55ns 6.01ns 0.0877 0 0 554 B
#5414 StartFinishScope net6.0 555ns 0.478ns 1.79ns 0.00934 0 0 672 B
#5414 StartFinishScope netcoreapp3.1 955ns 0.947ns 3.54ns 0.00899 0 0 672 B
#5414 StartFinishScope net472 948ns 2.36ns 9.12ns 0.101 0 0 634 B
Benchmarks.Trace.TraceAnnotationsBenchmark - Slower ⚠️ Same allocations ✔️

Slower ⚠️ in #5414

Benchmark diff/base Base Median (ns) Diff Median (ns) Modality
Benchmarks.Trace.TraceAnnotationsBenchmark.RunOnMethodBegin‑net6.0 1.140 640.77 730.16

Faster 🎉 in #5414

Benchmark base/diff Base Median (ns) Diff Median (ns) Modality
Benchmarks.Trace.TraceAnnotationsBenchmark.RunOnMethodBegin‑net472 1.148 1,167.53 1,017.30

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master RunOnMethodBegin net6.0 640ns 1.19ns 4.6ns 0.00941 0 0 672 B
master RunOnMethodBegin netcoreapp3.1 946ns 1.64ns 6.35ns 0.00861 0 0 672 B
master RunOnMethodBegin net472 1.17μs 3.27ns 12.7ns 0.101 0 0 634 B
#5414 RunOnMethodBegin net6.0 730ns 1.08ns 4.19ns 0.0094 0 0 672 B
#5414 RunOnMethodBegin netcoreapp3.1 976ns 1.97ns 7.62ns 0.00877 0 0 672 B
#5414 RunOnMethodBegin net472 1.02μs 1.86ns 7.21ns 0.101 0 0 634 B

@andrewlock
Copy link
Member

Throughput/Crank Report:zap:

Throughput results for AspNetCoreSimpleController comparing the following branches/commits:

Cases where throughput results for the PR are worse than latest master (5% drop or greater), results are shown in red.

Note that these results are based on a single point-in-time result for each branch. For full results, see one of the many, many dashboards!

gantt
    title Throughput Linux x64 (Total requests) 
    dateFormat  X
    axisFormat %s
    section Baseline
    This PR (5414) (11.288M)   : 0, 11287724
    master (11.335M)   : 0, 11335496
    benchmarks/2.9.0 (11.259M)   : 0, 11258931

    section Automatic
    This PR (5414) (7.739M)   : 0, 7738591
    master (7.719M)   : 0, 7718661
    benchmarks/2.9.0 (8.404M)   : 0, 8403894

    section Trace stats
    This PR (5414) (8.066M)   : 0, 8066032
    master (8.251M)   : 0, 8251395

    section Manual
    This PR (5414) (9.739M)   : 0, 9739057
    master (9.790M)   : 0, 9789936

    section Manual + Automatic
    This PR (5414) (7.370M)   : 0, 7370284
    master (7.328M)   : 0, 7327602

    section Version Conflict
    This PR (5414) (6.629M)   : 0, 6628702
    master (6.586M)   : 0, 6586053

Loading
gantt
    title Throughput Linux arm64 (Total requests) 
    dateFormat  X
    axisFormat %s
    section Baseline
    This PR (5414) (9.579M)   : 0, 9578897
    master (9.677M)   : 0, 9676732
    benchmarks/2.9.0 (9.538M)   : 0, 9538176

    section Automatic
    This PR (5414) (6.560M)   : 0, 6560317
    master (6.700M)   : 0, 6700130

    section Trace stats
    This PR (5414) (6.854M)   : 0, 6853847
    master (6.891M)   : 0, 6891070

    section Manual
    This PR (5414) (8.169M)   : 0, 8169404
    master (8.307M)   : 0, 8307285

    section Manual + Automatic
    This PR (5414) (6.145M)   : 0, 6144558
    master (6.287M)   : 0, 6287286

    section Version Conflict
    This PR (5414) (5.670M)   : 0, 5669824
    master (5.677M)   : 0, 5677039

Loading
gantt
    title Throughput Windows x64 (Total requests) 
    dateFormat  X
    axisFormat %s
    section Baseline
    This PR (5414) (10.052M)   : 0, 10052005

    section Automatic
    This PR (5414) (7.147M)   : 0, 7147394

    section Trace stats
    This PR (5414) (7.493M)   : 0, 7493098

    section Manual
    This PR (5414) (8.869M)   : 0, 8869470

    section Manual + Automatic
    This PR (5414) (7.017M)   : 0, 7016555

    section Version Conflict
    This PR (5414) (6.221M)   : 0, 6221261

Loading

Copy link
Contributor

@shurivich shurivich left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice

@dudikeleti dudikeleti merged commit 5c66b91 into master Apr 8, 2024
61 of 62 checks passed
@dudikeleti dudikeleti deleted the dudik/fix-type-detection-in-async-method branch April 8, 2024 12:54
@github-actions github-actions bot added this to the vNext-v2 milestone Apr 8, 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