Replace custom Marvin copy with string.GetHashCode (#48410)
authorStephen Toub <stoub@microsoft.com>
Tue, 9 Mar 2021 21:33:05 +0000 (16:33 -0500)
committerGitHub <noreply@github.com>
Tue, 9 Mar 2021 21:33:05 +0000 (16:33 -0500)
* Use HashCode instead of custom Marvin implementation

* Update src/libraries/System.Formats.Cbor/src/System/Formats/Cbor/CborConformanceLevel.cs

src/libraries/Common/src/System/Marvin.cs [deleted file]
src/libraries/Common/tests/Common.Tests.csproj
src/libraries/Common/tests/Tests/System/MarvinTests.cs [deleted file]
src/libraries/System.Formats.Cbor/src/System.Formats.Cbor.csproj
src/libraries/System.Formats.Cbor/src/System/Formats/Cbor/CborConformanceLevel.cs
src/libraries/System.Net.Primitives/src/System.Net.Primitives.csproj
src/libraries/System.Net.Primitives/src/System/Net/IPAddress.cs
src/libraries/System.Net.Primitives/tests/PalTests/System.Net.Primitives.Pal.Tests.csproj
src/libraries/System.Net.Primitives/tests/UnitTests/System.Net.Primitives.UnitTests.Tests.csproj

diff --git a/src/libraries/Common/src/System/Marvin.cs b/src/libraries/Common/src/System/Marvin.cs
deleted file mode 100644 (file)
index b36e2a6..0000000
+++ /dev/null
@@ -1,124 +0,0 @@
-// Licensed to the .NET Foundation under one or more agreements.
-// The .NET Foundation licenses this file to you under the MIT license.
-
-#nullable disable
-using System.Diagnostics;
-using System.Runtime.CompilerServices;
-using System.Runtime.InteropServices;
-using System.Security.Cryptography;
-
-namespace System
-{
-    internal static class Marvin
-    {
-        /// <summary>
-        /// Convenience method to compute a Marvin hash and collapse it into a 32-bit hash.
-        /// </summary>
-        [MethodImpl(MethodImplOptions.AggressiveInlining)]
-        public static int ComputeHash32(ReadOnlySpan<byte> data, ulong seed)
-        {
-            long hash64 = ComputeHash(data, seed);
-            return ((int)(hash64 >> 32)) ^ (int)hash64;
-        }
-
-        /// <summary>
-        /// Computes a 64-hash using the Marvin algorithm.
-        /// </summary>
-        [MethodImpl(MethodImplOptions.AggressiveInlining)]
-        public static long ComputeHash(ReadOnlySpan<byte> data, ulong seed)
-            => ComputeHash(ref MemoryMarshal.GetReference(data), (uint)data.Length, p0: (uint)seed, p1: (uint)(seed >> 32));
-
-        private static unsafe long ComputeHash(ref byte rBuffer, nuint cbBuffer, uint p0, uint p1)
-        {
-            nuint currentOffset = 0;
-
-            fixed (byte* pbBuffer = &rBuffer)
-            {
-                // Consume as many 4-byte chunks as possible.
-
-                if (cbBuffer >= 4)
-                {
-                    nuint stopOffset = cbBuffer & ~(nuint)3;
-                    do
-                    {
-                        p0 += Unsafe.ReadUnaligned<uint>(pbBuffer + currentOffset);
-                        currentOffset += 4;
-                        Block(ref p0, ref p1);
-                    } while (currentOffset < stopOffset);
-                }
-
-                // Fewer than 4 bytes remain; drain remaining bytes.
-
-                Debug.Assert(cbBuffer - currentOffset < 4, "Should have 0 - 3 bytes remaining.");
-                switch ((int)cbBuffer & 3)
-                {
-                    case 0:
-                        p0 += 0x80u;
-                        break;
-
-                    case 1:
-                        p0 += 0x8000u | pbBuffer[currentOffset];
-                        break;
-
-                    case 2:
-                        p0 += 0x800000u | Unsafe.ReadUnaligned<ushort>(pbBuffer + currentOffset);
-                        break;
-
-                    case 3:
-                        p0 += 0x80000000u | Unsafe.ReadUnaligned<ushort>(pbBuffer + currentOffset) | ((uint)pbBuffer[currentOffset + 2] << 16);
-                        break;
-
-                    default:
-                        Debug.Fail("Should not get here.");
-                        break;
-                }
-            }
-
-            Block(ref p0, ref p1);
-            Block(ref p0, ref p1);
-
-            return (((long)p1) << 32) | p0;
-        }
-
-        [MethodImpl(MethodImplOptions.AggressiveInlining)]
-        private static void Block(ref uint rp0, ref uint rp1)
-        {
-            uint p0 = rp0;
-            uint p1 = rp1;
-
-            p1 ^= p0;
-            p0 = _rotl(p0, 20);
-
-            p0 += p1;
-            p1 = _rotl(p1, 9);
-
-            p1 ^= p0;
-            p0 = _rotl(p0, 27);
-
-            p0 += p1;
-            p1 = _rotl(p1, 19);
-
-            rp0 = p0;
-            rp1 = p1;
-        }
-
-        [MethodImpl(MethodImplOptions.AggressiveInlining)]
-        private static uint _rotl(uint value, int shift)
-        {
-            // This is expected to be optimized into a single rol (or ror with negated shift value) instruction
-            return (value << shift) | (value >> (32 - shift));
-        }
-
-        public static ulong DefaultSeed { get; } = GenerateSeed();
-
-        private static ulong GenerateSeed()
-        {
-            using (RandomNumberGenerator rng = RandomNumberGenerator.Create())
-            {
-                var bytes = new byte[sizeof(ulong)];
-                rng.GetBytes(bytes);
-                return BitConverter.ToUInt64(bytes, 0);
-            }
-        }
-    }
-}
index 6cbd38f..8a8a8b0 100644 (file)
@@ -18,8 +18,6 @@
              Link="Common\Interop\Linux\Interop.ProcFsStat.TryReadStatusFile.cs" />
     <Compile Include="$(CommonPath)System\CharArrayHelpers.cs"
              Link="Common\System\CharArrayHelpers.cs" />
-    <Compile Include="$(CommonPath)System\Marvin.cs"
-             Link="Common\System\Marvin.cs" />
     <Compile Include="$(CommonPath)System\StringExtensions.cs"
              Link="Common\System\StringExtensions.cs" />
     <Compile Include="$(CommonPath)System\Collections\Generic\ArrayBuilder.cs"
@@ -79,7 +77,6 @@
     <Compile Include="Tests\System\CharArrayHelpersTests.cs" />
     <Compile Include="Tests\System\IO\PathInternal.Tests.cs" />
     <Compile Include="Tests\System\IO\StringParserTests.cs" />
-    <Compile Include="Tests\System\MarvinTests.cs" />
     <Compile Include="Tests\System\Net\HttpDateParserTests.cs" />
     <Compile Include="Tests\System\PasteArgumentsTests.cs" />
     <Compile Include="Tests\System\Security\IdentityHelperTests.cs" />
diff --git a/src/libraries/Common/tests/Tests/System/MarvinTests.cs b/src/libraries/Common/tests/Tests/System/MarvinTests.cs
deleted file mode 100644 (file)
index a70ae1a..0000000
+++ /dev/null
@@ -1,71 +0,0 @@
-// Licensed to the .NET Foundation under one or more agreements.
-// The .NET Foundation licenses this file to you under the MIT license.
-
-using System;
-using Test.Cryptography;
-
-using Xunit;
-
-namespace Tests.System
-{
-    public class MarvinTests
-    {
-        private const ulong Seed1 = 0x4FB61A001BDBCCLU;
-        private const ulong Seed2 = 0x804FB61A001BDBCCLU;
-        private const ulong Seed3 = 0x804FB61A801BDBCCLU;
-
-        private const string TestDataString0Byte = "";
-        private const string TestDataString1Byte = "af";
-        private const string TestDataString2Byte = "e70f";
-        private const string TestDataString3Byte = "37f495";
-        private const string TestDataString4Byte = "8642dc59";
-        private const string TestDataString5Byte = "153fb79826";
-        private const string TestDataString6Byte = "0932e6246c47";
-        private const string TestDataString7Byte = "ab427ea8d10fc7";
-
-        [Theory]
-        [MemberData(nameof(TestDataAndExpectedHashes))]
-        public void ComputeHash_Success(ulong seed, string testDataString, ulong expectedHash)
-        {
-            var testDataSpan = new Span<byte>(testDataString.HexToByteArray());
-            long hash = Marvin.ComputeHash(testDataSpan, seed);
-            Assert.Equal((long)expectedHash, hash);
-        }
-
-        public static object[][] TestDataAndExpectedHashes =
-        {
-            new object[] { Seed1, TestDataString0Byte, 0x30ED35C100CD3C7DLU },
-            new object[] { Seed1, TestDataString1Byte, 0x48E73FC77D75DDC1LU },
-            new object[] { Seed1, TestDataString2Byte, 0xB5F6E1FC485DBFF8LU },
-            new object[] { Seed1, TestDataString3Byte, 0xF0B07C789B8CF7E8LU },
-            new object[] { Seed1, TestDataString4Byte, 0x7008F2E87E9CF556LU },
-            new object[] { Seed1, TestDataString5Byte, 0xE6C08C6DA2AFA997LU },
-            new object[] { Seed1, TestDataString6Byte, 0x6F04BF1A5EA24060LU },
-            new object[] { Seed1, TestDataString7Byte, 0xE11847E4F0678C41LU },
-
-            new object[] { Seed2, TestDataString0Byte, 0x10A9D5D3996FD65DLU },
-            new object[] { Seed2, TestDataString1Byte, 0x68201F91960EBF91LU },
-            new object[] { Seed2, TestDataString2Byte, 0x64B581631F6AB378LU },
-            new object[] { Seed2, TestDataString3Byte, 0xE1F2DFA6E5131408LU },
-            new object[] { Seed2, TestDataString4Byte, 0x36289D9654FB49F6LU },
-            new object[] { Seed2, TestDataString5Byte, 0xA06114B13464DBDLU },
-            new object[] { Seed2, TestDataString6Byte, 0xD6DD5E40AD1BC2EDLU },
-            new object[] { Seed2, TestDataString7Byte, 0xE203987DBA252FB3LU },
-
-            new object[] { Seed3, "00", 0xA37FB0DA2ECAE06CLU },
-            new object[] { Seed3, "FF", 0xFECEF370701AE054LU },
-            new object[] { Seed3, "00FF", 0xA638E75700048880LU },
-            new object[] { Seed3, "FF00", 0xBDFB46D969730E2ALU },
-            new object[] { Seed3, "FF00FF", 0x9D8577C0FE0D30BFLU },
-            new object[] { Seed3, "00FF00", 0x4F9FBDDE15099497LU },
-            new object[] { Seed3, "00FF00FF", 0x24EAA279D9A529CALU },
-            new object[] { Seed3, "FF00FF00", 0xD3BEC7726B057943LU },
-            new object[] { Seed3, "FF00FF00FF", 0x920B62BBCA3E0B72LU },
-            new object[] { Seed3, "00FF00FF00", 0x1D7DDF9DFDF3C1BFLU },
-            new object[] { Seed3, "00FF00FF00FF", 0xEC21276A17E821A5LU },
-            new object[] { Seed3, "FF00FF00FF00", 0x6911A53CA8C12254LU },
-            new object[] { Seed3, "FF00FF00FF00FF", 0xFDFD187B1D3CE784LU },
-            new object[] { Seed3, "00FF00FF00FF00", 0x71876F2EFB1B0EE8LU },
-        };
-    }
-}
index aff4f21..2346299 100644 (file)
@@ -6,7 +6,6 @@
     <Nullable>enable</Nullable>
   </PropertyGroup>
   <ItemGroup>
-    <Compile Include="$(CommonPath)System\Marvin.cs" Link="Common\System\Marvin.cs" />
     <Compile Include="$(CommonPath)System\Memory\PointerMemoryManager.cs" Link="Common\System\Memory\PointerMemoryManager.cs" />
     <Compile Include="System\Formats\Cbor\HalfHelpers.cs" />
     <Compile Include="System\Formats\Cbor\Reader\CborReaderState.cs" />
index 7455ddd..1ac8444 100644 (file)
@@ -2,6 +2,7 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 
 using System.Diagnostics;
+using System.Runtime.InteropServices;
 using System.Text;
 
 namespace System.Formats.Cbor
@@ -209,7 +210,21 @@ namespace System.Formats.Cbor
 
         public static int GetKeyEncodingHashCode(ReadOnlySpan<byte> encoding)
         {
-            return System.Marvin.ComputeHash32(encoding, System.Marvin.DefaultSeed);
+            HashCode hash = default;
+
+            // TODO: Use https://github.com/dotnet/runtime/issues/48702 if/when it's available
+            while (encoding.Length >= sizeof(int))
+            {
+                hash.Add(MemoryMarshal.Read<int>(encoding));
+                encoding = encoding.Slice(sizeof(int));
+            }
+
+            foreach (byte b in encoding)
+            {
+                hash.Add(b);
+            }
+
+            return hash.ToHashCode();
         }
 
         public static bool AreEqualKeyEncodings(ReadOnlySpan<byte> left, ReadOnlySpan<byte> right)
index 60cd3de..39c385c 100644 (file)
@@ -68,8 +68,6 @@
              Link="Common\System\Net\NegotiationInfoClass.cs" />
     <Compile Include="$(CommonPath)System\Net\NetworkInformation\HostInformation.cs"
              Link="Common\System\Net\NetworkInformation\HostInformation.cs" />
-    <Compile Include="$(CommonPath)System\Marvin.cs"
-             Link="Common\System\Marvin.cs" />
     <Compile Include="$(CommonPath)System\Text\StringBuilderCache.cs"
              Link="Common\System\Text\StringBuilderCache.cs" />
     <Compile Include="$(CommonPath)System\HexConverter.cs"
index a799ba1..66be8c8 100644 (file)
@@ -588,37 +588,26 @@ namespace System.Net
 
         public override int GetHashCode()
         {
-            if (_hashCode != 0)
+            if (_hashCode == 0)
             {
-                return _hashCode;
-            }
-
-            // For IPv6 addresses, we calculate the hashcode by using Marvin
-            // on a stack-allocated array containing the Address bytes and ScopeId.
-            int hashCode;
-            if (IsIPv6)
-            {
-                const int AddressAndScopeIdLength = IPAddressParserStatics.IPv6AddressBytes + sizeof(uint);
-                Span<byte> addressAndScopeIdSpan = stackalloc byte[AddressAndScopeIdLength];
-
-                MemoryMarshal.AsBytes(new ReadOnlySpan<ushort>(_numbers)).CopyTo(addressAndScopeIdSpan);
-                Span<byte> scopeIdSpan = addressAndScopeIdSpan.Slice(IPAddressParserStatics.IPv6AddressBytes);
-                bool scopeWritten = BitConverter.TryWriteBytes(scopeIdSpan, _addressOrScopeId);
-                Debug.Assert(scopeWritten);
-
-                hashCode = Marvin.ComputeHash32(
-                    addressAndScopeIdSpan,
-                    Marvin.DefaultSeed);
-            }
-            else
-            {
-                // For IPv4 addresses, we use Marvin on the integer representation of the Address.
-                hashCode = Marvin.ComputeHash32(
-                    MemoryMarshal.AsBytes(MemoryMarshal.CreateReadOnlySpan(ref _addressOrScopeId, 1)),
-                    Marvin.DefaultSeed);
+                // For IPv4 addresses, we calculate the hashcode based on address bytes.
+                // For IPv6 addresses, we also factor in scope ID.
+                if (IsIPv6)
+                {
+                    ReadOnlySpan<byte> numbers = MemoryMarshal.AsBytes<ushort>(_numbers);
+                    _hashCode = HashCode.Combine(
+                        MemoryMarshal.Read<uint>(numbers),
+                        MemoryMarshal.Read<uint>(numbers.Slice(4)),
+                        MemoryMarshal.Read<uint>(numbers.Slice(8)),
+                        MemoryMarshal.Read<uint>(numbers.Slice(12)),
+                        _addressOrScopeId);
+                }
+                else
+                {
+                    _hashCode = HashCode.Combine(_addressOrScopeId);
+                }
             }
 
-            _hashCode = hashCode;
             return _hashCode;
         }
 
index 0d48b73..4652679 100644 (file)
@@ -32,8 +32,6 @@
              Link="Common\System\Net\SocketAddress.cs" />
     <Compile Include="..\..\src\System\Net\EndPoint.cs"
              Link="ProductionCode\System\Net\EndPoint.cs" />
-    <Compile Include="$(CommonPath)System\Marvin.cs"
-             Link="ProductionCode\Common\System\Marvin.cs" />
     <Compile Include="$(CommonPath)System\Text\StringBuilderCache.cs"
              Link="ProductionCode\Common\System\Text\StringBuilderCache.cs" />
     <Compile Include="$(CommonPath)System\HexConverter.cs"
index 2623c69..077938e 100644 (file)
@@ -64,8 +64,6 @@
              Link="ProductionCode\Common\System\Net\TcpValidationHelpers.cs" />
     <Compile Include="$(CommonPath)System\Net\UriScheme.cs"
              Link="ProductionCode\Common\System\Net\UriScheme.cs" />
-    <Compile Include="$(CommonPath)System\Marvin.cs"
-             Link="Common\System\Marvin.cs" />
     <Compile Include="$(CommonPath)System\Text\StringBuilderCache.cs"
              Link="Common\System\Text\StringBuilderCache.cs" />
     <!-- Logging -->