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.

Sign up for GitHub

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

Jump to bottom

Minor refactor in ThreadAbortCodeFixProvider #6673

Merged
merged 1 commit into from
Feb 17, 2025
Merged

Minor refactor in ThreadAbortCodeFixProvider #6673

merged 1 commit into from
Feb 17, 2025

Conversation

andrewlock
Copy link
Member

Summary of changes

Don't re-do work in code fix

Reason for change

Was looking at something else, and noticed that this was doing more work than necessary

Implementation details

The diagnostic is already located at the "problematic" catch, so we don't need to try finding it again in the code fix

Test coverage

There are existing unit tests for this, and they all pass, so :shipit:

@andrewlock andrewlock added area:builds project files, build scripts, pipelines, versioning, releases, packages type:refactor area:tools labels Feb 17, 2025
@andrewlock andrewlock requested a review from a team as a code owner February 17, 2025 10:55
@andrewlock
Copy link
Member Author

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 (6673) - mean (69ms)  : 66, 71
     .   : milestone, 69,
    master - mean (69ms)  : 66, 71
     .   : milestone, 69,

    section CallTarget+Inlining+NGEN
    This PR (6673) - mean (1,000ms)  : 978, 1022
     .   : milestone, 1000,
    master - mean (996ms)  : 972, 1019
     .   : milestone, 996,
Loading 
gantt
    title Execution time (ms) FakeDbCommand (.NET Core 3.1) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (6673) - mean (102ms)  : 99, 104
     .   : milestone, 102,
    master - mean (101ms)  : 99, 103
     .   : milestone, 101,

    section CallTarget+Inlining+NGEN
    This PR (6673) - mean (672ms)  : 653, 691
     .   : milestone, 672,
    master - mean (675ms)  : 660, 691
     .   : milestone, 675,
Loading 
gantt
    title Execution time (ms) FakeDbCommand (.NET 6) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (6673) - mean (89ms)  : 87, 90
     .   : milestone, 89,
    master - mean (89ms)  : 87, 91
     .   : milestone, 89,

    section CallTarget+Inlining+NGEN
    This PR (6673) - mean (634ms)  : 606, 662
     .   : milestone, 634,
    master - mean (634ms)  : 614, 654
     .   : milestone, 634,
Loading 
gantt
    title Execution time (ms) HttpMessageHandler (.NET Framework 4.6.2) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (6673) - mean (191ms)  : 188, 195
     .   : milestone, 191,
    master - mean (191ms)  : 187, 195
     .   : milestone, 191,

    section CallTarget+Inlining+NGEN
    This PR (6673) - mean (1,109ms)  : 1079, 1138
     .   : milestone, 1109,
    master - mean (1,113ms)  : 1085, 1140
     .   : milestone, 1113,
Loading 
gantt
    title Execution time (ms) HttpMessageHandler (.NET Core 3.1) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (6673) - mean (272ms)  : 267, 277
     .   : milestone, 272,
    master - mean (271ms)  : 266, 276
     .   : milestone, 271,

    section CallTarget+Inlining+NGEN
    This PR (6673) - mean (863ms)  : 837, 889
     .   : milestone, 863,
    master - mean (868ms)  : 837, 898
     .   : milestone, 868,
Loading 
gantt
    title Execution time (ms) HttpMessageHandler (.NET 6) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (6673) - mean (263ms)  : 260, 267
     .   : milestone, 263,
    master - mean (262ms)  : 258, 267
     .   : milestone, 262,

    section CallTarget+Inlining+NGEN
    This PR (6673) - mean (845ms)  : 809, 881
     .   : milestone, 845,
    master - mean (848ms)  : 818, 878
     .   : milestone, 848,
Loading

@andrewlock
Copy link
Member Author

Benchmarks Report for tracer 🐌

Benchmarks for #6673 compared to master:

  • 1 benchmarks are faster, with geometric mean 1.115
  • 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 7.89μs 45ns 321ns 0.0148 0.00738 0 5.61 KB
