Date: Tue, 26 Nov 2019 21:05:59 +0100
From: Tomas Vondra <tomas.vondra@2ndquadrant.com>
To: Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
Cc: Michael Paquier <michael@paquier.xyz>,
	Andrey Borodin <x4mmm@yandex-team.ru>,
	Alvaro Herrera <alvherre@2ndquadrant.com>,
	Gasper Zejn <zejn@owca.info>, pgsql-hackers@lists.postgresql.org
Subject: Re: pglz performance
Message-ID: <20191126200559.7ol33csmuwl6la2h@development>
References: <3c2d92d0-e715-f9ed-e1a7-3c510ed65940@2ndquadrant.com>
 <20191101154828.GA15829@alvherre.pgsql>
 <20191101165950.b5azncggaqfan5fi@development>
 <35c2a448-06b8-eab0-05ea-34a3f51ba9a8@2ndquadrant.com>
 <20191125080309.GK99720@paquier.xyz>
 <13C6049A-C3D9-4021-9934-41488DE38BE9@yandex-team.ru>
 <20191125082940.GP99720@paquier.xyz>
 <20191126094324.7y2n3n3houetavwd@development>
 <c52bb971-ae60-81eb-be66-57f12d3ae102@2ndquadrant.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii; format=flowed
Content-Disposition: inline
In-Reply-To: <c52bb971-ae60-81eb-be66-57f12d3ae102@2ndquadrant.com>
Precedence: bulk

On Tue, Nov 26, 2019 at 08:17:13PM +0100, Peter Eisentraut wrote:
>On 2019-11-26 10:43, Tomas Vondra wrote:
>>In general, I think the results for both patches seem clearly a win, but
>>maybe patch 1 is  bit better, especially on the newer (xeon) CPU. So I'd
>>probably go with that one.
>
>Patch 1 is also the simpler patch, so it seems clearly preferable.
>

Yeah, although the difference is minimal. We could probably construct a
benchmark where #2 wins, but I think these queries are fairly realistic.
So I'd just go with #1.

Code-wise I think the patches are mostly fine, although the comments
might need some proof-reading.

1) I wasn't really sure what a "nibble" is, but maybe it's just me and
it's a well-known term.

2) First byte use lower -> First byte uses lower

3) nibble contain upper -> nibble contains upper

4) to preven possible uncertanity -> to prevent possible uncertainty

5) I think we should briefly explain why memmove would be incompatible
with pglz, it's not quite clear to me.

6) I'm pretty sure the comment in the 'while (off < len)' branch will be
badly mangled by pgindent.

7) The last change moving "copy" to the next line seems unnecessary.


regards

-- 
Tomas Vondra                  http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services