Vectorize ProbabilisticMap.IndexOfAny#80963
Conversation
|
Tagging subscribers to this area: @dotnet/area-system-memory Issue Details
Benchmark sourcepublic class ReplaceLineEndingsBenchmark
{
private string _input;
[Params(0.0, 0.05, 0.1, 0.2, 1.0)]
public double NewLineFrequency;
[Params(10_000)]
public int Length;
[GlobalSetup]
public void Setup()
{
char[] input = new char[Length];
var rng = new Random(42);
for (int i = 0; i < input.Length; i++)
{
if (rng.NextDouble() < NewLineFrequency)
{
input[i] = '\n';
}
else
{
char c;
do
{
c = (char)rng.Next(0, 65536);
}
while ("\r\n\f\u0085\u2028\u2029".Contains(c));
input[i] = c;
}
}
_input = new string(input);
}
[Benchmark]
public string ReplaceLineEndings() => _input.ReplaceLineEndings();
}If we care, we could also do this for I don't know if it's possible to do something similar efficiently on ARM given that the bloom filter in this case is 256-bit.
|
|
/azp run runtime-libraries-coreclr outerloop |
|
Azure Pipelines successfully started running 1 pipeline(s). |
src/libraries/System.Private.CoreLib/src/System/IndexOfAnyValues/ProbabilisticMap.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Private.CoreLib/src/System/IndexOfAnyValues/ProbabilisticMap.cs
Outdated
Show resolved
Hide resolved
f96031d to
9af50f6
Compare
|
Added |
9af50f6 to
9c67c08
Compare
9c67c08 to
399770b
Compare
399770b to
8761d76
Compare
|
@radekdoulik we should add PackedSimd versions of the Unsafe* functions wasm supports to ease handling all the calling callers. I'm not sure we have a great pattern to simplify the *.IsHardwareAccelerated paths that works for all the cases yet but that is worth considering too. |
|
Any thoughts on this @dotnet/area-system-memory? |
src/libraries/System.Private.CoreLib/src/System/IndexOfAnyValues/ProbabilisticMap.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Private.CoreLib/src/System/IndexOfAnyValues/ProbabilisticMap.cs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
These are different because shifting via byte isn't accelerated on xarch, right?
There was a problem hiding this comment.
Yes. We do the same thing in IndexOfAnyAsciiSearcher as well, where that also feeds into a shuffle
There was a problem hiding this comment.
👍, probably worth opening an issue so an efficient implementation can be done and we can avoid the separate paths here.
src/libraries/System.Private.CoreLib/src/System/IndexOfAnyValues/ProbabilisticMap.cs
Outdated
Show resolved
Hide resolved
|
Anything else that should be changed here, or is this one good to merge @tannergooding? |
|
@tannergooding can this be merged? (I'm trying to avoid more merge conflicts as this was already rebased a fair number of times) |
|
Another merge conflict popped up. Would be good to see some more benchmark numbers to better display where the cutoff is for the index ratio. Likely also needs a secondary review from someone like @stephentoub given the code its touching. |
8482f56 to
a753250
Compare
src/libraries/System.Private.CoreLib/src/System/IndexOfAnyValues/ProbabilisticMap.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Private.CoreLib/src/System/Runtime/Intrinsics/Vector128.cs
Show resolved
Hide resolved
|
Thanks! |
AVX2
ARM64
Benchmark source
If we care, we could also do this for
LastIndexOfAny.#80297 could be interesting for ARM if we could do a 256-bit lookup instead of blending together smaller lookups.