Enable Regex to use SearchValues<string> in compiled / source generator TryFindNextStartingPosition #88400
Conversation
ghost
assigned 
|
Tagging subscribers to this area: @dotnet/area-system-text-regularexpressions |
| // final length of the string, so keeping this small helps avoid problematic complexity. | ||
| const int MaxPrefixLength = 8; | ||
|
|
||
| // Limit based on what IndexOfAny is able to handle most efficiently. This can be updated in future if/when IndexOfAny improves further. |
There was a problem hiding this comment.
Should this point to the private constant in IndexOfAny that defines the value so that it aids in keeping the two values in sync?
There was a problem hiding this comment.
Which constant are you referring to?
There was a problem hiding this comment.
Sorry, so long since I reviewed this and now I'm trying to page back what I was thinking and can't recall. Should be fine to ignore, I believe I was just trying to suggest to add a pointer here to where that limit is set.
...ies/System.Text.RegularExpressions/src/System/Text/RegularExpressions/RegexPrefixAnalyzer.cs
Show resolved
Hide resolved
...libraries/System.Text.RegularExpressions/src/System/Text/RegularExpressions/RegexCompiler.cs
Show resolved
Hide resolved
There was a problem hiding this comment.
Very minor comments, but this LGTM.
Do we plan to add a benchmark for cases similar to the ones you are adding tests for? Also, if you have, can you share how this impacts the benchmarks we already have? I'm assuming with this we might be faster in some cases while having a slightly bigger memory footprint(for all arrays, strings, etc being allocated)?
stephentoub
force-pushed
the
regexteddy
branch
from
9f0a16b to
e421750
Compare
.../System.Text.RegularExpressions/src/System/Text/RegularExpressions/RegexFindOptimizations.cs
Outdated
Show resolved
Hide resolved
…or TryFindNextStartingPosition The analyzer determines a set of prefixes that can start any match, and then uses SearchValues with IndexOfAny to find the next one from that set.
stephentoub
force-pushed
the
regexteddy
branch
from
e421750 to
40298bd
Compare
|
@stephentoub is this PR blocked with something? Looks having a test failure. |
There are some regressions to be investigated and fixed before it can be merged. I will close it for now and revisit it soon. |
Depends on #88394
Fixes #85693
The analyzer determines a set of prefixes that can start any match, and then uses SearchValues with IndexOfAny to find the next one from that set.