From 562657ea3f7be124a6c6b6d1e7450da2431a54a0 Mon Sep 17 00:00:00 2001 From: Thomas Munro Date: Thu, 11 Mar 2021 23:09:11 +1300 Subject: [PATCH v5 2/2] Remove PGPROC's redundant pgprocno member. It's derivable with pointer arithmetic. Author: Soumyadeep Chakraborty Discussion: https://postgr.es/m/CA%2BhUKGLYRyDaneEwz5Uya_OgFLMx5BgJfkQSD%3Dq9HmwsfRRb-w%40mail.gmail.com --- src/backend/access/transam/clog.c | 2 +- src/backend/access/transam/twophase.c | 3 +-- src/backend/access/transam/xlog.c | 2 +- src/backend/postmaster/bgwriter.c | 2 +- src/backend/postmaster/pgarch.c | 2 +- src/backend/storage/buffer/bufmgr.c | 6 +++--- src/backend/storage/ipc/procarray.c | 6 +++--- src/backend/storage/lmgr/condition_variable.c | 12 ++++++------ src/backend/storage/lmgr/lwlock.c | 6 +++--- src/backend/storage/lmgr/predicate.c | 2 +- src/backend/storage/lmgr/proc.c | 8 +++----- src/backend/utils/adt/lockfuncs.c | 1 + src/include/storage/lock.h | 2 +- src/include/storage/proc.h | 6 +++++- 14 files changed, 31 insertions(+), 29 deletions(-) diff --git a/src/backend/access/transam/clog.c b/src/backend/access/transam/clog.c index 3ea16a270a..d0557f6c55 100644 --- a/src/backend/access/transam/clog.c +++ b/src/backend/access/transam/clog.c @@ -464,7 +464,7 @@ TransactionGroupUpdateXidStatus(TransactionId xid, XidStatus status, if (pg_atomic_compare_exchange_u32(&procglobal->clogGroupFirst, &nextidx, - (uint32) proc->pgprocno)) + (uint32) GetPGProcNumber(proc))) break; } diff --git a/src/backend/access/transam/twophase.c b/src/backend/access/transam/twophase.c index 6d3efb49a4..0fe8a59cf0 100644 --- a/src/backend/access/transam/twophase.c +++ b/src/backend/access/transam/twophase.c @@ -279,7 +279,7 @@ TwoPhaseShmemInit(void) TwoPhaseState->freeGXacts = &gxacts[i]; /* associate it with a PGPROC assigned by InitProcGlobal */ - gxacts[i].pgprocno = PreparedXactProcs[i].pgprocno; + gxacts[i].pgprocno = GetPGProcNumber(&PreparedXactProcs[i]); /* * Assign a unique ID for each dummy proc, so that the range of @@ -456,7 +456,6 @@ MarkAsPreparingGuts(GlobalTransaction gxact, TransactionId xid, const char *gid, /* Initialize the PGPROC entry */ MemSet(proc, 0, sizeof(PGPROC)); - proc->pgprocno = gxact->pgprocno; SHMQueueElemInit(&(proc->links)); proc->waitStatus = PROC_WAIT_STATUS_OK; /* We set up the gxact's VXID as InvalidBackendId/XID */ diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c index 1574362724..576ba061d8 100644 --- a/src/backend/access/transam/xlog.c +++ b/src/backend/access/transam/xlog.c @@ -1666,7 +1666,7 @@ WALInsertLockAcquire(void) static int lockToTry = -1; if (lockToTry == -1) - lockToTry = MyProc->pgprocno % NUM_XLOGINSERT_LOCKS; + lockToTry = GetPGProcNumber(MyProc) % NUM_XLOGINSERT_LOCKS; MyLockNo = lockToTry; /* diff --git a/src/backend/postmaster/bgwriter.c b/src/backend/postmaster/bgwriter.c index 5584f4bc24..8c812e906d 100644 --- a/src/backend/postmaster/bgwriter.c +++ b/src/backend/postmaster/bgwriter.c @@ -333,7 +333,7 @@ BackgroundWriterMain(void) if (rc == WL_TIMEOUT && can_hibernate && prev_hibernate) { /* Ask for notification at next buffer allocation */ - StrategyNotifyBgWriter(MyProc->pgprocno); + StrategyNotifyBgWriter(GetPGProcNumber(MyProc)); /* Sleep ... */ (void) WaitLatch(MyLatch, WL_LATCH_SET | WL_TIMEOUT | WL_EXIT_ON_PM_DEATH, diff --git a/src/backend/postmaster/pgarch.c b/src/backend/postmaster/pgarch.c index 74a7d7c4d0..68de7ee3b8 100644 --- a/src/backend/postmaster/pgarch.c +++ b/src/backend/postmaster/pgarch.c @@ -196,7 +196,7 @@ PgArchiverMain(void) * Advertise our pgprocno so that backends can use our latch to wake us up * while we're sleeping. */ - PgArch->pgprocno = MyProc->pgprocno; + PgArch->pgprocno = GetPGProcNumber(MyProc); pgarch_MainLoop(); diff --git a/src/backend/storage/buffer/bufmgr.c b/src/backend/storage/buffer/bufmgr.c index fb86c8706c..ad95e2f938 100644 --- a/src/backend/storage/buffer/bufmgr.c +++ b/src/backend/storage/buffer/bufmgr.c @@ -3995,7 +3995,7 @@ UnlockBuffers(void) * got a cancel/die interrupt before getting the signal. */ if ((buf_state & BM_PIN_COUNT_WAITER) != 0 && - buf->wait_backend_pgprocno == MyProc->pgprocno) + buf->wait_backend_pgprocno == GetPGProcNumber(MyProc)) buf_state &= ~BM_PIN_COUNT_WAITER; UnlockBufHdr(buf, buf_state); @@ -4131,7 +4131,7 @@ LockBufferForCleanup(Buffer buffer) LockBuffer(buffer, BUFFER_LOCK_UNLOCK); elog(ERROR, "multiple backends attempting to wait for pincount 1"); } - bufHdr->wait_backend_pgprocno = MyProc->pgprocno; + bufHdr->wait_backend_pgprocno = GetPGProcNumber(MyProc); PinCountWaitBuf = bufHdr; buf_state |= BM_PIN_COUNT_WAITER; UnlockBufHdr(bufHdr, buf_state); @@ -4202,7 +4202,7 @@ LockBufferForCleanup(Buffer buffer) */ buf_state = LockBufHdr(bufHdr); if ((buf_state & BM_PIN_COUNT_WAITER) != 0 && - bufHdr->wait_backend_pgprocno == MyProc->pgprocno) + bufHdr->wait_backend_pgprocno == GetPGProcNumber(MyProc)) buf_state &= ~BM_PIN_COUNT_WAITER; UnlockBufHdr(bufHdr, buf_state); diff --git a/src/backend/storage/ipc/procarray.c b/src/backend/storage/ipc/procarray.c index c7816fcfb3..a2f53a9ce9 100644 --- a/src/backend/storage/ipc/procarray.c +++ b/src/backend/storage/ipc/procarray.c @@ -492,7 +492,7 @@ ProcArrayAdd(PGPROC *proc) Assert(allProcs[procno].pgxactoff == index); /* If we have found our right position in the array, break */ - if (arrayP->pgprocnos[index] > proc->pgprocno) + if (arrayP->pgprocnos[index] > GetPGProcNumber(proc)) break; } @@ -510,7 +510,7 @@ ProcArrayAdd(PGPROC *proc) &ProcGlobal->statusFlags[index], movecount * sizeof(*ProcGlobal->statusFlags)); - arrayP->pgprocnos[index] = proc->pgprocno; + arrayP->pgprocnos[index] = GetPGProcNumber(proc); proc->pgxactoff = index; ProcGlobal->xids[index] = proc->xid; ProcGlobal->subxidStates[index] = proc->subxidStatus; @@ -789,7 +789,7 @@ ProcArrayGroupClearXid(PGPROC *proc, TransactionId latestXid) if (pg_atomic_compare_exchange_u32(&procglobal->procArrayGroupFirst, &nextidx, - (uint32) proc->pgprocno)) + (uint32) GetPGProcNumber(proc))) break; } diff --git a/src/backend/storage/lmgr/condition_variable.c b/src/backend/storage/lmgr/condition_variable.c index 80d70c154c..d0728d17bd 100644 --- a/src/backend/storage/lmgr/condition_variable.c +++ b/src/backend/storage/lmgr/condition_variable.c @@ -57,7 +57,7 @@ ConditionVariableInit(ConditionVariable *cv) void ConditionVariablePrepareToSleep(ConditionVariable *cv) { - int pgprocno = MyProc->pgprocno; + int pgprocno = GetPGProcNumber(MyProc); /* * If some other sleep is already prepared, cancel it; this is necessary @@ -181,10 +181,10 @@ ConditionVariableTimedSleep(ConditionVariable *cv, long timeout, * guarantee not to return spuriously, we'll avoid this obvious case. */ SpinLockAcquire(&cv->mutex); - if (!proclist_contains(&cv->wakeup, MyProc->pgprocno, cvWaitLink)) + if (!proclist_contains(&cv->wakeup, GetPGProcNumber(MyProc), cvWaitLink)) { done = true; - proclist_push_tail(&cv->wakeup, MyProc->pgprocno, cvWaitLink); + proclist_push_tail(&cv->wakeup, GetPGProcNumber(MyProc), cvWaitLink); } SpinLockRelease(&cv->mutex); @@ -234,8 +234,8 @@ ConditionVariableCancelSleep(void) return; SpinLockAcquire(&cv->mutex); - if (proclist_contains(&cv->wakeup, MyProc->pgprocno, cvWaitLink)) - proclist_delete(&cv->wakeup, MyProc->pgprocno, cvWaitLink); + if (proclist_contains(&cv->wakeup, GetPGProcNumber(MyProc), cvWaitLink)) + proclist_delete(&cv->wakeup, GetPGProcNumber(MyProc), cvWaitLink); else signaled = true; SpinLockRelease(&cv->mutex); @@ -285,7 +285,7 @@ ConditionVariableSignal(ConditionVariable *cv) void ConditionVariableBroadcast(ConditionVariable *cv) { - int pgprocno = MyProc->pgprocno; + int pgprocno = GetPGProcNumber(MyProc); PGPROC *proc = NULL; bool have_sentinel = false; diff --git a/src/backend/storage/lmgr/lwlock.c b/src/backend/storage/lmgr/lwlock.c index 862097352b..e3cef6700a 100644 --- a/src/backend/storage/lmgr/lwlock.c +++ b/src/backend/storage/lmgr/lwlock.c @@ -1080,9 +1080,9 @@ LWLockQueueSelf(LWLock *lock, LWLockMode mode) /* LW_WAIT_UNTIL_FREE waiters are always at the front of the queue */ if (mode == LW_WAIT_UNTIL_FREE) - proclist_push_head(&lock->waiters, MyProc->pgprocno, lwWaitLink); + proclist_push_head(&lock->waiters, GetPGProcNumber(MyProc), lwWaitLink); else - proclist_push_tail(&lock->waiters, MyProc->pgprocno, lwWaitLink); + proclist_push_tail(&lock->waiters, GetPGProcNumber(MyProc), lwWaitLink); /* Can release the mutex now */ LWLockWaitListUnlock(lock); @@ -1122,7 +1122,7 @@ LWLockDequeueSelf(LWLock *lock) */ proclist_foreach_modify(iter, &lock->waiters, lwWaitLink) { - if (iter.cur == MyProc->pgprocno) + if (iter.cur == GetPGProcNumber(MyProc)) { found = true; proclist_delete(&lock->waiters, iter.cur, lwWaitLink); diff --git a/src/backend/storage/lmgr/predicate.c b/src/backend/storage/lmgr/predicate.c index fbc0caca5c..508c8bb37f 100644 --- a/src/backend/storage/lmgr/predicate.c +++ b/src/backend/storage/lmgr/predicate.c @@ -3674,7 +3674,7 @@ ReleasePredicateLocks(bool isCommit, bool isReadOnlySafe) PGPROC *proc; proc = BackendIdGetProc(roXact->vxid.backendId); - ProcSendSignal(proc->pgprocno); + ProcSendSignal(GetPGProcNumber(proc)); } possibleUnsafeConflict = nextConflict; diff --git a/src/backend/storage/lmgr/proc.c b/src/backend/storage/lmgr/proc.c index 7f2111c6e0..1321786857 100644 --- a/src/backend/storage/lmgr/proc.c +++ b/src/backend/storage/lmgr/proc.c @@ -227,7 +227,6 @@ InitProcGlobal(void) InitSharedLatch(&(procs[i].procLatch)); LWLockInitialize(&(procs[i].fpInfoLock), LWTRANCHE_LOCK_FASTPATH); } - procs[i].pgprocno = i; /* * Newly created PGPROCs for normal backends, autovacuum and bgworkers @@ -1947,10 +1946,9 @@ BecomeLockGroupMember(PGPROC *leader, int pid) /* * Get lock protecting the group fields. Note LockHashPartitionLockByProc - * accesses leader->pgprocno in a PGPROC that might be free. This is safe - * because all PGPROCs' pgprocno fields are set during shared memory - * initialization and never change thereafter; so we will acquire the - * correct lock even if the leader PGPROC is in process of being recycled. + * takes a PGPROC that might be free, but it uses only its address. We + * will acquire the correct lock even if the leader PGPROC is in the + * process of being recycled. */ leader_lwlock = LockHashPartitionLockByProc(leader); LWLockAcquire(leader_lwlock, LW_EXCLUSIVE); diff --git a/src/backend/utils/adt/lockfuncs.c b/src/backend/utils/adt/lockfuncs.c index 5dc0a5882c..020f76e431 100644 --- a/src/backend/utils/adt/lockfuncs.c +++ b/src/backend/utils/adt/lockfuncs.c @@ -18,6 +18,7 @@ #include "funcapi.h" #include "miscadmin.h" #include "storage/predicate_internals.h" +#include "storage/proc.h" #include "utils/array.h" #include "utils/builtins.h" diff --git a/src/include/storage/lock.h b/src/include/storage/lock.h index 9b2a421c32..c8e1b0ce1a 100644 --- a/src/include/storage/lock.h +++ b/src/include/storage/lock.h @@ -531,7 +531,7 @@ typedef enum * used for a given lock group is determined by the group leader's pgprocno. */ #define LockHashPartitionLockByProc(leader_pgproc) \ - LockHashPartitionLock((leader_pgproc)->pgprocno) + LockHashPartitionLock(GetPGProcNumber(leader_pgproc)) /* * function prototypes diff --git a/src/include/storage/proc.h b/src/include/storage/proc.h index 7c1e2efe30..435876772b 100644 --- a/src/include/storage/proc.h +++ b/src/include/storage/proc.h @@ -147,7 +147,6 @@ struct PGPROC int pgxactoff; /* offset into various ProcGlobal->arrays with * data mirrored from this PGPROC */ - int pgprocno; /* These fields are zero while a backend is still starting up: */ BackendId backendId; /* This backend's backend ID (if assigned) */ @@ -311,6 +310,9 @@ extern PGDLLIMPORT PGPROC *MyProc; * When entering a PGPROC for 2PC transactions with ProcArrayAdd(), the data * in the dense arrays is initialized from the PGPROC while it already holds * ProcArrayLock. + * + * Shared memory data structures can refer to PGPROCs by index in allProcs. + * See GetPGProcNumber() and GetPGProcByNumber(). */ typedef struct PROC_HDR { @@ -362,6 +364,8 @@ extern PGPROC *PreparedXactProcs; /* Accessor for PGPROC given a pgprocno. */ #define GetPGProcByNumber(n) (&ProcGlobal->allProcs[(n)]) +/* Accessor for pgprocno given a pointer to PGPROC. */ +#define GetPGProcNumber(proc) ((proc) - ProcGlobal->allProcs) /* * We set aside some extra PGPROC structures for auxiliary processes, -- 2.30.2