v0-0001-Reduce-size-of-LOCK-by-16-bytes.patch

From 1abaf5c17ddb92a14bd20afc0e43ad3fc21a8475 Mon Sep 17 00:00:00 2001
From: Matthias van de Meent <boekewurm+postgres@gmail.com>
Date: Wed, 20 Nov 2024 03:27:32 +0100
Subject: [PATCH v0 1/4] Reduce size of LOCK by 16 bytes

We only ever need 8 lockmodes, rather than 10, as NoLock is never registered,
and MaxLockmode is AccessExclusiveLock (8).

Also adjust various locations where we assume MAX_LOCKMODES > MaxLockMode.
---
 src/include/storage/lock.h           |  4 +-
 src/backend/access/common/relation.c |  6 +--
 src/backend/access/index/indexam.c   |  2 +-
 src/backend/storage/lmgr/README      | 15 +++---
 src/backend/storage/lmgr/lock.c      | 76 ++++++++++++++++------------
 src/backend/utils/adt/lockfuncs.c    |  2 +-
 6 files changed, 59 insertions(+), 46 deletions(-)

diff --git a/src/include/storage/lock.h b/src/include/storage/lock.h
index 787f3db06a..b2523bf79d 100644
--- a/src/include/storage/lock.h
+++ b/src/include/storage/lock.h
@@ -79,11 +79,13 @@ typedef struct
 		 (vxid_dst).localTransactionId = (proc).vxid.lxid)
 
 /* MAX_LOCKMODES cannot be larger than the # of bits in LOCKMASK */
-#define MAX_LOCKMODES		10
+#define MAX_LOCKMODES		MaxLockMode
 
 #define LOCKBIT_ON(lockmode) (1 << (lockmode))
 #define LOCKBIT_OFF(lockmode) (~(1 << (lockmode)))
 
+#define LOCK_VALID_LOCKMODE(lockmode) \
+	((lockmode) > NoLock && (lockmode) <= MaxLockMode)
 
 /*
  * This data structure defines the locking semantics associated with a
diff --git a/src/backend/access/common/relation.c b/src/backend/access/common/relation.c
index d8a313a2c9..78e58d13ce 100644
--- a/src/backend/access/common/relation.c
+++ b/src/backend/access/common/relation.c
@@ -48,7 +48,7 @@ relation_open(Oid relationId, LOCKMODE lockmode)
 {
 	Relation	r;
 
-	Assert(lockmode >= NoLock && lockmode < MAX_LOCKMODES);
+	Assert(lockmode >= NoLock && lockmode <= MAX_LOCKMODES);
 
 	/* Get the lock before trying to open the relcache entry */
 	if (lockmode != NoLock)
