| 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
| 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 |
| 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 |