[libcxx-commits] [PATCH] D59572: Fix and speedup __libcpp_locale_guard on Windows

[Tom Anderson via Phabricator via libcxx-commits]

thomasanderson created this revision.
thomasanderson added a reviewer: EricWF.
Herald added a subscriber: ldionne.

The old implementation assumed the POSIX `setlocale()` API where the old
locale is returned.  On Windows, the _new_ locale is returned.  This meant
that `__libcpp_locale_guard` wasn't resetting the locale on destruction.

The new implementation fixes the above issue and takes advantage of
`setlocale(LC_ALL)` to reduce the number of calls, and also avoids setting
the locale at all if it's not necessary.


Repository:
  rCXX libc++

https://reviews.llvm.org/D59572

Files:
  include/__locale


Index: include/__locale
===================================================================
--- include/__locale
+++ include/__locale
@@ -19,6 +19,7 @@
 #include <cctype>
 #include <locale.h>
 #if defined(_LIBCPP_MSVCRT_LIKE)
+# include <cstring>
 # include <support/win32/locale_win32.h>
 #elif defined(_AIX)
 # include <support/ibm/xlocale.h>
@@ -63,28 +64,29 @@
 #elif defined(_LIBCPP_MSVCRT_LIKE)
 struct __libcpp_locale_guard {
     __libcpp_locale_guard(locale_t __l) :
-        __status(_configthreadlocale(_ENABLE_PER_THREAD_LOCALE)),
-        __locale_collate(setlocale(LC_COLLATE, __l.__get_locale())),
-        __locale_ctype(setlocale(LC_CTYPE, __l.__get_locale())),
-        __locale_monetary(setlocale(LC_MONETARY, __l.__get_locale())),
-        __locale_numeric(setlocale(LC_NUMERIC, __l.__get_locale())),
-        __locale_time(setlocale(LC_TIME, __l.__get_locale()))
-        // LC_MESSAGES is not supported on Windows.
-    {}
+        __status(_configthreadlocale(_ENABLE_PER_THREAD_LOCALE)) {
+      // Setting the locale can be expensive even when the locale given is
+      // already the current locale, so do an explicit check to see if the
+      // current locale is already the one we want.
+      char* __lc = setlocale(LC_ALL, nullptr);
+      // If every category is the same, the locale string will simply be the
+      // locale name, otherwise it will be a semicolon-separated string listing
+      // each category.  In the second case, we know at least one category won't
+      // be what we want, so we only have to check the first case.
+      if (strcmp(__l.__get_locale(), __lc) != 0) {
+        __locale_all = _strdup(__lc);
+        setlocale(LC_ALL, __l.__get_locale());
+      }
+    }
     ~__libcpp_locale_guard() {
-        setlocale(LC_COLLATE, __locale_collate);
-        setlocale(LC_CTYPE, __locale_ctype);
-        setlocale(LC_MONETARY, __locale_monetary);
-        setlocale(LC_NUMERIC, __locale_numeric);
-        setlocale(LC_TIME, __locale_time);
-        _configthreadlocale(__status);
+      if (__locale_all) {
+        setlocale(LC_ALL, __locale_all);
+        free(__locale_all);
+      }
+      _configthreadlocale(__status);
     }
     int __status;
-    char* __locale_collate;
-    char* __locale_ctype;
-    char* __locale_monetary;
-    char* __locale_numeric;
-    char* __locale_time;
+    char* __locale_all = nullptr;
 };
 #endif
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D59572.191427.patch
Type: text/x-patch
Size: 2405 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/libcxx-commits/attachments/20190320/cfb41c2f/attachment.bin>