@@ -89,7 +89,7 @@ try_relation_open(Oid relationId, LOCKMODE lockmode)
 {
 	Relation	r;
 
-	Assert(lockmode >= NoLock && lockmode < MAX_LOCKMODES);
+	Assert(lockmode >= NoLock && lockmode <= MAX_LOCKMODES);
 
 	/* Get the lock first */
 	if (lockmode != NoLock)
@@ -206,7 +206,7 @@ relation_close(Relation relation, LOCKMODE lockmode)
 {
 	LockRelId	relid = relation->rd_lockInfo.lockRelId;
 
-	Assert(lockmode >= NoLock && lockmode < MAX_LOCKMODES);
+	Assert(lockmode >= NoLock && lockmode <= MAX_LOCKMODES);
 
 	/* The relcache does the real work... */
 	RelationClose(relation);
diff --git a/src/backend/access/index/indexam.c b/src/backend/access/index/indexam.c
index 1859be614c..70b9ac120e 100644
--- a/src/backend/access/index/indexam.c
+++ b/src/backend/access/index/indexam.c
@@ -178,7 +178,7 @@ index_close(Relation relation, LOCKMODE lockmode)
 {
 	LockRelId	relid = relation->rd_lockInfo.lockRelId;
 
-	Assert(lockmode >= NoLock && lockmode < MAX_LOCKMODES);
+	Assert(lockmode >= NoLock && lockmode <= MAX_LOCKMODES);
 
 	/* The relcache does the real work... */
 	RelationClose(relation);
diff --git a/src/backend/storage/lmgr/README b/src/backend/storage/lmgr/README
index 45de0fd2bd..f85a0d6020 100644
--- a/src/backend/storage/lmgr/README
+++ b/src/backend/storage/lmgr/README
@@ -105,7 +105,7 @@ grantMask -
     table) to determine if a new lock request will conflict with existing
     lock types held.  Conflicts are determined by bitwise AND operations
     between the grantMask and the conflict table entry for the requested
-    lock type.  Bit i of grantMask is 1 if and only if granted[i] > 0.
+    lock type.  Bit i of grantMask is 1 if and only if granted[i - 1] > 0.
 
 waitMask -
     This bitmask shows the types of locks being waited for.  Bit i of waitMask
@@ -133,10 +133,10 @@ nRequested -
     only in the backend's LOCALLOCK structure.)
 
 requested -
-    Keeps a count of how many locks of each type have been attempted.  Only
-    elements 1 through MAX_LOCKMODES-1 are used as they correspond to the lock
-    type defined constants.  Summing the values of requested[] should come out
-    equal to nRequested.
+    Keeps a count of how many locks of each type have been attempted.
+    Elements 0 through MAX_LOCKMODES (inclusive) are used, and correspond to
+    lock type definded constants with value elem+1.  Summing the values of
+    requested[] should come out equal to nRequested.
 
 nGranted -
     Keeps count of how many times this lock has been successfully acquired.
@@ -145,9 +145,8 @@ nGranted -
 
 granted -
     Keeps count of how many locks of each type are currently held.  Once again
-    only elements 1 through MAX_LOCKMODES-1 are used (0 is not).  Also, like
-    requested[], summing the values of granted[] should total to the value
-    of nGranted.
+    only elements 0 through MAX_LOCKMODES are used.  Also, like requested[],
+    summing the values of granted[] should total to the value of nGranted.
 
 We should always have 0 <= nGranted <= nRequested, and
 0 <= granted[i] <= requested[i] for each i.  When all the request counts
diff --git a/src/backend/storage/lmgr/lock.c b/src/backend/storage/lmgr/lock.c
index edc5020c6a..9bf6fbf976 100644
--- a/src/backend/storage/lmgr/lock.c
+++ b/src/backend/storage/lmgr/lock.c
@@ -367,19 +367,19 @@ LOCK_PRINT(const char *where, const LOCK *lock, LOCKMODE type)
 	if (LOCK_DEBUG_ENABLED(&lock->tag))
 		elog(LOG,
 			 "%s: lock(%p) id(%u,%u,%u,%u,%u,%u) grantMask(%x) "
-			 "req(%d,%d,%d,%d,%d,%d,%d)=%d "
-			 "grant(%d,%d,%d,%d,%d,%d,%d)=%d wait(%d) type(%s)",
+			 "req(%d,%d,%d,%d,%d,%d,%d,%d)=%d "
+			 "grant(%d,%d,%d,%d,%d,%d,%d,%d)=%d wait(%d) type(%s)",
 			 where, lock,
 			 lock->tag.locktag_field1, lock->tag.locktag_field2,
 			 lock->tag.locktag_field3, lock->tag.locktag_field4,
 			 lock->tag.locktag_type, lock->tag.locktag_lockmethodid,
 			 lock->grantMask,
-			 lock->requested[1], lock->requested[2], lock->requested[3],
-			 lock->requested[4], lock->requested[5], lock->requested[6],
-			 lock->requested[7], lock->nRequested,
-			 lock->granted[1], lock->granted[2], lock->granted[3],
-			 lock->granted[4], lock->granted[5], lock->granted[6],
-			 lock->granted[7], lock->nGranted,
+			 lock->requested[0], lock->requested[1], lock->requested[2],
+			 lock->requested[3], lock->requested[4], lock->requested[5],
+			 lock->requested[6], lock->requested[7], lock->nRequested,
+			 lock->granted[0], lock->granted[1], lock->granted[2],
+			 lock->granted[3], lock->granted[4], lock->granted[5],
+			 lock->granted[6], lock->granted[7], lock->nGranted,
 			 dclist_count(&lock->waitProcs),
 			 LockMethods[LOCK_LOCKMETHOD(*lock)]->lockModeNames[type]);
 }
@@ -704,6 +704,8 @@ LockHasWaiters(const LOCKTAG *locktag, LOCKMODE lockmode, bool sessionLock)
 	if (lockmode <= 0 || lockmode > lockMethodTable->numLockModes)
 		elog(ERROR, "unrecognized lock mode: %d", lockmode);
 
+	Assert(LOCK_VALID_LOCKMODE(lockmode));
+
 #ifdef LOCK_DEBUG
 	if (LOCK_DEBUG_ENABLED(locktag))
 		elog(LOG, "LockHasWaiters: lock [%u,%u] %s",
@@ -851,6 +853,8 @@ LockAcquireExtended(const LOCKTAG *locktag,
 	if (lockmode <= 0 || lockmode > lockMethodTable->numLockModes)
 		elog(ERROR, "unrecognized lock mode: %d", lockmode);
 
+	Assert(LOCK_VALID_LOCKMODE(lockmode));
+
 	if (RecoveryInProgress() && !InRecovery &&
 		(locktag->locktag_type == LOCKTAG_OBJECT ||
 		 locktag->locktag_type == LOCKTAG_RELATION) &&
@@ -1133,11 +1137,11 @@ LockAcquireExtended(const LOCKTAG *locktag,
 		else
 			PROCLOCK_PRINT("LockAcquire: did not join wait queue", proclock);
 		lock->nRequested--;
-		lock->requested[lockmode]--;
+		lock->requested[lockmode - 1]--;
 		LOCK_PRINT("LockAcquire: did not join wait queue",
 				   lock, lockmode);
 		Assert((lock->nRequested > 0) &&
-			   (lock->requested[lockmode] >= 0));
+			   (lock->requested[lockmode - 1] >= 0));
 		Assert(lock->nGranted <= lock->nRequested);
 		LWLockRelease(partitionLock);
 		if (locallock->nLocks == 0)
@@ -1237,6 +1241,7 @@ SetupLockInTable(LockMethod lockMethodTable, PGPROC *proc,
 	PROCLOCKTAG proclocktag;
 	uint32		proclock_hashcode;
 	bool		found;
+	Assert(LOCK_VALID_LOCKMODE(lockmode));
 
 	/*
 	 * Find or create a lock with this tag.
@@ -1267,8 +1272,8 @@ SetupLockInTable(LockMethod lockMethodTable, PGPROC *proc,
 	else
 	{
 		LOCK_PRINT("LockAcquire: found", lock, lockmode);
-		Assert((lock->nRequested >= 0) && (lock->requested[lockmode] >= 0));
-		Assert((lock->nGranted >= 0) && (lock->granted[lockmode] >= 0));
+		Assert((lock->nRequested >= 0) && (lock->requested[lockmode - 1] >= 0));
+		Assert((lock->nGranted >= 0) && (lock->granted[lockmode - 1] >= 0));
 		Assert(lock->nGranted <= lock->nRequested);
 	}
 
@@ -1385,8 +1390,8 @@ SetupLockInTable(LockMethod lockMethodTable, PGPROC *proc,
 	 * The other counts don't increment till we get the lock.
 	 */
 	lock->nRequested++;
-	lock->requested[lockmode]++;
-	Assert((lock->nRequested > 0) && (lock->requested[lockmode] > 0));
+	lock->requested[lockmode - 1]++;
+	Assert((lock->nRequested > 0) && (lock->requested[lockmode - 1] > 0));
 
 	/*
 	 * We shouldn't already hold the desired lock; else locallock table is
@@ -1483,7 +1488,7 @@ LockCheckConflicts(LockMethod lockMethodTable,
 	int			numLockModes = lockMethodTable->numLockModes;
 	LOCKMASK	myLocks;
 	int			conflictMask = lockMethodTable->conflictTab[lockmode];
-	int			conflictsRemaining[MAX_LOCKMODES];
+	int			conflictsRemaining[MAX_LOCKMODES + 1];
 	int			totalConflictsRemaining = 0;
 	dlist_iter	proclock_iter;
 	int			i;
@@ -1516,7 +1521,7 @@ LockCheckConflicts(LockMethod lockMethodTable,
 			conflictsRemaining[i] = 0;
 			continue;
 		}
-		conflictsRemaining[i] = lock->granted[i];
+		conflictsRemaining[i] = lock->granted[i - 1];
 		if (myLocks & LOCKBIT_ON(i))
 			--conflictsRemaining[i];
 		totalConflictsRemaining += conflictsRemaining[i];
@@ -1606,14 +1611,16 @@ LockCheckConflicts(LockMethod lockMethodTable,
 void
 GrantLock(LOCK *lock, PROCLOCK *proclock, LOCKMODE lockmode)
 {
+	Assert(LOCK_VALID_LOCKMODE(lockmode));
+
 	lock->nGranted++;
-	lock->granted[lockmode]++;
+	lock->granted[lockmode - 1]++;
 	lock->grantMask |= LOCKBIT_ON(lockmode);
-	if (lock->granted[lockmode] == lock->requested[lockmode])
+	if (lock->granted[lockmode - 1] == lock->requested[lockmode - 1])
 		lock->waitMask &= LOCKBIT_OFF(lockmode);
 	proclock->holdMask |= LOCKBIT_ON(lockmode);
 	LOCK_PRINT("GrantLock", lock, lockmode);
-	Assert((lock->nGranted > 0) && (lock->granted[lockmode] > 0));
+	Assert((lock->nGranted > 0) && (lock->granted[lockmode - 1] > 0));
 	Assert(lock->nGranted <= lock->nRequested);
 }
 
@@ -1631,20 +1638,21 @@ UnGrantLock(LOCK *lock, LOCKMODE lockmode,
 			PROCLOCK *proclock, LockMethod lockMethodTable)
 {
 	bool		wakeupNeeded = false;
+	Assert(LOCK_VALID_LOCKMODE(lockmode));
 
-	Assert((lock->nRequested > 0) && (lock->requested[lockmode] > 0));
-	Assert((lock->nGranted > 0) && (lock->granted[lockmode] > 0));
+	Assert((lock->nRequested > 0) && (lock->requested[lockmode - 1] > 0));
+	Assert((lock->nGranted > 0) && (lock->granted[lockmode - 1] > 0));
 	Assert(lock->nGranted <= lock->nRequested);
 
 	/*
 	 * fix the general lock stats
 	 */
 	lock->nRequested--;
-	lock->requested[lockmode]--;
+	lock->requested[lockmode - 1]--;
 	lock->nGranted--;
-	lock->granted[lockmode]--;
+	lock->granted[lockmode - 1]--;
 
-	if (lock->granted[lockmode] == 0)
+	if (lock->granted[lockmode - 1] == 0)
 	{
 		/* change the conflict mask.  No more of this lock type. */
 		lock->grantMask &= LOCKBIT_OFF(lockmode);
@@ -1656,7 +1664,7 @@ UnGrantLock(LOCK *lock, LOCKMODE lockmode,
 	 * We need only run ProcLockWakeup if the released lock conflicts with at
 	 * least one of the lock types requested by waiter(s).  Otherwise whatever
 	 * conflict made them wait must still exist.  NOTE: before MVCC, we could
-	 * skip wakeup if lock->granted[lockmode] was still positive. But that's
+	 * skip wakeup if lock->granted[lockmode - 1] was still positive. But that's
 	 * not true anymore, because the remaining granted locks might belong to
 	 * some waiter, who could now be awakened because he doesn't conflict with
 	 * his own locks.
@@ -1973,10 +1981,10 @@ RemoveFromWaitQueue(PGPROC *proc, uint32 hashcode)
 	Assert(waitLock->nRequested > 0);
 	Assert(waitLock->nRequested > proc->waitLock->nGranted);
 	waitLock->nRequested--;
-	Assert(waitLock->requested[lockmode] > 0);
-	waitLock->requested[lockmode]--;
+	Assert(waitLock->requested[lockmode - 1] > 0);
+	waitLock->requested[lockmode - 1]--;
 	/* don't forget to clear waitMask bit if appropriate */
-	if (waitLock->granted[lockmode] == waitLock->requested[lockmode])
+	if (waitLock->granted[lockmode - 1] == waitLock->requested[lockmode - 1])
 		waitLock->waitMask &= LOCKBIT_OFF(lockmode);
 
 	/* Clean up the proc's own state, and pass it the ok/fail signal */
@@ -2025,6 +2033,8 @@ LockRelease(const LOCKTAG *locktag, LOCKMODE lockmode, bool sessionLock)
 	if (lockmode <= 0 || lockmode > lockMethodTable->numLockModes)
 		elog(ERROR, "unrecognized lock mode: %d", lockmode);
 
+	Assert(LOCK_VALID_LOCKMODE(lockmode));
+
 #ifdef LOCK_DEBUG
 	if (LOCK_DEBUG_ENABLED(locktag))
 		elog(LOG, "LockRelease: lock [%u,%u] %s",
@@ -4311,6 +4321,8 @@ lock_twophase_recover(TransactionId xid, uint16 info,
 				 errhint("You might need to increase \"%s\".", "max_locks_per_transaction")));
 	}
 
+	Assert(LOCK_VALID_LOCKMODE(lockmode));
+
 	/*
 	 * if it's a new lock object, initialize it
 	 */
@@ -4329,8 +4341,8 @@ lock_twophase_recover(TransactionId xid, uint16 info,
 	else
 	{
 		LOCK_PRINT("lock_twophase_recover: found", lock, lockmode);
-		Assert((lock->nRequested >= 0) && (lock->requested[lockmode] >= 0));
-		Assert((lock->nGranted >= 0) && (lock->granted[lockmode] >= 0));
+		Assert((lock->nRequested >= 0) && (lock->requested[lockmode - 1] >= 0));
+		Assert((lock->nGranted >= 0) && (lock->granted[lockmode - 1] >= 0));
 		Assert(lock->nGranted <= lock->nRequested);
 	}
 
@@ -4402,8 +4414,8 @@ lock_twophase_recover(TransactionId xid, uint16 info,
 	 * requests, whether granted or waiting, so increment those immediately.
 	 */
 	lock->nRequested++;
-	lock->requested[lockmode]++;
-	Assert((lock->nRequested > 0) && (lock->requested[lockmode] > 0));
+	lock->requested[lockmode - 1]++;
+	Assert((lock->nRequested > 0) && (lock->requested[lockmode - 1] > 0));
 
 	/*
 	 * We shouldn't already hold the desired lock.
diff --git a/src/backend/utils/adt/lockfuncs.c b/src/backend/utils/adt/lockfuncs.c
index e790f856ab..f21f348464 100644
--- a/src/backend/utils/adt/lockfuncs.c
+++ b/src/backend/utils/adt/lockfuncs.c
@@ -189,7 +189,7 @@ pg_lock_status(PG_FUNCTION_ARGS)
 		granted = false;
 		if (instance->holdMask)
 		{
-			for (mode = 0; mode < MAX_LOCKMODES; mode++)
+			for (mode = 1; mode <= MAX_LOCKMODES; mode++)
 			{
 				if (instance->holdMask & LOCKBIT_ON(mode))
 				{
-- 
2.45.2