Horton
Menu

Re: ICU for global collation

Peter Eisentraut <peter.eisentraut@enterprisedb.com>

View thread
From: Peter Eisentraut <peter.eisentraut@enterprisedb.com>
To: Julien Rouhaud <rjuju123@gmail.com>
Cc: pgsql-hackers <pgsql-hackers@postgresql.org>, Daniel Verite <daniel@manitou-mail.org>
Date: 2022-01-11T09:10:25Z
Lists: pgsql-hackers 
On 10.01.22 07:00, Julien Rouhaud wrote:
>>> And then I changed in varstr_cmp():
>>>
>>>          if (collid != DEFAULT_COLLATION_OID)
>>>              mylocale = pg_newlocale_from_collation(collid);
>>>
>>> to just
>>>
>>>          mylocale = pg_newlocale_from_collation(collid);
>>>
>>> I find that the \timing results are indistinguishable.  (I used locale
>>> "en_US.UTF-8" and made sure that that code path is actually hit.)
>>>
>>> Does anyone have other insights?
>>
>> Looking at the git history, you added this comment in 414c5a2ea65.
>>
>> After a bit a digging in the lists, I found that you introduced it to fix a
>> reported 13% slowdown in varstr_cmp():
>> https://www.postgresql.org/message-id/20110129075253.GA18784%40tornado.leadboat.com
>> https://www.postgresql.org/message-id/1296748408.6442.1.camel%40vanquo.pezone.net
> 
> So I tried to run Noah's benchmark to see if I could reproduce the slowdown.
> Unfortunately the results I'm getting don't really make sense as removing the
> optimisation brings a 15% speedup, and with a few more runs I can see that I
> have about 25% noise, so there isn't much I can do to help.

Heh, I had that same experience, it actually got faster without the 
optimization, but then got lost in the noise on further testing.

Looking back at those discussions, I don't think those old test results 
are relevant anymore.  In the patch that was being tested there, 
pg_newlocale_from_collation(), did not contain

     if (collid == DEFAULT_COLLATION_OID)
         return (pg_locale_t) 0;

so the default collation actually went through most or all of the 
function and did a lot of work.  That would understandably be quite 
slow.  But just calling a function and returning immediately should not 
be a problem.  Otherwise, the call to check_collation_set() in 
varstr_cmp() and elsewhere would be just as bad.

So, unless there are concerns, I'm going to see about making a patch to 
call pg_newlocale_from_collation() even with the default collation. 
That would make the actual feature patch quite a bit smaller, since we 
won't have to patch every call site of pg_newlocale_from_collation().

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Add CHECK_FOR_INTERRUPTS while restoring changes during decoding.

  2. Improve ICU option handling in CREATE DATABASE

  3. Don't allow creation of database with ICU locale with unsupported encoding

  4. Make locale option behavior more consistent

  5. pg_dump: Dump colliculocale

  6. Remove further unwanted linker flags from perl_embed_ldflags

  7. Doc: document possible need to raise kernel's somaxconn limit.

  8. Reduce warnings with -Wshadow=compatible-local builds

  9. Remove redundant spaces in _outA_Expr() output

  10. Fix outdated --help message for postgres -f

  11. pg_upgrade: Fix version comparison for global ICU support

  12. Silence -Wmaybe-uninitialized compiler warning in dbcommands.c.

  13. Add option to use ICU as global locale provider

  14. DefineCollation() code cleanup

  15. Change collate and ctype fields to type text

  16. Call pg_newlocale_from_collation() also with default collation

  17. Use libc version as a collation version on glibc systems.

  18. Silence -Wmaybe-uninitialized compiler warnings in dbcommands.c.

  19. Make LC_COLLATE and LC_CTYPE database-level settings. Collation and