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

[Perf] Regressions in System.Buffers.Text.Tests.Base64Tests #72866

Closed
performanceautofiler bot opened this issue Jul 26, 2022 · 11 comments
Closed

[Perf] Regressions in System.Buffers.Text.Tests.Base64Tests #72866

performanceautofiler bot opened this issue Jul 26, 2022 · 11 comments
Assignees
Labels
arch-x64 area-Meta os-linux Linux OS (any supported distro) runtime-coreclr specific to the CoreCLR runtime tenet-performance Performance related issue tenet-performance-benchmarks Issue from performance benchmark
Milestone

Comments

@performanceautofiler
Copy link

Run Information

Architecture x64
OS ubuntu 18.04
Baseline b5fd812c63f2d548395b33e556f0e68cf7598dcf
Compare 02ffbcdb785d405e3d4e8457c05ab08f6b745e51
Diff Diff

Regressions in System.Perf_Convert

Benchmark Baseline Test Test/Base Test Quality Edge Detector Baseline IR Compare IR IR Ratio Baseline ETL Compare ETL
ToBase64CharArray - Duration of single invocation 889.24 ns 988.56 ns 1.11 0.00 False

graph
Test Report

Repro

git clone https://github.com/dotnet/performance.git
python3 .\performance\scripts\benchmarks_ci.py -f net6.0 --filter 'System.Perf_Convert*'

Payloads

Baseline
Compare

Histogram

System.Perf_Convert.ToBase64CharArray(binaryDataSize: 1024, formattingOptions: None)


Description of detection logic

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 988.5578792093395 > 934.2436747398181.
IsChangePoint: Marked as a change because one of 7/24/2022 8:36:55 PM, 7/26/2022 6:20:16 AM falls between 7/17/2022 2:02:19 PM and 7/26/2022 6:20:16 AM.
IsRegressionStdDev: Marked as regression because -157.90484221330118 (T) = (0 -985.1798912135928) / Math.Sqrt((1.4283183589673323 / (69)) + (2.4194046780736205 / (7))) is less than -1.9925434951804422 = MathNet.Numerics.Distributions.StudentT.InvCDF(0, 1, (69) + (7) - 2, .025) and -0.10743179921890782 = (889.6077319691004 - 985.1798912135928) / 889.6077319691004 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
Benchmarking workflow for dotnet/runtime repository

### Run Information
Architecture x64
OS ubuntu 18.04
Baseline b5fd812c63f2d548395b33e556f0e68cf7598dcf
Compare 02ffbcdb785d405e3d4e8457c05ab08f6b745e51
Diff Diff

Regressions in System.Buffers.Text.Tests.Base64Tests

Benchmark Baseline Test Test/Base Test Quality Edge Detector Baseline IR Compare IR IR Ratio Baseline ETL Compare ETL
ConvertToBase64CharArray - Duration of single invocation 866.07 ns 958.87 ns 1.11 0.00 False

graph
Test Report

Repro

git clone https://github.com/dotnet/performance.git
python3 .\performance\scripts\benchmarks_ci.py -f net6.0 --filter 'System.Buffers.Text.Tests.Base64Tests*'

Payloads

Baseline
Compare

Histogram

System.Buffers.Text.Tests.Base64Tests.ConvertToBase64CharArray(NumberOfBytes: 1000)


Description of detection logic

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 958.8704418275734 > 910.0775226681224.
IsChangePoint: Marked as a change because one of 7/24/2022 8:36:55 PM, 7/26/2022 6:20:16 AM falls between 7/17/2022 2:02:19 PM and 7/26/2022 6:20:16 AM.
IsRegressionStdDev: Marked as regression because -368.28363235682093 (T) = (0 -958.7521004129097) / Math.Sqrt((0.49035060946694337 / (70)) + (0.385901776692054 / (7))) is less than -1.9921021540016632 = MathNet.Numerics.Distributions.StudentT.InvCDF(0, 1, (70) + (7) - 2, .025) and -0.10588923110301522 = (866.9512944408087 - 958.7521004129097) / 866.9512944408087 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
Benchmarking workflow for dotnet/runtime repository

