Re: pglz compression performance, take two

2023-02-13 Thread Andrey Borodin
On Mon, Feb 13, 2023 at 4:03 PM Andres Freund wrote: > > Due to the sanitizer changes, and this feedback, I'm marking the entry as > waiting on author. > Thanks Andres! Yes, I plan to make another attempt to refactor this patch on the weekend. If this attempt fails, I think we should just reject i

Re: pglz compression performance, take two

2023-02-13 Thread Andres Freund
Hi, On 2023-02-08 11:16:47 +0100, Tomas Vondra wrote: > On 2/7/23 21:18, Andres Freund wrote: > > > > Independent of this failure, I'm worried about the cost/benefit analysis of > > a > > pglz change that changes this much at once. It's quite hard to review. > > > > I agree. > > I think I man

Re: pglz compression performance, take two

2023-02-08 Thread Tomas Vondra
On 2/7/23 21:18, Andres Freund wrote: > Hi, > > On 2023-02-05 10:36:39 -0800, Andrey Borodin wrote: >> On Fri, Jan 6, 2023 at 10:02 PM Andrey Borodin wrote: >>> >>> Hello! Please find attached v8. >> >> I got some interesting feedback from some patch users. >> There was an oversight that frequ

Re: pglz compression performance, take two

2023-02-07 Thread Andres Freund
Hi, On 2023-02-05 10:36:39 -0800, Andrey Borodin wrote: > On Fri, Jan 6, 2023 at 10:02 PM Andrey Borodin wrote: > > > > Hello! Please find attached v8. > > I got some interesting feedback from some patch users. > There was an oversight that frequently yielded results that are 1,2 or > 3 bytes lon

Re: pglz compression performance, take two

2023-02-06 Thread Andrey Borodin
On Mon, Feb 6, 2023 at 11:57 AM Tomas Vondra wrote: > > I wonder what that means for the patch. I haven't investigated this at > all, but it seems as if the optimization means we fail to find a match, > producing a tad larger output. That may be still be a good tradeoff, as > long as the output is

Re: pglz compression performance, take two

2023-02-06 Thread Tomas Vondra
On 2/6/23 03:00, Andrey Borodin wrote: > On Sun, Feb 5, 2023 at 5:51 PM Tomas Vondra > wrote: >> >> On 2/5/23 19:36, Andrey Borodin wrote: >>> On Fri, Jan 6, 2023 at 10:02 PM Andrey Borodin >>> wrote: Hello! Please find attached v8. >>> >>> I got some interesting feedback from some

Re: pglz compression performance, take two

2023-02-05 Thread Andrey Borodin
On Sun, Feb 5, 2023 at 5:51 PM Tomas Vondra wrote: > > On 2/5/23 19:36, Andrey Borodin wrote: > > On Fri, Jan 6, 2023 at 10:02 PM Andrey Borodin > > wrote: > >> > >> Hello! Please find attached v8. > > > > I got some interesting feedback from some patch users. > > There was an oversight that fre

Re: pglz compression performance, take two

2023-02-05 Thread Tomas Vondra
On 2/5/23 19:36, Andrey Borodin wrote: > On Fri, Jan 6, 2023 at 10:02 PM Andrey Borodin wrote: >> >> Hello! Please find attached v8. > > I got some interesting feedback from some patch users. > There was an oversight that frequently yielded results that are 1,2 or > 3 bytes longer than expected.

Re: pglz compression performance, take two

2023-02-05 Thread Andrey Borodin
On Fri, Jan 6, 2023 at 10:02 PM Andrey Borodin wrote: > > Hello! Please find attached v8. I got some interesting feedback from some patch users. There was an oversight that frequently yielded results that are 1,2 or 3 bytes longer than expected. Looking closer I found that the correctness of the

Re: pglz compression performance, take two

2023-01-06 Thread Andrey Borodin
On Sun, Nov 27, 2022 at 10:43 AM Andrey Borodin wrote: > > PFA review fixes (step 1 is unchanged). Hello! Please find attached v8. Changes are mostly cosmetic: 1. 2 steps from previous message were squashed together 2. I tried to do a better commit message Thanks! Best regards, Andrey Borodin.

Re: pglz compression performance, take two

2022-11-27 Thread Andrey Borodin
Hi Tomas, On Sun, Nov 27, 2022 at 8:02 AM Tomas Vondra wrote: > > 1) For PGLZ_HISTORY_SIZE it uses literal 0x0fff, with the explanation: > > /* to avoid compare in iteration */ > > which I think means intent to use this value as a bit mask, but then the > only place using PGLZ_HISTORY_SIZE do

Re: pglz compression performance, take two

2022-11-27 Thread Tomas Vondra
On 11/27/22 17:02, Tomas Vondra wrote: > Hi, > > I took a look at the v6 patch, with the intention to get it committed. I > have a couple minor comments: > > 1) For PGLZ_HISTORY_SIZE it uses literal 0x0fff, with the explanation: > > /* to avoid compare in iteration */ > > which I think mean

Re: pglz compression performance, take two

2022-11-27 Thread Tomas Vondra
Hi, I took a look at the v6 patch, with the intention to get it committed. I have a couple minor comments: 1) For PGLZ_HISTORY_SIZE it uses literal 0x0fff, with the explanation: /* to avoid compare in iteration */ which I think means intent to use this value as a bit mask, but then the only

Re: pglz compression performance, take two

2022-11-16 Thread Ian Lawrence Barwick
2021年11月5日(金) 14:51 Andrey Borodin : > > Thanks for the review Mark! Sorry it took too long to reply on my side. > > > 28 июня 2021 г., в 21:05, Mark Dilger > > написал(а): > > > >> #define PGLZ_HISTORY_SIZE 0x0fff - 1 /* to avoid compare in > >> iteration */ > > ... > >> static PGLZ_Hist

Re: pglz compression performance, take two

2021-11-04 Thread Andrey Borodin
Thanks for the review Mark! Sorry it took too long to reply on my side. > 28 июня 2021 г., в 21:05, Mark Dilger > написал(а): > >> #define PGLZ_HISTORY_SIZE 0x0fff - 1 /* to avoid compare in iteration >> */ > ... >> static PGLZ_HistEntry hist_entries[PGLZ_HISTORY_SIZE + 1]; > ... >>

Re: pglz compression performance, take two

2021-11-04 Thread Tomas Vondra
Hi, I've looked at this patch again and did some testing. I don't have any comments to the code (I see there are two comments from Mark after the last version, though). For the testing, I did a fairly simple benchmark loading either random or compressible data into a bytea column. The tables

Re: pglz compression performance, take two

2021-06-28 Thread Mark Dilger
> On Jun 28, 2021, at 9:05 AM, Mark Dilger wrote: > > Is it worth sharting a static inline function that uses your optimization in > other places? s/sharting/sharing/ > How confident are you that your optimization really helps? By which I mean, is the optimization worth the extra branch

Re: pglz compression performance, take two

2021-06-28 Thread Mark Dilger
> On Jun 27, 2021, at 3:41 AM, Andrey Borodin wrote: > > And here's what I've come up with. I have not tested the patch yet, but here are some quick review comments: > #define PGLZ_HISTORY_SIZE 0x0fff - 1 /* to avoid compare in iteration > */ ... > static PGLZ_HistEntry hist_entries

Re: pglz compression performance, take two

2021-06-27 Thread Andrey Borodin
> 20 марта 2021 г., в 00:35, Mark Dilger > написал(а): > > > >> On Jan 21, 2021, at 6:48 PM, Justin Pryzby wrote: >> >> @cfbot: rebased >> <0001-Reorganize-pglz-compression-code.patch> > > Review comments. Thanks for the review, Mark! And sorry for such a long delay, I've been trying to

Re: pglz compression performance, take two

2021-06-25 Thread Michael Paquier
On Sat, Mar 20, 2021 at 12:19:45AM -0500, Justin Pryzby wrote: > I think it's still relevant, since many people may not end up with binaries > --with-lz4 (I'm thinking of cloud providers). PGLZ is what existing data > uses, > and people may not want to/know to migrate to shiny new features, but t

Re: pglz compression performance, take two

2021-03-19 Thread Justin Pryzby
On Fri, Mar 19, 2021 at 01:29:14PM -0700, Mark Dilger wrote: > Robert Haas just committed Dilip Kumar's LZ4 compression, > bbe0a81db69bd10bd166907c3701492a29aca294. > > Is this pglz compression patch still relevant? How does the LZ4 compression > compare on your hardware? I think it's still re

