Crc32.NET icon indicating copy to clipboard operation
Crc32.NET copied to clipboard

Add .NET Core 2.1 and 3.0 perf improvements

Open brantburnett opened this issue 5 years ago • 15 comments

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 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

brantburnett avatar Oct 17 '20 04:10 brantburnett

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.

Skyppid avatar Nov 10 '20 08:11 Skyppid

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

force-net avatar Nov 10 '20 09:11 force-net

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

Skyppid avatar Nov 10 '20 10:11 Skyppid

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

brantburnett avatar Aug 11 '21 14:08 brantburnett

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

force-net avatar Aug 11 '21 18:08 force-net

@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[].

lugospod avatar Oct 06 '21 22:10 lugospod

@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.

brantburnett avatar Oct 07 '21 11:10 brantburnett

@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 :))

lugospod avatar Oct 07 '21 12:10 lugospod

@force-net any updates regarding this merge?

arnoldsi-vii avatar Feb 26 '22 21:02 arnoldsi-vii

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

force-net avatar Feb 27 '22 10:02 force-net

@force-net thank you

arnoldsi-vii avatar Mar 09 '22 22:03 arnoldsi-vii

@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>.

neon-sunset avatar Aug 22 '22 16:08 neon-sunset

@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: https://github.com/dotnet/runtime/pull/61558

brantburnett avatar Aug 22 '22 16:08 brantburnett

@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 https://github.com/dotnet/runtime/pull/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.

neon-sunset avatar Aug 22 '22 17:08 neon-sunset

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.

https://github.com/dotnet/runtime/pull/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.

brantburnett avatar Apr 23 '23 13:04 brantburnett