public inbox for [email protected]  
help / color / mirror / Atom feed
From: Bertrand Drouvot <[email protected]>
To: Andres Freund <[email protected]>
Cc: Heikki Linnakangas <[email protected]>
Cc: [email protected] <[email protected]>
Subject: Re: PGPROC alignment (was Re: pgsql: Separate RecoveryConflictReasons from procsignals)
Date: Tue, 10 Feb 2026 19:15:27 +0000
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>

Hi,

On Tue, Feb 10, 2026 at 01:15:01PM -0500, Andres Freund wrote:
> Hi,
> 
> On 2026-02-10 19:14:44 +0200, Heikki Linnakangas wrote:
> Yea, I don't think we need to be perfect here. Just a bit less bad. And, as
> you say, the current order doesn't make a lot of sense.
> Just grouping things like
> - pid, pgxactoff, backendType (i.e. barely if ever changing)
> - wait_event_info, waitStart (i.e. very frequently changing, but typically
>   accessed within one proc)
> - sem, lwWaiting, waitLockMode (i.e. stuff that is updated frequently and
>   accessed across processes)

With an ordering like in the attached (to apply on top of Heikki's patch), we're
back to 832 bytes.

But, then the pg_attribute_aligned() added in Heikki's patch makes it 896 bytes...

"
/*    816      |      16 */    dlist_node lockGroupLink;
/* XXX 64-byte padding   */

                               /* total size (bytes):  896 */
                             }
"

What about applying this new ordering and remove the pg_attribute_aligned()? (I
thought the aligned attribute would be smarter than that and not add this 64 padding
bytes).

Regards,

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

commit ea3aded2c4dec487c8a2ff5470cd7bae83e8dd47
Author: Bertrand Drouvot <[email protected]>
Date:   Tue Feb 10 18:54:29 2026 +0000

    reorder

diff --git a/src/include/storage/proc.h b/src/include/storage/proc.h
index 53acce8a5a1..5d41301c966 100644
--- a/src/include/storage/proc.h
+++ b/src/include/storage/proc.h
@@ -184,29 +184,16 @@ typedef enum
  */
 typedef struct PGPROC
 {
-	dlist_node	links;			/* list link if process is in a list */
-	dlist_head *procgloballist; /* procglobal list that owns this PGPROC */
-
-	PGSemaphore sem;			/* ONE semaphore to sleep on */
-	ProcWaitStatus waitStatus;
-
-	Latch		procLatch;		/* generic latch for process */
-
-
-	TransactionId xid;			/* id of top-level transaction currently being
-								 * executed by this proc, if running and XID
-								 * is assigned; else InvalidTransactionId.
-								 * mirrored in ProcGlobal->xids[pgxactoff] */
-
-	TransactionId xmin;			/* minimal running XID as it was when we were
-								 * starting our xact, excluding LAZY VACUUM:
-								 * vacuum must not remove tuples deleted by
-								 * xid >= xmin ! */
-
 	int			pid;			/* Backend's process ID; 0 if prepared xact */
-
 	int			pgxactoff;		/* offset into various ProcGlobal->arrays with
 								 * data mirrored from this PGPROC */
+	BackendType backendType;	/* what kind of process is this? */
+
+	/* These fields are zero while a backend is still starting up: */
+	Oid			databaseId;		/* OID of database this backend is using */
+	Oid			roleId;			/* OID of role using this backend */
+	Oid			tempNamespaceId;	/* OID of temp schema this backend is
+									 * using */
 
 	/*
 	 * Currently running top-level transaction's virtual xid. Together these
@@ -227,24 +214,44 @@ typedef struct PGPROC
 									 * InvalidLocalTransactionId */
 	}			vxid;
 
-	/* These fields are zero while a backend is still starting up: */
-	Oid			databaseId;		/* OID of database this backend is using */
-	Oid			roleId;			/* OID of role using this backend */
+	dlist_node	links;			/* list link if process is in a list */
+	dlist_head *procgloballist; /* procglobal list that owns this PGPROC */
 
-	Oid			tempNamespaceId;	/* OID of temp schema this backend is
-									 * using */
+	uint32		wait_event_info;	/* proc's wait information */
+	pg_atomic_uint64 waitStart; /* time at which wait for lock acquisition
+								 * started */
 
-	BackendType backendType;	/* what kind of process is this? */
+	TransactionId xid;			/* id of top-level transaction currently being
+								 * executed by this proc, if running and XID
+								 * is assigned; else InvalidTransactionId.
+								 * mirrored in ProcGlobal->xids[pgxactoff] */
 
