From: Chao Li <li.evan.chao@gmail.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Dimitrios Apostolou <jimis@gmx.net>
Cc: Nathan Bossart <nathandbossart@gmail.com>
Cc: Thomas Munro <thomas.munro@gmail.com>
Cc: pgsql-hackers@lists.postgresql.org
Subject: Re: [PING] [PATCH v2] parallel pg_restore: avoid disk seeks when jumping short distance forward
Date: Mon, 13 Oct 2025 11:33:06 +0800
Message-ID: <CBF70398-279C-4BA7-9FA1-2879A49F4EC9@gmail.com> (raw)
In-Reply-To: <3686.1760232320@sss.pgh.pa.us>
References: <2edb7a57-b225-3b23-a680-62ba90658fec@gmx.net>
<a7344d8f-927c-1f97-6cc0-ac630deb98c4@gmx.net>
<c0e531f9-accc-3a93-e9e6-2cec1471f9e9@gmx.net>
<1282067.1744651649@sss.pgh.pa.us>
<c6bb93c1-4042-743c-096c-27c5e7c73347@gmx.net>
<f7a23708-c336-c189-f155-4b3197bf6ea0@gmx.net>
<aEioDuePEBRnfJYk@nathan>
<5ed51587-5a24-4d79-e36e-927620d0e3a2@gmx.net>
<aEygf8AchBk04gOA@nathan>
<f99e9774-da49-60f0-20e0-b97d9f12a04e@gmx.net>
<72d2c81b-a0fe-3d99-9dd4-0d771e6e673e@gmx.net>
<1po8os49-r63o-2923-p37n-12698o1qn7p0@tzk.arg>
<3515357.1760128017@sss.pgh.pa.us>
<3698234.1760149336@sss.pgh.pa.us>
<3686.1760232320@sss.pgh.pa.us>
> On Oct 12, 2025, at 09:25, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
> While playing around with the test cases for pg_dump compression,
> I was startled to discover that the performance of compress_lz4's
> "stream API" code is absolutely abysmal. Here is a simple test
> case to demonstrate, using the regression database as test data:
>
> $ pg_dump -Fd --compress=lz4 -f rlz4.dir regression
> $ time pg_restore -f /dev/null rlz4.dir
>
> real 0m0.023s
> user 0m0.017s
> sys 0m0.006s
>
> So far so good, but now let's compress the toc.dat file:
>
> $ lz4 -f -m --rm rlz4.dir/toc.dat
> $ time pg_restore -f /dev/null rlz4.dir
>
> real 0m1.335s
> user 0m1.326s
> sys 0m0.008s
>
> Considering that lz4 prides itself on fast decompression speed,
> that is not a sane result. Decompressing the file only requires
> a couple ms on my machine:
>
> $ time lz4cat rlz4.dir/toc.dat.lz4 >/dev/null
>
> real 0m0.002s
> user 0m0.000s
> sys 0m0.002s
>
> So on this example, pg_restore is something more than 600x slower
> to read the TOC data than it ought to be.
>
I tested the patch on my MacBook M4 (Sequoia 15.6.1), compiled with -O2 optimization:
- For LZ4 performance improvement:
Without the fix (latest master):
```
chaol@ChaodeMacBook-Air tmp % pg_dump -Fd --compress=lz4 -f rlz4.dir regression
chaol@ChaodeMacBook-Air tmp % time pg_restore -f /dev/null rlz4.dir
pg_restore -f /dev/null rlz4.dir 0.03s user 0.03s system 11% cpu 0.463 total
chaol@ChaodeMacBook-Air tmp % lz4 -f -m --rm rlz4.dir/toc.dat
# It took 1.59s to restore from compressed data, much slower than from uncompressed data.
chaol@ChaodeMacBook-Air tmp % time pg_restore -f /dev/null rlz4.dir
pg_restore -f /dev/null rlz4.dir 1.59s user 0.02s system 97% cpu 1.653 total
```
With the fix:
```
chaol@ChaodeMacBook-Air tmp % pg_dump -Fd --compress=lz4 -f rlz4.dir regression
chaol@ChaodeMacBook-Air tmp % time pg_restore -f /dev/null rlz4.dir
pg_restore -f /dev/null rlz4.dir 0.02s user 0.03s system 16% cpu 0.305 total
chaol@ChaodeMacBook-Air tmp % lz4 -f -m --rm rlz4.dir/toc.dat
# Much faster !!!
chaol@ChaodeMacBook-Air tmp % time pg_restore -f /dev/null rlz4.dir
pg_restore -f /dev/null rlz4.dir 0.02s user 0.02s system 93% cpu 0.043 total
```
- For Gzip zero read bug:
Without the fix:
```
chaol@ChaodeMacBook-Air tmp % pg_dump -Fd --compress=gzip -f rgzip.dir regression
chaol@ChaodeMacBook-Air tmp % gzip rgzip.dir/toc.dat
# Failed
chaol@ChaodeMacBook-Air tmp % pg_restore -f /dev/null rgzip.dir
pg_restore: error: could not read from input file:
```
With the fix:
```
chaol@ChaodeMacBook-Air tmp % pg_dump -Fd --compress=gzip -f rgzip.dir regression
chaol@ChaodeMacBook-Air tmp % gzip rgzip.dir/toc.dat
# Ran successfully
chaol@ChaodeMacBook-Air tmp % pg_restore -f /dev/null rgzip.dir
```
I also reviewed the code change, basically LGTM. Just a couple of trivial comments:
1 - 0001
In WriteDataToArchiveLZ4()
```
+ required = LZ4F_compressBound(chunk, &state->prefs);
+ if (required > state->buflen - state->bufdata)
+ {
+ cs->writeF(AH, state->buffer, state->bufdata);
+ state->bufdata = 0;
+ }
```
And in EndCompressorLZ4()
```
+ required = LZ4F_compressBound(0, &state->prefs);
+ if (required > state->buflen - state->bufdata)
+ {
+ cs->writeF(AH, state->buffer, state->bufdata);
+ state->bufdata = 0;
+ }
```
These two code pieces are similar, only difference is the first parameter passed to LZ4F_compressBound().
I think we can create an inline function for it. But these are just 5 lines, I don’t have a strong option on that.
Same thing for the other code pieces in LZ4Stream_write() and LZ4Stream_close().
2 - 0003
```
/*
* LZ4 equivalent to feof() or gzeof(). Return true iff there is no
- * decompressed output in the overflow buffer and the end of the backing file
```
This doesn’t belong to the current patch. But “iff” seems a typo of “if”. You may fix it as you are touching this piece of code.
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: li.evan.chao@gmail.com, tgl@sss.pgh.pa.us, jimis@gmx.net, nathandbossart@gmail.com, thomas.munro@gmail.com, pgsql-hackers@lists.postgresql.org
Subject: Re: [PING] [PATCH v2] parallel pg_restore: avoid disk seeks when jumping short distance forward
In-Reply-To: <CBF70398-279C-4BA7-9FA1-2879A49F4EC9@gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox