Use span APIs to compute StringSegment hashes when available

Fixes #457
dotnet · Jan 16, 2019 · 1822a2c · 1822a2c
1 parent 1e3f27b
commit 1822a2c
Show file tree

Hide file tree

Showing 4 changed files with 95 additions and 20 deletions.
diff --git a/src/Primitives/src/Microsoft.Extensions.Primitives.csproj b/src/Primitives/src/Microsoft.Extensions.Primitives.csproj
@@ -1,11 +1,11 @@
-<Project Sdk="Microsoft.NET.Sdk">
+<Project Sdk="Microsoft.NET.Sdk">
 
   <PropertyGroup>
     <Description>Primitives shared by framework extensions. Commonly used types include:
 Microsoft.Extensions.Primitives.IChangeToken
 Microsoft.Extensions.Primitives.StringValues
 Microsoft.Extensions.Primitives.StringSegment</Description>
-    <TargetFramework>netstandard2.0</TargetFramework>
+    <TargetFrameworks>netstandard2.0;netcoreapp3.0</TargetFrameworks>
     <NoWarn>$(NoWarn);CS1591</NoWarn>
     <AllowUnsafeBlocks>true</AllowUnsafeBlocks>
     <GenerateDocumentationFile>true</GenerateDocumentationFile>

diff --git a/src/Primitives/src/StringSegment.cs b/src/Primitives/src/StringSegment.cs
@@ -1,4 +1,4 @@
-// Copyright (c) .NET Foundation. All rights reserved.
+// Copyright (c) .NET Foundation. All rights reserved.
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
 using System;
@@ -219,7 +219,7 @@ public bool Equals(string text, StringComparison comparisonType)
                 ThrowHelper.ThrowArgumentNullException(ExceptionArgument.text);
             }
 
-            int textLength = text.Length;
+            var textLength = text.Length;
             if (!HasValue || Length != textLength)
             {
                 return false;
@@ -229,16 +229,20 @@ public bool Equals(string text, StringComparison comparisonType)
         }
 
         /// <inheritdoc />
-        /// <remarks>
-        /// This GetHashCode is expensive since it allocates on every call.
-        /// However this is required to ensure we retain any behavior (such as hash code randomization) that
-        /// string.GetHashCode has.
-        /// </remarks>
         [MethodImpl(MethodImplOptions.AggressiveInlining)]
         public override int GetHashCode()
         {
-            // TODO: PERF; Note that .NET Core strings use randomized hash codes for security reasons.
+#if NETCOREAPP
+            return string.GetHashCode(AsSpan());
+#elif NETSTANDARD
+            // This GetHashCode is expensive since it allocates on every call.
+            // However this is required to ensure we retain any behavior (such as hash code randomization) that
+            // string.GetHashCode has.
             return Value?.GetHashCode() ?? 0;
+#else
+#error Target frameworks need to be updated.
+#endif
+
         }
 
         /// <summary>
@@ -290,8 +294,8 @@ public bool StartsWith(string text, StringComparison comparisonType)
                 ThrowHelper.ThrowArgumentNullException(ExceptionArgument.text);
             }
 
-            bool result = false;
-            int textLength = text.Length;
+            var result = false;
+            var textLength = text.Length;
 
             if (HasValue && Length >= textLength)
             {
@@ -315,9 +319,9 @@ public bool EndsWith(string text, StringComparison comparisonType)
                 ThrowHelper.ThrowArgumentNullException(ExceptionArgument.text);
             }
 
-            bool result = false;
-            int textLength = text.Length;
-            int comparisonLength = Offset + Length - textLength;
+            var result = false;
+            var textLength = text.Length;
+            var comparisonLength = Offset + Length - textLength;
 
             if (HasValue && comparisonLength > 0)
             {
@@ -531,7 +535,7 @@ public unsafe StringSegment TrimStart()
             {
                 while (trimmedStart < length)
                 {
-                    char c = p[trimmedStart];
+                    var c = p[trimmedStart];
 
                     if (!char.IsWhiteSpace(c))
                     {
@@ -558,7 +562,7 @@ public unsafe StringSegment TrimEnd()
             {
                 while (trimmedEnd >= offset)
                 {
-                    char c = p[trimmedEnd];
+                    var c = p[trimmedEnd];
 
                     if (!char.IsWhiteSpace(c))
                     {

diff --git a/src/Primitives/src/StringSegmentComparer.cs b/src/Primitives/src/StringSegmentComparer.cs
@@ -1,4 +1,4 @@
-// Copyright (c) .NET Foundation. All rights reserved.
+// Copyright (c) .NET Foundation. All rights reserved.
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
 using System;
@@ -10,6 +10,7 @@ public class StringSegmentComparer : IComparer<StringSegment>, IEqualityComparer
     {
         public static StringSegmentComparer Ordinal { get; }
             = new StringSegmentComparer(StringComparison.Ordinal, StringComparer.Ordinal);
+
         public static StringSegmentComparer OrdinalIgnoreCase { get; }
             = new StringSegmentComparer(StringComparison.OrdinalIgnoreCase, StringComparer.OrdinalIgnoreCase);
 
@@ -34,8 +35,17 @@ public bool Equals(StringSegment x, StringSegment y)
 
         public int GetHashCode(StringSegment obj)
         {
-            // TODO: PERF; Note that .NET Core strings use randomized hash codes for security reasons.
+#if NETCOREAPP
+            return string.GetHashCode(obj.AsSpan(), Comparison);
+#else
+            if (!obj.HasValue)
+            {
+                return 0;
+            }
+
+            // .NET Core strings use randomized hash codes for security reasons. Consequently we must materialize the StringSegment as a string
             return Comparer.GetHashCode(obj.Value);
+#endif
         }
     }
 }
diff --git a/src/Primitives/test/StringSegmentTest.cs b/src/Primitives/test/StringSegmentTest.cs
@@ -1,4 +1,4 @@
-// Copyright (c) .NET Foundation. All rights reserved.
+// Copyright (c) .NET Foundation. All rights reserved.
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
 using System;
@@ -847,6 +847,67 @@ public void StringSegment_Compare_Greater(StringSegment candidate, StringSegment
             Assert.True(result > 0, $"{segment} should be greater than {candidate}");
         }
 
+        [Theory]
+        [MemberData(nameof(GetHashCode_ReturnsSameValueForEqualSubstringsData))]
+        public void StringSegmentComparerOrdinal_GetHashCode_ReturnsSameValueForEqualSubstrings(StringSegment segment1, StringSegment segment2)
+        {
+            // Arrange
+            var comparer = StringSegmentComparer.Ordinal;
+
+            // Act
+            var hashCode1 = comparer.GetHashCode(segment1);
+            var hashCode2 = comparer.GetHashCode(segment2);
+
+            // Assert
+            Assert.Equal(hashCode1, hashCode2);
+        }
+
+        [Theory]
+        [MemberData(nameof(GetHashCode_ReturnsSameValueForEqualSubstringsData))]
+        public void StringSegmentComparerOrdinalIgnoreCase_GetHashCode_ReturnsSameValueForEqualSubstrings(StringSegment segment1, StringSegment segment2)
+        {
+            // Arrange
+            var comparer = StringSegmentComparer.OrdinalIgnoreCase;
+
+            // Act
+            var hashCode1 = comparer.GetHashCode(segment1);
+            var hashCode2 = comparer.GetHashCode(segment2);
+
+            // Assert
+            Assert.Equal(hashCode1, hashCode2);
+        }
+
+        [Fact]
+        public void StringSegmentComparerOrdinalIgnoreCase_GetHashCode_ReturnsSameValueForDifferentlyCasedStrings()
+        {
+            // Arrange
+            var segment1 = new StringSegment("abc");
+            var segment2 = new StringSegment("Abcd", 0, 3);
+            var comparer = StringSegmentComparer.OrdinalIgnoreCase;
+
+            // Act
+            var hashCode1 = comparer.GetHashCode(segment1);
+            var hashCode2 = comparer.GetHashCode(segment2);
+
+            // Assert
+            Assert.Equal(hashCode1, hashCode2);
+        }
+
+        [Theory]
+        [MemberData(nameof(GetHashCode_ReturnsDifferentValuesForInequalSubstringsData))]
+        public void StringSegmentComparerOrdinal_GetHashCode_ReturnsDifferentValuesForInequalSubstrings(StringSegment segment1, StringSegment segment2)
+        {
+            // Arrange
+            var comparer = StringSegmentComparer.Ordinal;
+
+            // Act
+            var hashCode1 = comparer.GetHashCode(segment1);
+            var hashCode2 = comparer.GetHashCode(segment2);
+
+            // Assert
+            Assert.NotEqual(hashCode1, hashCode2);
+        }
+
         [Fact]
         public void IndexOf_ComputesIndex_RelativeToTheCurrentSegment()
         {