-	/*
-	 * While in hot standby mode, shows that a conflict signal has been sent
-	 * for the current transaction. Set/cleared while holding ProcArrayLock,
-	 * though not required. Accessed without lock, if needed.
-	 *
-	 * This is a bitmask; each bit corresponds to a RecoveryConflictReason
-	 * enum value.
-	 */
-	pg_atomic_uint32 pendingRecoveryConflicts;
+	TransactionId xmin;			/* minimal running XID as it was when we were
+								 * starting our xact, excluding LAZY VACUUM:
+								 * vacuum must not remove tuples deleted by
+								 * xid >= xmin ! */
+
+	ProcWaitStatus waitStatus;
+	int			delayChkptFlags;	/* for DELAY_CHKPT_* flags */
+
+	LOCKMODE	waitLockMode;	/* type of lock we're waiting for */
+	LOCKMASK	heldLocks;		/* bitmask for lock types already held on this
+								 * lock object by this backend */
+
+	uint8		statusFlags;	/* this backend's status flags, see PROC_*
+								 * above. mirrored in
+								 * ProcGlobal->statusFlags[pgxactoff] */
+	XidCacheStatus subxidStatus;	/* mirrored with
+									 * ProcGlobal->subxidStates[i] */
+
+	bool		procArrayGroupMember;	/* true, if member of ProcArray group
+										 * waiting for XID clear */
+	bool		clogGroupMember;	/* true, if member of clog group */
+	bool		fpVXIDLock;		/* are we holding a fast-path VXID lock? */
+
+	LocalTransactionId fpLocalTransactionId;	/* lxid for fast-path VXID
+												 * lock */
+	PGSemaphore sem;			/* ONE semaphore to sleep on */
 
 	/*
 	 * Info about LWLock the process is currently waiting for, if any.
@@ -255,6 +262,19 @@ typedef struct PGPROC
 	 */
 	uint8		lwWaiting;		/* see LWLockWaitState */
 	uint8		lwWaitMode;		/* lwlock mode being waited for */
+
+	Latch		procLatch;		/* generic latch for process */
+
+	/*
+	 * While in hot standby mode, shows that a conflict signal has been sent
+	 * for the current transaction. Set/cleared while holding ProcArrayLock,
+	 * though not required. Accessed without lock, if needed.
+	 *
+	 * This is a bitmask; each bit corresponds to a RecoveryConflictReason
+	 * enum value.
+	 */
+	pg_atomic_uint32 pendingRecoveryConflicts;
+
 	proclist_node lwWaitLink;	/* position in LW lock wait list */
 
 	/* Support for condition variables. */
@@ -264,17 +284,6 @@ typedef struct PGPROC
 	/* waitLock and waitProcLock are NULL if not currently waiting. */
 	LOCK	   *waitLock;		/* Lock object we're sleeping on ... */
 	PROCLOCK   *waitProcLock;	/* Per-holder info for awaited lock */
-	LOCKMODE	waitLockMode;	/* type of lock we're waiting for */
-	LOCKMASK	heldLocks;		/* bitmask for lock types already held on this
-								 * lock object by this backend */
-	pg_atomic_uint64 waitStart; /* time at which wait for lock acquisition
-								 * started */
-
-	int			delayChkptFlags;	/* for DELAY_CHKPT_* flags */
-
-	uint8		statusFlags;	/* this backend's status flags, see PROC_*
-								 * above. mirrored in
-								 * ProcGlobal->statusFlags[pgxactoff] */
 
 	/*
 	 * Info to allow us to wait for synchronous replication, if needed.
@@ -293,15 +302,11 @@ typedef struct PGPROC
 	 */
 	dlist_head	myProcLocks[NUM_LOCK_PARTITIONS];
 
-	XidCacheStatus subxidStatus;	/* mirrored with
-									 * ProcGlobal->subxidStates[i] */
 	struct XidCache subxids;	/* cache for subtransaction XIDs */
 
 	/* Support for group XID clearing. */
-	/* true, if member of ProcArray group waiting for XID clear */
-	bool		procArrayGroupMember;
-	/* next ProcArray group member waiting for XID clear */
-	pg_atomic_uint32 procArrayGroupNext;
+	pg_atomic_uint32 procArrayGroupNext;	/* next ProcArray group member
+											 * waiting for XID clear */
 
 	/*
 	 * latest transaction id among the transaction's main XID and
@@ -309,10 +314,7 @@ typedef struct PGPROC
 	 */
 	TransactionId procArrayGroupMemberXid;
 
-	uint32		wait_event_info;	/* proc's wait information */
-
 	/* Support for group transaction status update. */
