Performance improvement in Count<T> extension

[Sergio0694]

PR Type

What kind of change does this PR introduce?

• Performance improvement

What is the new behavior?

About 20% improvement on .NET 5 when working on char types (or larger):

This was done by adding an unrolled loop for the vectorized path of the SIMD accelerated version of Count<T>.

PR Checklist

Please check if your PR fulfills the following requirements:

• Tested code with current supported SDKs
• Pull Request has been submitted to the documentation repository instructions. Link:
• Sample in sample app has been added / updated (for bug fixes / features)
  • Icon has been created (if new sample) following the Thumbnail Style Guide and templates
• Tests for the changes have been added (for bug fixes / features) (if applicable)
• Header has been added to all new source files (run build/UpdateHeaders.bat)
• Contains NO breaking changes

[ghost]

Thanks Sergio0694 for opening a Pull Request! The reviewers will test the PR and highlight if there is any conflict or changes required. If the PR is approved we will proceed to merge the pull request 🙌

[Rosuavio]

I like optimization, but could the code be wirten in a more maintainable way so that RyuJit would unroll it for us? There seems to be some ongoing discussion (dotnet/runtime#8107) on expanding loop unrolling optimizations.

[michael-hawker]

Thanks @RosarioPulella! @Sergio0694 would it make sense to find/file dotnet issues for any hand-optimizations like this you're doing that could maybe be improved in the compiler?

At least then we could mark out the section of code and link to the issue so we could remove it later when it gets optimized in the runtime?

[Sergio0694]

Oh, whoops, forgot to reply to this, I'm sorry! 😅

@RosarioPulella The discussion is ongoing but as you can see it's still planned for future (not even .NET 6), and furthermore this case is particularly tricky because the JIT would need to unroll a loop that itself is also already using vectorized instructions, which adds another layer of complexity on top for it to handle. I would argue this is worth it for a few reasons:

• Regardless of when the runtime will get support for this, this is a clean ~20% perf improvement we can have today.
• It doesn't really add much complexity to the rest of the codebase, or that class in particular.
• We only need this to trigger for specific T instantiations, so doing this manually gives us better control anyway. As in, even if that was supported I wouldn't want the JIT to just always unroll this, as when T is a sbyte that loop would not help at all (I've profiled that to verify that, that's why I've added that check to exclude that type and only for types >= 2 bytes in size).
• Since we're targeting multiple platforms, I'd argue it makes sense to include as many optimizations as we can upstream, so that consumers will be able to benefit from them on all platforms. I'm doing the same elsewhere too, so that devs not using .NET 5 will not be penalized (eg. by them not having access to specific JIT intrinsics).

@michael-hawker I've already opened a few issues related to things I'm doing in the HighPerformance package (or rather, things I discovered while working on it), I can definitely start also linking to issues from comments in the future 😊

[Rosuavio]

@Sergio0694 You make some really good points and I see the value in these changes.

[Sergio0694]

@RosarioPulella Awesome! Glad we all agree - and those were some absolutely valid questions! 😄

[ghost]

Hello @michael-hawker!

Because this pull request has the auto merge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.