[PATCH] oleaut32: Remove caching from VARIANT_GetLocalisedNumberChars().

Francois Gouget fgouget at codeweavers.com
Thu Aug 5 08:20:45 CDT 2021 

GetLocaleInfoW() already caches the registry information and is fast
enough. This also avoids using out-of-date information after a
SetLocaleInfoW() call.

Signed-off-by: Francois Gouget <fgouget at codeweavers.com>
---
GetLocaleInfoW() already caches the registry data which provides a 40x 
speedup over the no-cache case:
  https://www.winehq.org/pipermail/wine-devel/2021-August/192041.html

If that performance is considered sufficient this alternative to the 
above patch can be used. Its main advantage is that it simplifies the 
code by totally removing caching from VARIANT_GetLocalisedNumberChars().
---
 dlls/oleaut32/tests/vartest.c | 12 +++---------
 dlls/oleaut32/variant.c       | 28 ----------------------------
 2 files changed, 3 insertions(+), 37 deletions(-)

diff --git a/dlls/oleaut32/tests/vartest.c b/dlls/oleaut32/tests/vartest.c
index bed4de469df..89859988f1f 100644
--- a/dlls/oleaut32/tests/vartest.c
+++ b/dlls/oleaut32/tests/vartest.c
@@ -2198,7 +2198,7 @@ static void test_VarParseNumFromStrMisc(void)
       /* But SMONDECIMALSEP has no default! */
       SetLocaleInfoW(LOCALE_USER_DEFAULT, LOCALE_SMONDECIMALSEP, L"");
       hres = wconvert_str(L"3.9", ARRAY_SIZE(rgb), NUMPRS_DECIMAL|NUMPRS_CURRENCY|NUMPRS_USE_ALL, &np, rgb, LOCALE_USER_DEFAULT, 0);
-      todo_wine EXPECTFAIL;
+      EXPECTFAIL;
       SetLocaleInfoW(LOCALE_USER_DEFAULT, LOCALE_SDECIMAL, L".");
       SetLocaleInfoW(LOCALE_USER_DEFAULT, LOCALE_SMONDECIMALSEP, L".");
 
@@ -2206,9 +2206,7 @@ static void test_VarParseNumFromStrMisc(void)
       SetLocaleInfoW(LOCALE_USER_DEFAULT, LOCALE_STHOUSAND, L" ");
 
       hres = wconvert_str(L"1 000", ARRAY_SIZE(rgb), NUMPRS_THOUSANDS, &np, rgb, LOCALE_USER_DEFAULT, 0);
-      if (broken(1)) /* FIXME Reenable once Wine is less broken */
       EXPECT(1,NUMPRS_THOUSANDS,NUMPRS_THOUSANDS,5,0,3);
-      todo_wine ok(np.dwOutFlags == NUMPRS_THOUSANDS, "Got dwOutFlags=%08x\n", np.dwOutFlags);
       EXPECTRGB(0,1); /* Don't test extra digits, see "1,000" test */
       EXPECTRGB(4,FAILDIG);
 
@@ -2225,9 +2223,7 @@ static void test_VarParseNumFromStrMisc(void)
       SetLocaleInfoW(LOCALE_USER_DEFAULT, LOCALE_SMONTHOUSANDSEP, L" ");
 
       hres = wconvert_str(L"1|000", ARRAY_SIZE(rgb), NUMPRS_THOUSANDS, &np, rgb, LOCALE_USER_DEFAULT, 0);
-      if (broken(1)) /* FIXME Reenable once Wine is less broken */
       EXPECT(1,NUMPRS_THOUSANDS,NUMPRS_THOUSANDS,5,0,3);
-      todo_wine ok(np.dwOutFlags == NUMPRS_THOUSANDS, "Got dwOutFlags=%08x\n", np.dwOutFlags);
       EXPECTRGB(0,1); /* Don't test extra digits, see "1,000" test */
       EXPECTRGB(4,FAILDIG);
 
@@ -2235,9 +2231,7 @@ static void test_VarParseNumFromStrMisc(void)
       EXPECTFAIL;
 
       hres = wconvert_str(L"1|000", ARRAY_SIZE(rgb), NUMPRS_THOUSANDS|NUMPRS_CURRENCY, &np, rgb, LOCALE_USER_DEFAULT, 0);
-      if (broken(1)) /* FIXME Reenable once Wine is less broken */
       EXPECT(1,NUMPRS_THOUSANDS|NUMPRS_CURRENCY,NUMPRS_THOUSANDS,5,0,3);
-      todo_wine ok(np.dwOutFlags == NUMPRS_THOUSANDS, "Got dwOutFlags=%08x\n", np.dwOutFlags);
       EXPECTRGB(0,1); /* Don't test extra digits, see "1,000" test */
       EXPECTRGB(4,FAILDIG);
 