Re: pglz compression performance, take two

2021-03-19 Thread Mark Dilger
> On Jan 28, 2021, at 2:56 AM, Andrey Borodin wrote: > > > >> 22 янв. 2021 г., в 07:48, Justin Pryzby написал(а): >> >> @cfbot: rebased >> <0001-Reorganize-pglz-compression-code.patch> > > Thanks! > > I'm experimenting with TPC-C over PostgreSQL 13 on production-like cluster in > the cl

Re: pglz compression performance, take two

2021-03-19 Thread Mark Dilger
> On Jan 21, 2021, at 6:48 PM, Justin Pryzby wrote: > > @cfbot: rebased > <0001-Reorganize-pglz-compression-code.patch> Review comments. First, I installed a build from master without this patch, created a test installation with lots of compressed text and array columns, upgraded the binar

Re: pglz compression performance, take two

2021-01-28 Thread Andrey Borodin
> 22 янв. 2021 г., в 07:48, Justin Pryzby написал(а): > > @cfbot: rebased > <0001-Reorganize-pglz-compression-code.patch> Thanks! I'm experimenting with TPC-C over PostgreSQL 13 on production-like cluster in the cloud. Overall performance is IO-bound, but compression is burning a lot energ

Re: pglz compression performance, take two

2021-01-21 Thread Justin Pryzby
@cfbot: rebased >From 03fec5d2587cf34a1d1a75d7afdcfbad9cb7ec68 Mon Sep 17 00:00:00 2001 From: Andrey Date: Thu, 27 Jun 2019 23:18:21 +0500 Subject: [PATCH] Reorganize pglz compression code This patch accumulates several changes: 1. Convert macro-functions to regular functions for readability 2. U

Re: pglz compression performance, take two

2020-12-30 Thread Andrey Borodin
Thanks for looking into this, Justin! > 30 дек. 2020 г., в 09:39, Justin Pryzby написал(а): > > There's some typos in the current patch; > > farer (further: but it's not your typo) > positiion > reduce a => reduce the > monotonicity what => monotonicity, which > lesser good => less good > allig

Re: pglz compression performance, take two

2020-12-29 Thread Justin Pryzby
On Sat, Dec 26, 2020 at 12:06:59PM +0500, Andrey Borodin wrote: > > 12 дек. 2020 г., в 22:47, Andrey Borodin написал(а): > I've cleaned up comments, checked that memory alignment stuff actually make > sense for 32-bit ARM (according to Godbolt) and did some more code cleanup. > PFA v2 patch.

Re: pglz compression performance, take two

2020-12-26 Thread Tom Lane
Tomas Vondra writes: > On 12/26/20 8:06 AM, Andrey Borodin wrote: >> I'm still in doubt should I register this patch on CF or not. I'm willing to >> work on this, but it's not clear will it hit PGv14. And I hope for PGv15 we >> will have lz4 or something better for WAL compression. > I'd sugges

Re: pglz compression performance, take two

2020-12-26 Thread Tomas Vondra
On 12/26/20 8:06 AM, Andrey Borodin wrote: 12 дек. 2020 г., в 22:47, Andrey Borodin написал(а): I've cleaned up comments, checked that memory alignment stuff actually make sense for 32-bit ARM (according to Godbolt) and did some more code cleanup. PFA v2 patch. I'm still in doubt sh

Re: pglz compression performance, take two

2020-12-25 Thread Andrey Borodin
> 12 дек. 2020 г., в 22:47, Andrey Borodin написал(а): > > I've cleaned up comments, checked that memory alignment stuff actually make sense for 32-bit ARM (according to Godbolt) and did some more code cleanup. PFA v2 patch. I'm still in doubt should I register this patch on CF or not. I'm

Re: pglz compression performance, take two

2020-12-12 Thread Andrey Borodin
> 9 дек. 2020 г., в 12:44, Andrey Borodin написал(а): > PFA the patch with some editorialisation by me. > I saw some reports of bottlenecking in pglz WAL compression [1]. I've checked that on my machine simple test echo "wal_compression = on" >> $PGDATA/postgresql.conf pgbench -i -s 20 && pgbe