Remove always defined FEATURE_RANDOMIZE_STRING_HASHING define (dotnet…

…#13491) * Remove always defined FEATURE_RANDOMIZE_STRING_HASHING * Fully remove randomized hashing * Leftovers * Remove additionalEntropy from HashSortKey as it was always 0 * Remove additionalEntropy from HashString as it was always 0 * Change to private, as not in fact called by reflection in .NET Core * Fix build break due to FCDECLn * Revert removed QCALL * Remove unused strlen parameter
dotnet-bot · Aug 28, 2017 · 5c07c5a · 5c07c5a
1 parent 0db6d91
commit 5c07c5a
Show file tree

Hide file tree

Showing 26 changed files with 38 additions and 331 deletions.
diff --git a/clr.coreclr.props b/clr.coreclr.props
@@ -45,7 +45,6 @@
     <FeatureCominteropWinRTManagedActivation>true</FeatureCominteropWinRTManagedActivation>
     <FeatureLazyCOWPages Condition="('$(TargetArch)' == 'i386') or ('$(TargetArch)' == 'arm')">true</FeatureLazyCOWPages>
     <FeatureLegacyNetCFDbgHostControl>true</FeatureLegacyNetCFDbgHostControl>
-    <FeatureRandomizedStringHashing>true</FeatureRandomizedStringHashing>
     <!-- The rejit feature is available only on supported architectures (x86 & x64) -->
     <FeatureReJIT Condition="('$(TargetArch)' == 'i386') or ('$(TargetArch)' == 'amd64')">true</FeatureReJIT>
     <FeatureManagedEtw>false</FeatureManagedEtw>

diff --git a/clr.defines.targets b/clr.defines.targets
@@ -25,7 +25,6 @@
         <DefineConstants Condition="'$(FeaturePathCompat)' == 'true'">$(DefineConstants);FEATURE_PATHCOMPAT</DefineConstants>
         <DefineConstants Condition="'$(FeaturePerfTracing)' == 'true'">$(DefineConstants);FEATURE_PERFTRACING</DefineConstants>
         <DefineConstants Condition="'$(FeatureXplatEventSource)' == 'true'">$(DefineConstants);FEATURE_EVENTSOURCE_XPLAT</DefineConstants>
-        <DefineConstants Condition="'$(FeatureRandomizedStringHashing)' == 'true'">$(DefineConstants);FEATURE_RANDOMIZED_STRING_HASHING</DefineConstants>
         <DefineConstants Condition="'$(FeatureSortTables)' == 'true'">$(DefineConstants);FEATURE_SORT_TABLES</DefineConstants>
         <DefineConstants Condition="'$(FeatureTypeEquivalence)' == 'true'">$(DefineConstants);FEATURE_TYPEEQUIVALENCE</DefineConstants>
         <DefineConstants Condition="'$(FeatureUseLcid)' == 'true'">$(DefineConstants);FEATURE_USE_LCID</DefineConstants>

diff --git a/clrdefinitions.cmake b/clrdefinitions.cmake
@@ -169,7 +169,6 @@ if(CLR_CMAKE_PLATFORM_LINUX)
     add_definitions(-DFEATURE_PERFMAP)
 endif(CLR_CMAKE_PLATFORM_LINUX)
 add_definitions(-DFEATURE_PREJIT)
-add_definitions(-DFEATURE_RANDOMIZED_STRING_HASHING)
 
 add_definitions(-DFEATURE_READYTORUN)
 set(FEATURE_READYTORUN 1)

diff --git a/crossgen.cmake b/crossgen.cmake
@@ -13,7 +13,6 @@ remove_definitions(
     -DFEATURE_LOADER_OPTIMIZATION
     -DFEATURE_MULTICOREJIT
     -DFEATURE_PERFMAP
-    -DFEATURE_RANDOMIZED_STRING_HASHING
     -DFEATURE_REJIT
     -DFEATURE_TIERED_COMPILATION
     -DFEATURE_VERSIONING_LOG

diff --git a/src/classlibnative/bcltype/stringnative.cpp b/src/classlibnative/bcltype/stringnative.cpp
@@ -155,15 +155,13 @@ FCIMPL4(Object *, COMString::StringInitCharPtrPartial, StringObject *stringThis,
 }
 FCIMPLEND
 
-#ifdef FEATURE_RANDOMIZED_STRING_HASHING
-
 inline COMNlsHashProvider * GetCurrentNlsHashProvider()
 {
     LIMITED_METHOD_CONTRACT;
     return &COMNlsHashProvider::s_NlsHashProvider;
 }
 
-FCIMPL3(INT32, COMString::Marvin32HashString, StringObject* thisRefUNSAFE, INT32 strLen, INT64 additionalEntropy) {
+FCIMPL1(INT32, COMString::Marvin32HashString, StringObject* thisRefUNSAFE) {
     FCALL_CONTRACT;
 
     int iReturnHash = 0;
@@ -173,7 +171,7 @@ FCIMPL3(INT32, COMString::Marvin32HashString, StringObject* thisRefUNSAFE, INT32
     }
 
     BEGIN_SO_INTOLERANT_CODE_NOTHROW(GetThread(), FCThrow(kStackOverflowException));
-    iReturnHash = GetCurrentNlsHashProvider()->HashString(thisRefUNSAFE->GetBuffer(), thisRefUNSAFE->GetStringLength(), TRUE, additionalEntropy);
+    iReturnHash = GetCurrentNlsHashProvider()->HashString(thisRefUNSAFE->GetBuffer(), thisRefUNSAFE->GetStringLength());
     END_SO_INTOLERANT_CODE;
 
     FC_GC_POLL_RET();
@@ -182,21 +180,6 @@ FCIMPL3(INT32, COMString::Marvin32HashString, StringObject* thisRefUNSAFE, INT32
 }
 FCIMPLEND
 
-BOOL QCALLTYPE COMString::UseRandomizedHashing() {
-    QCALL_CONTRACT;
-
-    BOOL bUseRandomizedHashing = FALSE;
-
-    BEGIN_QCALL;
-
-    bUseRandomizedHashing = GetCurrentNlsHashProvider()->GetUseRandomHashing();
-
-    END_QCALL;
-
-    return bUseRandomizedHashing;
-}
-#endif
-
 /*===============================IsFastSort===============================
 **Action: Call the helper to walk the string and see if we have any high chars.
 **Returns: void.  The appropriate bits are set on the String.

diff --git a/src/classlibnative/bcltype/stringnative.h b/src/classlibnative/bcltype/stringnative.h
@@ -85,10 +85,7 @@ class COMString {
     static FCDECL2(VOID,        FCSetTrailByte,    StringObject* thisRefUNSAFE, UINT8 bData);
 #endif // FEATURE_COMINTEROP
 
-#ifdef FEATURE_RANDOMIZED_STRING_HASHING
-    static FCDECL3(INT32, Marvin32HashString, StringObject* thisRefUNSAFE, INT32 strLen, INT64 additionalEntropy);
-    static BOOL QCALLTYPE UseRandomizedHashing();
-#endif // FEATURE_RANDOMIZED_STRING_HASHING
+    static FCDECL1(INT32, Marvin32HashString, StringObject* thisRefUNSAFE);
 
 };
 

diff --git a/src/classlibnative/inc/nlsinfo.h b/src/classlibnative/inc/nlsinfo.h
@@ -54,7 +54,7 @@ class COMNlsInfo
     //  Native helper functions for CultureData
     //
 
-    static INT32 QCALLTYPE InternalGetGlobalizedHashCode(INT_PTR handle, LPCWSTR localeName, LPCWSTR pString, INT32 length, INT32 dwFlagsIn, INT64 additionalEntropy);
+    static INT32 QCALLTYPE InternalGetGlobalizedHashCode(INT_PTR handle, LPCWSTR localeName, LPCWSTR pString, INT32 length, INT32 dwFlagsIn);
 
     //
     //  Native helper function for methods in EncodingTable

diff --git a/src/classlibnative/nls/nlsinfo.cpp b/src/classlibnative/nls/nlsinfo.cpp
@@ -91,7 +91,7 @@ INT32 COMNlsInfo::CallGetUserDefaultUILanguage()
 //  InternalGetGlobalizedHashCode
 //
 ////////////////////////////////////////////////////////////////////////////
-INT32 QCALLTYPE COMNlsInfo::InternalGetGlobalizedHashCode(INT_PTR handle, LPCWSTR localeName, LPCWSTR string, INT32 length, INT32 dwFlagsIn, INT64 additionalEntropy)
+INT32 QCALLTYPE COMNlsInfo::InternalGetGlobalizedHashCode(INT_PTR handle, LPCWSTR localeName, LPCWSTR string, INT32 length, INT32 dwFlagsIn)
 {
     CONTRACTL
     {
@@ -140,7 +140,7 @@ INT32 QCALLTYPE COMNlsInfo::InternalGetGlobalizedHashCode(INT_PTR handle, LPCWST
             NewApis::LCMapStringEx(handle != NULL ? NULL : localeName, dwFlags, string, length, (LPWSTR)pByte, byteCount, NULL,NULL, (LPARAM) handle);
         }
 
-        iReturnHash = COMNlsHashProvider::s_NlsHashProvider.HashSortKey(pByte, byteCount, true, additionalEntropy);
+        iReturnHash = COMNlsHashProvider::s_NlsHashProvider.HashSortKey(pByte, byteCount);
     }
     END_QCALL;
     return(iReturnHash);

diff --git a/src/inc/MSCOREE.IDL b/src/inc/MSCOREE.IDL
@@ -164,7 +164,7 @@ typedef enum {
   STARTUP_ARM                   = 0x400000,                     // Enable the ARM feature.
   STARTUP_SINGLE_APPDOMAIN      = 0x800000,                      // application runs in default domain, no more domains are created 
   STARTUP_APPX_APP_MODEL        = 0x1000000,                     // jupiter app
-  STARTUP_DISABLE_RANDOMIZED_STRING_HASHING     = 0x2000000      // Disable the randomized string hashing
+  STARTUP_DISABLE_RANDOMIZED_STRING_HASHING     = 0x2000000      // Disable the randomized string hashing (not supported)
 } STARTUP_FLAGS;
 
 typedef enum {

diff --git a/src/inc/clrconfigvalues.h b/src/inc/clrconfigvalues.h
@@ -164,9 +164,6 @@ RETAIL_CONFIG_DWORD_INFO(EXTERNAL_NetFx45_LegacyManagedDeflateStream, W("NetFx45
 RETAIL_CONFIG_DWORD_INFO(EXTERNAL_DateTime_NetFX35ParseMode, W("DateTime_NetFX35ParseMode"), 0, "Flag to enable the .NET 3.5 System.DateTime Token Replacement Policy")
 RETAIL_CONFIG_DWORD_INFO(EXTERNAL_ThrowUnobservedTaskExceptions, W("ThrowUnobservedTaskExceptions"), 0, "Flag to propagate unobserved task exceptions on the finalizer thread.")
 RETAIL_CONFIG_DWORD_INFO(EXTERNAL_DateTime_NetFX40AmPmParseAdjustment, W("EnableAmPmParseAdjustment"), 0, "Flag to enable the .NET 4.0 DateTimeParse to correctly parse AM/PM cases")
-#ifdef FEATURE_RANDOMIZED_STRING_HASHING
-RETAIL_CONFIG_DWORD_INFO(EXTERNAL_UseRandomizedStringHashAlgorithm, W("UseRandomizedStringHashAlgorithm"), 0, "Flag to use a string hashing algorithm who's behavior differs between AppDomains")
-#endif // FEATURE_RANDOMIZED_STRING_HASHING
 
 // 
 // Conditional breakpoints

diff --git a/src/mscorlib/src/System/AppDomainSetup.cs b/src/mscorlib/src/System/AppDomainSetup.cs
@@ -63,10 +63,6 @@ internal enum LoaderInformation
         // to an AppDomain. We only use the keys, the values are ignored.
         private Dictionary<string, object> _CompatFlags;
 
-#if FEATURE_RANDOMIZED_STRING_HASHING
-        private bool _UseRandomizedStringHashing;
-#endif
-
         internal AppDomainSetup(AppDomainSetup copy, bool copyDomainBoundData)
         {
             string[] mine = Value;
@@ -92,11 +88,6 @@ internal AppDomainSetup(AppDomainSetup copy, bool copyDomainBoundData)
                 {
                     SetCompatibilitySwitches(copy._CompatFlags.Keys);
                 }
-
-#if FEATURE_RANDOMIZED_STRING_HASHING
-                _UseRandomizedStringHashing = copy._UseRandomizedStringHashing;
-#endif
-
             }
         }
 
@@ -157,20 +148,11 @@ internal Dictionary<string, object> GetCompatibilityFlags()
 
         public void SetCompatibilitySwitches(IEnumerable<String> switches)
         {
-#if FEATURE_RANDOMIZED_STRING_HASHING
-            _UseRandomizedStringHashing = false;
-#endif
             if (switches != null)
             {
                 _CompatFlags = new Dictionary<string, object>();
                 foreach (String str in switches)
                 {
-#if FEATURE_RANDOMIZED_STRING_HASHING
-                    if (StringComparer.OrdinalIgnoreCase.Equals("UseRandomizedStringHashAlgorithm", str))
-                    {
-                        _UseRandomizedStringHashing = true;
-                    }
-#endif
                     _CompatFlags.Add(str, null);
                 }
             }

diff --git a/src/mscorlib/src/System/Collections/Concurrent/ConcurrentDictionary.cs b/src/mscorlib/src/System/Collections/Concurrent/ConcurrentDictionary.cs
@@ -636,11 +636,7 @@ private bool TryAddInternal(TKey key, TValue value, bool updateIfExists, bool ac
                 //
                 if (resizeDesired)
                 {
-#if FEATURE_RANDOMIZED_STRING_HASHING
                     GrowTable(tables, tables.m_comparer, false, m_keyRehashCount);
-#else
-                    GrowTable(tables, tables.m_comparer, false, m_keyRehashCount);
-#endif
                 }
 
                 resultingValue = value;

diff --git a/src/mscorlib/src/System/Collections/Generic/Dictionary.cs b/src/mscorlib/src/System/Collections/Generic/Dictionary.cs
@@ -92,12 +92,10 @@ public Dictionary(int capacity, IEqualityComparer<TKey> comparer)
             if (capacity > 0) Initialize(capacity);
             this.comparer = comparer ?? EqualityComparer<TKey>.Default;
 
-#if FEATURE_RANDOMIZED_STRING_HASHING
-            if (HashHelpers.s_UseRandomizedStringHashing && this.comparer == EqualityComparer<string>.Default)
+            if (this.comparer == EqualityComparer<string>.Default)
             {
                 this.comparer = (IEqualityComparer<TKey>)NonRandomizedStringEqualityComparer.Default;
             }
-#endif // FEATURE_RANDOMIZED_STRING_HASHING
         }
 
         public Dictionary(IDictionary<TKey, TValue> dictionary) : this(dictionary, null) { }
@@ -408,9 +406,7 @@ private bool TryInsert(TKey key, TValue value, InsertionBehavior behavior)
             if (buckets == null) Initialize(0);
             int hashCode = comparer.GetHashCode(key) & 0x7FFFFFFF;
             int targetBucket = hashCode % buckets.Length;            
-#if FEATURE_RANDOMIZED_STRING_HASHING
             int collisionCount = 0;
-#endif
 
             for (int i = buckets[targetBucket]; i >= 0; i = entries[i].next)
             {
@@ -430,9 +426,8 @@ private bool TryInsert(TKey key, TValue value, InsertionBehavior behavior)
 
                     return false;
                 }
-#if FEATURE_RANDOMIZED_STRING_HASHING
+
                 collisionCount++;
-#endif
             }
             int index;
             if (freeCount > 0)
@@ -459,18 +454,14 @@ private bool TryInsert(TKey key, TValue value, InsertionBehavior behavior)
             buckets[targetBucket] = index;
             version++;
 
-#if FEATURE_RANDOMIZED_STRING_HASHING
-            // In case we hit the collision threshold we'll need to switch to the comparer which is using randomized string hashing
-            // in this case will be EqualityComparer<string>.Default.
-            // Note, randomized string hashing is turned on by default on coreclr so EqualityComparer<string>.Default will 
-            // be using randomized string hashing
+            // If we hit the collision threshold we'll need to switch to the comparer which is using randomized string hashing
+            // i.e. EqualityComparer<string>.Default.
 
             if (collisionCount > HashHelpers.HashCollisionThreshold && comparer == NonRandomizedStringEqualityComparer.Default)
             {
                 comparer = (IEqualityComparer<TKey>)EqualityComparer<string>.Default;
                 Resize(entries.Length, true);
             }
-#endif
 
             return true;
         }

diff --git a/src/mscorlib/src/System/Collections/Generic/EqualityComparer.cs b/src/mscorlib/src/System/Collections/Generic/EqualityComparer.cs
@@ -272,11 +272,8 @@ public override int GetHashCode() =>
             GetType().GetHashCode();
     }
 
-    // NonRandomizedStringEqualityComparer is the comparer used by default with the Dictionary<string,...> 
-    // As the randomized string hashing is turned on by default on coreclr, we need to keep the performance not affected 
-    // as much as possible in the main stream scenarios like Dictionary<string,>
     // We use NonRandomizedStringEqualityComparer as default comparer as it doesnt use the randomized string hashing which 
-    // keep the perofrmance not affected till we hit collision threshold and then we switch to the comparer which is using 
+    // keeps the performance unaffected till we hit collision threshold and then we switch to the comparer which is using 
     // randomized string hashing GenericEqualityComparer<string>
     [Serializable]
     [System.Runtime.CompilerServices.TypeForwardedFrom("mscorlib, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089")] 

diff --git a/src/mscorlib/src/System/Collections/Hashtable.cs b/src/mscorlib/src/System/Collections/Hashtable.cs
@@ -1392,10 +1392,7 @@ internal class HashtableDebugView
     [FriendAccessAllowed]
     internal static class HashHelpers
     {
-#if FEATURE_RANDOMIZED_STRING_HASHING
         public const int HashCollisionThreshold = 100;
-        public static bool s_UseRandomizedStringHashing = String.UseRandomizedHashing();
-#endif
 
         // Table of prime numbers to use as hash table sizes. 
         // A typical resize algorithm would pick the smallest prime number in this array

diff --git a/src/mscorlib/src/System/Globalization/CompareInfo.Unix.cs b/src/mscorlib/src/System/Globalization/CompareInfo.Unix.cs
@@ -137,14 +137,6 @@ internal static unsafe int LastIndexOfOrdinalCore(string source, string value, i
             return -1;
         }
 
-        private int GetHashCodeOfStringCore(string source, CompareOptions options)
-        {
-            Debug.Assert(source != null);
-            Debug.Assert((options & (CompareOptions.Ordinal | CompareOptions.OrdinalIgnoreCase)) == 0);
-
-            return GetHashCodeOfStringCore(source, options, forceRandomizedHashing: false, additionalEntropy: 0);
-        }
-
         private static unsafe int CompareStringOrdinalIgnoreCase(char* string1, int count1, char* string2, int count2)
         {
             Debug.Assert(!GlobalizationMode.Invariant);
@@ -357,7 +349,7 @@ private unsafe static bool IsSortable(char *text, int length)
         // ---- PAL layer ends here ----
         // -----------------------------
 
-        internal unsafe int GetHashCodeOfStringCore(string source, CompareOptions options, bool forceRandomizedHashing, long additionalEntropy)
+        internal unsafe int GetHashCodeOfStringCore(string source, CompareOptions options)
         {
             Debug.Assert(!_invariantMode);
 
@@ -376,21 +368,21 @@ internal unsafe int GetHashCodeOfStringCore(string source, CompareOptions option
             {
                 byte* pSortKey = stackalloc byte[sortKeyLength];
                 Interop.GlobalizationInterop.GetSortKey(_sortHandle, source, source.Length, pSortKey, sortKeyLength, options);
-                return InternalHashSortKey(pSortKey, sortKeyLength, false, additionalEntropy);
+                return InternalHashSortKey(pSortKey, sortKeyLength);
             }
 
             byte[] sortKey = new byte[sortKeyLength];
 
             fixed (byte* pSortKey = sortKey)
             {
                 Interop.GlobalizationInterop.GetSortKey(_sortHandle, source, source.Length, pSortKey, sortKeyLength, options);
-                return InternalHashSortKey(pSortKey, sortKeyLength, false, additionalEntropy);
+                return InternalHashSortKey(pSortKey, sortKeyLength);
             }
         }
 
         [DllImport(JitHelpers.QCall)]
         [SuppressUnmanagedCodeSecurity]
-        private static unsafe extern int InternalHashSortKey(byte* sortKey, int sortKeyLength, [MarshalAs(UnmanagedType.Bool)] bool forceRandomizedHashing, long additionalEntropy);
+        private static unsafe extern int InternalHashSortKey(byte* sortKey, int sortKeyLength);
 
         private static CompareOptions GetOrdinalCompareOptions(CompareOptions options)
         {

diff --git a/src/mscorlib/src/System/Globalization/CompareInfo.Windows.cs b/src/mscorlib/src/System/Globalization/CompareInfo.Windows.cs
@@ -92,7 +92,7 @@ private unsafe int GetHashCodeOfStringCore(string source, CompareOptions options
             int flags = GetNativeCompareFlags(options);
             int tmpHash = 0;
 #if CORECLR
-            tmpHash = InternalGetGlobalizedHashCode(_sortHandle, _sortName, source, source.Length, flags, 0);
+            tmpHash = InternalGetGlobalizedHashCode(_sortHandle, _sortName, source, source.Length, flags);
 #else
             fixed (char* pSource = source)
             {
@@ -481,7 +481,7 @@ private unsafe SortVersion GetSortVersion()
         // Get a locale sensitive sort hash code from native code -- COMNlsInfo::InternalGetGlobalizedHashCode
         [DllImport(JitHelpers.QCall, CharSet = CharSet.Unicode)]
         [SuppressUnmanagedCodeSecurity]
-        private static extern int InternalGetGlobalizedHashCode(IntPtr handle, string localeName, string source, int length, int dwFlags, long additionalEntropy);
+        private static extern int InternalGetGlobalizedHashCode(IntPtr handle, string localeName, string source, int length, int dwFlags);
 #endif
     }
 }
diff --git a/src/mscorlib/src/System/String.Comparison.cs b/src/mscorlib/src/System/String.Comparison.cs
@@ -1002,34 +1002,14 @@ public static bool Equals(String a, String b, StringComparison comparisonType)
             return !String.Equals(a, b);
         }
 
-#if FEATURE_RANDOMIZED_STRING_HASHING
-        // Do not remove!
-        // This method is called by reflection in System.Xml
         [MethodImplAttribute(MethodImplOptions.InternalCall)]
-        internal static extern int InternalMarvin32HashString(string s, int strLen, long additionalEntropy);
-
-        internal static bool UseRandomizedHashing()
-        {
-            return InternalUseRandomizedHashing();
-        }
-
-        [System.Security.SuppressUnmanagedCodeSecurity]
-        [DllImport(JitHelpers.QCall, CharSet = CharSet.Unicode)]
-        private static extern bool InternalUseRandomizedHashing();
-#endif
+        private static extern int InternalMarvin32HashString(string s);
 
         // Gets a hash code for this string.  If strings A and B are such that A.Equals(B), then
         // they will return the same hash code.
         public override int GetHashCode()
         {
-#if FEATURE_RANDOMIZED_STRING_HASHING
-            if (HashHelpers.s_UseRandomizedStringHashing)
-            {
-                return InternalMarvin32HashString(this, this.Length, 0);
-            }
-#endif // FEATURE_RANDOMIZED_STRING_HASHING
-
-            return GetLegacyNonRandomizedHashCode();
+            return InternalMarvin32HashString(this);
         }
 
         // Gets a hash code for this string and this comparison. If strings A and B and comparition C are such

diff --git a/src/pal/prebuilt/inc/mscoree.h b/src/pal/prebuilt/inc/mscoree.h
@@ -317,7 +317,7 @@ enum __MIDL___MIDL_itf_mscoree_0000_0000_0002
         STARTUP_ARM	= 0x400000,
         STARTUP_SINGLE_APPDOMAIN	= 0x800000,
         STARTUP_APPX_APP_MODEL	= 0x1000000,
-        STARTUP_DISABLE_RANDOMIZED_STRING_HASHING	= 0x2000000
+        STARTUP_DISABLE_RANDOMIZED_STRING_HASHING	= 0x2000000 // not supported
     } 	STARTUP_FLAGS;
 
 typedef /* [public] */