@performanceautofiler performanceautofiler bot added CoreClr untriaged New issue has not been triaged by the area owner labels Jul 26, 2022
@DrewScoggins DrewScoggins transferred this issue from dotnet/perf-autofiling-issues Jul 26, 2022
@dotnet-issue-labeler
Copy link

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.

@DrewScoggins DrewScoggins changed the title [Perf] Changes at 7/24/2022 10:50:09 PM [Perf] Regressions in System.Buffers.Text.Tests.Base64Tests Jul 26, 2022
@DrewScoggins
Copy link
Member

Seems related to #72727

@DrewScoggins DrewScoggins added tenet-performance Performance related issue tenet-performance-benchmarks Issue from performance benchmark labels Jul 26, 2022
@DrewScoggins
Copy link
Member

Seeing the same impact on Alpine: dotnet/perf-autofiling-issues#6782

@stephentoub stephentoub self-assigned this Jul 26, 2022
@DrewScoggins
Copy link
Member

DrewScoggins commented Jul 26, 2022

Seeing some impact from this change on x64 as well: dotnet/perf-autofiling-issues#6840 dotnet/perf-autofiling-issues#6841

@DrewScoggins
Copy link
Member

Seeing impact on Amd hardware as well: dotnet/perf-autofiling-issues#6849

@EgorBo
Copy link
Member

EgorBo commented Jul 26, 2022

@stephentoub I've just checked this one locally and was able to reproduce the ~15% perf regression. Then I re-run it again but with --envvars DOTNET_JitDisablePGO:1 and it didn't show any regression.

It means that your change in C# slightly changed layout (the way you pin that string) and invalidated the built-in profile. Once a new profile is collected and updated in this repo the regression should be gone

@stephentoub
Copy link
Member

Thanks, @EgorBo. I suspected something like that.

Should we just close this then? Or wait to see it drop?

Moving forward, is there any way we can avoid noise like this? I see these kinds of things not infrequently.

@EgorBo
Copy link
Member

EgorBo commented Jul 26, 2022

Moving forward, is there any way we can avoid noise like this?

Hard to tell 🙁
We need to benchmark the code we ship so we can't only measure JitDisablePgo
We update PGO profile (ideally) once a week. PGO profiles are not very deterministic and two subsequent collections on top of the same codebase might produce different results and if we'll be updating PGO profile too often it will be painful to triage perf issues.

cc @AndyAyersMS @DrewScoggins maybe prior filing an issue we can have a separate run of the benchmark in question with --envvars DOTNET_JitDisablePGO:1 and if it doesn't produce the same diffrience - leave a note or something?

The things is - I closed many perf issues assigned to me as "looks like that thing fixed itself"

@AndyAyersMS
Copy link
Member

We are set up to do no pgo runs in the perf lab (at least for windows x64) but we hijacked that run to measure OSR. Now that OSR is enabled, we could switch that leg back.

PGO profiles are not very deterministic

I think in fact the collections are fairly consistent, at least they were last we looked. But the jit is overly sensitive to small differences in the data. This is something I hope to work on during .NET 8.

@ghost
Copy link

ghost commented Jul 27, 2022

Tagging subscribers to this area: @dotnet/area-meta
See info in area-owners.md if you want to be subscribed.

Issue Details

Run Information

Architecture x64
OS ubuntu 18.04
Baseline b5fd812c63f2d548395b33e556f0e68cf7598dcf
Compare 02ffbcdb785d405e3d4e8457c05ab08f6b745e51
Diff Diff

Regressions in System.Perf_Convert

Benchmark Baseline Test Test/Base Test Quality Edge Detector Baseline IR Compare IR IR Ratio Baseline ETL Compare ETL
ToBase64CharArray - Duration of single invocation 889.24 ns 988.56 ns 1.11 0.00 False

