Message-ID: <c3093547-6ce2-66ca-71de-3a3c633cfb02@enterprisedb.com>
Date: Tue, 11 Jan 2022 10:10:25 +0100
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:91.0)
 Gecko/20100101 Thunderbird/91.4.1
Subject: Re: ICU for global collation
Content-Language: en-US
To: Julien Rouhaud <rjuju123@gmail.com>
Cc: pgsql-hackers <pgsql-hackers@postgresql.org>,
 Daniel Verite <daniel@manitou-mail.org>
References: <374f8048-9d87-462c-a8aa-d02ffca6525f@manitou-mail.org>
 <d7060bf1-177c-e0f2-f13f-b857cdd1182e@2ndquadrant.com>
 <525ef44f-52bf-505f-a491-07835d039424@enterprisedb.com>
 <YdOvEAFGbfGY/47A@jrouhaud>
 <bba413de-5548-3480-e8f8-1ced562a866b@enterprisedb.com>
 <ed3baa81-7fac-7788-cc12-41e3f7917e34@enterprisedb.com>
 <20220110032508.4eyz6m6mpvt622la@jrouhaud>
 <20220110060027.egvaly4lskylo22f@jrouhaud>
From: Peter Eisentraut <peter.eisentraut@enterprisedb.com>
In-Reply-To: <20220110060027.egvaly4lskylo22f@jrouhaud>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Archived-At: 
 <https://www.postgresql.org/message-id/c3093547-6ce2-66ca-71de-3a3c633cfb02%40enterprisedb.com>
Precedence: bulk


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().