dotnet / perf-autofiling-issues

A landing place for auto-filed performance issues before they receive triage
MIT License
9 stars 4 forks source link

[Perf] Changes at 7/7/2022 11:40:51 PM #6640

Open performanceautofiler[bot] opened 2 years ago

performanceautofiler[bot] commented 2 years ago

Run Information

Architecture arm64
OS ubuntu 20.04
Baseline 21a69aa7f70cad997959cce2c33e60d821daca61
Compare 4a63cb28b69e1c48bccf592150be7ba297b67950
Diff Diff

Regressions in System.Threading.Tests.Perf_Monitor

Benchmark Baseline Test Test/Base Test Quality Edge Detector Baseline IR Compare IR IR Ratio Baseline ETL Compare ETL
[EnterExit - Duration of single invocation](<https://pvscmdupload.z22.web.core.windows.net/reports/allTestHistory/refs/heads/main_arm64_ubuntu 20.04/System.Threading.Tests.Perf_Monitor.EnterExit.html>) 19.89 ns 22.04 ns 1.11 0.21 False

graph Test Report

Repro

git clone https://github.com/dotnet/performance.git
py .\performance\scripts\benchmarks_ci.py -f net6.0 --filter 'System.Threading.Tests.Perf_Monitor*'
### Payloads [Baseline]() [Compare]() ### Histogram #### System.Threading.Tests.Perf_Monitor.EnterExit ```log ``` ### Description of detection logic ```IsRegressionBase: Marked as regression because the compare was 5% greater than the baseline, and the value was not too small. IsRegressionChecked: Marked as regression because the three check build points were 0.05 greater than the baseline. IsImprovementBase: Marked as not an improvement because the compare was not 5% less than the baseline, or the value was too small. IsRegressionBase: Marked as regression because the compare was 5% greater than the baseline, and the value was not too small. IsRegressionChecked: Marked as regression because the three check build points were 0.05 greater than the baseline. IsRegressionWindowed: Marked as regression because 22.04067352234627 > 20.58504262920396. IsChangePoint: Marked as a change because one of 7/7/2022 7:47:15 PM, 7/11/2022 5:54:09 PM falls between 7/3/2022 4:26:07 AM and 7/11/2022 5:54:09 PM. IsRegressionStdDev: Marked as regression because -7.2688174840201105 (T) = (0 -21.97191346917649) / Math.Sqrt((1.6872586448060853 / (30)) + (0.1767391199318404 / (16))) is less than -2.0153675744421933 = MathNet.Numerics.Distributions.StudentT.InvCDF(0, 1, (30) + (16) - 2, .025) and -0.0938709626300066 = (20.086385158583205 - 21.97191346917649) / 20.086385158583205 is less than -0.05. IsImprovementBase: Marked as not an improvement because the compare was not 5% less than the baseline, or the value was too small. IsChangeEdgeDetector: Marked not as a regression because Edge Detector said so. ``` ### Docs [Profiling workflow for dotnet/runtime repository](https://github.com/dotnet/performance/blob/master/docs/profiling-workflow-dotnet-runtime.md) [Benchmarking workflow for dotnet/runtime repository](https://github.com/dotnet/performance/blob/master/docs/benchmarking-workflow-dotnet-runtime.md)
AndyAyersMS commented 2 years ago

Maybe from https://github.com/dotnet/runtime/pull/70992? I can no longer see the diffs.

AndyAyersMS commented 2 years ago

@jakobbotsch maybe take a look?

jakobbotsch commented 2 years ago

On my Raspberry Pi 400 I get:

Method Job Toolchain Mean Error StdDev Median Min Max Ratio RatioSD Allocated Alloc Ratio
EnterExit Job-DXHTWZ /base/corerun 47.79 ns 0.008 ns 0.007 ns 47.79 ns 47.78 ns 47.81 ns 1.00 0.00 - NA
EnterExit Job-CINUBA /diff-no-71512/corerun 49.37 ns 0.201 ns 0.178 ns 49.43 ns 48.94 ns 49.52 ns 1.03 0.00 - NA
EnterExit Job-PAERIH /diff/corerun 58.67 ns 0.823 ns 0.770 ns 58.96 ns 56.76 ns 58.97 ns 1.23 0.02 - NA

So this seems to be caused by https://github.com/dotnet/runtime/pull/71512 (the benchmark just calls Monitor.Enter followed by Monitor.Exit). cc @kunalspathak

base = 21a69aa7f70cad997959cce2c33e60d821daca61 diff = 4a63cb28b69e1c48bccf592150be7ba297b67950 diff-no-71512 = 4a63cb28b69e1c48bccf592150be7ba297b67950 with PR linked above reverted

kunalspathak commented 2 years ago

On my Raspberry Pi 400

Does it have atomics?

jakobbotsch commented 2 years ago

Does it have atomics?

This is what I get out of lscpu, so I don't think so:

Architecture:                    aarch64
CPU op-mode(s):                  32-bit, 64-bit
Byte Order:                      Little Endian
CPU(s):                          4
On-line CPU(s) list:             0-3
Thread(s) per core:              1
Core(s) per socket:              4
Socket(s):                       1
Vendor ID:                       ARM
Model:                           3
Model name:                      Cortex-A72
Stepping:                        r0p3
CPU max MHz:                     1800.0000
CPU min MHz:                     600.0000
BogoMIPS:                        108.00
Vulnerability Itlb multihit:     Not affected
Vulnerability L1tf:              Not affected
Vulnerability Mds:               Not affected
Vulnerability Meltdown:          Not affected
Vulnerability Spec store bypass: Vulnerable
Vulnerability Spectre v1:        Mitigation; __user pointer sanitization
Vulnerability Spectre v2:        Vulnerable
Vulnerability Srbds:             Not affected
Vulnerability Tsx async abort:   Not affected
Flags:                           fp asimd evtstrm crc32 cpuid

Do the benchmark machines have atomics? I assume so since there were improvements, which means maybe the uncontended case regressed in either case?

kunalspathak commented 2 years ago

Model name: Cortex-A72 Flags: fp asimd evtstrm crc32 cpuid

https://developer.arm.com/Processors/Cortex-A72 is based on Armv8-A and doesn't have atomics so yes, https://github.com/dotnet/runtime/pull/71512 regresses such scenarios, because in addition to the logic we had, we would be doing an extra check to see if atomics is present. However, the machine where the regression is reported has atomics, so unlikely that it is because of https://github.com/dotnet/runtime/pull/71512.

image

Narrowing down the commit range for the improvement and regression, this is the commit range:

  1. https://github.com/dotnet/runtime/compare/21a69aa7f70cad997959cce2c33e60d821daca61...79d5e091bb1d5c02a4ff992f0e055f650467bc77
  2. https://github.com/dotnet/runtime/compare/79d5e091bb1d5c02a4ff992f0e055f650467bc77...8cd9d63693d75a526ddd72b372ffc12cc2ab3735