search

dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
15.19k stars 4.72k forks source link

Regressions in System.Text.RegularExpressions.Tests.Perf_Regex_Industry_RustLang_Sherlock #71070

Closed performanceautofiler[bot] closed 1 year ago

performanceautofiler[bot] commented 2 years ago

Run Information

Architecture x64
OS Windows 10.0.19042
Baseline 3fc61ebb562afc327a8fc6de5c82d76e86bf6f5d
Compare eca86deef0f485bec5f3c8230ef4f7f3ad157559
Diff Diff

Regressions in System.Text.RegularExpressions.Tests.Perf_Regex_Industry_RustLang_Sherlock

Benchmark Baseline Test Test/Base Test Quality Edge Detector Baseline IR Compare IR IR Ratio Baseline ETL Compare ETL
[Count - Duration of single invocation](<https://pvscmdupload.blob.core.windows.net/reports/allTestHistory/refs/heads/main_x64_Windows 10.0.19042/amd/System.Text.RegularExpressions.Tests.Perf_Regex_Industry_RustLang_Sherlock.Count(Pattern%3a%20%22(%3fi)Sherlock%7cHolmes%7cWatson%7cIrene%7cAdler%7cJohn%7cBaker%22%2c%20Options%3a%20Compiled).html>) 2.44 ms 2.65 ms 1.08 0.01 False

graph Test Report

Repro

git clone https://github.com/dotnet/performance.git
py .\performance\scripts\benchmarks_ci.py -f net6.0 --filter 'System.Text.RegularExpressions.Tests.Perf_Regex_Industry_RustLang_Sherlock*'
### Payloads [Baseline]() [Compare]() ### Histogram #### System.Text.RegularExpressions.Tests.Perf_Regex_Industry_RustLang_Sherlock.Count(Pattern: "(?i)Sherlock|Holmes|Watson|Irene|Adler|John|Baker", Options: Compiled) ```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 2.649909513888889 > 2.5637261562500004. IsChangePoint: Marked as a change because one of 4/16/2022 10:12:41 PM, 5/11/2022 9:09:01 PM, 6/16/2022 2:51:40 PM, 6/21/2022 6:07:15 AM falls between 6/12/2022 5:56:07 PM and 6/21/2022 6:07:15 AM. IsRegressionStdDev: Marked as regression because -149.93728756574984 (T) = (0 -2647563.7113278955) / Math.Sqrt((43793409.01966206 / (42)) + (22597757.113547586 / (27))) is less than -1.9960083540247138 = MathNet.Numerics.Distributions.StudentT.InvCDF(0, 1, (42) + (27) - 2, .025) and -0.08417888158379576 = (2441998.969266278 - 2647563.7113278955) / 2441998.969266278 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)

category:performance theme:benchmarks skill-level:beginner cost:small impact:small

ghost commented 2 years ago

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

Issue Details
### Run Information Architecture | x64 -- | -- OS | Windows 10.0.19042 Baseline | [3fc61ebb562afc327a8fc6de5c82d76e86bf6f5d](https://github.com/dotnet/runtime/commit/3fc61ebb562afc327a8fc6de5c82d76e86bf6f5d) Compare | [eca86deef0f485bec5f3c8230ef4f7f3ad157559](https://github.com/dotnet/runtime/commit/eca86deef0f485bec5f3c8230ef4f7f3ad157559) Diff | [Diff](https://github.com/dotnet/runtime/compare/3fc61ebb562afc327a8fc6de5c82d76e86bf6f5d...eca86deef0f485bec5f3c8230ef4f7f3ad157559) ### Regressions in System.Text.RegularExpressions.Tests.Perf_Regex_Industry_RustLang_Sherlock Benchmark | Baseline | Test | Test/Base | Test Quality | Edge Detector | Baseline IR | Compare IR | IR Ratio | Baseline ETL | Compare ETL -- | -- | -- | -- | -- | -- | -- | -- | -- | -- | -- [Count - Duration of single invocation]() | 2.44 ms | 2.65 ms | 1.08 | 0.01 | False | | | ![graph]() [Test Report]() ### Repro ```cmd git clone https://github.com/dotnet/performance.git py .\performance\scripts\benchmarks_ci.py -f net6.0 --filter 'System.Text.RegularExpressions.Tests.Perf_Regex_Industry_RustLang_Sherlock*' ```
### Payloads [Baseline]() [Compare]() ### Histogram #### System.Text.RegularExpressions.Tests.Perf_Regex_Industry_RustLang_Sherlock.Count(Pattern: "(?i)Sherlock|Holmes|Watson|Irene|Adler|John|Baker", Options: Compiled) ```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 2.649909513888889 > 2.5637261562500004. IsChangePoint: Marked as a change because one of 4/16/2022 10:12:41 PM, 5/11/2022 9:09:01 PM, 6/16/2022 2:51:40 PM, 6/21/2022 6:07:15 AM falls between 6/12/2022 5:56:07 PM and 6/21/2022 6:07:15 AM. IsRegressionStdDev: Marked as regression because -149.93728756574984 (T) = (0 -2647563.7113278955) / Math.Sqrt((43793409.01966206 / (42)) + (22597757.113547586 / (27))) is less than -1.9960083540247138 = MathNet.Numerics.Distributions.StudentT.InvCDF(0, 1, (42) + (27) - 2, .025) and -0.08417888158379576 = (2441998.969266278 - 2647563.7113278955) / 2441998.969266278 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)
Author: performanceautofiler[bot]
Assignees: EgorBo
Labels: `area-System.Text.RegularExpressions`, `refs/heads/main`, `RunKind=micro`, `Windows 10.0.19042`, `Regression`, `CoreClr`, `x64`
Milestone: -
EgorBo commented 2 years ago

https://github.com/dotnet/runtime/pull/67141 ?

EgorBo commented 2 years ago

Probably related: https://github.com/dotnet/perf-autofiling-issues/issues/6201 win-x64: https://github.com/dotnet/perf-autofiling-issues/issues/6215

ghost commented 2 years ago

Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch See info in area-owners.md if you want to be subscribed.

Issue Details
### Run Information Architecture | x64 -- | -- OS | Windows 10.0.19042 Baseline | [3fc61ebb562afc327a8fc6de5c82d76e86bf6f5d](https://github.com/dotnet/runtime/commit/3fc61ebb562afc327a8fc6de5c82d76e86bf6f5d) Compare | [eca86deef0f485bec5f3c8230ef4f7f3ad157559](https://github.com/dotnet/runtime/commit/eca86deef0f485bec5f3c8230ef4f7f3ad157559) Diff | [Diff](https://github.com/dotnet/runtime/compare/3fc61ebb562afc327a8fc6de5c82d76e86bf6f5d...eca86deef0f485bec5f3c8230ef4f7f3ad157559) ### Regressions in System.Text.RegularExpressions.Tests.Perf_Regex_Industry_RustLang_Sherlock Benchmark | Baseline | Test | Test/Base | Test Quality | Edge Detector | Baseline IR | Compare IR | IR Ratio | Baseline ETL | Compare ETL -- | -- | -- | -- | -- | -- | -- | -- | -- | -- | -- [Count - Duration of single invocation]() | 2.44 ms | 2.65 ms | 1.08 | 0.01 | False | | | ![graph]() [Test Report]() ### Repro ```cmd git clone https://github.com/dotnet/performance.git py .\performance\scripts\benchmarks_ci.py -f net6.0 --filter 'System.Text.RegularExpressions.Tests.Perf_Regex_Industry_RustLang_Sherlock*' ```
### Payloads [Baseline]() [Compare]() ### Histogram #### System.Text.RegularExpressions.Tests.Perf_Regex_Industry_RustLang_Sherlock.Count(Pattern: "(?i)Sherlock|Holmes|Watson|Irene|Adler|John|Baker", Options: Compiled) ```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 2.649909513888889 > 2.5637261562500004. IsChangePoint: Marked as a change because one of 4/16/2022 10:12:41 PM, 5/11/2022 9:09:01 PM, 6/16/2022 2:51:40 PM, 6/21/2022 6:07:15 AM falls between 6/12/2022 5:56:07 PM and 6/21/2022 6:07:15 AM. IsRegressionStdDev: Marked as regression because -149.93728756574984 (T) = (0 -2647563.7113278955) / Math.Sqrt((43793409.01966206 / (42)) + (22597757.113547586 / (27))) is less than -1.9960083540247138 = MathNet.Numerics.Distributions.StudentT.InvCDF(0, 1, (42) + (27) - 2, .025) and -0.08417888158379576 = (2441998.969266278 - 2647563.7113278955) / 2441998.969266278 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)
Author: performanceautofiler[bot]
Assignees: EgorBo
Labels: `tenet-performance`, `tenet-performance-benchmarks`, `area-CodeGen-coreclr`, `refs/heads/main`, `RunKind=micro`, `Windows 10.0.19042`, `Regression`, `CoreClr`, `x64`
Milestone: 7.0.0
joperezr commented 2 years ago

https://github.com/dotnet/runtime/pull/67141 ?

Moving to CodeGen area.

EgorBo commented 2 years ago

I took a look at the codegen diff (the issue reproduces only in Compiled mode) and it looks like this: https://www.diffchecker.com/HaCxdrD8 (my PR https://github.com/dotnet/runtime/pull/67141 is on the right) - it removes the bound check as was expected but for some reason for this particular pattern it makes code slow and I don't understand why exactly - probably misaligned loop or something like that

EgorBo commented 1 year ago

Not an issue any more:

image

stephentoub commented 1 year ago

Not an issue any more

That said, the graph shows a significant improvement from https://github.com/dotnet/runtime/commit/aea2eb7fea7c7c460b54afc86dfe838123e80410, but then we regress back more than half of that gain, due to what looks like https://github.com/dotnet/runtime/pull/78863 and https://github.com/dotnet/runtime/pull/80830. Am I reading that right? @MihaZupan, is that expected? Maybe this was already discussed elsewhere?

MihaZupan commented 1 year ago

Maybe this was already discussed elsewhere

I think that's effectively #81105.

Avx2 support made the searching throughput for longer strings a lot faster (as expected) but added a noticeable penalty when a match is found. #80830 got a chunk of that difference back, but there's still a gap, so regex patterns where we match a lot are impacted. It's on my todo to see if I can do something about that.

stephentoub commented 1 year ago

https://github.com/dotnet/runtime/pull/80830 got a chunk of that difference back

That's interesting, since that actually seems to go in the wrong direction here?

image
MihaZupan commented 1 year ago

That part is not expected, that wasn't what I saw locally -- that PR should have been just improvements across the board. I'll try to take a look soon™️

EgorBo commented 1 year ago

@stephentoub right, but this regression was filed for June 2022 spike so I closed as irrelevant, for the new regressions there is https://github.com/dotnet/runtime/issues/81105

stephentoub commented 1 year ago

Ok