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

[CI Visibility] BenchmarkDotNet instrumentation refactor and fixes #4628

Conversation

tonyredondo
Copy link
Member

Summary of changes

This PR contains a refactor, fixes and improvements for the BenchmarkDotNet instrumentation.

Reason for change

Before:

image

After:

image

Test coverage

We are dogfooding the instrumentation for our benchmark tests.

@tonyredondo tonyredondo requested review from a team as code owners September 14, 2023 22:20
@tonyredondo tonyredondo self-assigned this Sep 14, 2023
@tonyredondo tonyredondo changed the title [CI Visibility] BenchmarkDotNet instrumentation refactor and improvements [CI Visibility] BenchmarkDotNet instrumentation refactor and fixes Sep 14, 2023
Copy link
Member

@andrewlock andrewlock left a comment

Choose a reason for hiding this comment

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

Too many breaking changes unfortunately 😉

Copy link
Member

@andrewlock andrewlock left a comment

Choose a reason for hiding this comment

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

Tony cheated by using science to prove we're currently the only person using this package (or at least, the only one successfully using the package) so I guess we can allow these breaking changes

grmbl..grmbl...what's the world coming to..grmbl

@DataDog DataDog deleted a comment from andrewlock Sep 15, 2023
@DataDog DataDog deleted a comment from andrewlock Sep 15, 2023
@DataDog DataDog deleted a comment from andrewlock Sep 15, 2023
@DataDog DataDog deleted a comment from datadog-ddstaging bot Sep 15, 2023
@DataDog DataDog deleted a comment from andrewlock Sep 15, 2023
@DataDog DataDog deleted a comment from andrewlock Sep 15, 2023
@DataDog DataDog deleted a comment from andrewlock Sep 15, 2023
@DataDog DataDog deleted a comment from datadog-ddstaging bot Sep 15, 2023
@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 (4628) - mean (68ms)  : 65, 71
     .   : milestone, 68,
    master - mean (68ms)  : 65, 71
     .   : milestone, 68,

    section CallTarget+Inlining+NGEN
    This PR (4628) - mean (1,037ms)  : 1010, 1064
     .   : milestone, 1037,
    master - mean (1,032ms)  : 1008, 1055
     .   : milestone, 1032,

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

    section CallTarget+Inlining+NGEN
    This PR (4628) - mean (749ms)  : 724, 773
     .   : milestone, 749,
    master - mean (747ms)  : 727, 766
     .   : milestone, 747,

Loading
gantt
    title Execution time (ms) FakeDbCommand (.NET 6) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (4628) - mean (88ms)  : 85, 91
     .   : milestone, 88,
    master - mean (89ms)  : 83, 95
     .   : milestone, 89,

    section CallTarget+Inlining+NGEN
    This PR (4628) - mean (711ms)  : 691, 731
     .   : milestone, 711,
    master - mean (718ms)  : 695, 741
     .   : milestone, 718,

Loading
gantt
    title Execution time (ms) HttpMessageHandler (.NET Framework 4.6.2) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (4628) - mean (187ms)  : 178, 197
     .   : milestone, 187,
    master - mean (186ms)  : 178, 194
     .   : milestone, 186,

    section CallTarget+Inlining+NGEN
    This PR (4628) - mean (1,131ms)  : 1103, 1159
     .   : milestone, 1131,
    master - mean (1,114ms)  : 1079, 1150
     .   : milestone, 1114,

Loading
gantt
    title Execution time (ms) HttpMessageHandler (.NET Core 3.1) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (4628) - mean (268ms)  : 261, 276
     .   : milestone, 268,
    master - mean (266ms)  : 254, 277
     .   : milestone, 266,

    section CallTarget+Inlining+NGEN
    This PR (4628) - mean (1,100ms)  : 1060, 1140
     .   : milestone, 1100,
    master - mean (1,095ms)  : 1056, 1134
     .   : milestone, 1095,

Loading
gantt
    title Execution time (ms) HttpMessageHandler (.NET 6) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (4628) - mean (257ms)  : 246, 268
     .   : milestone, 257,
    master - mean (258ms)  : 248, 268
     .   : milestone, 258,

    section CallTarget+Inlining+NGEN
    This PR (4628) - mean (1,050ms)  : 1016, 1083
     .   : milestone, 1050,
    master - mean (1,043ms)  : 1008, 1079
     .   : milestone, 1043,

Loading

@datadog-ddstaging
Copy link

datadog-ddstaging bot commented Sep 15, 2023

Datadog Report

Branch report: tony/refactor-and-improvements-of-benchmarkdotnet-instrumentation
Commit report: 40a5ca5

dd-trace-dotnet: 1 Failed (1 Known Flaky), 0 New Flaky, 302143 Passed, 1154 Skipped, 23m 24.52s Wall Time

❌ Failed Tests (1)

  • CheckEndpointsAreAttached - Datadog.Profiler.IntegrationTests.CodeHotspot.CodeHotspotTest - ❄️ Known flaky - Details

    Expand for error
     Expected endpoints.Distinct() to be a collection with 1 item(s), but found an empty collection.
     
     With configuration:
     - Use declared types and members
     - Compare enums by value
     - Include all non-private properties
     - Include all non-private fields
     - Match member by name (or throw)
     - Without automatic conversion.
     - Without automatic conversion.
     ...
    

Copy link
Contributor

@bouwkast bouwkast left a comment

