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