graph
Test Report

Repro

git clone https://github.com/dotnet/performance.git
python3 .\performance\scripts\benchmarks_ci.py -f net6.0 --filter 'System.Perf_Convert*'

Payloads

Baseline
Compare

Histogram

System.Perf_Convert.ToBase64CharArray(binaryDataSize: 1024, formattingOptions: None)


Description of detection logic

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 988.5578792093395 > 934.2436747398181.
IsChangePoint: Marked as a change because one of 7/24/2022 8:36:55 PM, 7/26/2022 6:20:16 AM falls between 7/17/2022 2:02:19 PM and 7/26/2022 6:20:16 AM.
IsRegressionStdDev: Marked as regression because -157.90484221330118 (T) = (0 -985.1798912135928) / Math.Sqrt((1.4283183589673323 / (69)) + (2.4194046780736205 / (7))) is less than -1.9925434951804422 = MathNet.Numerics.Distributions.StudentT.InvCDF(0, 1, (69) + (7) - 2, .025) and -0.10743179921890782 = (889.6077319691004 - 985.1798912135928) / 889.6077319691004 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
Benchmarking workflow for dotnet/runtime repository

### Run Information
Architecture x64
OS ubuntu 18.04
Baseline b5fd812c63f2d548395b33e556f0e68cf7598dcf
Compare 02ffbcdb785d405e3d4e8457c05ab08f6b745e51
Diff Diff

Regressions in System.Buffers.Text.Tests.Base64Tests

Benchmark Baseline Test Test/Base Test Quality Edge Detector Baseline IR Compare IR IR Ratio Baseline ETL Compare ETL
ConvertToBase64CharArray - Duration of single invocation 866.07 ns 958.87 ns 1.11 0.00 False

graph
Test Report

Repro

git clone https://github.com/dotnet/performance.git
python3 .\performance\scripts\benchmarks_ci.py -f net6.0 --filter 'System.Buffers.Text.Tests.Base64Tests*'

Payloads

Baseline
Compare

Histogram

System.Buffers.Text.Tests.Base64Tests.ConvertToBase64CharArray(NumberOfBytes: 1000)


Description of detection logic

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 958.8704418275734 > 910.0775226681224.
IsChangePoint: Marked as a change because one of 7/24/2022 8:36:55 PM, 7/26/2022 6:20:16 AM falls between 7/17/2022 2:02:19 PM and 7/26/2022 6:20:16 AM.
IsRegressionStdDev: Marked as regression because -368.28363235682093 (T) = (0 -958.7521004129097) / Math.Sqrt((0.49035060946694337 / (70)) + (0.385901776692054 / (7))) is less than -1.9921021540016632 = MathNet.Numerics.Distributions.StudentT.InvCDF(0, 1, (70) + (7) - 2, .025) and -0.10588923110301522 = (866.9512944408087 - 958.7521004129097) / 866.9512944408087 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
Benchmarking workflow for dotnet/runtime repository

Author: performanceautofiler[bot]
Assignees: stephentoub, DrewScoggins
Labels:

area-Meta, tenet-performance, tenet-performance-benchmarks, untriaged, refs/heads/main, ubuntu 18.04, RunKind=micro, Regression, CoreClr, x64

Milestone: -

@DrewScoggins
Copy link
Member

We are also seeing improvements on Arm64: dotnet/perf-autofiling-issues#6859

@ghost ghost locked as resolved and limited conversation to collaborators Sep 10, 2022
@jeffhandley jeffhandley added runtime-coreclr specific to the CoreCLR runtime os-linux Linux OS (any supported distro) arch-x64 and removed CoreClr labels Dec 28, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
arch-x64 area-Meta os-linux Linux OS (any supported distro) runtime-coreclr specific to the CoreCLR runtime tenet-performance Performance related issue tenet-performance-benchmarks Issue from performance benchmark
Projects
None yet
Development

No branches or pull requests

6 participants