Choose a reason for hiding this comment

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

Nice looks good to me!

@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 (4628) (11.441M)   : 0, 11440942
    master (11.476M)   : 0, 11475995
    benchmarks/2.37.0 (11.517M)   : 0, 11517370
    benchmarks/2.9.0 (11.506M)   : 0, 11506345

    section Automatic
    This PR (4628) (8.127M)   : 0, 8127116
    master (8.074M)   : 0, 8073632
    benchmarks/2.37.0 (8.143M)   : 0, 8142506
    benchmarks/2.9.0 (8.280M)   : 0, 8279564

    section Trace stats
    master (8.038M)   : 0, 8038306
    benchmarks/2.37.0 (8.044M)   : 0, 8044129

    section Manual
    This PR (4628) (10.280M)   : 0, 10279536
    master (10.163M)   : 0, 10163147
    benchmarks/2.37.0 (10.278M)   : 0, 10278226

    section Manual + Automatic
    This PR (4628) (7.787M)   : 0, 7787388
    master (7.783M)   : 0, 7782808
    benchmarks/2.37.0 (7.786M)   : 0, 7785678

    section Version Conflict
    master (6.984M)   : 0, 6983730
    benchmarks/2.37.0 (7.067M)   : 0, 7066760

Loading
gantt
    title Throughput Linux arm64 (Total requests) 
    dateFormat  X
    axisFormat %s
    section Baseline
    This PR (4628) (9.605M)   : 0, 9604707
    master (9.588M)   : 0, 9588243
    benchmarks/2.37.0 (9.619M)   : 0, 9618848
    benchmarks/2.9.0 (9.542M)   : 0, 9542061

    section Automatic
    This PR (4628) (6.595M)   : 0, 6595013
    master (6.780M)   : 0, 6780012
    benchmarks/2.37.0 (6.844M)   : 0, 6843850

    section Trace stats
    master (6.755M)   : 0, 6754543
    benchmarks/2.37.0 (6.801M)   : 0, 6801072

    section Manual
    This PR (4628) (8.531M)   : 0, 8531476
    master (8.679M)   : 0, 8678767
    benchmarks/2.37.0 (8.587M)   : 0, 8587283

    section Manual + Automatic
    This PR (4628) (6.639M)   : 0, 6638580
    master (6.417M)   : 0, 6417386
    benchmarks/2.37.0 (6.537M)   : 0, 6536939

    section Version Conflict
    master (5.991M)   : 0, 5990527
    benchmarks/2.37.0 (5.769M)   : 0, 5768881

Loading
gantt
    title Throughput Windows x64 (Total requests) 
    dateFormat  X
    axisFormat %s
    section Baseline
    This PR (4628) (7.007M)   : 0, 7006964
    master (9.247M)   : 0, 9247194
    benchmarks/2.37.0 (9.309M)   : 0, 9308797
    benchmarks/2.9.0 (9.366M)   : 0, 9366476

    section Automatic
    This PR (4628) (6.632M)   : 0, 6632444
    master (6.622M)   : 0, 6621852
    benchmarks/2.37.0 (6.552M)   : 0, 6552099
    benchmarks/2.9.0 (6.864M)   : 0, 6864235

    section Trace stats
    master (6.632M)   : 0, 6632286
    benchmarks/2.37.0 (6.685M)   : 0, 6685340

    section Manual
    This PR (4628) (8.629M)   : 0, 8629127
    master (8.465M)   : 0, 8464839
    benchmarks/2.37.0 (8.493M)   : 0, 8492505

    section Manual + Automatic
    This PR (4628) (6.592M)   : 0, 6592428
    master (6.505M)   : 0, 6504598
    benchmarks/2.37.0 (6.373M)   : 0, 6372588

    section Version Conflict
    master (5.869M)   : 0, 5868509
    benchmarks/2.37.0 (5.938M)   : 0, 5937795

Loading
gantt
    title Throughput Linux x64 (ASM) (Total requests) 
    dateFormat  X
    axisFormat %s
    section Baseline
    This PR (4628) (5.444M)   : 0, 5443807
    master (7.489M)   : 0, 7489317
    benchmarks/2.37.0 (7.239M)   : 0, 7239363
    benchmarks/2.9.0 (7.508M)   : 0, 7508023

    section No attack
    This PR (4628) (2.133M)   : 0, 2132593
    master (2.070M)   : 0, 2069897
    benchmarks/2.37.0 (2.105M)   : 0, 2105389
    benchmarks/2.9.0 (3.275M)   : 0, 3275098

    section Attack
    This PR (4628) (1.649M)   : 0, 1649312
    master (1.674M)   : 0, 1674160
    benchmarks/2.37.0 (1.686M)   : 0, 1685550
    benchmarks/2.9.0 (2.570M)   : 0, 2570112

    section Blocking
    This PR (4628) (3.220M)   : 0, 3219692
    master (3.325M)   : 0, 3324769
    benchmarks/2.37.0 (3.048M)   : 0, 3048473

Loading

@tonyredondo tonyredondo merged commit 9de8e1e into master Sep 15, 2023
48 of 51 checks passed
@tonyredondo tonyredondo deleted the tony/refactor-and-improvements-of-benchmarkdotnet-instrumentation branch September 15, 2023 15:20
@github-actions github-actions bot added this to the vNext milestone Sep 15, 2023
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.

None yet

4 participants