Add XxHash128

[xoofx]

Fixes #77885

Lots of changes in XxHash3 because I had to extract a good chunk of the shared implementation for XXH3 to a shared class XxHashShared.

Performance wise, it is overall under 10% slower compared to C++, but also sometimes 10% faster. On the large item, it is closer to 30% slower, but I noticed that the warmup was only 10% slower, while the actual run was getting slower, not sure if it is a tiered compilation issue.

You will notice that I have introduced constants for all the secrets (e.g DefaultSecretUInt64_0), as they are shared between XXH3-64 and XXH3-128 (before they were inlined and hardcoded).

I have added similar tests to XxHash3, by using the native implementation to generate the expected test results.

BenchmarkDotNet=v0.13.2, OS=Windows 11 (10.0.22621.674)
AMD Ryzen 9 5950X, 1 CPU, 32 logical and 16 physical cores
.NET SDK=7.0.100-rc.1.22431.12
  [Host]     : .NET 7.0.0 (7.0.22.42610), X64 RyuJIT AVX2
  DefaultJob : .NET 7.0.0 (7.0.22.42610), X64 RyuJIT AVX2

Method	data	Mean	Error	StdDev	Ratio
XXH3 native	Byte[0]	7.138 ns	0.0139 ns	0.0130 ns	1.00
XXH3 C#	Byte[0]	7.677 ns	0.0121 ns	0.0113 ns	1.08
XXH3 native	Byte[3]	7.726 ns	0.0359 ns	0.0336 ns	1.00
XXH3 C#	Byte[3]	7.592 ns	0.0317 ns	0.0281 ns	0.98
XXH3 native	Byte[8]	7.265 ns	0.0099 ns	0.0087 ns	1.00
XXH3 C#	Byte[8]	7.411 ns	0.0108 ns	0.0095 ns	1.02
XXH3 native	Byte[16]	7.784 ns	0.0282 ns	0.0264 ns	1.00
XXH3 C#	Byte[16]	7.075 ns	0.0270 ns	0.0253 ns	0.91
XXH3 native	Byte[128]	10.483 ns	0.0250 ns	0.0234 ns	1.00
XXH3 C#	Byte[128]	10.671 ns	0.0412 ns	0.0344 ns	1.02
XXH3 native	Byte[240]	21.07 ns	0.043 ns	0.036 ns	1.00
XXH3 C#	Byte[240]	18.94 ns	0.033 ns	0.031 ns	0.90
XXH3 native	Byte[1048576]	27,361.578 ns	48.3501 ns	45.2267 ns	1.00
XXH3 C#	Byte[1048576]	35,092.181 ns	304.4434 ns	284.7766 ns	1.28

cc: @stephentoub, @EgorBo

[ghost]

Tagging subscribers to this area: @dotnet/area-system-security, @vcsjones
See info in area-owners.md if you want to be subscribed.

Issue Details

---

Fixes #77885

Lots of changes in XxHash3 because I had to extract a good chunk of the shared implementation for XXH3 to a shared class XxHashShared.

For large buffers, performance is on par with XxHash3, as it is using the shared implementation.
Compare to native, for intermediate buffers, I haven't checked XxHash3, but the performance is okish, but might require more optimizations here and there. I will try to add some of them as part of this PR If I find some (e.g see for example the 240 bytes buffer that is almost 6 times slower)

You will notice that I have introduced constants for all the secrets (e.g DefaultSecretUInt64_0), as they are shared between XXH3-64 and XXH3-128.

I have added similar tests to XxHash3, by using the native implementation to generate the expected test results.

This PR will have to wait for #77881 to be merged (and will have to be rebased, because XxHash3 file will conflict).

BenchmarkDotNet=v0.13.2, OS=Windows 11 (10.0.22621.674)
AMD Ryzen 9 5950X, 1 CPU, 32 logical and 16 physical cores
.NET SDK=7.0.100-rc.1.22431.12
  [Host]     : .NET 7.0.0 (7.0.22.42610), X64 RyuJIT AVX2
  DefaultJob : .NET 7.0.0 (7.0.22.42610), X64 RyuJIT AVX2

