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

From: Bertrand Drouvot <bertranddrouvot(dot)pg(at)gmail(dot)com>
To: Heikki Linnakangas <hlinnaka(at)iki(dot)fi>
Cc: Andres Freund <andres(at)anarazel(dot)de>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: PGPROC alignment (was Re: pgsql: Separate RecoveryConflictReasons from procsignals)
Date: 2026-02-13 08:03:04
Message-ID: aY7auF2/xuT58vRI@ip-10-97-1-34.eu-west-3.compute.internal
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-committers pgsql-hackers

Hi,

On Wed, Feb 11, 2026 at 12:03:51PM +0200, Heikki Linnakangas wrote:
> On 11/02/2026 06:40, Bertrand Drouvot wrote:
> > A few comments:
> >
> > 0001:
> >
> > + * and (b) to make the multiplication / division to convert between PGPROC *
> > + * and ProcNumber be a little cheaper
> >
> > Is that correct if PGPROC size is not a power of 2?
>
> You're right, it's not.

Looking more closely at:

"
/* GCC supports aligned and packed */
#if defined(__GNUC__)
#define pg_attribute_aligned(a) __attribute__((aligned(a)))
#define pg_attribute_packed() __attribute__((packed))
#elif defined(_MSC_VER)
/*
* MSVC supports aligned.
*
* Packing is also possible but only by wrapping the entire struct definition
* which doesn't fit into our current macro declarations.
*/
#define pg_attribute_aligned(a) __declspec(align(a))
#else
/*
* NB: aligned and packed are not given default definitions because they
* affect code functionality; they *must* be implemented by the compiler
* if they are to be used.
*/
#endif
"

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?

Regards,

--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com

In response to

Responses

Browse pgsql-committers by date

  From Date Subject
Next Message Daniel Gustafsson 2026-02-13 10:43:22 pgsql: Restart BackgroundPsql's timer more nicely.
Previous Message Michael Paquier 2026-02-13 03:18:03 pgsql: Improve error message for checksum failures in pgstat_database.c

Browse pgsql-hackers by date

  From Date Subject
Next Message Michael Paquier 2026-02-13 08:22:38 Re: use the malloc macros in pg_dump.c
Previous Message Michael Paquier 2026-02-13 07:36:57 Re: Adding locks statistics