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

Fix ASM WAF benchmarks #4547

Merged
merged 1 commit into from
Aug 24, 2023
Merged

Fix ASM WAF benchmarks #4547

merged 1 commit into from
Aug 24, 2023

Conversation

andrewlock
Copy link
Member

@andrewlock andrewlock commented Aug 24, 2023 •
edited
Loading

Summary of changes

  • Update the ASM benchmark so it doesn't break the benchmark comparer

Reason for change

The current implementation generates the same name for each benchmark run, breaking the comparerer.

Implementation details

When using complex objects as arguments to a benchmark you have to override ToString() to fix the display names

Test coverage

  • I'll check that the names are generated correctly in the CI run

Confirmed this fixes the naming:

  • Benchmarks.Trace.Asm.AppSecWafBenchmark.RunWaf(args: NestedMap (10))
  • Benchmarks.Trace.Asm.AppSecWafBenchmark.RunWaf(args: NestedMap (100))
  • Benchmarks.Trace.Asm.AppSecWafBenchmark.RunWaf(args: NestedMap (1000))

Other details

Broken in #4534

@andrewlock andrewlock requested a review from a team as a code owner August 24, 2023 13:34
@andrewlock andrewlock added area:builds project files, build scripts, pipelines, versioning, releases, packages area:asm labels Aug 24, 2023
@github-actions github-actions bot added the area:tests unit tests, integration tests label Aug 24, 2023
@datadog-ddstaging
Copy link

datadog-ddstaging bot commented Aug 24, 2023 •
edited
Loading

Datadog Report

Branch report: andrew/ci/fix-benchmark-comparison
Commit report: 2781a6a

✅ dd-trace-dotnet: 0 Failed, 0 New Flaky, 298959 Passed, 1115 Skipped, 21m 22.86s Wall Time

@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 (4547) - mean (69ms)  : 65, 73
     .   : milestone, 69,
    master - mean (69ms)  : 64, 74
     .   : milestone, 69,

    section CallTarget+Inlining+NGEN
    This PR (4547) - mean (1,040ms)  : 1016, 1064
     .   : milestone, 1040,
    master - mean (1,033ms)  : 1004, 1062
     .   : milestone, 1033,
Loading 
gantt
    title Execution time (ms) FakeDbCommand (.NET Core 3.1) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (4547) - mean (108ms)  : 96, 120
     .   : milestone, 108,
    master - mean (105ms)  : 100, 111
     .   : milestone, 105,

    section CallTarget+Inlining+NGEN
    This PR (4547) - mean (732ms)  : 700, 764
     .   : milestone, 732,
    master - mean (733ms)  : 700, 766
     .   : milestone, 733,
Loading 
gantt
    title Execution time (ms) FakeDbCommand (.NET 6) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (4547) - mean (91ms)  : 84, 97
     .   : milestone, 91,
    master - mean (90ms)  : 83, 97
     .   : milestone, 90,

    section CallTarget+Inlining+NGEN
    This PR (4547) - mean (694ms)  : 663, 725
     .   : milestone, 694,
    master - mean (689ms)  : 666, 711
     .   : milestone, 689,
Loading 
gantt
    title Execution time (ms) HttpMessageHandler (.NET Framework 4.6.2) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (4547) - mean (195ms)  : 191, 200
     .   : milestone, 195,
    master - mean (188ms)  : 183, 193
     .   : milestone, 188,

    section CallTarget+Inlining+NGEN
    This PR (4547) - mean (1,149ms)  : 1126, 1173
     .   : milestone, 1149,
    master - mean (1,127ms)  : 1087, 1168
     .   : milestone, 1127,
Loading 
gantt
    title Execution time (ms) HttpMessageHandler (.NET Core 3.1) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (4547) - mean (277ms)  : 271, 282
     .   : milestone, 277,
    master - mean (271ms)  : 263, 278
     .   : milestone, 271,

    section CallTarget+Inlining+NGEN
    This PR (4547) - mean (1,098ms)  : 1061, 1135
     .   : milestone, 1098,
    master - mean (1,077ms)  : 1049, 1105
     .   : milestone, 1077,
Loading 
gantt
    title Execution time (ms) HttpMessageHandler (.NET 6) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (4547) - mean (267ms)  : 261, 273
     .   : milestone, 267,
    master - mean (257ms)  : 249, 266
     .   : milestone, 257,

    section CallTarget+Inlining+NGEN
    This PR (4547) - mean (1,060ms)  : 1020, 1101
     .   : milestone, 1060,
    master - mean (1,033ms)  : 994, 1072
     .   : milestone, 1033,
Loading

@andrewlock
Copy link
Member Author

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 (4547) (11.565M)   : 0, 11564889
    master (11.644M)   : 0, 11644043
    benchmarks/2.36.0 (11.752M)   : 0, 11752454
    benchmarks/2.9.0 (11.752M)   : 0, 11752427

    section Automatic
    This PR (4547) (8.100M)   : 0, 8100379
    master (8.291M)   : 0, 8291142
    benchmarks/2.36.0 (8.280M)   : 0, 8280416
    benchmarks/2.9.0 (8.494M)   : 0, 8494490

    section Trace stats
    master (8.262M)   : 0, 8262399
    benchmarks/2.36.0 (8.148M)   : 0, 8147852

    section Manual
    This PR (4547) (10.296M)   : 0, 10295640
    master (10.527M)   : 0, 10527241
    benchmarks/2.36.0 (10.336M)   : 0, 10335525

    section Manual + Automatic
    This PR (4547) (7.716M)   : 0, 7715726
    master (7.944M)   : 0, 7944369
    benchmarks/2.36.0 (7.907M)   : 0, 7907307

    section Version Conflict
    master (7.251M)   : 0, 7250761
    benchmarks/2.36.0 (7.084M)   : 0, 7084420