master StartStopWithChild netcoreapp3.1 10.2μs 56ns 331ns 0.0244 0.00977 0 5.8 KB
master StartStopWithChild net472 16μs 61ns 236ns 1.04 0.299 0.102 6.22 KB
#6673 StartStopWithChild net6.0 7.77μs 44.4ns 323ns 0.0147 0.00735 0 5.61 KB
#6673 StartStopWithChild netcoreapp3.1 10.1μs 55.8ns 330ns 0.0204 0.0102 0 5.81 KB
#6673 StartStopWithChild net472 16μs 48ns 186ns 1.03 0.296 0.0879 6.21 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 491μs 526ns 2.04μs 0 0 0 2.7 KB
master WriteAndFlushEnrichedTraces netcoreapp3.1 648μs 459ns 1.78μs 0 0 0 2.7 KB
master WriteAndFlushEnrichedTraces net472 870μs 491ns 1.77μs 0.431 0 0 3.3 KB
#6673 WriteAndFlushEnrichedTraces net6.0 471μs 281ns 1.01μs 0 0 0 2.7 KB
#6673 WriteAndFlushEnrichedTraces netcoreapp3.1 690μs 821ns 3.18μs 0 0 0 2.7 KB
#6673 WriteAndFlushEnrichedTraces net472 849μs 697ns 2.7μs 0.425 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 130μs 525ns 2.03μs 0.198 0 0 14.47 KB
master SendRequest netcoreapp3.1 143μs 500ns 1.94μs 0.218 0 0 17.27 KB
master SendRequest net472 0.00978ns 0.00216ns 0.00835ns 0 0 0 0 b
#6673 SendRequest net6.0 130μs 118ns 410ns 0.197 0 0 14.47 KB
#6673 SendRequest netcoreapp3.1 146μs 217ns 841ns 0.22 0 0 17.27 KB
#6673 SendRequest net472 0ns 0ns 0ns 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 583μs 3.25μs 20.3μs 0.598 0 0 41.63 KB
master WriteAndFlushEnrichedTraces netcoreapp3.1 651μs 2.86μs 11.1μs 0.316 0 0 41.74 KB
master WriteAndFlushEnrichedTraces net472 836μs 3.85μs 14.4μs 8.19 2.59 0.431 53.27 KB
#6673 WriteAndFlushEnrichedTraces net6.0 575μs 3.16μs 19.5μs 0.553 0 0 41.56 KB
#6673 WriteAndFlushEnrichedTraces netcoreapp3.1 684μs 3.83μs 25.4μs 0.329 0 0 41.81 KB
#6673 WriteAndFlushEnrichedTraces net472 850μs 3.44μs 13.3μs 8.33 2.5 0.417 53.28 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.35μs 1.2ns 4.65ns 0.0143 0 0 1.02 KB
master ExecuteNonQuery netcoreapp3.1 1.71μs 1.13ns 4.36ns 0.0138 0 0 1.02 KB
master ExecuteNonQuery net472 2.11μs 2.75ns 10.7ns 0.156 0.00106 0 987 B
#6673 ExecuteNonQuery net6.0 1.3μs 2.91ns 11.3ns 0.0145 0 0 1.02 KB
#6673 ExecuteNonQuery netcoreapp3.1 1.85μs 1.53ns 5.92ns 0.0129 0 0 1.02 KB
#6673 ExecuteNonQuery net472 2.03μs 1.58ns 5.71ns 0.157 0.00101 0 987 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.31μs 0.593ns 2.22ns 0.0139 0 0 976 B
master CallElasticsearch netcoreapp3.1 1.54μs 0.821ns 3.18ns 0.0128 0 0 976 B
master CallElasticsearch net472 2.51μs 1.61ns 6.23ns 0.157 0 0 995 B
master CallElasticsearchAsync net6.0 1.29μs 0.767ns 2.97ns 0.013 0 0 952 B
master CallElasticsearchAsync netcoreapp3.1 1.65μs 0.879ns 3.17ns 0.014 0 0 1.02 KB
master CallElasticsearchAsync net472 2.51μs 0.746ns 2.69ns 0.167 0 0 1.05 KB
#6673 CallElasticsearch net6.0 1.28μs 0.667ns 2.5ns 0.0134 0 0 976 B
#6673 CallElasticsearch netcoreapp3.1 1.52μs 0.589ns 2.28ns 0.0136 0 0 976 B
#6673 CallElasticsearch net472 2.61μs 2.53ns 9.81ns 0.157 0 0 995 B
#6673 CallElasticsearchAsync net6.0 1.29μs 0.819ns 3.17ns 0.013 0 0 952 B
#6673 CallElasticsearchAsync netcoreapp3.1 1.77μs 0.966ns 3.74ns 0.0141 0 0 1.02 KB
#6673 CallElasticsearchAsync net472 2.6μs 1.75ns 6.54ns 0.167 0 0 1.05 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.19μs 0.383ns 1.48ns 0.0137 0 0 952 B
master ExecuteAsync netcoreapp3.1 1.62μs 1.5ns 5.79ns 0.0122 0 0 952 B
master ExecuteAsync net472 1.84μs 0.6ns 2.16ns 0.145 0 0 915 B
#6673 ExecuteAsync net6.0 1.28μs 4.56ns 17.7ns 0.0132 0 0 952 B
#6673 ExecuteAsync netcoreapp3.1 1.6μs 0.779ns 2.92ns 0.0128 0 0 952 B
#6673 ExecuteAsync net472 1.91μs 0.77ns 2.98ns 0.144 0 0 915 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.42μs 1.17ns 4.37ns 0.0309 0 0 2.31 KB
master SendAsync netcoreapp3.1 5.35μs 5.4ns 19.5ns 0.0376 0 0 2.85 KB
master SendAsync net472 7.42μs 1.17ns 4.37ns 0.494 0 0 3.12 KB
#6673 SendAsync net6.0 4.38μs 1.99ns 7.18ns 0.0329 0 0 2.31 KB
#6673 SendAsync netcoreapp3.1 5.34μs 2.85ns 11ns 0.0376 0 0 2.85 KB
#6673 SendAsync net472 7.51μs 1.6ns 6.2ns 0.496 0 0 3.12 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.47μs 0.602ns 2.33ns 0.0227 0 0 1.64 KB
master EnrichedLog netcoreapp3.1 2.18μs 0.913ns 3.41ns 0.0216 0 0 1.64 KB
master EnrichedLog net472 2.54μs 0.954ns 3.57ns 0.249 0 0 1.57 KB
#6673 EnrichedLog net6.0 1.57μs 1.37ns 5.13ns 0.0228 0 0 1.64 KB
#6673 EnrichedLog netcoreapp3.1 2.23μs 0.785ns 2.94ns 0.0225 0 0 1.64 KB
#6673 EnrichedLog net472 2.61μs 3.46ns 12.9ns 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 140ns 525ns 0.0562 0 0 4.28 KB
master EnrichedLog netcoreapp3.1 119μs 152ns 590ns 0.0595 0 0 4.28 KB
master EnrichedLog net472 149μs 119ns 460ns 0.669 0.223 0 4.46 KB
#6673 EnrichedLog net6.0 115μs 237ns 919ns 0.0566 0 0 4.28 KB
#6673 EnrichedLog netcoreapp3.1 118μs 186ns 719ns 0 0 0 4.28 KB
#6673 EnrichedLog net472 150μs 195ns 754ns 0.68 0.227 0 4.46 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.09μs 0.966ns 3.74ns 0.0307 0 0 2.2 KB
master EnrichedLog netcoreapp3.1 4.1μs 1.99ns 7.69ns 0.0287 0 0 2.2 KB
master EnrichedLog net472 4.82μs 1.91ns 7.15ns 0.319 0 0 2.02 KB
#6673 EnrichedLog net6.0 3.14μs 1.32ns 5.13ns 0.0308 0 0 2.2 KB
#6673 EnrichedLog netcoreapp3.1 4.22μs 2.97ns 11.5ns 0.0296 0 0 2.2 KB
#6673 EnrichedLog net472 4.87μs 2.18ns 8.17ns 0.319 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.33μs 0.776ns 3.01ns 0.0159 0 0 1.14 KB
master SendReceive netcoreapp3.1 1.88μs 2.78ns 10.8ns 0.015 0 0 1.14 KB
master SendReceive net472 2.05μs 0.792ns 2.96ns 0.183 0 0 1.16 KB
#6673 SendReceive net6.0 1.33μs 0.673ns 2.61ns 0.0159 0 0 1.14 KB
#6673 SendReceive netcoreapp3.1 1.81μs 1.19ns 4.29ns 0.0153 0 0 1.14 KB
#6673 SendReceive net472 2.05μs 5.86ns 22.7ns 0.183 0 0 1.16 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.88μs 0.712ns 2.76ns 0.0217 0 0 1.6 KB
master EnrichedLog netcoreapp3.1 3.82μs 1.98ns 7.67ns 0.021 0 0 1.65 KB
master EnrichedLog net472 4.29μs 4.29ns 16ns 0.322 0 0 2.04 KB
#6673 EnrichedLog net6.0 2.86μs 1.83ns 7.08ns 0.0222 0 0 1.6 KB
#6673 EnrichedLog netcoreapp3.1 3.83μs 1.17ns 4.37ns 0.0211 0 0 1.65 KB
#6673 EnrichedLog net472 4.31μs 2.9ns 10.9ns 0.323 0 0 2.04 KB
Benchmarks.Trace.SpanBenchmark - Faster 🎉 Same allocations ✔️

