-
Notifications
You must be signed in to change notification settings - Fork 4.5k
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
Regressions in System.Buffers.Binary.Tests.BinaryReadAndWriteTests #68338
Comments
This is a clear regression but the range e0560ba...12dd2ab doesn't have any related commit. |
There was also an arcade update of dotnet/arcade@d2715c6...2e24ed0 in performance repo and it might be one of the cause. |
I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label. |
Tagging subscribers to this area: @dotnet/area-system-buffers Issue DetailsRun Information
Regressions in System.Buffers.Binary.Tests.BinaryReadAndWriteTests
Reprogit clone https://github.com/dotnet/performance.git
py .\performance\scripts\benchmarks_ci.py -f net6.0 --filter 'System.Buffers.Binary.Tests.BinaryReadAndWriteTests*' PayloadsHistogramSystem.Buffers.Binary.Tests.BinaryReadAndWriteTests.ReadStructFieldByFieldUsingBitConverterBE
Description of detection logic
DocsProfiling workflow for dotnet/runtime repository
|
More regressions in arm64: dotnet/perf-autofiling-issues#4727 |
Another regression (windows x64): dotnet/perf-autofiling-issues#4715 |
Prompted by @kunalspathak observation above, in the perf repo over this period was not much of interest there except maybe 2b90a3bb0e8705c8af2a06447657d7c1debde312 which updated Arcade
in Arcade that is |
The actual diff is seemingly: https://github.com/dotnet/arcade/compare/549523c3fc8929da1a3073d1a97f298e0d1dc342..d2715c6ef2c3e28479 which bumped roslyn from 4.2.0-3.22201.5 (4f4a757510f260eb8ac121dd9f8e7706d13751fb) to 4.2.0-4.22208.7 (9a3459303679328da9ccc529888e3c576ce3efec) I may be reading the diffs wrong, but I don't see anything in the roslyn diffs that looks like it would change IL emission: We should look at the IL and see if anything's changed. |
More regressions detected across all configs, for both BE and LE. Here is the data: System.Buffers.Binary.Tests.BinaryReadAndWriteTests.ReadStructFieldByFieldUsingBitConverterLE
System.Buffers.Binary.Tests.BinaryReadAndWriteTests.ReadStructFieldByFieldUsingBitConverterBE
|
Any idea why - did it coincide with another arcade update? |
@danmoseley We haven't determined why. I think we should check in on this in a bit, and also monitor the rest of the configs to see if they also recovered. Unfortunately the recovery won't show up in the perf report until preview6 because it seems to have missed the cutoff. EDIT: Nevermind, I thought preview5 snapped already. We will see these changes in the preview5 report. |
Out of the range 9d972ed...7b5f40f during which performance was improved, we have a theory that #68739 might have impacted, but it is hard to believe that the performance came back to around same point. Also, even if it is because of #68739, it means that we still need to investigate the cause of the initial spike. |
As expected, preview5 shows the fix to this regression. System.Buffers.Binary.Tests.BinaryReadAndWriteTests.ReadStructFieldByFieldUsingBitConverterBE
System.Buffers.Binary.Tests.BinaryReadAndWriteTests.ReadStructFieldByFieldUsingBitConverterLE
|
Run Information
Regressions in System.Buffers.Binary.Tests.BinaryReadAndWriteTests
Test Report
Repro
Payloads
Baseline
Compare
Histogram
System.Buffers.Binary.Tests.BinaryReadAndWriteTests.ReadStructFieldByFieldUsingBitConverterBE
Description of detection logic
Docs
Profiling workflow for dotnet/runtime repository
Benchmarking workflow for dotnet/runtime repository
The text was updated successfully, but these errors were encountered: