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

Don't require additional Windows SDK for Nuke desktop notification #5192

Merged
merged 2 commits into from
Feb 15, 2024

Conversation

andrewlock
Copy link
Member

Summary of changes

  • Change the targeted version of Windows
  • Don't add the extra dependencies etc unless you're actually using the Windows notification

Reason for change

Targeting a different version of Windows (10.0.17763 instead of 10.0.19041) and don't want to have the extra dependencies if you're not using the nuke notification.

Implementation details

Changed some versions, check for the NUKE_NOTIFY env-var in the build

Test coverage

Tested locally, it works

Other details

We have space issues on the benchmark agents, and part of the issue is a tonne of Windows SDKs, so I'm trying to clear some out

@andrewlock andrewlock added the area:builds project files, build scripts, pipelines, versioning, releases, packages label Feb 14, 2024
@andrewlock andrewlock requested a review from a team as a code owner February 14, 2024 13:06
@datadog-ddstaging
Copy link

datadog-ddstaging bot commented Feb 14, 2024

Datadog Report

Branch report: andrew/ci/dont-use-windows-sdk
Commit report: fe90f60
Test service: dd-trace-dotnet

✅ 0 Failed, 311325 Passed, 1586 Skipped, 41m 31.08s Wall Time
❄️ 1 New Flaky
⌛ 3 Performance Regressions

New Flaky Tests (1)

  • HttpClient_SubmitsTraces - Datadog.Trace.ClrProfiler.IntegrationTests.HttpMessageHandlerTests - Last Failure

    Expand for error
     Expected collection to contain a single item matching x.IsRequestType("app-closing"), but no such item was found.
     Expected varintegration=latestIntegrations not to be empty.
     Expected varintegration=latestIntegrations {empty} to contain key "HttpMessageHandler".
     Expected varintegration=latestIntegrations to be True because should only be enabled if we generate a span, but found False.
     Expected True because should only be auto-enabled if available, but found <null>.
     Expected collection to contain a single item matching x.IsRequestType("app-closing"), but no such item was found.
     Expected varintegration=latestIntegrations not to be empty.
     Expected varintegration=latestIntegrations {empty} to contain key "HttpSocketsHandler".
     Expected collection to contain a single item matching x.IsRequestType("app-closing"), but no such item was found.
     Expected varintegration=latestIntegrations not to be empty.
     ...
    

⌛ Performance Regressions vs Default Branch (3)

  • CallTarget+Inlining+NGEN - Samples.FakeDbCommand.windows.netcoreapp31.json.scenarios 765.08ms (+56.9ms, +8%) - Details
  • Baseline - Samples.FakeDbCommand.windows.net462.json.scenarios 86.71ms (+12.58ms, +17%) - Details
  • Baseline - Samples.FakeDbCommand.windows.netcoreapp31.json.scenarios 124.46ms (+13.5ms, +12%) - Details

@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 (5192) - mean (87ms)  : 66, 107
     .   : milestone, 87,
    master - mean (73ms)  : 66, 79
     .   : milestone, 73,

    section CallTarget+Inlining+NGEN
    This PR (5192) - mean (970ms)  : 940, 1000
     .   : milestone, 970,
    master - mean (972ms)  : 940, 1004
     .   : milestone, 972,

Loading
gantt
    title Execution time (ms) FakeDbCommand (.NET Core 3.1) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (5192) - mean (124ms)  : 114, 135
     .   : milestone, 124,
    master - mean (110ms)  : 108, 113
     .   : milestone, 110,

    section CallTarget+Inlining+NGEN
    This PR (5192) - mean (765ms)  : crit, 655, 876
     .   : crit, milestone, 765,
    master - mean (680ms)  : 660, 700
     .   : milestone, 680,

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

    section CallTarget+Inlining+NGEN
    This PR (5192) - mean (647ms)  : 610, 685
     .   : milestone, 647,
    master - mean (646ms)  : 624, 669
     .   : milestone, 646,

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

    section CallTarget+Inlining+NGEN
    This PR (5192) - mean (1,067ms)  : 1047, 1086
     .   : milestone, 1067,
    master - mean (1,059ms)  : 1036, 1082
     .   : milestone, 1059,

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

    section CallTarget+Inlining+NGEN
    This PR (5192) - mean (1,064ms)  : 1042, 1085
     .   : milestone, 1064,
    master - mean (1,060ms)  : 1034, 1087
     .   : milestone, 1060,

