From 6108fd7b67a8ee501b484cf55e47771f0e9f73b2 Mon Sep 17 00:00:00 2001 From: MihaZupan Date: Sun, 23 Feb 2020 15:00:33 +0100 Subject: [PATCH 1/6] Optimize Uri.GetHashCode --- .../System.Private.Uri/src/System/Uri.cs | 41 ++++++------------- 1 file changed, 12 insertions(+), 29 deletions(-) diff --git a/src/libraries/System.Private.Uri/src/System/Uri.cs b/src/libraries/System.Private.Uri/src/System/Uri.cs index e8d936349be31f..db129ea1aab872 100644 --- a/src/libraries/System.Private.Uri/src/System/Uri.cs +++ b/src/libraries/System.Private.Uri/src/System/Uri.cs @@ -1505,39 +1505,27 @@ public static int FromHex(char digit) => (uint)(digit - 'a') <= 'f' - 'a' ? digit - 'a' + 10 : throw new ArgumentException(nameof(digit)); - // - // GetHashCode - // - // Overrides default function (in Object class) - // - // public override int GetHashCode() { if (IsNotAbsoluteUri) { - return CalculateCaseInsensitiveHashCode(OriginalString); - } - - // Consider moving hash code storage from m_Info.MoreInfo to m_Info - UriInfo info = EnsureUriInfo(); - if ((object?)info.MoreInfo == null) - { - info.MoreInfo = new MoreInfo(); + MoreInfo moreInfo = (_info ??= new UriInfo()).MoreInfo ??= new MoreInfo(); + if (moreInfo.Hash == 0) + { + moreInfo.Hash = OriginalString.GetHashCode(StringComparison.OrdinalIgnoreCase); + } + return moreInfo.Hash; } - int tempHash = info.MoreInfo.Hash; - if (tempHash == 0) + else { - string? chkString = info.MoreInfo.RemoteUrl; - if ((object?)chkString == null) - chkString = GetParts(UriComponents.HttpRequestUrl, UriFormat.SafeUnescaped); - tempHash = CalculateCaseInsensitiveHashCode(chkString); - if (tempHash == 0) + MoreInfo moreInfo = EnsureUriInfo().MoreInfo ??= new MoreInfo(); + if (moreInfo.Hash == 0) { - tempHash = 0x1000000; //making it not zero still large enough to be mapped to zero by a hashtable + string? chkString = moreInfo.RemoteUrl ??= GetParts(UriComponents.HttpRequestUrl, UriFormat.SafeUnescaped); + moreInfo.Hash = chkString.GetHashCode(StringComparison.OrdinalIgnoreCase); } - info.MoreInfo.Hash = tempHash; + return moreInfo.Hash; } - return tempHash; } // @@ -4943,11 +4931,6 @@ private static char[] Compress(char[] dest, ushort start, ref int destLength, Ur return dest; } - internal static int CalculateCaseInsensitiveHashCode(string text) - { - return text.ToLowerInvariant().GetHashCode(); - } - // // CombineUri // From 80c19518fcce343b61a6188e4cc8d6fe4fa24c3d Mon Sep 17 00:00:00 2001 From: MihaZupan Date: Sun, 23 Feb 2020 17:36:48 +0100 Subject: [PATCH 2/6] Don't use case-insensitive string hashing when not necessary --- src/libraries/System.Private.Uri/src/System/Uri.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/libraries/System.Private.Uri/src/System/Uri.cs b/src/libraries/System.Private.Uri/src/System/Uri.cs index db129ea1aab872..7420e136eaaafc 100644 --- a/src/libraries/System.Private.Uri/src/System/Uri.cs +++ b/src/libraries/System.Private.Uri/src/System/Uri.cs @@ -1512,7 +1512,7 @@ public override int GetHashCode() MoreInfo moreInfo = (_info ??= new UriInfo()).MoreInfo ??= new MoreInfo(); if (moreInfo.Hash == 0) { - moreInfo.Hash = OriginalString.GetHashCode(StringComparison.OrdinalIgnoreCase); + moreInfo.Hash = OriginalString.GetHashCode(); } return moreInfo.Hash; } @@ -1521,8 +1521,8 @@ public override int GetHashCode() MoreInfo moreInfo = EnsureUriInfo().MoreInfo ??= new MoreInfo(); if (moreInfo.Hash == 0) { - string? chkString = moreInfo.RemoteUrl ??= GetParts(UriComponents.HttpRequestUrl, UriFormat.SafeUnescaped); - moreInfo.Hash = chkString.GetHashCode(StringComparison.OrdinalIgnoreCase); + string remoteUrl = moreInfo.RemoteUrl ??= GetParts(UriComponents.HttpRequestUrl, UriFormat.SafeUnescaped); + moreInfo.Hash = remoteUrl.GetHashCode(IsUncOrDosPath ? StringComparison.OrdinalIgnoreCase : StringComparison.Ordinal); } return moreInfo.Hash; } From 4f15c66bfc9485dbe588dee760083c0754f5d02e Mon Sep 17 00:00:00 2001 From: Miha Zupan Date: Mon, 24 Feb 2020 11:06:09 +0100 Subject: [PATCH 3/6] Don't expect Uri.GetHashCode to always be case-insensitive in tests --- .../System.Runtime/tests/System/Uri.MethodsTests.cs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/libraries/System.Runtime/tests/System/Uri.MethodsTests.cs b/src/libraries/System.Runtime/tests/System/Uri.MethodsTests.cs index 633baab3ae544b..6985889eb3fd22 100644 --- a/src/libraries/System.Runtime/tests/System/Uri.MethodsTests.cs +++ b/src/libraries/System.Runtime/tests/System/Uri.MethodsTests.cs @@ -389,15 +389,17 @@ public static IEnumerable Equals_TestData() public void Equals(Uri uri1, object obj, bool expected) { Uri uri2 = obj as Uri; + if (uri1 != null) { Assert.Equal(expected, uri1.Equals(obj)); + if (uri2 != null) { - bool onlyCaseDifference = string.Equals(uri1.OriginalString, uri2.OriginalString, StringComparison.OrdinalIgnoreCase); - Assert.Equal(expected || onlyCaseDifference, uri1.GetHashCode().Equals(uri2.GetHashCode())); + Assert.Equal(expected, uri1.GetHashCode() == uri2.GetHashCode()); } } + if (!(obj is string)) { Assert.Equal(expected, uri1 == uri2); From eca5fd3eaf99e487ad1a770060f318412a627641 Mon Sep 17 00:00:00 2001 From: MihaZupan Date: Mon, 24 Feb 2020 22:52:58 +0100 Subject: [PATCH 4/6] Correct Uri.Equals test Hash codes being equal doesn't mean that the values are equal --- .../System.Runtime/tests/System/Uri.MethodsTests.cs | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/libraries/System.Runtime/tests/System/Uri.MethodsTests.cs b/src/libraries/System.Runtime/tests/System/Uri.MethodsTests.cs index 6985889eb3fd22..8f22f9f90cbf25 100644 --- a/src/libraries/System.Runtime/tests/System/Uri.MethodsTests.cs +++ b/src/libraries/System.Runtime/tests/System/Uri.MethodsTests.cs @@ -396,7 +396,17 @@ public void Equals(Uri uri1, object obj, bool expected) if (uri2 != null) { - Assert.Equal(expected, uri1.GetHashCode() == uri2.GetHashCode()); + bool hashCodesAreEqual = uri1.GetHashCode() == uri2.GetHashCode(); + + if (expected) + { + Assert.True(hashCodesAreEqual); + } + + if (!hashCodesAreEqual) + { + Assert.False(expected); + } } } From 684854d36a9e2d515d102716c2823f044ea4bf96 Mon Sep 17 00:00:00 2001 From: MihaZupan Date: Mon, 2 Mar 2020 01:40:15 +0100 Subject: [PATCH 5/6] Do not cache Uri GetHashCode --- .../System.Private.Uri/src/System/Uri.cs | 20 +++++++++---------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/src/libraries/System.Private.Uri/src/System/Uri.cs b/src/libraries/System.Private.Uri/src/System/Uri.cs index 7420e136eaaafc..88f9df2992f551 100644 --- a/src/libraries/System.Private.Uri/src/System/Uri.cs +++ b/src/libraries/System.Private.Uri/src/System/Uri.cs @@ -149,7 +149,6 @@ private class MoreInfo public string? Query; public string? Fragment; public string? AbsoluteUri; - public int Hash; public string? RemoteUrl; }; @@ -1509,22 +1508,21 @@ public override int GetHashCode() { if (IsNotAbsoluteUri) { - MoreInfo moreInfo = (_info ??= new UriInfo()).MoreInfo ??= new MoreInfo(); - if (moreInfo.Hash == 0) - { - moreInfo.Hash = OriginalString.GetHashCode(); - } - return moreInfo.Hash; + return OriginalString.GetHashCode(); } else { MoreInfo moreInfo = EnsureUriInfo().MoreInfo ??= new MoreInfo(); - if (moreInfo.Hash == 0) + string remoteUrl = moreInfo.RemoteUrl ??= GetParts(UriComponents.HttpRequestUrl, UriFormat.SafeUnescaped); + + if (IsUncOrDosPath) + { + return remoteUrl.GetHashCode(StringComparison.OrdinalIgnoreCase); + } + else { - string remoteUrl = moreInfo.RemoteUrl ??= GetParts(UriComponents.HttpRequestUrl, UriFormat.SafeUnescaped); - moreInfo.Hash = remoteUrl.GetHashCode(IsUncOrDosPath ? StringComparison.OrdinalIgnoreCase : StringComparison.Ordinal); + return remoteUrl.GetHashCode(); } - return moreInfo.Hash; } } From 450be7816fc94b28ec1861d0c79adbb4145ef52e Mon Sep 17 00:00:00 2001 From: MihaZupan Date: Mon, 2 Mar 2020 01:43:09 +0100 Subject: [PATCH 6/6] Simplify Uri.Equals test --- .../tests/System/Uri.MethodsTests.cs | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-) diff --git a/src/libraries/System.Runtime/tests/System/Uri.MethodsTests.cs b/src/libraries/System.Runtime/tests/System/Uri.MethodsTests.cs index 8f22f9f90cbf25..64efed81d4d69c 100644 --- a/src/libraries/System.Runtime/tests/System/Uri.MethodsTests.cs +++ b/src/libraries/System.Runtime/tests/System/Uri.MethodsTests.cs @@ -394,19 +394,9 @@ public void Equals(Uri uri1, object obj, bool expected) { Assert.Equal(expected, uri1.Equals(obj)); - if (uri2 != null) + if (uri2 != null && expected) { - bool hashCodesAreEqual = uri1.GetHashCode() == uri2.GetHashCode(); - - if (expected) - { - Assert.True(hashCodesAreEqual); - } - - if (!hashCodesAreEqual) - { - Assert.False(expected); - } + Assert.Equal(uri1.GetHashCode(), uri2.GetHashCode()); } }