-	bool		clogGroupMember;	/* true, if member of clog group */
 	pg_atomic_uint32 clogGroupNext; /* next clog group member */
 	TransactionId clogGroupMemberXid;	/* transaction id of clog group member */
 	XidStatus	clogGroupMemberXidStatus;	/* transaction status of clog
@@ -326,9 +328,6 @@ typedef struct PGPROC
 	LWLock		fpInfoLock;		/* protects per-backend fast-path state */
 	uint64	   *fpLockBits;		/* lock modes held for each fast-path slot */
 	Oid		   *fpRelId;		/* slots for rel oids */
-	bool		fpVXIDLock;		/* are we holding a fast-path VXID lock? */
-	LocalTransactionId fpLocalTransactionId;	/* lxid for fast-path VXID
-												 * lock */
 
 	/*
 	 * Support for lock groups.  Use LockHashPartitionLockByProc on the group


Attachments:

  [text/plain] order.txt (7.1K, 2-order.txt)
  download | inline diff:
commit ea3aded2c4dec487c8a2ff5470cd7bae83e8dd47
Author: Bertrand Drouvot <[email protected]>
Date:   Tue Feb 10 18:54:29 2026 +0000

    reorder

diff --git a/src/include/storage/proc.h b/src/include/storage/proc.h
index 53acce8a5a1..5d41301c966 100644
--- a/src/include/storage/proc.h
+++ b/src/include/storage/proc.h
@@ -184,29 +184,16 @@ typedef enum
  */
 typedef struct PGPROC
 {
-	dlist_node	links;			/* list link if process is in a list */
-	dlist_head *procgloballist; /* procglobal list that owns this PGPROC */
-
-	PGSemaphore sem;			/* ONE semaphore to sleep on */
-	ProcWaitStatus waitStatus;
-
-	Latch		procLatch;		/* generic latch for process */
-
-
-	TransactionId xid;			/* id of top-level transaction currently being
-								 * executed by this proc, if running and XID
-								 * is assigned; else InvalidTransactionId.
-								 * mirrored in ProcGlobal->xids[pgxactoff] */
-
-	TransactionId xmin;			/* minimal running XID as it was when we were
-								 * starting our xact, excluding LAZY VACUUM:
-								 * vacuum must not remove tuples deleted by
-								 * xid >= xmin ! */
-
 	int			pid;			/* Backend's process ID; 0 if prepared xact */
-
 	int			pgxactoff;		/* offset into various ProcGlobal->arrays with
 								 * data mirrored from this PGPROC */
+	BackendType backendType;	/* what kind of process is this? */
+
+	/* These fields are zero while a backend is still starting up: */
+	Oid			databaseId;		/* OID of database this backend is using */
+	Oid			roleId;			/* OID of role using this backend */
+	Oid			tempNamespaceId;	/* OID of temp schema this backend is
+									 * using */
 
 	/*
 	 * Currently running top-level transaction's virtual xid. Together these
@@ -227,24 +214,44 @@ typedef struct PGPROC
 									 * InvalidLocalTransactionId */
 	}			vxid;
 
-	/* These fields are zero while a backend is still starting up: */
-	Oid			databaseId;		/* OID of database this backend is using */
-	Oid			roleId;			/* OID of role using this backend */
+	dlist_node	links;			/* list link if process is in a list */
+	dlist_head *procgloballist; /* procglobal list that owns this PGPROC */
 
-	Oid			tempNamespaceId;	/* OID of temp schema this backend is
-									 * using */
+	uint32		wait_event_info;	/* proc's wait information */
+	pg_atomic_uint64 waitStart; /* time at which wait for lock acquisition
+								 * started */
 
-	BackendType backendType;	/* what kind of process is this? */
+	TransactionId xid;			/* id of top-level transaction currently being
+								 * executed by this proc, if running and XID
+								 * is assigned; else InvalidTransactionId.
+								 * mirrored in ProcGlobal->xids[pgxactoff] */
 
