Re: Making aggregate deserialization (and WAL receive) functions slightly faster

From: David Rowley <dgrowleyml(at)gmail(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, Michael Paquier <michael(at)paquier(dot)xyz>, PostgreSQL Developers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Making aggregate deserialization (and WAL receive) functions slightly faster
Date: 2023-10-12 23:22:54
Message-ID: CAApHDvoK_nmeO-CxKpNf_AuUbFFNY6GeZboGt4A1p9R5_AoVuw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Wed, 11 Oct 2023 at 08:52, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>
> David Rowley <dgrowleyml(at)gmail(dot)com> writes:
> > I've attached a slightly more worked on patch that makes maxlen == 0
> > mean read-only. Unsure if a macro is worthwhile there or not.
>
> A few thoughts:

Thank you for the review.

I spent more time on this and did end up with 2 new init functions as
you mentioned. One for strictly read-only (initReadOnlyStringInfo),
which cannot be appended to, and as you mentioned, another
(initStringInfoFromString) which can accept a palloc'd buffer which
becomes managed by the stringinfo code. I know these names aren't
exactly as you mentioned. I'm open to adjusting still.

This means I got rid of the read-only conversion code in
enlargeStringInfo(). I didn't do anything to try to handle buffer
enlargement more efficiently in enlargeStringInfo() for the case where
initStringInfoFromString sets maxlen to some non-power-of-2. The
doubling code seems like it'll work ok without power-of-2 values,
it'll just end up calling repalloc() with non-power-of-2 values.

I did also wonder if resetStringInfo() would have any business
touching the existing buffer in a read-only StringInfo and came to the
conclusion that it wouldn't be very read-only if we allowed
resetStringInfo() to do its thing on it. I added an Assert to fail if
resetStringInfo() receives a read-only StringInfo.

Also, since it's still being discussed, I left out the adjustment to
LogicalParallelApplyLoop(). That also allows the tests to pass
without the failing Assert that was checking for the NUL terminator.

David

Attachment Content-Type Size
initstringinfo_changes_v3.patch text/plain 10.1 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Michael Paquier 2023-10-12 23:30:02 Re: Test 026_overwrite_contrecord fails on very slow machines (under Valgrind)
Previous Message Peter Geoghegan 2023-10-12 23:21:15 Re: interval_ops shall stop using btequalimage (deduplication)