Method	data	Mean	Error	StdDev
XXH3Native	Byte[0]	7.137 ns	0.0157 ns	0.0147 ns
XXH3	Byte[0]	12.110 ns	0.0894 ns	0.0836 ns
XXH3Native	Byte[1048576]	27,354.139 ns	92.2719 ns	86.3112 ns
XXH3	Byte[1048576]	31,880.918 ns	33.4602 ns	31.2987 ns
XXH3Native	Byte[128]	10.459 ns	0.0208 ns	0.0184 ns
XXH3	Byte[128]	28.779 ns	0.2224 ns	0.1857 ns
XXH3Native	Byte[16]	7.715 ns	0.0150 ns	0.0140 ns
XXH3	Byte[16]	17.824 ns	0.0928 ns	0.0868 ns
XXH3Native	Byte[240]	21.043 ns	0.0578 ns	0.0512 ns
XXH3	Byte[240]	118.980 ns	0.4931 ns	0.4612 ns
XXH3Native	Byte[3]	7.745 ns	0.0273 ns	0.0255 ns
XXH3	Byte[3]	13.438 ns	0.0067 ns	0.0059 ns
XXH3Native	Byte[8]	7.286 ns	0.0131 ns	0.0122 ns
XXH3	Byte[8]	14.442 ns	0.0680 ns	0.0636 ns

cc: @stephentoub, @EgorBo

Author:	xoofx
Assignees:	-
Labels:	area-System.Security
Milestone:	-

[xoofx]

Commit 548c2e8 is fixing the performance difference now on part with the C++ version. Should have done before submitting this PR! 😅
I have updated the benchmarks in the PR description.

[GrabYourPitchforks]

I've added the NO MERGE label temporarily since this hasn't yet gone through API review. Once that happens, anybody should feel free to remove that label and continue with the standard PR review. :)

(Normal practice is to close PRs where API review hasn't yet taken place, but given that this API is likely to go through as-is I don't think closing this PR is warranted.)

[xoofx]

I ran the tests on a ARM64 and apart the optimized versions that is using vectorized HW intrinsincs, most of the scalar paths are quite slower (from 30% to 2x times slower). I haven't checked the generated code, but the ARM64 JIT is definitely struggling there.

That would be a good playground to optimize things further for ARM64 actually?

BenchmarkDotNet=v0.13.2, OS=Windows 11 (10.0.22621.755)
Snapdragon Compute Platform, 1 CPU, 8 logical and 8 physical cores
.NET SDK=7.0.100-rc.2.22477.23
  [Host]     : .NET 7.0.0 (7.0.22.47203), Arm64 RyuJIT AdvSIMD
  DefaultJob : .NET 7.0.0 (7.0.22.47203), Arm64 RyuJIT AdvSIMD

Method	data	Mean	Error	StdDev	Ratio
XXH3 native	Byte[0]	3.010 ns	0.0077 ns	0.0072 ns	1.00
XXH3 C#	Byte[0]	6.586 ns	0.0124 ns	0.0110 ns	2.19
XXH3 native	Byte[3]	4.215 ns	0.0123 ns	0.0115 ns	1.00
XXH3 C#	Byte[3]	8.335 ns	0.0105 ns	0.0094 ns	1.98
XXH3 native	Byte[8]	4.042 ns	0.0117 ns	0.0104 ns	1.00
XXH3 C#	Byte[8]	5.312 ns	0.0139 ns	0.0130 ns	1.31
XXH3 native	Byte[16]	5.787 ns	0.0107 ns	0.0100 ns	1.00
XXH3 C#	Byte[16]	5.212 ns	0.0131 ns	0.0116 ns	0.90
XXH3 native	Byte[128]	13.538 ns	0.0233 ns	0.0218 ns	1.00
XXH3 C#	Byte[128]	19.959 ns	0.0399 ns	0.0374 ns	1.47
XXH3 native	Byte[240]	28.492 ns	0.0492 ns	0.0460 ns	1.00
XXH3 C#	Byte[240]	36.120 ns	0.0753 ns	0.0704 ns	1.27
XXH3 native	Byte[1048576]	70,504.056 ns	269.1166 ns	251.7318 ns	1.00
XXH3 C#	Byte[1048576]	61,561.207 ns	284.8987 ns	237.9033 ns	0.87

[xoofx]

Thanks for the PR review! I'm on a business travel this week but I will get back to this PR hopefully next week.

[stephentoub]

The API was approved. As were the one-shot helpers in #76279. When you swing back around to working on this, can you please add those HashToUInt128 and GetCurrentHashAsUInt128 members as well? Thanks.

[xoofx]

I'm not sure about how to add the new API to ref. Is it correct to use #if/#endif with NET7_0_OR_GREATER?

    public sealed partial class XxHash128 : System.IO.Hashing.NonCryptographicHashAlgorithm
    {
        public XxHash128() : base (default(int)) { }
        public XxHash128(long seed) : base (default(int)) { }
        public override void Append(System.ReadOnlySpan<byte> source) { }
        protected override void GetCurrentHashCore(System.Span<byte> destination) { }
#if NET7_0_OR_GREATER
        [System.CLSCompliantAttribute(false)] public UInt128 GetCurrentHashAsUInt128() { throw null; }
#endif
        public static byte[] Hash(byte[] source) { throw null; }
        public static byte[] Hash(byte[] source, long seed) { throw null; }
        public static byte[] Hash(System.ReadOnlySpan<byte> source, long seed = (long)0) { throw null; }
        public static int Hash(System.ReadOnlySpan<byte> source, System.Span<byte> destination, long seed = (long)0) { throw null; }
#if NET7_0_OR_GREATER
        [System.CLSCompliantAttribute(false)] public static UInt128 HashToUInt128(ReadOnlySpan<byte> source, long seed = 0) { throw null; }
#endif
        public override void Reset() { }
        public static bool TryHash(System.ReadOnlySpan<byte> source, System.Span<byte> destination, out int bytesWritten, long seed = (long)0) { throw null; }
    }