-	/*
-	 * While in hot standby mode, shows that a conflict signal has been sent
-	 * for the current transaction. Set/cleared while holding ProcArrayLock,
-	 * though not required. Accessed without lock, if needed.
-	 *
-	 * This is a bitmask; each bit corresponds to a RecoveryConflictReason
-	 * enum value.
-	 */
-	pg_atomic_uint32 pendingRecoveryConflicts;
+	TransactionId xmin;			/* minimal running XID as it was when we were
+								 * starting our xact, excluding LAZY VACUUM:
+								 * vacuum must not remove tuples deleted by
+								 * xid >= xmin ! */
+
+	ProcWaitStatus waitStatus;
+	int			delayChkptFlags;	/* for DELAY_CHKPT_* flags */
+
+	LOCKMODE	waitLockMode;	/* type of lock we're waiting for */
+	LOCKMASK	heldLocks;		/* bitmask for lock types already held on this
+								 * lock object by this backend */
+
+	uint8		statusFlags;	/* this backend's status flags, see PROC_*
+								 * above. mirrored in
+								 * ProcGlobal->statusFlags[pgxactoff] */
+	XidCacheStatus subxidStatus;	/* mirrored with
+									 * ProcGlobal->subxidStates[i] */
+
+	bool		procArrayGroupMember;	/* true, if member of ProcArray group
+										 * waiting for XID clear */
+	bool		clogGroupMember;	/* true, if member of clog group */
+	bool		fpVXIDLock;		/* are we holding a fast-path VXID lock? */
+
+	LocalTransactionId fpLocalTransactionId;	/* lxid for fast-path VXID
+												 * lock */
+	PGSemaphore sem;			/* ONE semaphore to sleep on */
 
 	/*
 	 * Info about LWLock the process is currently waiting for, if any.
@@ -255,6 +262,19 @@ typedef struct PGPROC
 	 */
 	uint8		lwWaiting;		/* see LWLockWaitState */
 	uint8		lwWaitMode;		/* lwlock mode being waited for */
+
+	Latch		procLatch;		/* generic latch for process */
+
+	/*
+	 * While in hot standby mode, shows that a conflict signal has been sent
+	 * for the current transaction. Set/cleared while holding ProcArrayLock,
+	 * though not required. Accessed without lock, if needed.
+	 *
+	 * This is a bitmask; each bit corresponds to a RecoveryConflictReason
+	 * enum value.
+	 */
+	pg_atomic_uint32 pendingRecoveryConflicts;
+
 	proclist_node lwWaitLink;	/* position in LW lock wait list */
 
 	/* Support for condition variables. */
@@ -264,17 +284,6 @@ typedef struct PGPROC
 	/* waitLock and waitProcLock are NULL if not currently waiting. */
 	LOCK	   *waitLock;		/* Lock object we're sleeping on ... */
 	PROCLOCK   *waitProcLock;	/* Per-holder info for awaited lock */
-	LOCKMODE	waitLockMode;	/* type of lock we're waiting for */
-	LOCKMASK	heldLocks;		/* bitmask for lock types already held on this
-								 * lock object by this backend */
-	pg_atomic_uint64 waitStart; /* time at which wait for lock acquisition
-								 * started */
-
-	int			delayChkptFlags;	/* for DELAY_CHKPT_* flags */
-
-	uint8		statusFlags;	/* this backend's status flags, see PROC_*
-								 * above. mirrored in
-								 * ProcGlobal->statusFlags[pgxactoff] */
 
 	/*
 	 * Info to allow us to wait for synchronous replication, if needed.
@@ -293,15 +302,11 @@ typedef struct PGPROC
 	 */
 	dlist_head	myProcLocks[NUM_LOCK_PARTITIONS];
 
-	XidCacheStatus subxidStatus;	/* mirrored with
-									 * ProcGlobal->subxidStates[i] */
 	struct XidCache subxids;	/* cache for subtransaction XIDs */
 
 	/* Support for group XID clearing. */
-	/* true, if member of ProcArray group waiting for XID clear */
-	bool		procArrayGroupMember;
-	/* next ProcArray group member waiting for XID clear */
-	pg_atomic_uint32 procArrayGroupNext;
+	pg_atomic_uint32 procArrayGroupNext;	/* next ProcArray group member
+											 * waiting for XID clear */
 
 	/*
 	 * latest transaction id among the transaction's main XID and
@@ -309,10 +314,7 @@ typedef struct PGPROC
 	 */
 	TransactionId procArrayGroupMemberXid;
 
-	uint32		wait_event_info;	/* proc's wait information */
-
 	/* Support for group transaction status update. */
-	bool		clogGroupMember;	/* true, if member of clog group */
 	pg_atomic_uint32 clogGroupNext; /* next clog group member */
 	TransactionId clogGroupMemberXid;	/* transaction id of clog group member */
 	XidStatus	clogGroupMemberXidStatus;	/* transaction status of clog
@@ -326,9 +328,6 @@ typedef struct PGPROC
 	LWLock		fpInfoLock;		/* protects per-backend fast-path state */
 	uint64	   *fpLockBits;		/* lock modes held for each fast-path slot */
 	Oid		   *fpRelId;		/* slots for rel oids */
-	bool		fpVXIDLock;		/* are we holding a fast-path VXID lock? */
-	LocalTransactionId fpLocalTransactionId;	/* lxid for fast-path VXID
-												 * lock */
 
 	/*
 	 * Support for lock groups.  Use LockHashPartitionLockByProc on the group


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