Loading
gantt
    title Execution time (ms) HttpMessageHandler (.NET 6) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (5192) - mean (260ms)  : 256, 263
     .   : milestone, 260,
    master - mean (259ms)  : 252, 265
     .   : milestone, 259,

    section CallTarget+Inlining+NGEN
    This PR (5192) - mean (999ms)  : 972, 1027
     .   : milestone, 999,
    master - mean (1,009ms)  : 977, 1042
     .   : milestone, 1009,

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 (5192) (11.107M)   : 0, 11107215
    master (11.256M)   : 0, 11256375
    benchmarks/2.9.0 (11.308M)   : 0, 11307837

    section Automatic
    This PR (5192) (7.731M)   : 0, 7730761
    master (7.619M)   : 0, 7619109
    benchmarks/2.9.0 (7.998M)   : 0, 7998252

    section Trace stats
    This PR (5192) (7.920M)   : 0, 7920347
    master (8.008M)   : 0, 8007505

    section Manual
    This PR (5192) (9.649M)   : 0, 9649359
    master (9.723M)   : 0, 9723486

    section Manual + Automatic
    This PR (5192) (7.161M)   : 0, 7160797
    master (7.310M)   : 0, 7310355

    section Version Conflict
    This PR (5192) (6.499M)   : 0, 6499465
    master (6.587M)   : 0, 6587239

Loading
gantt
    title Throughput Linux arm64 (Total requests) 
    dateFormat  X
    axisFormat %s
    section Baseline
    This PR (5192) (9.643M)   : 0, 9642921
    master (9.600M)   : 0, 9600069
    benchmarks/2.9.0 (9.577M)   : 0, 9577145

    section Automatic
    This PR (5192) (6.485M)   : 0, 6485176
    master (6.607M)   : 0, 6606924

    section Trace stats
    This PR (5192) (6.931M)   : 0, 6930569
    master (6.927M)   : 0, 6927149

    section Manual
    This PR (5192) (8.422M)   : 0, 8422378
    master (7.984M)   : 0, 7983517

    section Manual + Automatic
    This PR (5192) (6.326M)   : 0, 6326071
    master (6.318M)   : 0, 6318027

    section Version Conflict
    This PR (5192) (5.752M)   : 0, 5751753
    master (5.728M)   : 0, 5728462

Loading
gantt
    title Throughput Windows x64 (Total requests) 
    dateFormat  X
    axisFormat %s
    section Baseline
    This PR (5192) (10.516M)   : 0, 10515850
    master (10.316M)   : 0, 10316082
    benchmarks/2.9.0 (10.394M)   : 0, 10393876

    section Automatic
    This PR (5192) (7.242M)   : 0, 7242462
    master (7.380M)   : 0, 7380100
    benchmarks/2.9.0 (7.621M)   : 0, 7621208

    section Trace stats
    This PR (5192) (7.589M)   : 0, 7589408
    master (7.728M)   : 0, 7728420

    section Manual
    This PR (5192) (9.008M)   : 0, 9007664
    master (9.047M)   : 0, 9047148

    section Manual + Automatic
    This PR (5192) (6.912M)   : 0, 6911754
    master (7.194M)   : 0, 7193672

    section Version Conflict
    This PR (5192) (6.294M)   : 0, 6294279
    master (6.344M)   : 0, 6344145

Loading
gantt
    title Throughput Linux x64 (ASM) (Total requests) 
    dateFormat  X
    axisFormat %s
    section Baseline
    master (7.569M)   : 0, 7568842
    benchmarks/2.9.0 (7.633M)   : 0, 7632818

    section No attack
    master (1.856M)   : 0, 1856288
    benchmarks/2.9.0 (3.152M)   : 0, 3152273

    section Attack
    master (1.459M)   : 0, 1459232
    benchmarks/2.9.0 (2.469M)   : 0, 2468505

    section Blocking
    master (3.186M)   : 0, 3186214

    section IAST default
    master (6.503M)   : 0, 6502598

    section IAST full
    master (5.675M)   : 0, 5674990

    section Base vuln
    master (0.937M)   : 0, 936754

    section IAST vuln
    master (0.878M)   : 0, 877826

Loading

@andrewlock andrewlock merged commit 7308fac into master Feb 15, 2024
53 of 55 checks passed
@andrewlock andrewlock deleted the andrew/ci/dont-use-windows-sdk branch February 15, 2024 09:01
@github-actions github-actions bot added this to the vNext milestone Feb 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:builds project files, build scripts, pipelines, versioning, releases, packages
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants