-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
Light up IndexOfAnyAsciiSearcher for AVX512 #93222
Comments
Tagging subscribers to this area: @dotnet/area-system-runtime Issue DetailsThe implementation that backs many IndexOfAny searches with
|
For reference, the implementation could look something like this: MihaZupan/runtime@searchvalues-state...MihaZupan:runtime:searchvalues-avx512 (might need some cleanup after #91937). With that implementation, I was seeing higher throughput for longer values as expected (about ~1.65x), but noticeably more overhead for early matches (regressed more regex cases than it improved). |
Hi @stephentoub @MihaZupan From previous conversation,
As there is already a prototype, I would like to know if there is any plan to submit a PR based on the existing works. Or, if no, will it be okay for us to take the prototype and continue the work to form a PR? |
Hey @Ruihan-Yin, I've pushed an updated version of my change that compiles again to MihaZupan/runtime@main...MihaZupan:runtime:searchvalues-ascii-avx512. Benchmark results
I avoided opening a PR as the regressions for early matches are quite noticeable. |
Hi @MihaZupan , thanks for the update!
|
The benchmark numbers above are for this: public class IndexOfAnyAsciiBenchmark
{
private static readonly SearchValues<char> s_chars = SearchValues.Create("abcABC123");
private string _charBuffer;
private string _charBufferExcept;
[Params(8, 16, 32, 64, 128, 1000, 10_000)]
public int Length;
[Params(true, false)]
public bool MatchAtStart;
[GlobalSetup]
public void Setup()
{
var charBuffer = new char[Length];
var charBufferExcept = new char[Length];
charBuffer.AsSpan().Fill('\n');
charBufferExcept.AsSpan().Fill('b');
if (MatchAtStart)
{
charBuffer[0] = 'b';
charBufferExcept[0] = '\n';
}
_charBuffer = new string(charBuffer);
_charBufferExcept = new string(charBufferExcept);
}
[Benchmark]
public int IndexOfAny() => _charBuffer.AsSpan().IndexOfAny(s_chars);
[Benchmark]
public int IndexOfAnyExcept() => _charBufferExcept.AsSpan().IndexOfAnyExcept(s_chars);
}
I haven't looked at this under a profiler, but I can get the codegen for you if that'd help. |
Thanks for sharing! |
Example diffs for the
Part of the issue here is how the search logic is structured. Essentially it looks like this in if (length < 8)
{
// Scalar loop
return NotFound;
}
if (length > 16)
{
if (length > 32)
{
// Process the input in chunks of 32 characters (Vector256)
}
// Process the last [1-32] characters by loading two overlapping vectors (Vector256)
return NotFound;
}
// Process the [8-16] characters by loading two overlapping vectors (Vector128)
return NotFound; and like this in if (length < 8)
{
// Scalar loop
return NotFound;
}
if (length > 16)
{
if (length > 32)
{
if (length > 64)
{
// Process the input in chunks of 64 characters (Vector512)
}
// Process the last [1-64] characters by loading two overlapping vectors (Vector512)
return NotFound;
}
// Process the last [16-32] characters by loading two overlapping vectors (Vector256)
return NotFound;
}
// Process the [8-16] characters by loading two overlapping vectors (Vector128)
return NotFound; For the |
(sorry about the spam) With a small tweak, the results are much closer (varies quite a bit between runs, but still).
I'll test how this impacts the Regex benchmarks I initially mentioned. |
Reran the numbers with main...MihaZupan:runtime:searchvalues-ascii-avx512-4 Basic IndexOfAny benchmarks
Sherlock Regex benchmarks
|
@MihaZupan, we are reaching preview 7 snap soon. Do you plan to finsih this work for .NET 9? |
Change is being reverted in #104688 |
The implementation that backs many IndexOfAny searches with
SearchValues
today only leverages Vector128/256:https://github.com/dotnet/runtime/blob/a5461cbc8ed20e0981fd4e846a180f35b07dcc0a/src/libraries/System.Private.CoreLib/src/System/SearchValues/IndexOfAnyAsciiSearcher.cs
We should teach it about Vector512.
(@MihaZupan has a prototype.)
The text was updated successfully, but these errors were encountered: