Re: PGPROC alignment (was Re: pgsql: Separate RecoveryConflictReasons from procsignals)

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