Merge pull request #2112 from ellismg/cache-ucollators

Cache UCollators in a Locale

3 files changed, 124 insertions, 49 deletions

diff --git a/src/corefx/System.Globalization.Native/collation.cpp b/src/corefx/System.Globalization.Native/collation.cpp
index fadaa73eac..fd6e038ea7 100644
--- a/src/corefx/System.Globalization.Native/collation.cpp
+++ b/src/corefx/System.Globalization.Native/collation.cpp
@@ -19,31 +19,89 @@ const int32_t CompareOptionsIgnoreCase = 1;
 // const int32_t CompareOptionsStringSort = 0x20000000;
 
 /*
+ * For increased performance, we cache the UCollator objects for a locale and
+ * share them across threads. This is safe (and supported in ICU) if we ensure
+ * multiple threads are only ever dealing with const UCollators.
+ */
+typedef struct _sort_handle
+{
+    UCollator* regular;
+    UCollator* ignoreCase;
+
+    _sort_handle() : regular(nullptr), ignoreCase(nullptr)
+    {
+    }
+
+} SortHandle;
+
+/*
  * To collator returned by this function is owned by the callee and must be
- *closed when this method returns
- * with a U_SUCCESS UErrorCode.
+ * closed when this method returns with a U_SUCCESS UErrorCode.
  *
  * On error, the return value is undefined.
  */
-UCollator* GetCollatorForLocaleAndOptions(const char* lpLocaleName, int32_t options, UErrorCode* pErr)
+UCollator* CloneCollatorWithOptions(const UCollator* pCollator, int32_t options, UErrorCode* pErr)
+{
+    UCollator* pClonedCollator = ucol_safeClone(pCollator, nullptr, nullptr, pErr);
+
+    if ((options & CompareOptionsIgnoreCase) == CompareOptionsIgnoreCase)
+    {
+        ucol_setAttribute(pClonedCollator, UCOL_STRENGTH, UCOL_SECONDARY, pErr);
+    }
+
+    return pClonedCollator;
+}
+
+extern "C" SortHandle* GetSortHandle(const char* lpLocaleName)
+{
+    SortHandle* pSortHandle = new SortHandle();
+
+    UErrorCode err = U_ZERO_ERROR;
+
+    pSortHandle->regular = ucol_open(lpLocaleName, &err);
+    pSortHandle->ignoreCase = CloneCollatorWithOptions(pSortHandle->regular, CompareOptionsIgnoreCase, &err);
+
+    if (U_FAILURE(err))
+    {
+        if (pSortHandle->regular != nullptr)
+              ucol_close(pSortHandle->regular);
+
+        if (pSortHandle->ignoreCase != nullptr)
+            ucol_close(pSortHandle->ignoreCase);
+
+        delete pSortHandle;
+        pSortHandle = nullptr;
+    }
+
+    return pSortHandle;
+}
+
+extern "C" void CloseSortHandle(SortHandle* pSortHandle)
 {
-    UCollator* pColl = nullptr;
+    ucol_close(pSortHandle->regular);
+    ucol_close(pSortHandle->ignoreCase);
+
+    pSortHandle->regular = nullptr;
+    pSortHandle->ignoreCase = nullptr;
 
-    pColl = ucol_open(lpLocaleName, pErr);
+    delete pSortHandle;
+}
 
+const UCollator* GetCollatorFromSortHandle(const SortHandle* pSortHandle, int32_t options, UErrorCode* pErr)
+{
     if ((options & CompareOptionsIgnoreCase) == CompareOptionsIgnoreCase)
     {
-        ucol_setAttribute(pColl, UCOL_STRENGTH, UCOL_SECONDARY, pErr);
+        return pSortHandle->ignoreCase;
     }
 
-    return pColl;
+    return pSortHandle->regular;
 }
 
 /*
 Function:
 CompareString
 */
