Re: [PoC] Improve dead tuple storage for lazy vacuum

From: John Naylor <johncnaylorls(at)gmail(dot)com>
To: Pavel Borisov <pashkin(dot)elfe(at)gmail(dot)com>
Cc: Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>, Nathan Bossart <nathandbossart(at)gmail(dot)com>, Andres Freund <andres(at)anarazel(dot)de>, Matthias van de Meent <boekewurm+postgres(at)gmail(dot)com>, Yura Sokolov <y(dot)sokolov(at)postgrespro(dot)ru>
Subject: Re: [PoC] Improve dead tuple storage for lazy vacuum
Date: 2024-04-08 13:17:54
Message-ID: CANWCAZYkVqzpgWD5kTLvw0KNX2XSNxP_ZzTmuRfuYDAtd6j7jw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Mon, Apr 8, 2024 at 7:42 PM Pavel Borisov <pashkin(dot)elfe(at)gmail(dot)com> wrote:
>
>> I pushed both of these and see that mylodon complains that anonymous
>> unions are a C11 feature. I'm not actually sure that the union with
>> uintptr_t is actually needed, though, since that's not accessed as
>> such here. The simplest thing seems to get rid if the union and name
>> the inner struct "header", as in the attached.
>
>
> Provided uintptr_t is not accessed it might be good to get rid of it.
>
> Maybe this patch also need correction in this:
> +#define NUM_FULL_OFFSETS ((sizeof(uintptr_t) - sizeof(uint8) - sizeof(int8)) / sizeof(OffsetNumber))

For full context the diff was

-#define NUM_FULL_OFFSETS ((sizeof(bitmapword) - sizeof(uint16)) /
sizeof(OffsetNumber))
+#define NUM_FULL_OFFSETS ((sizeof(uintptr_t) - sizeof(uint8) -
sizeof(int8)) / sizeof(OffsetNumber))

I wanted the former, from f35bd9bf35 , to be independently useful (in
case the commit in question had some unresolvable issue), and its
intent is to fill struct padding when the array of bitmapword happens
to have length zero. Changing to uintptr_t for the size calculation
reflects the intent to fit in a (local) pointer, regardless of the
size of a bitmapword. (If a DSA pointer happens to be a different size
for some odd platform, it should still work, BTW.)

My thinking with the union was, for big-endian, to force the 'flags'
member to where it can be set, but thinking again, it should still
work if by happenstance the header was smaller than the child pointer:
A different bit would get tagged, but I believe that's irrelevant. The
'flags' member makes sure a byte is reserved for the tag, but it may
not be where the tag is actually located, if that makes sense.

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Robert Haas 2024-04-08 13:26:09 Re: PostgreSQL 17 Release Management Team & Feature Freeze
Previous Message Pavel Borisov 2024-04-08 12:42:01 Re: [PoC] Improve dead tuple storage for lazy vacuum