Re: [PATCH] Fix severe performance regression with gettext 0.20+ on Windows

On 10.12.25 01:45, Bryan Green wrote:
> The attached patch takes a pragmatic approach: for gettext 0.20.1+, we
> avoid triggering the bug by using Windows locale format instead of
> calling IsoLocaleName(). This works because gettext 0.20.1+ internally
> converts the Windows format back to POSIX for catalog lookups, whereas
> 0.19.8 and earlier need POSIX format directly.

A few comments on the patch itself:

1) The description from the commit message, 'gettext 0.20.1+ expects
Windows locale format ("English_United States") not POSIX format
("en_US")', or similar, should be added as a comment to the code.

2) Is the cutoff actually 0.20.1 or should it be 0.20?

3) Similarly some comment about why "C" and "POSIX" are handled specially.

4) There is already earlier in pg_perm_setlocale() code that handles
LC_MESSAGES specially on Windows, and it deals with the (locale == NULL
|| locale[0] == '\0') case, which is then repeated later on in your
patch. I wonder if this could be simplified or combined somehow.

5) With the patch applied, building with a new-enough gettext will leave
the function IsoLocaleName() unused, which could result in a compiler
warning. You could add pg_attribute_unused() to avoid that.