-extern "C" int32_t CompareString(const char* lpLocaleName,
+extern "C" int32_t CompareString(const SortHandle* pSortHandle,
                                  const UChar* lpStr1,
                                  int32_t cwStr1Length,
                                  const UChar* lpStr2,
@@ -56,12 +114,11 @@ extern "C" int32_t CompareString(const char* lpLocaleName,
 
     UCollationResult result = UCOL_EQUAL;
     UErrorCode err = U_ZERO_ERROR;
-    UCollator* pColl = GetCollatorForLocaleAndOptions(lpLocaleName, options, &err);
+    const UCollator* pColl = GetCollatorFromSortHandle(pSortHandle, options, &err);
 
     if (U_SUCCESS(err))
     {
         result = ucol_strcoll(pColl, lpStr1, cwStr1Length, lpStr2, cwStr2Length);
-        ucol_close(pColl);
     }
 
     return result;
@@ -72,13 +129,13 @@ Function:
 IndexOf
 */
 extern "C" int32_t
-IndexOf(const char* lpLocaleName, const UChar* lpTarget, int32_t cwTargetLength, const UChar* lpSource, int32_t cwSourceLength, int32_t options)
+IndexOf(const SortHandle* pSortHandle, const UChar* lpTarget, int32_t cwTargetLength, const UChar* lpSource, int32_t cwSourceLength, int32_t options)
 {
     static_assert(USEARCH_DONE == -1, "managed side requires -1 for not found");
 
     int32_t result = USEARCH_DONE;
     UErrorCode err = U_ZERO_ERROR;
-    UCollator* pColl = GetCollatorForLocaleAndOptions(lpLocaleName, options, &err);
+    const UCollator* pColl = GetCollatorFromSortHandle(pSortHandle, options, &err);
 
     if (U_SUCCESS(err))
     {
@@ -89,8 +146,6 @@ IndexOf(const char* lpLocaleName, const UChar* lpTarget, int32_t cwTargetLength,
             result = usearch_first(pSearch, &err);
             usearch_close(pSearch);
         }
-
-        ucol_close(pColl);
     }
 
     return result;
@@ -101,13 +156,13 @@ Function:
 LastIndexOf
 */
 extern "C" int32_t LastIndexOf(
-    const char* lpLocaleName, const UChar* lpTarget, int32_t cwTargetLength, const UChar* lpSource, int32_t cwSourceLength, int32_t options)
+    const SortHandle* pSortHandle, const UChar* lpTarget, int32_t cwTargetLength, const UChar* lpSource, int32_t cwSourceLength, int32_t options)
 {
     static_assert(USEARCH_DONE == -1, "managed side requires -1 for not found");
 
     int32_t result = USEARCH_DONE;
     UErrorCode err = U_ZERO_ERROR;
-    UCollator* pColl = GetCollatorForLocaleAndOptions(lpLocaleName, options, &err);
+    const UCollator* pColl = GetCollatorFromSortHandle(pSortHandle, options, &err);
 
     if (U_SUCCESS(err))
     {
@@ -118,8 +173,6 @@ extern "C" int32_t LastIndexOf(
             result = usearch_last(pSearch, &err);
             usearch_close(pSearch);
         }
-
-        ucol_close(pColl);
     }
 
     return result;
@@ -202,11 +255,11 @@ IndexOfOrdinalIgnoreCase(
  Return value is a "Win32 BOOL" (1 = true, 0 = false)
  */
 extern "C" int32_t StartsWith(
-    const char* lpLocaleName, const UChar* lpTarget, int32_t cwTargetLength, const UChar* lpSource, int32_t cwSourceLength, int32_t options)
+    const SortHandle* pSortHandle, const UChar* lpTarget, int32_t cwTargetLength, const UChar* lpSource, int32_t cwSourceLength, int32_t options)
 {
     int32_t result = FALSE;
     UErrorCode err = U_ZERO_ERROR;
-    UCollator* pColl = GetCollatorForLocaleAndOptions(lpLocaleName, options, &err);
+    const UCollator* pColl = GetCollatorFromSortHandle(pSortHandle, options, &err);
 
     if (U_SUCCESS(err))
     {
@@ -255,8 +308,6 @@ extern "C" int32_t StartsWith(
 
             usearch_close(pSearch);
         }
-
-        ucol_close(pColl);
     }
 
     return result;
@@ -266,11 +317,11 @@ extern "C" int32_t StartsWith(
  Return value is a "Win32 BOOL" (1 = true, 0 = false)
  */
 extern "C" int32_t EndsWith(
-    const char* lpLocaleName, const UChar* lpTarget, int32_t cwTargetLength, const UChar* lpSource, int32_t cwSourceLength, int32_t options)
+    const SortHandle* pSortHandle, const UChar* lpTarget, int32_t cwTargetLength, const UChar* lpSource, int32_t cwSourceLength, int32_t options)
 {
     int32_t result = FALSE;
     UErrorCode err = U_ZERO_ERROR;
-    UCollator* pColl = GetCollatorForLocaleAndOptions(lpLocaleName, options, &err);
+    const UCollator* pColl = GetCollatorFromSortHandle(pSortHandle, options, &err);
 
     if (U_SUCCESS(err))
     {
@@ -290,19 +341,17 @@ extern "C" int32_t EndsWith(
 
                 // TODO (dotnet/corefx#3467): We should do something similar to what
                 // StartsWith does where we can ignore
-                // some collation elements at the end of te string if they are zero.
+                // some collation elements at the end of the string if they are zero.
             }
 
             usearch_close(pSearch);
         }
-
-        ucol_close(pColl);
     }
 
     return result;
 }
 
-extern "C" int32_t GetSortKey(const char* lpLocaleName,
+extern "C" int32_t GetSortKey(const SortHandle* pSortHandle,
                               const UChar* lpStr,
                               int32_t cwStrLength,
                               uint8_t* sortKey,
@@ -310,14 +359,12 @@ extern "C" int32_t GetSortKey(const char* lpLocaleName,
                               int32_t options)
 {
     UErrorCode err = U_ZERO_ERROR;
-    UCollator* pColl = GetCollatorForLocaleAndOptions(lpLocaleName, options, &err);
+    const UCollator* pColl = GetCollatorFromSortHandle(pSortHandle, options, &err);
     int32_t result = 0;
 
     if (U_SUCCESS(err))
     {
         result = ucol_getSortKey(pColl, lpStr, cwStrLength, sortKey, cbSortKeyLength);
-
-        ucol_close(pColl);
     }
 
     return result;
diff --git a/src/mscorlib/corefx/Interop/Unix/System.Globalization.Native/Interop.Collation.cs b/src/mscorlib/corefx/Interop/Unix/System.Globalization.Native/Interop.Collation.cs
index 5b2f03e57c..677c24d07e 100644
--- a/src/mscorlib/corefx/Interop/Unix/System.Globalization.Native/Interop.Collation.cs
+++ b/src/mscorlib/corefx/Interop/Unix/System.Globalization.Native/Interop.Collation.cs
@@ -4,35 +4,64 @@
 using System;
 using System.Globalization;
 using System.Runtime.InteropServices;
+using System.Security;
 
 internal static partial class Interop
 {
     internal static partial class GlobalizationInterop
     {
         [DllImport(Libraries.GlobalizationInterop, CharSet = CharSet.Unicode)]
-        internal unsafe static extern int CompareString(byte[] localeName, char* lpStr1, int cwStr1Len, char* lpStr2, int cwStr2Len, CompareOptions options);
+        internal unsafe static extern SafeSortHandle GetSortHandle(byte[] localeName);
 
         [DllImport(Libraries.GlobalizationInterop, CharSet = CharSet.Unicode)]
-        internal unsafe static extern int IndexOf(byte[] localeName, string target, int cwTargetLength, char* pSource, int cwSourceLength, CompareOptions options);
+        internal unsafe static extern void CloseSortHandle(IntPtr handle);
 
         [DllImport(Libraries.GlobalizationInterop, CharSet = CharSet.Unicode)]
-        internal unsafe static extern int LastIndexOf(byte[] localeName, string target, int cwTargetLength, char* pSource, int cwSourceLength, CompareOptions options);
+        internal unsafe static extern int CompareString(SafeSortHandle sortHandle, char* lpStr1, int cwStr1Len, char* lpStr2, int cwStr2Len, CompareOptions options);
+
+        [DllImport(Libraries.GlobalizationInterop, CharSet = CharSet.Unicode)]
+        internal unsafe static extern int IndexOf(SafeSortHandle sortHandle, string target, int cwTargetLength, char* pSource, int cwSourceLength, CompareOptions options);
+
+        [DllImport(Libraries.GlobalizationInterop, CharSet = CharSet.Unicode)]
+        internal unsafe static extern int LastIndexOf(SafeSortHandle sortHandle, string target, int cwTargetLength, char* pSource, int cwSourceLength, CompareOptions options);
 
         [DllImport(Libraries.GlobalizationInterop, CharSet = CharSet.Unicode)]
         internal unsafe static extern int IndexOfOrdinalIgnoreCase(string target, int cwTargetLength, char* pSource, int cwSourceLength, bool findLast);
 
         [DllImport(Libraries.GlobalizationInterop, CharSet = CharSet.Unicode)]
         [return: MarshalAs(UnmanagedType.Bool)]
-        internal unsafe static extern bool StartsWith(byte[] localeName, string target, int cwTargetLength, string source, int cwSourceLength, CompareOptions options);
+        internal unsafe static extern bool StartsWith(SafeSortHandle sortHandle, string target, int cwTargetLength, string source, int cwSourceLength, CompareOptions options);
 
         [DllImport(Libraries.GlobalizationInterop, CharSet = CharSet.Unicode)]
         [return: MarshalAs(UnmanagedType.Bool)]
-        internal unsafe static extern bool EndsWith(byte[] localeName, string target, int cwTargetLength, string source, int cwSourceLength, CompareOptions options);
+        internal unsafe static extern bool EndsWith(SafeSortHandle sortHandle, string target, int cwTargetLength, string source, int cwSourceLength, CompareOptions options);
 
         [DllImport(Libraries.GlobalizationInterop, CharSet = CharSet.Unicode)]
-        internal unsafe static extern int GetSortKey(byte[] localeName, string str, int strLength, byte* sortKey, int sortKeyLength, CompareOptions options);
+        internal unsafe static extern int GetSortKey(SafeSortHandle sortHandle, string str, int strLength, byte* sortKey, int sortKeyLength, CompareOptions options);
 
         [DllImport(Libraries.GlobalizationInterop, CharSet = CharSet.Unicode)]
         internal unsafe static extern int CompareStringOrdinalIgnoreCase(char* lpStr1, int cwStr1Len, char* lpStr2, int cwStr2Len);
+
+        [SecurityCritical]
+        internal class SafeSortHandle : SafeHandle
+        {
+            private SafeSortHandle() :
+                base(IntPtr.Zero, true)
+            {
+            }
+
+            public override bool IsInvalid
+            {
+                get { return handle == IntPtr.Zero; }
+            }
+
+            [SecurityCritical]
+            protected override bool ReleaseHandle()
+            {
+                CloseSortHandle(handle);
+                SetHandle(IntPtr.Zero);
+                return true;
+            }
+        }
     }
 }
diff --git a/src/mscorlib/corefx/System/Globalization/CompareInfo.Unix.cs b/src/mscorlib/corefx/System/Globalization/CompareInfo.Unix.cs
index d65ac8b4e8..c6ddf8bfd2 100644
--- a/src/mscorlib/corefx/System/Globalization/CompareInfo.Unix.cs
+++ b/src/mscorlib/corefx/System/Globalization/CompareInfo.Unix.cs
@@ -10,14 +10,13 @@ namespace System.Globalization
 {
     public partial class CompareInfo
     {
-        // ICU uses a char* (UTF-8) to represent a locale name.
-        private readonly byte[] m_sortNameAsUtf8;
+        private readonly Interop.GlobalizationInterop.SafeSortHandle m_sortHandle;
 
-        internal unsafe CompareInfo(CultureInfo culture)
+        internal CompareInfo(CultureInfo culture)
         {
             m_name = culture.m_name;
             m_sortName = culture.SortName;
-            m_sortNameAsUtf8 = System.Text.Encoding.UTF8.GetBytes(m_sortName);
+            m_sortHandle = Interop.GlobalizationInterop.GetSortHandle(System.Text.Encoding.UTF8.GetBytes(m_sortName));
         }
 
         internal static unsafe int IndexOfOrdinal(string source, string value, int startIndex, int count, bool ignoreCase)
@@ -136,7 +135,7 @@ namespace System.Globalization
             {
                 fixed (char* pString2 = string2)
                 {
-                    return Interop.GlobalizationInterop.CompareString(m_sortNameAsUtf8, pString1 + offset1, length1, pString2 + offset2, length2, options);
+                    return Interop.GlobalizationInterop.CompareString(m_sortHandle, pString1 + offset1, length1, pString2 + offset2, length2, options);
                 }
             }
         }
@@ -160,7 +159,7 @@ namespace System.Globalization
 
             fixed (char* pSource = source)
             {
-                int index = Interop.GlobalizationInterop.IndexOf(m_sortNameAsUtf8, target, target.Length, pSource + startIndex, count, options);
+                int index = Interop.GlobalizationInterop.IndexOf(m_sortHandle, target, target.Length, pSource + startIndex, count, options);
 
                 return index != -1 ? index + startIndex : -1;
             }
@@ -188,7 +187,7 @@ namespace System.Globalization
 
             fixed (char* pSource = source)
             {
-                int lastIndex = Interop.GlobalizationInterop.LastIndexOf(m_sortNameAsUtf8, target, target.Length, pSource + (startIndex - count + 1), count, options);
+                int lastIndex = Interop.GlobalizationInterop.LastIndexOf(m_sortHandle, target, target.Length, pSource + (startIndex - count + 1), count, options);
 
                 return lastIndex != -1 ? lastIndex + leftStartIndex : -1;
             }
@@ -200,7 +199,7 @@ namespace System.Globalization
             Contract.Assert(!string.IsNullOrEmpty(prefix));
             Contract.Assert((options & (CompareOptions.Ordinal | CompareOptions.OrdinalIgnoreCase)) == 0);
 
-            return Interop.GlobalizationInterop.StartsWith(m_sortNameAsUtf8, prefix, prefix.Length, source, source.Length, options);
+            return Interop.GlobalizationInterop.StartsWith(m_sortHandle, prefix, prefix.Length, source, source.Length, options);
         }
 
         private bool EndsWith(string source, string suffix, CompareOptions options)
@@ -209,7 +208,7 @@ namespace System.Globalization
             Contract.Assert(!string.IsNullOrEmpty(suffix));
             Contract.Assert((options & (CompareOptions.Ordinal | CompareOptions.OrdinalIgnoreCase)) == 0);
 
-            return Interop.GlobalizationInterop.EndsWith(m_sortNameAsUtf8, suffix, suffix.Length, source, source.Length, options);
+            return Interop.GlobalizationInterop.EndsWith(m_sortHandle, suffix, suffix.Length, source, source.Length, options);
         }
 
         // -----------------------------
@@ -221,13 +220,13 @@ namespace System.Globalization
             Contract.Assert(source != null);
             Contract.Assert((options & (CompareOptions.Ordinal | CompareOptions.OrdinalIgnoreCase)) == 0);
 
-            int sortKeyLength = Interop.GlobalizationInterop.GetSortKey(m_sortNameAsUtf8, source, source.Length, null, 0, options);
+            int sortKeyLength = Interop.GlobalizationInterop.GetSortKey(m_sortHandle, source, source.Length, null, 0, options);
 
             // As an optimization, for small sort keys we allocate the buffer on the stack.
             if (sortKeyLength <= 256)
             {
                 byte* pSortKey = stackalloc byte[sortKeyLength];
-                Interop.GlobalizationInterop.GetSortKey(m_sortNameAsUtf8, source, source.Length, pSortKey, sortKeyLength, options);
+                Interop.GlobalizationInterop.GetSortKey(m_sortHandle, source, source.Length, pSortKey, sortKeyLength, options);
                 return InternalHashSortKey(pSortKey, sortKeyLength, false, additionalEntropy);
             }
 
@@ -235,7 +234,7 @@ namespace System.Globalization
 
             fixed(byte* pSortKey = sortKey)
             {
-                Interop.GlobalizationInterop.GetSortKey(m_sortNameAsUtf8, source, source.Length, pSortKey, sortKeyLength, options);
+                Interop.GlobalizationInterop.GetSortKey(m_sortHandle, source, source.Length, pSortKey, sortKeyLength, options);
                 return InternalHashSortKey(pSortKey, sortKeyLength, false, additionalEntropy);
             }
         }