Intrinsicify SpanHelpers.IndexOfAny(char,char)

[benaadams]

Updated Intrinsicify SpanHelpers.IndexOfAny(char,char) dotnet/coreclr#22877

Resolves: #12094

[gfoidl]

👍 thanks for the comment (cf. #11703).

[danmoseley]

Regex uses IndexOfAny(char, char) and IndexOfAny(char, char, char) so would presumably benefit from this.

[danmoseley]

Same request for perf numbers please - thanks!

[stephentoub]

Regex uses IndexOfAny(char, char) and IndexOfAny(char, char, char) so would presumably benefit from this.

Ideally we'd test this specifically with regex redux, which spends something like 30% of the time now inside of IndexOfAny(char, char). While it'd be great if it made it faster, I also want to confirm it doesn't make it slower. If memory serves, it was spending a lot of its time in the non-vectorized portion of the method.
#25023

[benaadams]

Ideally we'd test this specifically with regex redux, which ...

@stephentoub is that in the performance repo?

[danmoseley]

https://benchmarksgame-team.pages.debian.net/benchmarksgame/program/regexredux-csharpcore-5.html

https://github.com/dotnet/performance/blob/master/src/benchmarks/micro/runtime/BenchmarksGame/regex-redux-5.cs

[benaadams]

Dealing with the alignment loses most of the benefit, so have updated it to match the approach I took for SequenceCompareTo where if it goes vector it does vector for everything (tried a bunch of variations, aligning with loop, unaligned vector first read => aligned vectors etc), however this worked best

+10%

| Faster                                                  | base/diff | Base Median (ns) | Diff Median (ns) | Modality|
| ------------------------------------------------------- | ---------:| ----------------:| ----------------:| ------- |
| BenchmarksGame.RegexRedux_1.RunBench                    |      1.11 |      45419250.00 |      41004791.67 |         |
| BenchmarksGame.RegexRedux_5.RunBench(options: Compiled) |      1.10 |       8419890.74 |       7639460.29 | bimodal |
| BenchmarksGame.RegexRedux_5.RunBench(options: None)     |      1.02 |      25805255.56 |      25258357.14 |         |

[danmoseley]

10% is significant on that one. This might be worth getting creative to try to get into this release.

@benaadams was that with IndexOfAny(char, char, char) changes as well? I would expect that to be relevant also: https://github.com/dotnet/performance/blob/master/src/benchmarks/micro/runtime/BenchmarksGame/regex-redux-5.cs#L60-L68

[benaadams]

Just noticed that's with deleting IndexOfAny(char, char, char) to it uses the very slow path... 😅

[benaadams]

Let me put them all in one as they are basically identical methods

[danmoseley]

All your IndexOf PR's into one? That sounds like a good idea, thanks.

[danmoseley]

Your perf numbers above demonstrate that the interpreter doesn't use IndexOfAny at all. @stephentoub had this listed in #1349 and it's checked off so maybe he tried it and it didn't give a win somehow.

[stephentoub]

I probably checked it off in error, or having employed IndexOf but not IndexOfAny. There's a TODO in the code for the IndexOfAny (and some other related optimizations):

runtime/src/libraries/System.Text.RegularExpressions/src/System/Text/RegularExpressions/RegexInterpreter.cs

Lines 444 to 450 in 19a4297

	// TODO https://github.com/dotnet/runtime/issues/1349:
	// LeadingCharClasses may contain multiple sets, one for each of the first N characters in the expression,
	// but the interpreter currently only uses the first set for the first character. In fact, we currently
	// only run the analysis that can produce multiple sets if RegexOptions.Compiled was set.
	string set = _code.LeadingCharClasses[0].CharClass;
	if (RegexCharClass.IsSingleton(set))

I don't think I ever got around to evaluating the trade-off that would be required here, the overhead of extracting from the char class the 2 or 3 characters that could be used with IndexOfAny and then using IndexOfAny.

[danmoseley]

OK, I un-checked it for some future day. Presumably the value could only be higher in the interpreter, since the runtime is larger.

[stephentoub]

Presumably the value could only be higher in the interpreter, since the runtime is larger.

Not necessarily. With the compiler we get to compute the right thing to do once, and there's then no overhead at execution time, since the choice to use IndexOfAny and the arguments to it are all pre-computed. With the interpreter, we either need to redetermine that every time we invoke FindFirstChar, or we need to compute it once and store that data away somewhere, then check whether we have it and use it. It would certainly be beneficial when the things being searched for are few and far between, but if the IndexOfAny is used to search for something that frequently presents itself very quickly, it might be a net loss. Someone would need to do the work and then see whether it's fruitful on a variety of benchmarks.

[benaadams]

Superseded by #40729