From 98d69e37db722dde4cd4dceda1123f6fa0fb8d8b Mon Sep 17 00:00:00 2001 From: Matthias van de Meent Date: Wed, 20 Nov 2024 03:46:16 +0100 Subject: [PATCH v0 2/4] Reduce size of LOCK by 8 more bytes LOCKMASK will only use bits [1..8], and thus always fits in uint16. By changing the type from int to uint16, and moving the grant/wait masks in LOCK to the padding space of dclist_head, we save 8 bytes on the struct when the binary is compiled for a 64-bit architecture. --- src/include/storage/lock.h | 18 +++++++++-- src/include/storage/lockdefs.h | 2 +- src/backend/storage/lmgr/deadlock.c | 12 ++++---- src/backend/storage/lmgr/lock.c | 48 ++++++++++++++--------------- src/backend/storage/lmgr/proc.c | 8 ++--- 5 files changed, 50 insertions(+), 38 deletions(-) diff --git a/src/include/storage/lock.h b/src/include/storage/lock.h index b2523bf79d..345ded934f 100644 --- a/src/include/storage/lock.h +++ b/src/include/storage/lock.h @@ -289,6 +289,13 @@ typedef struct LOCKTAG (locktag).locktag_type = LOCKTAG_APPLY_TRANSACTION, \ (locktag).locktag_lockmethodid = DEFAULT_LOCKMETHOD) +/* + * On 64-bit architectures there are 4 bytes of padding in dclist_head. We + * reuse those 4 padding bytes to store some values. + */ +#define SIZEOF_PACKED_DCLIST_HEAD \ + (offsetof(dclist_head, count) + sizeof(uint32)) + /* * Per-locked-object lock information: * @@ -313,10 +320,15 @@ typedef struct LOCK LOCKTAG tag; /* unique identifier of lockable object */ /* data */ - LOCKMASK grantMask; /* bitmask for lock types already granted */ - LOCKMASK waitMask; /* bitmask for lock types awaited */ dlist_head procLocks; /* list of PROCLOCK objects assoc. with lock */ - dclist_head waitProcs; /* list of PGPROC objects waiting on lock */ + union { + dclist_head waitProcs; /* list of PGPROC objects waiting on lock */ + struct { + char pad[SIZEOF_PACKED_DCLIST_HEAD]; + LOCKMASK grantMask; /* bitmask for lock types already granted */ + LOCKMASK waitMask; /* bitmask for lock types awaited */ + } masks; + } packed; int requested[MAX_LOCKMODES]; /* counts of requested locks */ int nRequested; /* total of requested[] array */ int granted[MAX_LOCKMODES]; /* counts of granted locks */ diff --git a/src/include/storage/lockdefs.h b/src/include/storage/lockdefs.h index 810b297edf..c75b98960b 100644 --- a/src/include/storage/lockdefs.h +++ b/src/include/storage/lockdefs.h @@ -22,7 +22,7 @@ * mask indicating a set of held or requested lock types (the bit 1<waitProcs; + dclist_head *waitQueue = &lock->packed.waitProcs; Assert(nProcs == dclist_count(waitQueue)); @@ -697,7 +697,7 @@ FindLockCycleRecurseMember(PGPROC *checkProc, dclist_head *waitQueue; /* Use the true lock wait queue order */ - waitQueue = &lock->waitProcs; + waitQueue = &lock->packed.waitProcs; /* * Find the last member of the lock group that is present in the wait @@ -813,8 +813,8 @@ ExpandConstraints(EDGE *constraints, /* No, so allocate a new list */ waitOrders[nWaitOrders].lock = lock; waitOrders[nWaitOrders].procs = waitOrderProcs + nWaitOrderProcs; - waitOrders[nWaitOrders].nProcs = dclist_count(&lock->waitProcs); - nWaitOrderProcs += dclist_count(&lock->waitProcs); + waitOrders[nWaitOrders].nProcs = dclist_count(&lock->packed.waitProcs); + nWaitOrderProcs += dclist_count(&lock->packed.waitProcs); Assert(nWaitOrderProcs <= MaxBackends); /* @@ -861,7 +861,7 @@ TopoSort(LOCK *lock, int nConstraints, PGPROC **ordering) /* output argument */ { - dclist_head *waitQueue = &lock->waitProcs; + dclist_head *waitQueue = &lock->packed.waitProcs; int queue_size = dclist_count(waitQueue); PGPROC *proc; int i, @@ -1049,7 +1049,7 @@ TopoSort(LOCK *lock, static void PrintLockQueue(LOCK *lock, const char *info) { - dclist_head *waitQueue = &lock->waitProcs; + dclist_head *waitQueue = &lock->packed.waitProcs; dlist_iter proc_iter; printf("%s lock %p queue ", info, lock); diff --git a/src/backend/storage/lmgr/lock.c b/src/backend/storage/lmgr/lock.c index 9bf6fbf976..65349b1196 100644 --- a/src/backend/storage/lmgr/lock.c +++ b/src/backend/storage/lmgr/lock.c @@ -373,14 +373,14 @@ LOCK_PRINT(const char *where, const LOCK *lock, LOCKMODE type) 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->packed.masks.grantMask, 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), + dclist_count(&lock->packed.waitProcs), LockMethods[LOCK_LOCKMETHOD(*lock)]->lockModeNames[type]); } @@ -768,7 +768,7 @@ LockHasWaiters(const LOCKTAG *locktag, LOCKMODE lockmode, bool sessionLock) /* * Do the checking. */ - if ((lockMethodTable->conflictTab[lockmode] & lock->waitMask) != 0) + if ((lockMethodTable->conflictTab[lockmode] & lock->packed.masks.waitMask) != 0) hasWaiters = true; LWLockRelease(partitionLock); @@ -1085,7 +1085,7 @@ LockAcquireExtended(const LOCKTAG *locktag, * wait queue. Otherwise, check for conflict with already-held locks. * (That's last because most complex check.) */ - if (lockMethodTable->conflictTab[lockmode] & lock->waitMask) + if (lockMethodTable->conflictTab[lockmode] & lock->packed.masks.waitMask) found_conflict = true; else found_conflict = LockCheckConflicts(lockMethodTable, lockmode, @@ -1259,10 +1259,10 @@ SetupLockInTable(LockMethod lockMethodTable, PGPROC *proc, */ if (!found) { - lock->grantMask = 0; - lock->waitMask = 0; + lock->packed.masks.grantMask = 0; + lock->packed.masks.waitMask = 0; dlist_init(&lock->procLocks); - dclist_init(&lock->waitProcs); + dclist_init(&lock->packed.waitProcs); lock->nRequested = 0; lock->nGranted = 0; MemSet(lock->requested, 0, sizeof(int) * MAX_LOCKMODES); @@ -1344,7 +1344,7 @@ SetupLockInTable(LockMethod lockMethodTable, PGPROC *proc, else { PROCLOCK_PRINT("LockAcquire: found", proclock); - Assert((proclock->holdMask & ~lock->grantMask) == 0); + Assert((proclock->holdMask & ~lock->packed.masks.grantMask) == 0); #ifdef CHECK_DEADLOCK_RISK @@ -1497,12 +1497,12 @@ LockCheckConflicts(LockMethod lockMethodTable, * first check for global conflicts: If no locks conflict with my request, * then I get the lock. * - * Checking for conflict: lock->grantMask represents the types of + * Checking for conflict: lock->packed.masks.grantMask represents the types of * currently held locks. conflictTable[lockmode] has a bit set for each * type of lock that conflicts with request. Bitwise compare tells if * there is a conflict. */ - if (!(conflictMask & lock->grantMask)) + if (!(conflictMask & lock->packed.masks.grantMask)) { PROCLOCK_PRINT("LockCheckConflicts: no conflict", proclock); return false; @@ -1615,9 +1615,9 @@ GrantLock(LOCK *lock, PROCLOCK *proclock, LOCKMODE lockmode) lock->nGranted++; lock->granted[lockmode - 1]++; - lock->grantMask |= LOCKBIT_ON(lockmode); + lock->packed.masks.grantMask |= LOCKBIT_ON(lockmode); if (lock->granted[lockmode - 1] == lock->requested[lockmode - 1]) - lock->waitMask &= LOCKBIT_OFF(lockmode); + lock->packed.masks.waitMask &= LOCKBIT_OFF(lockmode); proclock->holdMask |= LOCKBIT_ON(lockmode); LOCK_PRINT("GrantLock", lock, lockmode); Assert((lock->nGranted > 0) && (lock->granted[lockmode - 1] > 0)); @@ -1655,7 +1655,7 @@ UnGrantLock(LOCK *lock, LOCKMODE lockmode, if (lock->granted[lockmode - 1] == 0) { /* change the conflict mask. No more of this lock type. */ - lock->grantMask &= LOCKBIT_OFF(lockmode); + lock->packed.masks.grantMask &= LOCKBIT_OFF(lockmode); } LOCK_PRINT("UnGrantLock: updated", lock, lockmode); @@ -1669,7 +1669,7 @@ UnGrantLock(LOCK *lock, LOCKMODE lockmode, * some waiter, who could now be awakened because he doesn't conflict with * his own locks. */ - if (lockMethodTable->conflictTab[lockmode] & lock->waitMask) + if (lockMethodTable->conflictTab[lockmode] & lock->packed.masks.waitMask) wakeupNeeded = true; /* @@ -1971,11 +1971,11 @@ RemoveFromWaitQueue(PGPROC *proc, uint32 hashcode) Assert(proc->waitStatus == PROC_WAIT_STATUS_WAITING); Assert(proc->links.next != NULL); Assert(waitLock); - Assert(!dclist_is_empty(&waitLock->waitProcs)); + Assert(!dclist_is_empty(&waitLock->packed.waitProcs)); Assert(0 < lockmethodid && lockmethodid < lengthof(LockMethods)); /* Remove proc from lock's wait queue */ - dclist_delete_from_thoroughly(&waitLock->waitProcs, &proc->links); + dclist_delete_from_thoroughly(&waitLock->packed.waitProcs, &proc->links); /* Undo increments of request counts by waiting process */ Assert(waitLock->nRequested > 0); @@ -1985,7 +1985,7 @@ RemoveFromWaitQueue(PGPROC *proc, uint32 hashcode) waitLock->requested[lockmode - 1]--; /* don't forget to clear waitMask bit if appropriate */ if (waitLock->granted[lockmode - 1] == waitLock->requested[lockmode - 1]) - waitLock->waitMask &= LOCKBIT_OFF(lockmode); + waitLock->packed.masks.waitMask &= LOCKBIT_OFF(lockmode); /* Clean up the proc's own state, and pass it the ok/fail signal */ proc->waitLock = NULL; @@ -2459,7 +2459,7 @@ LockReleaseAll(LOCKMETHODID lockmethodid, bool allLocks) Assert(lock->nRequested >= 0); Assert(lock->nGranted >= 0); Assert(lock->nGranted <= lock->nRequested); - Assert((proclock->holdMask & ~lock->grantMask) == 0); + Assert((proclock->holdMask & ~lock->packed.masks.grantMask) == 0); /* * Release the previously-marked lock modes @@ -3605,7 +3605,7 @@ PostPrepare_Locks(TransactionId xid) Assert(lock->nRequested >= 0); Assert(lock->nGranted >= 0); Assert(lock->nGranted <= lock->nRequested); - Assert((proclock->holdMask & ~lock->grantMask) == 0); + Assert((proclock->holdMask & ~lock->packed.masks.grantMask) == 0); /* Ignore it if nothing to release (must be a session lock) */ if (proclock->releaseMask == 0) @@ -4046,7 +4046,7 @@ GetSingleProcBlockerStatusData(PGPROC *blocked_proc, BlockedProcsData *data) } /* Enlarge waiter_pids[] if it's too small to hold all wait queue PIDs */ - waitQueue = &(theLock->waitProcs); + waitQueue = &(theLock->packed.waitProcs); queue_size = dclist_count(waitQueue); if (queue_size > data->maxpids - data->npids) @@ -4328,10 +4328,10 @@ lock_twophase_recover(TransactionId xid, uint16 info, */ if (!found) { - lock->grantMask = 0; - lock->waitMask = 0; + lock->packed.masks.grantMask = 0; + lock->packed.masks.waitMask = 0; dlist_init(&lock->procLocks); - dclist_init(&lock->waitProcs); + dclist_init(&lock->packed.waitProcs); lock->nRequested = 0; lock->nGranted = 0; MemSet(lock->requested, 0, sizeof(int) * MAX_LOCKMODES); @@ -4406,7 +4406,7 @@ lock_twophase_recover(TransactionId xid, uint16 info, else { PROCLOCK_PRINT("lock_twophase_recover: found", proclock); - Assert((proclock->holdMask & ~lock->grantMask) == 0); + Assert((proclock->holdMask & ~lock->packed.masks.grantMask) == 0); } /* diff --git a/src/backend/storage/lmgr/proc.c b/src/backend/storage/lmgr/proc.c index 720ef99ee8..a359c0be21 100644 --- a/src/backend/storage/lmgr/proc.c +++ b/src/backend/storage/lmgr/proc.c @@ -1088,7 +1088,7 @@ JoinWaitQueue(LOCALLOCK *locallock, LockMethod lockMethodTable, bool dontWait) PROCLOCK *proclock = locallock->proclock; uint32 hashcode = locallock->hashcode; LWLock *partitionLock PG_USED_FOR_ASSERTS_ONLY = LockHashPartitionLock(hashcode); - dclist_head *waitQueue = &lock->waitProcs; + dclist_head *waitQueue = &lock->packed.waitProcs; PGPROC *insert_before = NULL; LOCKMASK myProcHeldLocks; LOCKMASK myHeldLocks; @@ -1223,7 +1223,7 @@ JoinWaitQueue(LOCALLOCK *locallock, LockMethod lockMethodTable, bool dontWait) else dclist_push_tail(waitQueue, &MyProc->links); - lock->waitMask |= LOCKBIT_ON(lockmode); + lock->packed.masks.waitMask |= LOCKBIT_ON(lockmode); /* Set up wait information in PGPROC object, too */ MyProc->heldLocks = myProcHeldLocks; @@ -1708,7 +1708,7 @@ ProcWakeup(PGPROC *proc, ProcWaitStatus waitStatus) Assert(proc->waitStatus == PROC_WAIT_STATUS_WAITING); /* Remove process from wait queue */ - dclist_delete_from_thoroughly(&proc->waitLock->waitProcs, &proc->links); + dclist_delete_from_thoroughly(&proc->waitLock->packed.waitProcs, &proc->links); /* Clean up process' state and pass it the ok/fail signal */ proc->waitLock = NULL; @@ -1730,7 +1730,7 @@ ProcWakeup(PGPROC *proc, ProcWaitStatus waitStatus) void ProcLockWakeup(LockMethod lockMethodTable, LOCK *lock) { - dclist_head *waitQueue = &lock->waitProcs; + dclist_head *waitQueue = &lock->packed.waitProcs; LOCKMASK aheadRequests = 0; dlist_mutable_iter miter; -- 2.45.2