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

NonBacktracking inner matching loop optimizations #70217

Merged
merged 6 commits into from
Jun 7, 2022

Conversation

olsaarik
Copy link
Contributor

@olsaarik olsaarik commented Jun 3, 2022

This PR addresses the regressions related to the inner matching loop performance in #70022 introduced by #69839 and includes further optimizations. The changes in the PR are roughly:

  • Remove Or, And, Not + related fixes #69839 caused a regression by making fixed length marker evaluation character context aware, which made it more expensive. Fixed length markers were evaluated whenever a nullable state was found in the first phase of the search. This PR pulls the evaluation outside the inner matching loop by recording the state ID instead, which in DFA mode is just an int assignment. I've also implemented support for fixed length markers in NFA mode.
  • Pulls the decision to try to use RegexFindOptimizations out of the inner matching loop by using a IInitialStateHandler interface and the "templating" technique with structs implementing the interface as generic type arguments that we already use for DFA and NFA mode.
  • Introduces a decision outside the inner matching loop to never evaluate contextual nullability for patterns that don't have any anchors. This also uses the "templating" technique with a INullabilityHandler interface.
  • Tabulate an array of (bool IsInitial, bool IsDeadend, bool IsNullable, bool CanBeNullable) in the SymbolicRegexBuilder that can be accessed by state IDs. This reduces pointer chasing involved in looking these up from the DfaMatchingState and its related SymbolicRegexNode. The existing IsInitial field from DfaMatchingState is removed for some memory saving.
  • Makes CurrentState hold the ID (an int) of the current DFA state instead of the DfaMatchingState instance. This allows cached transitions to be taken in DFA mode without ever dereferencing the current DfaMatchingState. Coupled with the above optimization the DfaMatchingState is rarely needed in typical patterns.

One optimization that I evaluated, but that didn't decisively help was pulling out the decision to handle the "last end of line" special minterm outside the inner matching loop. I'm guessing the char == '\n' && pos == input.Length - 1 check is already cheap enough that it isn't really visible.

I compared performance after implementing each optimization to check that each was helping. Here's a total before and after comparison for NonBacktracking on all the relevant benchmarks in dotnet/performance. The baseline here is the situation after the regressions in #70022.
summary:
better: 58, geomean: 1.360
worse: 5, geomean: 1.057
total diff: 63