[stephentoub]

I'm not sure about how to add the new API to ref. Is it correct to use #if/#endif with NET7_0_OR_GREATER?

We generally split it across files, e.g.
https://github.com/dotnet/runtime/tree/main/src/libraries/System.Threading.Channels/ref

[xoofx]

I have merged latest master and fixed the ref file. (2 pipelines are failing but with a weird error on Azure, but don't think it's coming from this PR)

[stephentoub]

Thanks.

[stephentoub]

Thanks!

[ViktorHofer]

nit: It would have been better to condition on a compatibility check / version check:

<ItemGroup Condition="$([MSBuild]::IsTargetFrameworkCompatible('$(TargetFramework)', 'net7.0'))">

to not lose the API on net7.0 when $(NetCoreAppCurrent) becomes net8.0. This just started failing in the PR that upgrades to net8.0: https://github.com/dotnet/runtime/pull/78354/checks?check_run_id=9846188314.

I will change this in our PR.

[stephentoub]

Thanks

[uweigand]

Looks like this commit caused CI failures on the big-endian s390x target:
https://dev.azure.com/dnceng-public/public/_build/results?buildId=100464&view=logs&j=7ea23876-d932-5222-06a0-de72daa3cbf4&t=879dfc24-2817-51ab-29eb-686cb2c04225&l=55

/__w/1/s/.packages/microsoft.dotnet.helix.sdk/8.0.0-beta.22579.2/tools/azure-pipelines/AzurePipelines.MultiQueue.targets(43,5): error : Test System.IO.Hashing.Tests.XxHash128Tests.Hash_Streaming_Expected has failed. Check the Test tab or this console log: https://helix.dot.net/api/2019-06-17/jobs/ce5f39e5-2814-4f1d-88c3-9e58bd385d9f/workitems/System.IO.Hashing.Tests/console [/__w/1/s/src/libraries/sendtohelixhelp.proj]
/__w/1/s/.packages/microsoft.dotnet.helix.sdk/8.0.0-beta.22579.2/tools/azure-pipelines/AzurePipelines.MultiQueue.targets(43,5): error : Test System.IO.Hashing.Tests.XxHash128Tests.Hash_OneShot_Expected has failed. Check the Test tab or this console log: https://helix.dot.net/api/2019-06-17/jobs/ce5f39e5-2814-4f1d-88c3-9e58bd385d9f/workitems/System.IO.Hashing.Tests/console [/__w/1/s/src/libraries/sendtohelixhelp.proj]
/__w/1/s/.packages/microsoft.dotnet.helix.sdk/8.0.0-beta.22579.2/tools/azure-pipelines/AzurePipelines.MultiQueue.targets(43,5): error : Test System.IO.Hashing.Tests.XxHash3Tests.Hash_Streaming_Expected has failed. Check the Test tab or this console log: https://helix.dot.net/api/2019-06-17/jobs/ce5f39e5-2814-4f1d-88c3-9e58bd385d9f/workitems/System.IO.Hashing.Tests/console [/__w/1/s/src/libraries/sendtohelixhelp.proj]
/__w/1/s/.packages/microsoft.dotnet.helix.sdk/8.0.0-beta.22579.2/tools/azure-pipelines/AzurePipelines.MultiQueue.targets(43,5): error : Test System.IO.Hashing.Tests.XxHash3Tests.Hash_OneShot_Expected has failed. Check the Test tab or this console log: https://helix.dot.net/api/2019-06-17/jobs/ce5f39e5-2814-4f1d-88c3-9e58bd385d9f/workitems/System.IO.Hashing.Tests/console [/__w/1/s/src/libraries/sendtohelixhelp.proj]
    0 Warning(s)

I think the endian fixes provided by @stephentoub in #78084 got lost in the move from XXHash3.cs to XXHashShared.cs.

[xoofx]

I think the endian fixes provided by @stephentoub in #78084 got lost in the move from XXHash3.cs to XXHashShared.cs.

Ouch, indeed, my apologize for this regression, I pushed the PR #79186 to backport the fix.