Vectorized HttpCharacters #44041
Vectorized HttpCharacters #44041
Conversation
ghost
added
the
community-contribution
|
Thanks for your PR, @gfoidl. Someone from the team will get assigned to your PR shortly and we'll get it reviewed. |
|
|
||
| namespace Microsoft.AspNetCore.Http; | ||
|
|
||
| internal static class HttpCharacters | ||
| [SkipLocalsInit] | ||
| internal static unsafe partial class HttpCharacters |
There was a problem hiding this comment.
I developed this with a playground-project in https://github.com/gfoidl-Tests/SourceGenerator-Vectors
For best codegen, a code-generator is used (in the project it's still the non-incremental one, I didn't update that code).
Methods in the generator do "the same" as was done in the static init before.
Thus there's no need for static initialization at runtime, instead
ROS<byte>-- will refer to the assembly's static data segment -- can be used- for vectors it's best to use constants like
Vector128.Create((byte)0xF)inline instead of loading the fromstatic readonlyfields
In this draft-PR I just copied the output from the source-generator over to here (see file below).
What's the right strategy to generate such a file?
A source-generator, tt-file, etc. Please advise so I can update the PR here to make it suite.
|
|
||
| namespace Microsoft.AspNetCore.Http; | ||
|
|
||
| [CompilerGenerated] |
There was a problem hiding this comment.
See comment above -- right now this is just copy & paste (+ little editing) from my playgroud-project.
gfoidl
force-pushed
the
httpcharacters_vectorized
branch
from
a940f4a
to
903c988
Compare
|
I'll close this PR, as I'm pretty sure with dotnet/runtime#68328 we get a proper building block and the wire-up should become trivial. Only the "extended case" isn't covered by the runtime-issue. In the worst case it remains the scalar-approach as it's now, so no regression should happen, but also no improvement. |
|
Hi @gfoidl. It looks like you just commented on a closed PR. The team will most probably miss it. If you'd like to bring something important up to their attention, consider filing a new issue and add enough details to build context. |
|
dotnet/runtime#68328 is done 🎉, so in the next days I'll create a PR based on these new APIs. |
|
Hi @gfoidl. It looks like you just commented on a closed PR. The team will most probably miss it. If you'd like to bring something important up to their attention, consider filing a new issue and add enough details to build context. |
amcasey
added
area-networking
Note: after I created this PR I got pushed towards dotnet/runtime#68328, with which I think this can be solved in a more general runtime-like way. I don't know the outcome from that issue (yet), so please lets hold on with this PR in the meantime. Should we close it instead and eventually re-open if the mentioned issue doesn't fit?
At least the the "extended" case a special handling is needed -- so if the issue has a usable solution, this PR can be trimmed down if the "extended" case is worth it (which I doubt, as it's seldom?).
As outlined in #33776 (comment) these methods are vectorized now.
Vectorization is not a plain win, there are trade-offs due to a bit more overhead. In the numbers belows this can be seen quite good. There seems to be a pattern were perf regresses:
In these cases a scalar-loop is faster, as it has less work to do in comparison to vectorized code.
But the longer the input, the farther away from the beginning the invalid char is or if there's no invalid char at all, vectorization shows nice improvements.
I expect that in real-life most of the input is valid, thus no invalid char is found, as well as inputs are long enough for vectorization to show it's goodness.
If my assumption is wrong, at least for specific method, we could disable vectorizaiton on these.
For a description how the approach works, please see comments in code.