Slower diff/base Base Median (ns) Diff Median (ns) Modality
System.Text.RegularExpressions.Tests.Perf_Regex_Industry_Leipzig.Count(Pattern: "Tom|Sawyer|Huckleberry|Finn", Options: NonBacktracking) 1.11 4785248.08 5325761.46 several?
System.Text.RegularExpressions.Tests.Perf_Regex_Industry_Leipzig.Count(Pattern: "Tom.{10,25}river|river.{10,25}Tom", Options: NonBacktracking) 1.06 9628288.00 10195706.67
System.Text.RegularExpressions.Tests.Perf_Regex_Industry_BoostDocs_Simple.IsMatch(Id: 12, Options: NonBacktracking) 1.05 133.03 139.37
System.Text.RegularExpressions.Tests.Perf_Regex_Industry_BoostDocs_Simple.IsMatch(Id: 13, Options: NonBacktracking) 1.04 134.98 139.74
System.Text.RegularExpressions.Tests.Perf_Regex_Industry_SliceSlice.Count(Options: IgnoreCase, NonBacktracking) 1.03 2029181900.00 2094702200.00 several?
Faster base/diff Base Median (ns) Diff Median (ns) Modality
System.Text.RegularExpressions.Tests.Perf_Regex_Industry_RustLang_Sherlock.Count(Pattern: "[^\\n]*", Options: NonBacktracking) 2.59 15286261.76 5901962.79
System.Text.RegularExpressions.Tests.Perf_Regex_Industry_RustLang_Sherlock.Count(Pattern: "(?s).*", Options: NonBacktracking) 2.51 10022464.58 4000825.81
System.Text.RegularExpressions.Tests.Perf_Regex_Industry_Mariomkas.Count(Pattern: "[\w]+://[^/\\s?#]+[^\\s?#]+(?:\?[^\\s#])?(?:#[^\\s])?", Options: NonBacktracking) 2.22 6771215.28 3045911.11
System.Text.RegularExpressions.Tests.Perf_Regex_Industry_RustLang_Sherlock.Count(Pattern: "\b\w+n\b", Options: NonBacktracking) 2.17 10788114.58 4964147.06
System.Text.RegularExpressions.Tests.Perf_Regex_Industry_RustLang_Sherlock.Count(Pattern: ".*", Options: NonBacktracking) 1.96 12170225.00 6225097.67 bimodal
System.Text.RegularExpressions.Tests.Perf_Regex_Industry_RustLang_Sherlock.Count(Pattern: "\w+\s+Holmes", Options: NonBacktracking) 1.88 4073781.25 2171156.90
System.Text.RegularExpressions.Tests.Perf_Regex_Industry_SliceSlice.Count(Options: NonBacktracking) 1.86 1099632850.00 590537400.00 several?
System.Text.RegularExpressions.Tests.Perf_Regex_Industry_RustLang_Sherlock.Count(Pattern: "[a-zA-Z]+ing", Options: NonBacktracking) 1.82 8657798.33 4762550.98
System.Text.RegularExpressions.Tests.Perf_Regex_Industry_RustLang_Sherlock.Count(Pattern: "Holmes", Options: NonBacktracking) 1.80 159116.57 88287.89
System.Text.RegularExpressions.Tests.Perf_Regex_Industry_RustLang_Sherlock.Count(Pattern: "\s[a-zA-Z]{0,12}ing\s", Options: NonBacktracking) 1.78 4577136.36 2567926.42
System.Text.RegularExpressions.Tests.Perf_Regex_Industry_RustLang_Sherlock.Count(Pattern: "the\s+\w+", Options: NonBacktracking) 1.76 1961271.43 1115444.84
System.Text.RegularExpressions.Tests.Perf_Regex_Industry_RustLang_Sherlock.Count(Pattern: "Holmes.{0,25}Watson|Watson.{0,25}Holmes", Options: NonBacktracking) 1.72 199215.34 115912.40
System.Text.RegularExpressions.Tests.Perf_Regex_Industry_RustLang_Sherlock.Count(Pattern: "\w+", Options: NonBacktracking) 1.69 23110975.00 13655700.00
System.Text.RegularExpressions.Tests.Perf_Regex_Industry_RustLang_Sherlock.Count(Pattern: "\w+\s+Holmes\s+\w+", Options: NonBacktracking) 1.61 3377971.43 2104111.40 bimodal
System.Text.RegularExpressions.Tests.Perf_Regex_Industry_RustLang_Sherlock.Count(Pattern: "Sher[a-z]+|Hol[a-z]+", Options: NonBacktracking) 1.59 204743.55 128662.45
System.Text.RegularExpressions.Tests.Perf_Regex_Industry_RustLang_Sherlock.Count(Pattern: "[a-q][^u-z]{13}x", Options: NonBacktracking) 1.57 88111.27 56102.58
System.Text.RegularExpressions.Tests.Perf_Regex_Industry_RustLang_Sherlock.Count(Pattern: "the", Options: NonBacktracking) 1.50 1033139.24 687533.78 bimodal
System.Text.RegularExpressions.Tests.Perf_Regex_Industry_RustLang_Sherlock.Count(Pattern: "Sherlock\s+Holmes", Options: NonBacktracking) 1.47 86482.44 58707.08
System.Text.RegularExpressions.Tests.Perf_Regex_Industry_RustLang_Sherlock.Count(Pattern: "Sherlock|Holmes", Options: NonBacktracking) 1.47 177346.23 120786.17 bimodal
System.Text.RegularExpressions.Tests.Perf_Regex_Industry_Leipzig.Count(Pattern: ".{2,4}(Tom|Sawyer|Huckleberry|Finn)", Options: NonBacktracking) 1.41 67257150.00 47606437.50 several?
System.Text.RegularExpressions.Tests.Perf_Regex_Industry_RustLang_Sherlock.Count(Pattern: "Sherlock|Street", Options: NonBacktracking) 1.40 76250.69 54275.02
System.Text.RegularExpressions.Tests.Perf_Regex_Industry_Leipzig.Count(Pattern: ".{0,2}(Tom|Sawyer|Huckleberry|Finn)", Options: NonBacktracking) 1.40 67197850.00 47843350.00
System.Text.RegularExpressions.Tests.Perf_Regex_Industry_RustLang_Sherlock.Count(Pattern: "zqj", Options: NonBacktracking) 1.40 53884.39 38434.69 several?
System.Text.RegularExpressions.Tests.Perf_Regex_Industry_RustLang_Sherlock.Count(Pattern: "\p{Lu}", Options: NonBacktracking) 1.37 2540423.68 1849647.01 several?
System.Text.RegularExpressions.Tests.Perf_Regex_Industry_BoostDocs_Simple.IsMatch(Id: 5, Options: NonBacktracking) 1.37 206.84 150.82
System.Text.RegularExpressions.Tests.Perf_Regex_Industry_BoostDocs_Simple.IsMatch(Id: 3, Options: NonBacktracking) 1.37 242.66 177.47
System.Text.RegularExpressions.Tests.Perf_Regex_Industry_RustLang_Sherlock.Count(Pattern: "Sherlock", Options: NonBacktracking) 1.34 66087.55 49360.96 bimodal
System.Text.RegularExpressions.Tests.Perf_Regex_Industry_RustLang_Sherlock.Count(Pattern: "Sherlock|Holmes|Watson|Irene|Adler|John|Baker", Options: NonBacktracking) 1.33 3037911.41 2280343.52 several?
System.Text.RegularExpressions.Tests.Perf_Regex_Industry_RustLang_Sherlock.Count(Pattern: "\p{Ll}", Options: NonBacktracking) 1.33 40920612.50 30728875.00
System.Text.RegularExpressions.Tests.Perf_Regex_Industry_RustLang_Sherlock.Count(Pattern: "Sherlock|Holmes|Watson", Options: NonBacktracking) 1.32 211902.74 160457.22
System.Text.RegularExpressions.Tests.Perf_Regex_Industry_RustLang_Sherlock.Count(Pattern: "\p{L}", Options: NonBacktracking) 1.31 40560162.50 30977100.00
System.Text.RegularExpressions.Tests.Perf_Regex_Industry_RustLang_Sherlock.Count(Pattern: "Sherlock Holmes", Options: NonBacktracking) 1.31 68187.37 52127.82
System.Text.RegularExpressions.Tests.Perf_Regex_Industry_RustLang_Sherlock.Count(Pattern: "The", Options: NonBacktracking) 1.30 130572.40 100326.33 several?
System.Text.RegularExpressions.Tests.Perf_Regex_Industry_BoostDocs_Simple.IsMatch(Id: 4, Options: NonBacktracking) 1.29 181.44 140.99
System.Text.RegularExpressions.Tests.Perf_Regex_Industry_RustLang_Sherlock.Count(Pattern: "aqj", Options: NonBacktracking) 1.26 48508.51 38491.90
System.Text.RegularExpressions.Tests.Perf_Regex_Industry_RustLang_Sherlock.Count(Pattern: "aei", Options: NonBacktracking) 1.26 59747.35 47496.19
System.Text.RegularExpressions.Tests.Perf_Regex_Industry_RustLang_Sherlock.Count(Pattern: "(?m)^Sherlock Holmes|Sherlock Holmes$", Options: NonBacktracking) 1.25 64672.47 51940.21
System.Text.RegularExpressions.Tests.Perf_Regex_Industry_RustLang_Sherlock.Count(Pattern: "(?i)the", Options: NonBacktracking) 1.24 1523287.44 1232909.66
System.Text.RegularExpressions.Tests.Perf_Regex_Industry_BoostDocs_Simple.IsMatch(Id: 10, Options: NonBacktracking) 1.13 123.15 108.83
System.Text.RegularExpressions.Tests.Perf_Regex_Industry_BoostDocs_Simple.IsMatch(Id: 2, Options: NonBacktracking) 1.12 161.55 143.72
System.Text.RegularExpressions.Tests.Perf_Regex_Industry_BoostDocs_Simple.IsMatch(Id: 6, Options: NonBacktracking) 1.11 114.45 103.11 several?
System.Text.RegularExpressions.Tests.Perf_Regex_Industry_BoostDocs_Simple.IsMatch(Id: 7, Options: NonBacktracking) 1.09 101.74 93.39
System.Text.RegularExpressions.Tests.Perf_Regex_Industry_BoostDocs_Simple.IsMatch(Id: 9, Options: NonBacktracking) 1.08 112.40 103.77 bimodal
System.Text.RegularExpressions.Tests.Perf_Regex_Industry_Mariomkas.Count(Pattern: "(?:(?:25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9])\.){3}(?:25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9])", Options: NonBacktracking) 1.07 13209031.58 12307950.00
System.Text.RegularExpressions.Tests.Perf_Regex_Industry_RustLang_Sherlock.Count(Pattern: "(?i)Sher[a-z]+|Hol[a-z]+", Options: NonBacktracking) 1.06 984012.25 924302.87
System.Text.RegularExpressions.Tests.Perf_Regex_Industry_Leipzig.Count(Pattern: "[a-z]shing", Options: NonBacktracking) 1.06 2657380.00 2513476.53
System.Text.RegularExpressions.Tests.Perf_Regex_Industry_Mariomkas.Count(Pattern: "[\w\.+-]+@[\w\.-]+\.[\w\.-]+", Options: NonBacktracking) 1.06 638071.50 603662.24
System.Text.RegularExpressions.Tests.Perf_Regex_Industry_Leipzig.Count(Pattern: "Twain", Options: NonBacktracking) 1.05 2494026.73 2386224.76
System.Text.RegularExpressions.Tests.Perf_Regex_Industry_BoostDocs_Simple.IsMatch(Id: 0, Options: NonBacktracking) 1.04 83.28 79.90
System.Text.RegularExpressions.Tests.Perf_Regex_Industry_BoostDocs_Simple.IsMatch(Id: 8, Options: NonBacktracking) 1.04 106.23 102.07
System.Text.RegularExpressions.Tests.Perf_Regex_Industry_Leipzig.Count(Pattern: "([A-Za-z]awyer|[A-Za-z]inn)\s", Options: NonBacktracking) 1.04 15942657.14 15355812.50
System.Text.RegularExpressions.Tests.Perf_Regex_Industry_Leipzig.Count(Pattern: "(?i)Tom|Sawyer|Huckleberry|Finn", Options: NonBacktracking) 1.04 75034250.00 72294600.00
System.Text.RegularExpressions.Tests.Perf_Regex_Industry_RustLang_Sherlock.Count(Pattern: "(?i)Sherlock", Options: NonBacktracking) 1.04 103258.57 99506.47
System.Text.RegularExpressions.Tests.Perf_Regex_Industry_RustLang_Sherlock.Count(Pattern: "(?i)Sherlock|Holmes|Watson|Irene|Adler|John|Baker", Options: NonBacktracking) 1.03 3304392.00 3202143.59
System.Text.RegularExpressions.Tests.Perf_Regex_Industry_RustLang_Sherlock.Count(Pattern: "(?i)Holmes", Options: NonBacktracking) 1.03 507619.96 492948.24
System.Text.RegularExpressions.Tests.Perf_Regex_Industry_RustLang_Sherlock.Count(Pattern: "(?i)Sherlock|Holmes|Watson", Options: NonBacktracking) 1.03 2190175.00 2130422.17
System.Text.RegularExpressions.Tests.Perf_Regex_Industry_Leipzig.Count(Pattern: "(?i)Twain", Options: NonBacktracking) 1.03 6689778.38 6510337.84
System.Text.RegularExpressions.Tests.Perf_Regex_Industry_Mariomkas.Ctor(Pattern: "(?:(?:25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9])\.){3}(?:25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9])", Options: NonBacktracking) 1.02 212577.91 208770.25

@ghost
Copy link

ghost commented Jun 3, 2022

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

Issue Details

This PR addresses the regressions related to the inner matching loop performance in #70022 introduced by #69839 and includes further optimizations. The changes in the PR are roughly:

  • Remove Or, And, Not + related fixes #69839 caused a regression by making fixed length marker evaluation character context aware, which made it more expensive. Fixed length markers were evaluated whenever a nullable state was found in the first phase of the search. This PR pulls the evaluation outside the inner matching loop, which is possible because the state at the end position is recorded.
  • Pulls the decision to try to use RegexFindOptimizations out of the inner matching loop by using a IFindOptimizationsHandler interface and the "templating" technique with structs implementing the interface as generic type arguments that we already use for DFA and NFA mode.
  • Introduces a decision outside the inner matching loop to never evaluate contextual nullability for patterns that don't have any anchors. This also uses the "templating" technique with a INullabilityHandler interface.
  • Tabulate an array of (bool IsInitial, bool IsDeadend, bool IsNullable, bool CanBeNullable) in the SymbolicRegexBuilder that can be accessed by state IDs. This reduces pointer chasing involved in looking these up from the DfaMatchingState its related SymbolicRegexNode. The existing IsInitial field from DfaMatchingState is removed for some memory saving.
  • Makes CurrentState hold the ID (an int) of the current DFA state instead of the DfaMatchingState instance. This allows cached transitions to be taken without ever dereferencing the current DfaMatchingState. Coupled with the above optimization the DfaMatchingState is rarely needed. The only situation is when a pattern with anchors hits a state that isn't unconditionally nullable but can be nullable, which is when a call to the related node's IsNullableFor is required.

One optimization that I evaluated, but that didn't decisively help was pulling out the decision to handle the "last end of line" special minterm outside the inner matching loop. I'm guessing the char == '\n' && pos == input.Length - 1 check is already cheap enough that it isn't really visible.

I compared performance after implementing each optimization to check that each was helping. Here's a total before and after comparison for NonBacktracking on all the relevant benchmarks in dotnet/performance.
summary:
better: 54, geomean: 1.409
worse: 5, geomean: 1.042
total diff: 59

Slower diff/base Base Median (ns) Diff Median (ns) Modality
System.Text.RegularExpressions.Tests.Perf_Regex_Industry_BoostDocs_Simple.IsMatc 1.06 133.03 141.47
System.Text.RegularExpressions.Tests.Perf_Regex_Industry_BoostDocs_Simple.IsMatc 1.06 134.98 142.81
System.Text.RegularExpressions.Tests.Perf_Regex_Industry_Leipzig.Count(Pattern: 1.04 2494026.73 2586967.73
System.Text.RegularExpressions.Tests.Perf_Regex_Industry_BoostDocs_Simple.IsMatc 1.03 621.96 641.37
System.Text.RegularExpressions.Tests.Perf_Regex_Industry_Leipzig.Count(Pattern: 1.02 21974218.18 22465754.55 several?
Faster base/diff Base Median (ns) Diff Median (ns) Modality
System.Text.RegularExpressions.Tests.Perf_Regex_Industry_RustLang_Sherlock.Count 2.80 15286261.76 5455385.37
System.Text.RegularExpressions.Tests.Perf_Regex_Industry_RustLang_Sherlock.Count 2.63 10022464.58 3808660.00
System.Text.RegularExpressions.Tests.Perf_Regex_Industry_Mariomkas.Count(Pattern 2.29 6771215.28 2952460.47
System.Text.RegularExpressions.Tests.Perf_Regex_Industry_RustLang_Sherlock.Count 2.14 12170225.00 5693354.17
System.Text.RegularExpressions.Tests.Perf_Regex_Industry_RustLang_Sherlock.Count 2.12 10788114.58 5086388.46
System.Text.RegularExpressions.Tests.Perf_Regex_Industry_RustLang_Sherlock.Count 1.93 4073781.25 2110730.00
System.Text.RegularExpressions.Tests.Perf_Regex_Industry_SliceSlice.Count(Option 1.91 1099632850.00 575289350.00
System.Text.RegularExpressions.Tests.Perf_Regex_Industry_RustLang_Sherlock.Count 1.89 4577136.36 2422383.65
System.Text.RegularExpressions.Tests.Perf_Regex_Industry_RustLang_Sherlock.Count 1.88 159116.57 84593.72
System.Text.RegularExpressions.Tests.Perf_Regex_Industry_RustLang_Sherlock.Count 1.87 8657798.33 4639441.18
System.Text.RegularExpressions.Tests.Perf_Regex_Industry_RustLang_Sherlock.Count 1.80 1961271.43 1087434.02
System.Text.RegularExpressions.Tests.Perf_Regex_Industry_RustLang_Sherlock.Count 1.80 199215.34 110475.42
System.Text.RegularExpressions.Tests.Perf_Regex_Industry_RustLang_Sherlock.Count 1.75 23110975.00 13200073.68
System.Text.RegularExpressions.Tests.Perf_Regex_Industry_RustLang_Sherlock.Count 1.67 1033139.24 619551.44 bimodal
System.Text.RegularExpressions.Tests.Perf_Regex_Industry_RustLang_Sherlock.Count 1.65 88111.27 53410.99
System.Text.RegularExpressions.Tests.Perf_Regex_Industry_RustLang_Sherlock.Count 1.62 3377971.43 2081657.14 bimodal
System.Text.RegularExpressions.Tests.Perf_Regex_Industry_RustLang_Sherlock.Count 1.51 204743.55 135847.72
System.Text.RegularExpressions.Tests.Perf_Regex_Industry_RustLang_Sherlock.Count 1.49 40920612.50 27464887.50
System.Text.RegularExpressions.Tests.Perf_Regex_Industry_RustLang_Sherlock.Count 1.46 86482.44 59256.18
System.Text.RegularExpressions.Tests.Perf_Regex_Industry_RustLang_Sherlock.Count 1.46 2540423.68 1744844.14 several?
System.Text.RegularExpressions.Tests.Perf_Regex_Industry_RustLang_Sherlock.Count 1.45 53884.39 37107.66 several?
System.Text.RegularExpressions.Tests.Perf_Regex_Industry_RustLang_Sherlock.Count 1.45 40560162.50 28054600.00
System.Text.RegularExpressions.Tests.Perf_Regex_Industry_Leipzig.Count(Pattern: 1.42 67197850.00 47471937.50 several?
System.Text.RegularExpressions.Tests.Perf_Regex_Industry_RustLang_Sherlock.Count 1.38 130572.40 94735.13 several?
System.Text.RegularExpressions.Tests.Perf_Regex_Industry_RustLang_Sherlock.Count 1.37 66087.55 48179.84 bimodal
System.Text.RegularExpressions.Tests.Perf_Regex_Industry_BoostDocs_Simple.IsMatc 1.37 206.84 150.89
System.Text.RegularExpressions.Tests.Perf_Regex_Industry_RustLang_Sherlock.Count 1.36 3037911.41 2239705.17 several?
System.Text.RegularExpressions.Tests.Perf_Regex_Industry_RustLang_Sherlock.Count 1.34 68187.37 51054.16
System.Text.RegularExpressions.Tests.Perf_Regex_Industry_RustLang_Sherlock.Count 1.31 177346.23 134953.41 bimodal
System.Text.RegularExpressions.Tests.Perf_Regex_Industry_BoostDocs_Simple.IsMatc 1.30 242.66 187.25
System.Text.RegularExpressions.Tests.Perf_Regex_Industry_RustLang_Sherlock.Count 1.29 1523287.44 1180916.82
System.Text.RegularExpressions.Tests.Perf_Regex_Industry_RustLang_Sherlock.Count 1.28 76250.69 59610.21
System.Text.RegularExpressions.Tests.Perf_Regex_Industry_Leipzig.Count(Pattern: 1.28 67257150.00 52701412.50
System.Text.RegularExpressions.Tests.Perf_Regex_Industry_RustLang_Sherlock.Count 1.27 48508.51 38088.60
System.Text.RegularExpressions.Tests.Perf_Regex_Industry_BoostDocs_Simple.IsMatc 1.26 181.44 143.67
System.Text.RegularExpressions.Tests.Perf_Regex_Industry_RustLang_Sherlock.Count 1.26 211902.74 168012.23
System.Text.RegularExpressions.Tests.Perf_Regex_Industry_RustLang_Sherlock.Count 1.26 64672.47 51279.66
System.Text.RegularExpressions.Tests.Perf_Regex_Industry_RustLang_Sherlock.Count 1.24 59747.35 48143.89
System.Text.RegularExpressions.Tests.Perf_Regex_Industry_BoostDocs_Simple.IsMatc 1.11 101.74 91.25
System.Text.RegularExpressions.Tests.Perf_Regex_Industry_Leipzig.Count(Pattern: 1.11 2899242.35 2603272.22
System.Text.RegularExpressions.Tests.Perf_Regex_Industry_BoostDocs_Simple.IsMatc 1.10 123.15 111.45
System.Text.RegularExpressions.Tests.Perf_Regex_Industry_BoostDocs_Simple.IsMatc 1.10 112.40 102.38
System.Text.RegularExpressions.Tests.Perf_Regex_Industry_BoostDocs_Simple.IsMatc 1.10 114.45 104.46 several?
System.Text.RegularExpressions.Tests.Perf_Regex_Industry_BoostDocs_Simple.IsMatc 1.09 106.23 97.12
System.Text.RegularExpressions.Tests.Perf_Regex_Industry_Leipzig.Count(Pattern: 1.08 2657380.00 2456057.14
System.Text.RegularExpressions.Tests.Perf_Regex_Industry_BoostDocs_Simple.IsMatc 1.08 161.55 149.36
System.Text.RegularExpressions.Tests.Perf_Regex_Industry_Mariomkas.Count(Pattern 1.08 638071.50 593200.89
System.Text.RegularExpressions.Tests.Perf_Regex_Industry_Mariomkas.Count(Pattern 1.07 13209031.58 12390294.74
System.Text.RegularExpressions.Tests.Perf_Regex_Industry_Leipzig.Count(Pattern: 1.05 15942657.14 15209585.71
System.Text.RegularExpressions.Tests.Perf_Regex_Industry_Leipzig.Count(Pattern: 1.04 9628288.00 9238760.00
System.Text.RegularExpressions.Tests.Perf_Regex_Industry_RustLang_Sherlock.Count 1.04 984012.25 948066.73
System.Text.RegularExpressions.Tests.Perf_Regex_Industry_BoostDocs_Simple.IsMatc 1.04 83.28 80.34 several?
System.Text.RegularExpressions.Tests.Perf_Regex_Industry_RustLang_Sherlock.Count 1.03 102579.62 99430.16
System.Text.RegularExpressions.Tests.Perf_Regex_Industry_Leipzig.Count(Pattern: 1.03 4785248.08 4657136.11
Author: olsaarik
Assignees: olsaarik
Labels:

area-System.Text.RegularExpressions

Milestone: -

@olsaarik olsaarik marked this pull request as ready for review June 3, 2022 22:47
Copy link
Member

@stephentoub stephentoub left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Those improvements look great. Do we know why the cited regressions regress?

if (nextState is not null || builder.TryCreateNewTransition(dfaMatchingState, mintermId, dfaOffset, checkThreshold: true, out nextState))
int dfaOffset = (state.DfaStateId << builder._mintermsLog) | mintermId;
int nextStateId = builder._delta[dfaOffset];
if (nextStateId > 0 || builder.TryCreateNewTransition(state.DfaStateId, mintermId, dfaOffset, checkThreshold: true, out nextStateId))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like the only call site for the wrapper TryCreateNewTransition that was added. Any reason this can't instead just be:

if (nextStateId > 0 || builder.TryCreateNewTransition(state.DfaState!, mintermId, dfaOffset, checkThreshold: true, out DfaMatchingState<TSet> newState))
{
    state.DfaStateId = newState.Id;
    return true;
}

rather than having that extra wrapper?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right, this is cleaner.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, now I remember why I added it: having a wrapper for the state ID allowed a single if body here to handle both an existing transition and a new one. I'll still remove the wrapper, since expanding the code here makes the different cases a bit more apparent anyway.

@@ -367,7 +367,7 @@ public SymbolicMatch FindMatch(RegexRunnerMode mode, ReadOnlySpan<char> input, i
Debug.Assert(matchEnd >= startat - 1);
matchStart = matchEnd < startat ?
startat :
FindStartPosition(input, matchEnd, matchStartLowBoundary, perThreadData);
SpecializedFindStartPosition(input, matchEnd, matchStartLowBoundary, perThreadData);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does "Specialized" mean? Specialized for or to do what?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's essentially "curry with preferred generic type parameter and invoke", but I saw your other suggestion on getting rid of these functions and that does look cleaner.

Comment on lines 388 to 399
[MethodImpl(MethodImplOptions.AggressiveInlining)]
int SpecializedFindEndPosition(ReadOnlySpan<char> input, int pos, long timeoutOccursAt, RegexRunnerMode mode, out int initialStatePos, out int matchLength, PerThreadData perThreadData) =>
_findOpts is null ?
SpecializedFindEndPosition2<NoOptimizationsInitialStateHandler>(input, pos, timeoutOccursAt, mode, out initialStatePos, out matchLength, perThreadData) :
SpecializedFindEndPosition2<InitialStateFindOptimizationsHandler>(input, pos, timeoutOccursAt, mode, out initialStatePos, out matchLength, perThreadData);

[MethodImpl(MethodImplOptions.AggressiveInlining)]
int SpecializedFindEndPosition2<TFindOptimizationsHandler>(ReadOnlySpan<char> input, int pos, long timeoutOccursAt, RegexRunnerMode mode, out int initialStatePos, out int matchLength, PerThreadData perThreadData)
where TFindOptimizationsHandler : struct, IInitialStateHandler =>
_pattern._info.ContainsSomeAnchor ?
FindEndPosition<TFindOptimizationsHandler, FullNullabilityHandler>(input, pos, timeoutOccursAt, mode, out initialStatePos, out matchLength, perThreadData) :
FindEndPosition<TFindOptimizationsHandler, NoAnchorsNullabilityHandler>(input, pos, timeoutOccursAt, mode, out initialStatePos, out matchLength, perThreadData);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's only a single call site for this, these are both aggressively inlined, and the naming with both the "Specialized" prefix and "2" suffix is... suspect :) Can the single call site just be changed to:

int matchEnd = (_findOpts is null, _pattern._info.ContainsSomeAnchor) switch
{
    (false, false) => FindEndPosition<NoOptimizationsInitialStateHandler, NoAnchorsNullabilityHandler>(input, pos, timeoutOccursAt, mode, out initialStatePos, out matchLength, perThreadData),
    (true, false) => FindEndPosition<InitialStateFindOptimizationsHandler, NoAnchorsNullabilityHandler>(input, pos, timeoutOccursAt, mode, out initialStatePos, out matchLength, perThreadData),
    (false, true) => FindEndPosition<NoOptimizationsInitialStateHandler, FullNullabilityHandler>(input, pos, timeoutOccursAt, mode, out initialStatePos, out matchLength, perThreadData),
    (true, true) => FindEndPosition<InitialStateFindOptimizationsHandler, FullNullabilityHandler>(input, pos, timeoutOccursAt, mode, out initialStatePos, out matchLength, perThreadData),
};

FindEndPosition<TFindOptimizationsHandler, NoAnchorsNullabilityHandler>(input, pos, timeoutOccursAt, mode, out initialStatePos, out matchLength, perThreadData);

[MethodImpl(MethodImplOptions.AggressiveInlining)]
int SpecializedFindStartPosition(ReadOnlySpan<char> input, int i, int matchStartBoundary, PerThreadData perThreadData) =>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto here... can this just be manually inlined into the single call site?

matchStart =
    matchEnd < startat ? startat :
    _pattern._info.ContainsSomeAnchor ? FindStartPosition<FullNullabilityHandler>(input, i, matchStartBoundary, perThreadData) :
    FindStartPosition<NoAnchorsNullabilityHandler>(input, i, matchStartBoundary, perThreadData);

// state processing logic and optimizations), or failed to transition (which should only happen if we were in DFA mode and
// need to switch over to NFA mode). If we exited because we hit an initial state, find result will be 0, otherwise -1.
if (findResult < 0)
else
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why add the else block? Seems cleaner to just remove the else and unindent all of its contents.

try
{
// Loop through each character in the input, transitioning from state to state for each.
while (true)
{
var (isInitial, isDeadend, isNullable, canBeNullable) = TStateHandler.GetStateInfo(builder, ref state);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
var (isInitial, isDeadend, isNullable, canBeNullable) = TStateHandler.GetStateInfo(builder, ref state);
(bool isInitial, bool isDeadend, bool isNullable, bool canBeNullable) = TStateHandler.GetStateInfo(builder, ref state);

@@ -638,23 +649,29 @@ private bool FindStartPositionDeltas<TStateHandler>(SymbolicRegexBuilder<TSet> b
// Loop backwards through each character in the input, transitioning from state to state for each.
while (true)
{
var (isInitial, isDeadend, isNullable, canBeNullable) = TStateHandler.GetStateInfo(builder, ref state);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
var (isInitial, isDeadend, isNullable, canBeNullable) = TStateHandler.GetStateInfo(builder, ref state);
(bool isInitial, bool isDeadend, bool isNullable, bool canBeNullable) = TStateHandler.GetStateInfo(builder, ref state);

@olsaarik
Copy link
Contributor Author

olsaarik commented Jun 6, 2022

@stephentoub

Those improvements look great. Do we know why the cited regressions regress?

I don't think the regressions I cited actually do regress, but rather my laptop isn't stable enough as a benchmarking machine. I'll benchmark again before merging to make sure.

Copy link
Member

@stephentoub stephentoub left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, @olsaarik. Looks good. Left a few style comments, but I'm going to merge this to unblock subsequent changes and to get the regressions addressed.

{
return coreState.FixedLength(nextCharKind);
}
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
}
}

@DrewScoggins
Copy link
Member

Improvements on Arm64 Windows - dotnet/perf-autofiling-issues#6176

@EgorBo
Copy link
Member

EgorBo commented Jun 28, 2022

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants