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

Regressions in System.Memory.ReadOnlySpan.IndexOfString #85756

Closed
performanceautofiler bot opened this issue May 4, 2023 · 7 comments
Closed

Regressions in System.Memory.ReadOnlySpan.IndexOfString #85756

performanceautofiler bot opened this issue May 4, 2023 · 7 comments

Comments

@performanceautofiler
Copy link

Run Information

Name Value
Architecture arm64
OS Windows 10.0.25094
Queue AmpereWindows
Baseline 2b530d4abdab90801b9c3434d62e511d9f72e3e7
Compare 7a87fc1e18e53c03a17ae9105ad679d7906ae9ec
Diff Diff
Configs CompilationMode:tiered, RunKind:micro

Regressions in System.Memory.ReadOnlySpan

Benchmark Baseline Test Test/Base Test Quality Edge Detector Baseline IR Compare IR IR Ratio Baseline ETL Compare ETL
IndexOfString - Duration of single invocation 27.69 ns 44.13 ns 1.59 0.69 False

Test Report

Repro

General Docs link: https://github.com/dotnet/performance/blob/main/docs/benchmarking-workflow-dotnet-runtime.md

Payloads

Baseline
Compare

git clone https://github.com/dotnet/performance.git
py .\performance\scripts\benchmarks_ci.py -f net8.0 --filter 'System.Memory.ReadOnlySpan*'

Payloads

Baseline
Compare

Histogram

System.Memory.ReadOnlySpan.IndexOfString(input: "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAXAAAAAAAAAAAAAAAAAAAAAAAAAAAAA", value: "x", comparisonType: OrdinalIgnoreCase)


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 44.12726463340534 > 29.065976290113895.
IsChangePoint: Marked as a change because one of 5/1/2023 10:17:01 AM, 5/4/2023 2:44:24 AM falls between 4/25/2023 1:02:54 PM and 5/4/2023 2:44:24 AM.
IsRegressionStdDev: Marked as regression because -12.64424196287755 (T) = (0 -49.89955517156274) / Math.Sqrt((5.661681989159535 / (29)) + (22.471229788612526 / (8))) is less than -2.0301079282477414 = MathNet.Numerics.Distributions.StudentT.InvCDF(0, 1, (29) + (8) - 2, .025) and -0.783145851023553 = (27.98400094020331 - 49.89955517156274) / 27.98400094020331 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.

JIT Disasms

Docs

Profiling workflow for dotnet/runtime repository
Benchmarking workflow for dotnet/runtime repository

@performanceautofiler performanceautofiler bot added arch-arm64 os-windows runtime-coreclr specific to the CoreCLR runtime untriaged New issue has not been triaged by the area owner labels May 4, 2023
@cincuranet cincuranet removed the untriaged New issue has not been triaged by the area owner label May 4, 2023
@cincuranet cincuranet transferred this issue from dotnet/perf-autofiling-issues May 4, 2023
@dotnet-issue-labeler dotnet-issue-labeler bot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label May 4, 2023
@ghost ghost added the untriaged New issue has not been triaged by the area owner label May 4, 2023
@cincuranet
Copy link
Contributor

Looks like #85437 could the culprit. cc @stephentoub

@cincuranet cincuranet changed the title [Perf] Windows/arm64: 1 Regression on 5/1/2023 3:45:46 PM Regressions in System.Memory.ReadOnlySpan.IndexOfString May 4, 2023
@EgorBo
Copy link
Member

EgorBo commented May 4, 2023

I'd say it is expected - it is the worst case for the new algorithm

@stephentoub
Copy link
Member

Presumably there are corresponding significant improvements in other benchmarks?

@ghost
Copy link

ghost commented May 4, 2023

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

Issue Details

Run Information

Name Value
Architecture arm64
OS Windows 10.0.25094
Queue AmpereWindows
Baseline 2b530d4abdab90801b9c3434d62e511d9f72e3e7
Compare 7a87fc1e18e53c03a17ae9105ad679d7906ae9ec
Diff Diff
Configs CompilationMode:tiered, RunKind:micro

Regressions in System.Memory.ReadOnlySpan

Benchmark Baseline Test Test/Base Test Quality Edge Detector Baseline IR Compare IR IR Ratio Baseline ETL Compare ETL
IndexOfString - Duration of single invocation 27.69 ns 44.13 ns 1.59 0.69 False

Test Report

Repro

General Docs link: https://github.com/dotnet/performance/blob/main/docs/benchmarking-workflow-dotnet-runtime.md

Payloads

Baseline
Compare

git clone https://github.com/dotnet/performance.git
py .\performance\scripts\benchmarks_ci.py -f net8.0 --filter 'System.Memory.ReadOnlySpan*'

Payloads

Baseline
Compare

Histogram

System.Memory.ReadOnlySpan.IndexOfString(input: "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAXAAAAAAAAAAAAAAAAAAAAAAAAAAAAA", value: "x", comparisonType: OrdinalIgnoreCase)


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 44.12726463340534 > 29.065976290113895.
IsChangePoint: Marked as a change because one of 5/1/2023 10:17:01 AM, 5/4/2023 2:44:24 AM falls between 4/25/2023 1:02:54 PM and 5/4/2023 2:44:24 AM.
IsRegressionStdDev: Marked as regression because -12.64424196287755 (T) = (0 -49.89955517156274) / Math.Sqrt((5.661681989159535 / (29)) + (22.471229788612526 / (8))) is less than -2.0301079282477414 = MathNet.Numerics.Distributions.StudentT.InvCDF(0, 1, (29) + (8) - 2, .025) and -0.783145851023553 = (27.98400094020331 - 49.89955517156274) / 27.98400094020331 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.

JIT Disasms

Docs

Profiling workflow for dotnet/runtime repository
Benchmarking workflow for dotnet/runtime repository

Author: performanceautofiler[bot]
Assignees: -
Labels:

arch-arm64, area-System.Runtime, os-windows, untriaged, runtime-coreclr, needs-area-label

Milestone: -

@stephentoub
Copy link
Member

This should also be fixed by #85781.

@vcsjones vcsjones removed the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label May 4, 2023
@ghost ghost removed the untriaged New issue has not been triaged by the area owner label May 4, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Jun 4, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

6 participants