Skip to content
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

Add .NET Core 2.1 and 3.0 perf improvements #19

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

brantburnett
Copy link

The addition of Span in .NET Core 2.1 can offer some performance
improvements moving through the array in SafeProxy by reducing the
number of arithmetic operations.

.NET Core 3.0 also adds Span based overloads to HashAlgorithm
which can further improve performance if explicitly supported. If not
supported, any requests to the Span overloads are copied to an
array before processing.

A BenchmarkDotNet project was also added to assist with benchmarking.

Test results across several target frameworks comparing the pre and post
change performance against a 65536 byte array. These metrics are for
calls in via the array overloads, not the Span overloads. They
show an approximately 25% reduction in runtime on .NET Core 2.1 and 3.1.

Method Runtime Size Mean Error StdDev Ratio Rank
Array .NET 4.6.1 65536 48.08 us 0.192 us 0.170 us 1.00 1
Span .NET 4.6.1 65536 47.87 us 0.169 us 0.150 us 1.00 1
Array .NET Core 2.1 65536 48.99 us 0.260 us 0.217 us 1.00 2
Span .NET Core 2.1 65536 37.02 us 0.261 us 0.218 us 0.76 1
Array .NET Core 3.1 65536 50.01 us 0.335 us 0.297 us 1.00 2
Span .NET Core 3.1 65536 37.04 us 0.218 us 0.204 us 0.74 1

The addition of Span<T> in .NET Core 2.1 can offer some performance
improvements moving through the array in SafeProxy by reducing the
number of arithmetic operations.

.NET Core 3.0 also adds Span<byte> based overloads to HashAlgorithm
which can further improve performance if explicitly supported. If not
supported, any requests to the Span<byte> overloads are copied to an
array before processing.

A BenchmarkDotNet project was also added to assist with benchmarking.

Test results across several target frameworks comparing the pre and post
change performance against a 65536 byte array. These metrics are for
calls in via the array overloads, not the Span<byte> overloads. They
show an approximately 25% reduction in runtime on .NET Core 2.1 and 3.1.

| Method |       Runtime |  Size |     Mean |    Error |   StdDev | Ratio | Rank |
|------- |-------------- |------ |---------:|---------:|---------:|------:|-----:|
|  Array |    .NET 4.6.1 | 65536 | 48.08 us | 0.192 us | 0.170 us |  1.00 |    1 |
|   Span |    .NET 4.6.1 | 65536 | 47.87 us | 0.169 us | 0.150 us |  1.00 |    1 |
|        |               |       |          |          |          |       |      |
|  Array | .NET Core 2.1 | 65536 | 48.99 us | 0.260 us | 0.217 us |  1.00 |    2 |
|   Span | .NET Core 2.1 | 65536 | 37.02 us | 0.261 us | 0.218 us |  0.76 |    1 |
|        |               |       |          |          |          |       |      |
|  Array | .NET Core 3.1 | 65536 | 50.01 us | 0.335 us | 0.297 us |  1.00 |    2 |
|   Span | .NET Core 3.1 | 65536 | 37.04 us | 0.218 us | 0.204 us |  0.74 |    1 |
@Skyppid
Copy link

Skyppid commented Nov 10, 2020

Thanks for adding this as it has long been requested in #11. Kinda sad tho' that it's not really maintained anymore, I'd love to just use the nuget package instead of having to compile it myself.

@force-net
Copy link
Owner

Thanks for PR. I'll do my best to find time to merge it and publish new package.
I'm alive, but really has problems with spare time

@Skyppid
Copy link

Skyppid commented Nov 10, 2020

Great to hear, thanks @force-net. No worries, I guess everyone understands that.

@brantburnett
Copy link
Author

@force-net Have you had a chance to look at this yet?

@force-net
Copy link
Owner

@brantburnett I'm really sorry. I'm trying to take some vacation to fix issues and merge PR. Lot of work and other stuff.

@lugospod
Copy link

lugospod commented Oct 6, 2021

@force-net Any news regarding this merge?

Also, it would be usefull to include ReadOnlyMemory support..so we don't have to allocate memory to provide this library byte[].

@brantburnett
Copy link
Author

@force-net Any news regarding this merge?

Also, it would be usefull to include ReadOnlyMemory support..so we don't have to allocate memory to provide this library byte[].

This change adds ReadOnlyMemory support via the use of ReadOnlySpan. ReadOnlyMemory has a Span property.

@lugospod
Copy link

lugospod commented Oct 7, 2021

@brantburnett Tnx.. I found it last night... I decided to stop waiting for the new release and just extract the code I need because obviously nuget losses its benefits if one has to wait so long (not blaming the team, that's just life :))

@arnoldsi-vii
Copy link

@force-net any updates regarding this merge?

@force-net
Copy link
Owner

Sorry, I still do not have time to merge and review all changes. But I hope, I'll find it.

@arnoldsi-vii
Copy link

@force-net thank you

@neon-sunset
Copy link

neon-sunset commented Aug 22, 2022

@brantburnett @arnoldsi-vii it appears there is https://www.nuget.org/packages/System.IO.Hashing/ now.

It doesn't seem to use any kind of loop unrolling in its implementation however and I haven't yet tested its performance vs this library: https://github.com/dotnet/runtime/blob/main/src/libraries/System.IO.Hashing/src/System/IO/Hashing/Crc32.cs
It does however accept ROS<byte>.

@brantburnett
Copy link
Author

@neon-sunset

Based on my quick review, I agree that implementation looks slower on the calculation side, though the use of ReadOnlySpan may make up for some of that. It's also possible that modern JIT doesn't need the manual loop unrolling, not sure. It also doesn't use Intrinsics to use CPU optimizations (my next planned improvement) nor does it have a CRC32C algorithm.

However, it may make sense to try to move these optimizations to that official library rather than trying to get this library maintained again.

Note that there is other work in progress that adds some optimizations in a different spot: dotnet/runtime#61558

@neon-sunset
Copy link

neon-sunset commented Aug 22, 2022

@brantburnett
Unfortunately, JIT does not do any loop unrolling or auto-vectorization as of today. It can do loop cloning for independent operations but this doesn't seem to be applicable in our case.

As for the mentioned PR, its purpose is a little bit different: .NET 7 introduces fully cross-platform vector operations. What I mean by this is that pre-.NET 7 it was necessary to reference exact intrinsics like Avx2.DoStuff or AdvSimd.DoStuffButArm which meant a lot of code duplication. However, it is now possible to write an algorithm on top of Vector<T>/VectorXXX<T> and its extensions once, and it will compile to corresponding efficient codegen that uses vector instructions supported on the target platform (there are limitations - using Vector256 on arm64 will cause it to fallback to scalar code instead of producing unrolled Vector128 operations).

The reason I mention this is that dotnet/runtime#61558 appears to be following the same approach ensuring that we can just use a single method to compute checksum for a primitive value which will either use available CRC32 intrinsics or fallback fast implementation for specific platform.

However, our use case here is a little bit different so I totally agree with you on the suggestion to upstream improvements for System.IO.Hashing instead.

@brantburnett
Copy link
Author

For Crc32 (though unfortunately not yet Crc32C) there is now an even more performant implementation using vectorization and polynomial multiplication that has been merged into System.IO.Hashing.

dotnet/runtime#83321

This will be included in the .NET 8 release of the System.IO.Hashing NuGet (probably in preview 4). The vectorization improvements are also backward compatible to .NET 7 and the ARM scalar improvements to .NET 6 if you use the new package.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants