dotnet / runtime

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

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

Closed performanceautofiler[bot] closed 2 years ago

performanceautofiler[bot] commented 2 years ago

Run Information

Architecture x64
OS ubuntu 18.04
Baseline 20e8f905d05f2d8f390d0e2ef10f1a4a1289967c
Compare 78c6505cffe2558b036fbe44cd27038affbb6cce
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_ubuntu 18.04/System.Text.RegularExpressions.Tests.Perf_Regex_Industry_RustLang_Sherlock.Count(Pattern%3a%20%22(%3fi)Sherlock%20Holmes%22%2c%20Options%3a%20NonBacktracking).html>) 132.97 μs 150.70 μs 1.13 0.01 False

graph Test Report

Repro

git clone https://github.com/dotnet/performance.git
python3 .\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", Options: NonBacktracking) ```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 150.7005840544872 > 139.10515277691204. IsChangePoint: Marked as a change because one of 1/11/2022 1:49:04 AM, 2/8/2022 11:58:21 PM, 2/12/2022 1:03:18 PM, 2/15/2022 5:17:31 AM falls between 2/6/2022 2:48:36 PM and 2/15/2022 5:17:31 AM. IsRegressionStdDev: Marked as regression because -31.688061479666207 (T) = (0 -151376.4322737688) / Math.Sqrt((15642188.562759358 / (48)) + (1053701.8619441038 / (9))) is less than -2.0040447832881556 = MathNet.Numerics.Distributions.StudentT.InvCDF(0, 1, (48) + (9) - 2, .025) and -0.16187403613980636 = (130286.44032420217 - 151376.4322737688) / 130286.44032420217 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)
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 | ubuntu 18.04 Baseline | [20e8f905d05f2d8f390d0e2ef10f1a4a1289967c](https://github.com/dotnet/runtime/commit/20e8f905d05f2d8f390d0e2ef10f1a4a1289967c) Compare | [78c6505cffe2558b036fbe44cd27038affbb6cce](https://github.com/dotnet/runtime/commit/78c6505cffe2558b036fbe44cd27038affbb6cce) Diff | [Diff](https://github.com/dotnet/runtime/compare/20e8f905d05f2d8f390d0e2ef10f1a4a1289967c...78c6505cffe2558b036fbe44cd27038affbb6cce) ### 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]() | 132.97 μs | 150.70 μs | 1.13 | 0.01 | False | | | ![graph]() [Test Report]() ### Repro ```cmd git clone https://github.com/dotnet/performance.git python3 .\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", Options: NonBacktracking) ```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 150.7005840544872 > 139.10515277691204. IsChangePoint: Marked as a change because one of 1/11/2022 1:49:04 AM, 2/8/2022 11:58:21 PM, 2/12/2022 1:03:18 PM, 2/15/2022 5:17:31 AM falls between 2/6/2022 2:48:36 PM and 2/15/2022 5:17:31 AM. IsRegressionStdDev: Marked as regression because -31.688061479666207 (T) = (0 -151376.4322737688) / Math.Sqrt((15642188.562759358 / (48)) + (1053701.8619441038 / (9))) is less than -2.0040447832881556 = MathNet.Numerics.Distributions.StudentT.InvCDF(0, 1, (48) + (9) - 2, .025) and -0.16187403613980636 = (130286.44032420217 - 151376.4322737688) / 130286.44032420217 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: -
Labels: `area-System.Text.RegularExpressions`, `untriaged`, `refs/heads/main`, `x64`, `ubuntu 18.04`, `Regression`, `RunKind=micro`, `CoreClr`
Milestone: -
EgorBo commented 2 years ago

https://github.com/dotnet/runtime/pull/65129

stephentoub commented 2 years ago

@olsaarik, is this expected? The pattern here, "(?i)Sherlock Holmes", doesn't have any subcaptures.

olsaarik commented 2 years ago

This is expected, but can be fixed with additional optimizations. With the subcaptures changes in https://github.com/dotnet/runtime/pull/65129 we also fixed the length of captures preferred by NonBacktracking to match that of the backtracking engines. That partially disabled an optimization for patterns with fixed length fragments. For this pattern (?i)Sherlock Holmes there's no possibility of selecting the wrong length of match, which could be detected and the whole optimization recovered.

olsaarik commented 2 years ago

This is where the optimization in question is currently applied: https://github.com/dotnet/runtime/blob/ea5376adf7bb963f76272b5077df45c252a5c15b/src/libraries/System.Text.RegularExpressions/src/System/Text/RegularExpressions/Symbolic/SymbolicRegexMatcher.cs#L533-L555