public inbox for [email protected]  

help / color / mirror / Atom feed

From: Bertrand Drouvot <[email protected]>
To: Heikki Linnakangas <[email protected]>
Cc: Andres Freund <[email protected]>
Cc: [email protected] <[email protected]>
Subject: Re: PGPROC alignment (was Re: pgsql: Separate RecoveryConflictReasons from procsignals)
Date: Fri, 13 Feb 2026 08:03:04 +0000
Message-ID: <aY7auF2/[email protected]> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>

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

view thread (22+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected]
  Subject: Re: PGPROC alignment (was Re: pgsql: Separate RecoveryConflictReasons from procsignals)
  In-Reply-To: <aY7auF2/[email protected]>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox