On 11/02/2026 06:40, Bertrand Drouvot wrote:
> 0004:
>
> The grouping looks Ok to me. Just one nit for the added comments:
>
> + /*---- Backend identity ----*/
> + /*---- Transactions and snapshots ----*/
> + /*---- Inter-process signaling ----*/
> + /*---- LWLock waiting ----*/
> + /*---- Lock manager data ----*/
> + /*---- Synchronous replication waiting ----*/
> + /*---- Support for group XID clearing. ----*/
> + /*---- Support for group transaction status update. ----*/
> + /*---- Status reporting ----*/
>
> Some have period and some don't.
Fixed that, and changed to different style for the delimiter comments.
It's a matter of taste, but I think the new style is closer to what we
use elsewhere.
On 13/02/2026 10:03, Bertrand Drouvot wrote:
> and what the patch adds:
>
> +/*
> + * If compiler understands aligned pragma, use it to align the struct at cache
> + * line boundaries. This is just for performance, to (a) avoid false sharing
> + * and (b) to make the multiplication / division to convert between PGPROC *
> + * and ProcNumber be a little cheaper.
> + */
> +#if defined(pg_attribute_aligned)
> + pg_attribute_aligned(PG_CACHE_LINE_SIZE)
> +#endif
> +PGPROC;
>
> It means that PGPROC is "acceptable" without padding (on compiler that does not
> understand the aligned attribute).
>
> OTOH, looking at:
>
> "
> typedef union WALInsertLockPadded
> {
> WALInsertLock l;
> char pad[PG_CACHE_LINE_SIZE];
> } WALInsertLockPadded;
> "
>
> It seems to mean that WALInsertLockPadded is unacceptable without padding (since
> it's not using pg_attribute_aligned()).
>
> That looks ok to see PGPROC as an "acceptable" one, if not, should we use the
> union trick?
It seems acceptable to just not align it if the compiler doesn't support
it. This is just a performance optimization, after all.
Attached is new versions the remaining patches. I think these are ready
to be committed.
I don't have the hardware and test case that would be sensitive enough
for these changes in memory layout, so I haven't done any performance
testing of this. I'd guess this no worse than the old layout. Grouping
fields together which are used together is usually a good strategy. I
don't feel obliged to do performance testing of this, given that no one
did that for the old layout either, so if it happened to be really good
from a performance point of view, it was purely accidental. But if
anyone has the means and interest to do that, that'd be much appreciated
of course.
- Heikki
