Re: [PATCH] Cache localised number chars

[Robert Shearman]

Michael Karcher wrote:
> 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];
> -
>
> /* Asking for default locale entries is very expensive: It is a registry
> -
> server call. So cache one localy, as Microsoft does it too */
> -
>
> if(lcid == lastLcid && dwFlags == lastFlags)
> -
>
> {
> -
>
> memcpy(lpChars, &lastChars, sizeof(defaultChars));
> -
>
> return;
> -
>
> }
> -

This introduces a race condition.

[PATCH] Cache localised number chars

[Michael Karcher]

Am Dienstag, den 13.05.2008, 18:57 +0100 schrieb Robert Shearman:
> Michael Karcher wrote:
> > 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];
> > -
> >
> > /* Asking for default locale entries is very expensive: It is a registry
> > -
> > server call. So cache one localy, as Microsoft does it too */
> > -
> >
> > if(lcid == lastLcid && dwFlags == lastFlags)
> > -
> >
> > {
> > -
> >
> > memcpy(lpChars, &lastChars, sizeof(defaultChars));
> > -
> >
> > return;
> > -
> >
> > }
> > -
>
> This introduces a race condition.

Oops, you seem to be right. Thanks for spotting it! I suppose wrapping
the whole code into a critical section will fix it. I will resubmit the
patch with a critical section (It still will help. Without the critical
section, I measured a 1000x speedup on VarI2FromBstr, so even with the
section, this patch will add a lot of performance to wine.)

Kind regards,
  Michael Karcher