Loading 
gantt
    title Throughput Linux arm64 (Total requests) 
    dateFormat  X
    axisFormat %s
    section Baseline
    This PR (4547) (9.636M)   : 0, 9636264
    master (9.600M)   : 0, 9599884
    benchmarks/2.36.0 (9.582M)   : 0, 9582169
    benchmarks/2.9.0 (9.455M)   : 0, 9455151

    section Automatic
    This PR (4547) (6.606M)   : 0, 6605964
    master (6.795M)   : 0, 6794541
    benchmarks/2.36.0 (6.775M)   : 0, 6775278

    section Trace stats
    master (6.671M)   : 0, 6671385
    benchmarks/2.36.0 (6.725M)   : 0, 6725030

    section Manual
    This PR (4547) (8.659M)   : 0, 8659005
    master (8.468M)   : 0, 8467833
    benchmarks/2.36.0 (8.664M)   : 0, 8663817

    section Manual + Automatic
    This PR (4547) (6.516M)   : 0, 6515732
    master (6.192M)   : 0, 6191835
    benchmarks/2.36.0 (6.670M)   : 0, 6669993

    section Version Conflict
    master (5.780M)   : 0, 5780026
    benchmarks/2.36.0 (5.931M)   : 0, 5931025
Loading 
gantt
    title Throughput Windows x64 (Total requests) 
    dateFormat  X
    axisFormat %s
    section Baseline
    This PR (4547) (10.345M)   : 0, 10345023
    master (10.358M)   : 0, 10357887
    benchmarks/2.36.0 (10.209M)   : 0, 10209016
    benchmarks/2.9.0 (10.103M)   : 0, 10102653

    section Automatic
    This PR (4547) (7.339M)   : 0, 7339022
    master (7.273M)   : 0, 7273411
    benchmarks/2.36.0 (7.281M)   : 0, 7280896
    benchmarks/2.9.0 (7.341M)   : 0, 7340763

    section Trace stats
    master (7.316M)   : 0, 7316175
    benchmarks/2.36.0 (7.267M)   : 0, 7267042

    section Manual
    This PR (4547) (9.214M)   : 0, 9214044
    master (9.235M)   : 0, 9234639
    benchmarks/2.36.0 (9.078M)   : 0, 9077794

    section Manual + Automatic
    This PR (4547) (7.274M)   : 0, 7273901
    master (7.227M)   : 0, 7227130
    benchmarks/2.36.0 (7.075M)   : 0, 7075480

    section Version Conflict
    master (6.644M)   : 0, 6643598
    benchmarks/2.36.0 (6.479M)   : 0, 6478676
Loading 
gantt
    title Throughput Linux x64 (ASM) (Total requests) 
    dateFormat  X
    axisFormat %s
    section Baseline
    This PR (4547) (7.369M)   : 0, 7368782
    master (7.438M)   : 0, 7437662
    benchmarks/2.36.0 (7.453M)   : 0, 7453304
    benchmarks/2.9.0 (7.826M)   : 0, 7825811

    section No attack
    This PR (4547) (2.138M)   : 0, 2137564
    master (2.151M)   : 0, 2150930
    benchmarks/2.36.0 (2.136M)   : 0, 2136014
    benchmarks/2.9.0 (3.260M)   : 0, 3260143

    section Attack
    This PR (4547) (1.650M)   : 0, 1650212
    master (1.670M)   : 0, 1669947
    benchmarks/2.36.0 (1.687M)   : 0, 1687301
    benchmarks/2.9.0 (2.532M)   : 0, 2532213

    section Blocking
    This PR (4547) (3.094M)   : 0, 3093578
    master (3.105M)   : 0, 3105259
    benchmarks/2.36.0 (3.104M)   : 0, 3103809
Loading

@andrewlock andrewlock merged commit 9aea9ba into master Aug 24, 2023
@andrewlock andrewlock deleted the andrew/ci/fix-benchmark-comparison branch August 24, 2023 16:28
@github-actions github-actions bot added this to the vNext milestone Aug 24, 2023
NachoEchevarria pushed a commit that referenced this pull request Aug 29, 2023
@andrewlock @NachoEchevarria
Fix ASM WAF benchmarks (#4547)
49f8680
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@NachoEchevarria NachoEchevarria NachoEchevarria approved these changes

@bouwkast bouwkast bouwkast approved these changes

@lucaspimentel lucaspimentel lucaspimentel approved these changes

Assignees
No one assigned
Labels
area:asm area:builds project files, build scripts, pipelines, versioning, releases, packages area:tests unit tests, integration tests
Projects
None yet
Milestone
2.37.0
Development

Successfully merging this pull request may close these issues.
4 participants
@andrewlock @lucaspimentel @bouwkast @NachoEchevarria