(moving to pgsql-hackers)
On 10/02/2026 18:41, Andres Freund wrote:
On 2026-02-10 17:52:16 +0200, Heikki Linnakangas wrote:
If there's a performance reason to keep have it be aligned - and maybe there
is - we should pad it explicitly.
We should make it a power of two or such. There are some workloads where the
indexing from GetPGProcByNumber() shows up, because it ends up having to be
implemented as a 64 bit multiplication, which has a reasonably high latency
(3-5 cycles). Whereas a shift has a latency of 1 and typically higher
throughput too.
Power of two means going to 1024 bytes. That's a lot of padding. Where
have you seen that show up?
Attached is a patch to align to cache line boundary. That's
straightforward if that's what we want to do.
Re false sharing: We should really separate stuff that changes (like
e.g. pendingRecoveryConflicts) and never changing stuff (backendType). You
don't need overlapping structs to have false sharing issues if you mix
different access patterns inside a struct that's accessed across processes...
Makes sense, although I don't want to optimize too hard for performance,
at the expense of readability. The current order is pretty random
anyway, though.
It'd probably be good to move the subxids cache to the end of the
struct. That'd act as natural padding, as it's not very frequently used,
especially the tail end of the cache. Or come to think of it, it might
be good to move the subxids cache out of PGPROC altogether. It's mostly
frequently accessed in GetSnapshotData(), and for that it'd actually be
better if it was in a separate "mirrored" array, similar to the main xid
and subxidStates. That would eliminate the pgprocnos[pgxactoff] lookup
from GetSnapshotdata() altogether.
I'm a little reluctant to mess with this without a concrete benchmark
though. Got one in mind?
- Heikki
From a3e2d94f6a3ab215afd3e8cee624bd9c09ec5391 Mon Sep 17 00:00:00 2001
From: Heikki Linnakangas <[email protected]>
Date: Tue, 10 Feb 2026 18:53:31 +0200
Subject: [PATCH 1/1] Align PGPROC to cache line boundary
---
src/include/storage/proc.h | 16 ++++++++++++----
1 file changed, 12 insertions(+), 4 deletions(-)
diff --git a/src/include/storage/proc.h b/src/include/storage/proc.h
index ac0df4aeaaa..53acce8a5a1 100644
--- a/src/include/storage/proc.h
+++ b/src/include/storage/proc.h
@@ -182,7 +182,7 @@ typedef enum
*
* See PROC_HDR for details.
*/
-struct PGPROC
+typedef struct PGPROC
{
dlist_node links; /* list link if process is in a list */
dlist_head *procgloballist; /* procglobal list that owns this PGPROC */
@@ -337,10 +337,18 @@ struct PGPROC
PGPROC *lockGroupLeader; /* lock group leader, if I'm a member */
dlist_head lockGroupMembers; /* list of members, if I'm a leader */
dlist_node lockGroupLink; /* my member link, if I'm a member */
-};
-
-/* NOTE: "typedef struct PGPROC PGPROC" appears in storage/lock.h. */
+}
+/*
+ * 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;
extern PGDLLIMPORT PGPROC *MyProc;
--
2.47.3