Faster 🎉 in #6673

Benchmark base/diff Base Median (ns) Diff Median (ns) Modality
Benchmarks.Trace.SpanBenchmark.StartFinishSpan‑netcoreapp3.1 1.115 605.69 543.24

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master StartFinishSpan net6.0 397ns 0.466ns 1.8ns 0.00815 0 0 576 B
master StartFinishSpan netcoreapp3.1 607ns 0.892ns 3.45ns 0.0076 0 0 576 B
master StartFinishSpan net472 589ns 0.939ns 3.51ns 0.0918 0 0 578 B
master StartFinishScope net6.0 485ns 0.717ns 2.78ns 0.00983 0 0 696 B
master StartFinishScope netcoreapp3.1 742ns 2.23ns 8.62ns 0.00941 0 0 696 B
master StartFinishScope net472 828ns 1.73ns 6.72ns 0.104 0 0 658 B
#6673 StartFinishSpan net6.0 410ns 0.638ns 2.47ns 0.00812 0 0 576 B
#6673 StartFinishSpan netcoreapp3.1 543ns 0.943ns 3.65ns 0.00795 0 0 576 B
#6673 StartFinishSpan net472 577ns 0.851ns 3.3ns 0.0917 0 0 578 B
#6673 StartFinishScope net6.0 479ns 0.881ns 3.41ns 0.00975 0 0 696 B
#6673 StartFinishScope netcoreapp3.1 742ns 1.05ns 4.06ns 0.00908 0 0 696 B
#6673 StartFinishScope net472 792ns 1.4ns 5.43ns 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 643ns 0.873ns 3.38ns 0.0096 0 0 696 B
master RunOnMethodBegin netcoreapp3.1 947ns 1.87ns 7.23ns 0.00917 0 0 696 B
master RunOnMethodBegin net472 1.09μs 2.22ns 8.29ns 0.105 0 0 658 B
#6673 RunOnMethodBegin net6.0 620ns 0.368ns 1.42ns 0.00988 0 0 696 B
#6673 RunOnMethodBegin netcoreapp3.1 885ns 0.325ns 1.26ns 0.00933 0 0 696 B
#6673 RunOnMethodBegin net472 1.06μs 0.4ns 1.5ns 0.104 0 0 658 B

@andrewlock andrewlock merged commit d2ae16e into master Feb 17, 2025
62 of 67 checks passed
@andrewlock andrewlock deleted the andrew/analyzer-simplification branch February 17, 2025 13:32
@github-actions github-actions bot added this to the vNext-v3 milestone Feb 17, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kevingosse kevingosse kevingosse approved these changes

Assignees
No one assigned
Labels
area:builds project files, build scripts, pipelines, versioning, releases, packages area:tools type:refactor
Projects
None yet
Milestone
3.11.0
Development

Successfully merging this pull request may close these issues.
2 participants
@andrewlock @kevingosse