@@ -2298,14 +2292,14 @@ static void test_VarParseNumFromStrMisc(void)
 
       SetLocaleInfoW(LOCALE_USER_DEFAULT, LOCALE_STHOUSAND, L"\xa0");
       hres = wconvert_str(L"1 000", ARRAY_SIZE(rgb), NUMPRS_THOUSANDS|NUMPRS_CURRENCY|NUMPRS_USE_ALL, &np, rgb, LOCALE_USER_DEFAULT, 0);
-      todo_wine EXPECT(1,NUMPRS_THOUSANDS|NUMPRS_CURRENCY|NUMPRS_USE_ALL,NUMPRS_THOUSANDS,5,0,3);
+      EXPECT(1,NUMPRS_THOUSANDS|NUMPRS_CURRENCY|NUMPRS_USE_ALL,NUMPRS_THOUSANDS,5,0,3);
       EXPECTRGB(0,1); /* Don't test extra digits, see "1,000" test */
       EXPECTRGB(4,FAILDIG);
 
 
       /* Regular thousands separators also have precedence over the currency ones */
       hres = wconvert_str(L"1\xa0\x30\x30\x30", ARRAY_SIZE(rgb), NUMPRS_THOUSANDS|NUMPRS_CURRENCY|NUMPRS_USE_ALL, &np, rgb, LOCALE_USER_DEFAULT, 0);
-      todo_wine EXPECT(1,NUMPRS_THOUSANDS|NUMPRS_CURRENCY|NUMPRS_USE_ALL,NUMPRS_THOUSANDS,5,0,3);
+      EXPECT(1,NUMPRS_THOUSANDS|NUMPRS_CURRENCY|NUMPRS_USE_ALL,NUMPRS_THOUSANDS,5,0,3);
       EXPECTRGB(0,1); /* Don't test extra digits, see "1,000" test */
       EXPECTRGB(4,FAILDIG);
 
diff --git a/dlls/oleaut32/variant.c b/dlls/oleaut32/variant.c
index e4993de75a5..14ba63b9b2e 100644
--- a/dlls/oleaut32/variant.c
+++ b/dlls/oleaut32/variant.c
@@ -42,14 +42,6 @@
 
 WINE_DEFAULT_DEBUG_CHANNEL(variant);
 
-static CRITICAL_SECTION cache_cs;
-static CRITICAL_SECTION_DEBUG critsect_debug =
-{
-    0, 0, &cache_cs,
-    { &critsect_debug.ProcessLocksList, &critsect_debug.ProcessLocksList },
-      0, 0, { (DWORD_PTR)(__FILE__ ": cache_cs") }
-};
-static CRITICAL_SECTION cache_cs = { &critsect_debug, -1, 0, 0, 0, 0 };
 
 /* Convert a variant from one type to another */
 static inline HRESULT VARIANT_Coerce(VARIANTARG* pd, LCID lcid, USHORT wFlags,
@@ -1519,24 +1511,9 @@ HRESULT WINAPI VarUdateFromDate(DATE dateIn, ULONG dwFlags, UDATE *lpUdate)
 static void VARIANT_GetLocalisedNumberChars(VARIANT_NUMBER_CHARS *lpChars, LCID lcid, DWORD dwFlags)
 {
   static const VARIANT_NUMBER_CHARS defaultChars = { '-','+','.',',','$',0,'.',',' };
-  static VARIANT_NUMBER_CHARS lastChars;
-  static LCID lastLcid = -1;
-  static DWORD lastFlags = 0;
   LCTYPE lctype = dwFlags & LOCALE_NOUSEROVERRIDE;
   WCHAR buff[4];
 
-  /* To make caching thread-safe, a critical section is needed */
-  EnterCriticalSection(&cache_cs);
-
-  /* Asking for default locale entries is very expensive: It is a registry
-     server call. So cache one locally, as Microsoft does it too */
-  if(lcid == lastLcid && dwFlags == lastFlags)
-  {
-    memcpy(lpChars, &lastChars, sizeof(defaultChars));
-    LeaveCriticalSection(&cache_cs);
-    return;
-  }
-
   memcpy(lpChars, &defaultChars, sizeof(defaultChars));
   GET_NUMBER_TEXT(LOCALE_SNEGATIVESIGN, cNegativeSymbol);
   GET_NUMBER_TEXT(LOCALE_SPOSITIVESIGN, cPositiveSymbol);
@@ -1556,11 +1533,6 @@ static void VARIANT_GetLocalisedNumberChars(VARIANT_NUMBER_CHARS *lpChars, LCID
   }
   TRACE("lcid 0x%x, cCurrencyLocal=%d,%d %s\n", lcid, lpChars->cCurrencyLocal,
         lpChars->cCurrencyLocal2, wine_dbgstr_w(buff));
-
-  memcpy(&lastChars, lpChars, sizeof(defaultChars));
-  lastLcid = lcid;
-  lastFlags = dwFlags;
-  LeaveCriticalSection(&cache_cs);
 }
 
 /* Number Parsing States */
-- 
2.20.1

More information about the wine-devel mailing list