Remove superfluous 'pgprocno' field from PGPROC

It was always just the index of the PGPROC entry from the beginning of
the proc array. Introduce a macro to compute it from the pointer
instead.

Reviewed-by: Andres Freund
Discussion: https://www.postgresql.org/message-id/8171f1aa-496f-46a6-afc3-c46fe7a9b407@iki.fi

diff --git a/src/backend/access/transam/clog.c b/src/backend/access/transam/clog.c
index 06fc2989bab189bc4227a90e4c29d0852ed15ab8..97f7434da348dca7a4809813267dde831d4c773d 100644 (file)
--- a/src/backend/access/transam/clog.c
+++ b/src/backend/access/transam/clog.c
@@ -425,6 +425,7 @@ TransactionGroupUpdateXidStatus(TransactionId xid, XidStatus status,
 {
        volatile PROC_HDR *procglobal = ProcGlobal;
        PGPROC     *proc = MyProc;
+       int                     pgprocno = MyProcNumber;
        uint32          nextidx;
        uint32          wakeidx;
 
@@ -458,7 +459,7 @@ TransactionGroupUpdateXidStatus(TransactionId xid, XidStatus status,
                 * less efficiently.
                 */
                if (nextidx != INVALID_PGPROCNO &&
-                       ProcGlobal->allProcs[nextidx].clogGroupMemberPage != proc->clogGroupMemberPage)
+                       GetPGProcByNumber(nextidx)->clogGroupMemberPage != proc->clogGroupMemberPage)
                {
                        /*
                         * Ensure that this proc is not a member of any clog group that
@@ -473,7 +474,7 @@ TransactionGroupUpdateXidStatus(TransactionId xid, XidStatus status,
 
                if (pg_atomic_compare_exchange_u32(&procglobal->clogGroupFirst,
                                                                                   &nextidx,
-                                                                                  (uint32) proc->pgprocno))
+                                                                                  (uint32) pgprocno))
                        break;
        }
 

diff --git a/src/backend/access/transam/twophase.c b/src/backend/access/transam/twophase.c
index 8426458f7f5d0c48cdc71af5a8a01421fb071a1a..234c8d08ebc0e7149720f5c5d44d38d02e88570b 100644 (file)
--- a/src/backend/access/transam/twophase.c
+++ b/src/backend/access/transam/twophase.c
@@ -284,7 +284,7 @@ TwoPhaseShmemInit(void)
                        TwoPhaseState->freeGXacts = &gxacts[i];
 
                        /* associate it with a PGPROC assigned by InitProcGlobal */
-                       gxacts[i].pgprocno = PreparedXactProcs[i].pgprocno;
+                       gxacts[i].pgprocno = GetNumberFromPGProc(&PreparedXactProcs[i]);
 
                        /*
                         * Assign a unique ID for each dummy proc, so that the range of
@@ -461,7 +461,6 @@ MarkAsPreparingGuts(GlobalTransaction gxact, TransactionId xid, const char *gid,
 
        /* Initialize the PGPROC entry */
        MemSet(proc, 0, sizeof(PGPROC));
-       proc->pgprocno = gxact->pgprocno;
        dlist_node_init(&proc->links);
        proc->waitStatus = PROC_WAIT_STATUS_OK;
        if (LocalTransactionIdIsValid(MyProc->lxid))
@@ -780,7 +779,7 @@ pg_prepared_xact(PG_FUNCTION_ARGS)
        while (status->array != NULL && status->currIdx < status->ngxacts)
        {
                GlobalTransaction gxact = &status->array[status->currIdx++];
-               PGPROC     *proc = &ProcGlobal->allProcs[gxact->pgprocno];
+               PGPROC     *proc = GetPGProcByNumber(gxact->pgprocno);
                Datum           values[5] = {0};
                bool            nulls[5] = {0};
                HeapTuple       tuple;
@@ -935,7 +934,7 @@ TwoPhaseGetDummyProc(TransactionId xid, bool lock_held)
 {
        GlobalTransaction gxact = TwoPhaseGetGXact(xid, lock_held);
 
-       return &ProcGlobal->allProcs[gxact->pgprocno];
+       return GetPGProcByNumber(gxact->pgprocno);
 }
 
 /************************************************************************/
@@ -1080,7 +1079,7 @@ save_state_data(const void *data, uint32 len)
 void
 StartPrepare(GlobalTransaction gxact)
 {
-       PGPROC     *proc = &ProcGlobal->allProcs[gxact->pgprocno];
+       PGPROC     *proc = GetPGProcByNumber(gxact->pgprocno);
        TransactionId xid = gxact->xid;
        TwoPhaseFileHeader hdr;
        TransactionId *children;
@@ -1539,7 +1538,7 @@ FinishPreparedTransaction(const char *gid, bool isCommit)
         * try to commit the same GID at once.
         */
        gxact = LockGXact(gid, GetUserId());
-       proc = &ProcGlobal->allProcs[gxact->pgprocno];
+       proc = GetPGProcByNumber(gxact->pgprocno);
        xid = gxact->xid;
 
        /*

diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 50c347a6795313cfae387abc110c856c5a37f1bf..c1162d55bff8cc97f0e62984c5c350465dcbbe44 100644 (file)
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -1379,7 +1379,7 @@ WALInsertLockAcquire(void)
        static int      lockToTry = -1;
 
        if (lockToTry == -1)
-               lockToTry = MyProc->pgprocno % NUM_XLOGINSERT_LOCKS;
+               lockToTry = MyProcNumber % NUM_XLOGINSERT_LOCKS;
        MyLockNo = lockToTry;
 
        /*

diff --git a/src/backend/postmaster/bgwriter.c b/src/backend/postmaster/bgwriter.c
index f11ce27084eeac095a72ae08fbc8c2adad683db0..6364b16261fe323df019e33af6d3b8420914ba9b 100644 (file)
--- a/src/backend/postmaster/bgwriter.c
+++ b/src/backend/postmaster/bgwriter.c
@@ -326,7 +326,7 @@ BackgroundWriterMain(void)
                if (rc == WL_TIMEOUT && can_hibernate && prev_hibernate)
                {
                        /* Ask for notification at next buffer allocation */
-                       StrategyNotifyBgWriter(MyProc->pgprocno);
+                       StrategyNotifyBgWriter(MyProcNumber);
                        /* 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 02814cd2c8fceefd7cbe495dc9139a3445f05bd4..9c18e4b3efbc698b71925d474e14adbc75e1d00c 100644 (file)
--- a/src/backend/postmaster/pgarch.c
+++ b/src/backend/postmaster/pgarch.c
@@ -242,7 +242,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 = MyProcNumber;
 
        /* Create workspace for pgarch_readyXlog() */
        arch_files = palloc(sizeof(struct arch_files_state));

diff --git a/src/backend/postmaster/walsummarizer.c b/src/backend/postmaster/walsummarizer.c
index e85d4970347b41ddb8d30a08a693412a4c64f9b6..f295eff32f476fdd114522235551126f3b6891ef 100644 (file)
--- a/src/backend/postmaster/walsummarizer.c
+++ b/src/backend/postmaster/walsummarizer.c
@@ -248,7 +248,7 @@ WalSummarizerMain(void)
        /* Advertise ourselves. */
        on_shmem_exit(WalSummarizerShutdown, (Datum) 0);
        LWLockAcquire(WALSummarizerLock, LW_EXCLUSIVE);
-       WalSummarizerCtl->summarizer_pgprocno = MyProc->pgprocno;
+       WalSummarizerCtl->summarizer_pgprocno = MyProcNumber;
        LWLockRelease(WALSummarizerLock);
 
        /* Create and switch to a memory context that we can reset on error. */

diff --git a/src/backend/storage/buffer/bufmgr.c b/src/backend/storage/buffer/bufmgr.c
index 07575ef31299abdc70562ae50e983fe112bcf5d8..bdf89bbc4dc79e64e66b91362e067859f7a67adb 100644 (file)
--- a/src/backend/storage/buffer/bufmgr.c
+++ b/src/backend/storage/buffer/bufmgr.c
@@ -4780,7 +4780,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 == MyProcNumber)
                        buf_state &= ~BM_PIN_COUNT_WAITER;
 
                UnlockBufHdr(buf, buf_state);
@@ -4930,7 +4930,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 = MyProcNumber;
                PinCountWaitBuf = bufHdr;
                buf_state |= BM_PIN_COUNT_WAITER;
                UnlockBufHdr(bufHdr, buf_state);
@@ -4994,7 +4994,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 == MyProcNumber)
                        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 ee2d7f8585a8d01301af23423922c8a5c197724e..dd329a86ef4d57ba2304ac78ad52d9ef53b7e906 100644 (file)
--- a/src/backend/storage/ipc/procarray.c
+++ b/src/backend/storage/ipc/procarray.c
@@ -468,6 +468,7 @@ CreateSharedProcArray(void)
 void
 ProcArrayAdd(PGPROC *proc)
 {
+       int                     pgprocno = GetNumberFromPGProc(proc);
        ProcArrayStruct *arrayP = procArray;
        int                     index;
        int                     movecount;
@@ -499,13 +500,13 @@ ProcArrayAdd(PGPROC *proc)
         */
        for (index = 0; index < arrayP->numProcs; index++)
        {
-               int                     procno PG_USED_FOR_ASSERTS_ONLY = arrayP->pgprocnos[index];
+               int                     this_procno = arrayP->pgprocnos[index];
 
-               Assert(procno >= 0 && procno < (arrayP->maxProcs + NUM_AUXILIARY_PROCS));
-               Assert(allProcs[procno].pgxactoff == index);
+               Assert(this_procno >= 0 && this_procno < (arrayP->maxProcs + NUM_AUXILIARY_PROCS));
+               Assert(allProcs[this_procno].pgxactoff == index);
 
                /* If we have found our right position in the array, break */
-               if (arrayP->pgprocnos[index] > proc->pgprocno)
+               if (this_procno > pgprocno)
                        break;
        }
 
@@ -523,7 +524,7 @@ ProcArrayAdd(PGPROC *proc)
                        &ProcGlobal->statusFlags[index],
                        movecount * sizeof(*ProcGlobal->statusFlags));
 
-       arrayP->pgprocnos[index] = proc->pgprocno;
+       arrayP->pgprocnos[index] = GetNumberFromPGProc(proc);
        proc->pgxactoff = index;
        ProcGlobal->xids[index] = proc->xid;
        ProcGlobal->subxidStates[index] = proc->subxidStatus;
@@ -791,6 +792,7 @@ ProcArrayEndTransactionInternal(PGPROC *proc, TransactionId latestXid)
 static void
 ProcArrayGroupClearXid(PGPROC *proc, TransactionId latestXid)
 {
+       int                     pgprocno = GetNumberFromPGProc(proc);
        PROC_HDR   *procglobal = ProcGlobal;
        uint32          nextidx;
        uint32          wakeidx;
@@ -808,7 +810,7 @@ ProcArrayGroupClearXid(PGPROC *proc, TransactionId latestXid)
 
                if (pg_atomic_compare_exchange_u32(&procglobal->procArrayGroupFirst,
                                                                                   &nextidx,
-                                                                                  (uint32) proc->pgprocno))
+                                                                                  (uint32) pgprocno))
                        break;
        }
 

diff --git a/src/backend/storage/lmgr/condition_variable.c b/src/backend/storage/lmgr/condition_variable.c
index e2d6d6852207e2a950adf9484b65575aa301affd..10fdae19dcc07386ee2f31f0843b791f6e50d53c 100644 (file)
--- 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 = MyProcNumber;
 
        /*
         * 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, MyProcNumber, cvWaitLink))
                {
                        done = true;
-                       proclist_push_tail(&cv->wakeup, MyProc->pgprocno, cvWaitLink);
+                       proclist_push_tail(&cv->wakeup, MyProcNumber, cvWaitLink);
                }
                SpinLockRelease(&cv->mutex);
 
@@ -236,8 +236,8 @@ ConditionVariableCancelSleep(void)
                return false;
 
        SpinLockAcquire(&cv->mutex);
-       if (proclist_contains(&cv->wakeup, MyProc->pgprocno, cvWaitLink))
-               proclist_delete(&cv->wakeup, MyProc->pgprocno, cvWaitLink);
+       if (proclist_contains(&cv->wakeup, MyProcNumber, cvWaitLink))
+               proclist_delete(&cv->wakeup, MyProcNumber, cvWaitLink);
        else
                signaled = true;
        SpinLockRelease(&cv->mutex);
@@ -281,7 +281,7 @@ ConditionVariableSignal(ConditionVariable *cv)
 void
 ConditionVariableBroadcast(ConditionVariable *cv)
 {
-       int                     pgprocno = MyProc->pgprocno;
+       int                     pgprocno = MyProcNumber;
        PGPROC     *proc = NULL;
        bool            have_sentinel = false;
 

diff --git a/src/backend/storage/lmgr/lwlock.c b/src/backend/storage/lmgr/lwlock.c
index 71677cf031709fd304b0ca55bed28f0e127d9ad4..997857679ede1cd89948d45cca8bda3af299c6a2 100644 (file)
--- a/src/backend/storage/lmgr/lwlock.c
+++ b/src/backend/storage/lmgr/lwlock.c
@@ -1056,9 +1056,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, MyProcNumber, lwWaitLink);
        else
-               proclist_push_tail(&lock->waiters, MyProc->pgprocno, lwWaitLink);
+               proclist_push_tail(&lock->waiters, MyProcNumber, lwWaitLink);
 
        /* Can release the mutex now */
        LWLockWaitListUnlock(lock);
@@ -1097,7 +1097,7 @@ LWLockDequeueSelf(LWLock *lock)
         */
        on_waitlist = MyProc->lwWaiting == LW_WS_WAITING;
        if (on_waitlist)
-               proclist_delete(&lock->waiters, MyProc->pgprocno, lwWaitLink);
+               proclist_delete(&lock->waiters, MyProcNumber, lwWaitLink);
 
        if (proclist_is_empty(&lock->waiters) &&
                (pg_atomic_read_u32(&lock->state) & LW_FLAG_HAS_WAITERS) != 0)

diff --git a/src/backend/storage/lmgr/predicate.c b/src/backend/storage/lmgr/predicate.c
index eed63a05ed837f4c819baf9765353eabc3241ac0..d62060d58c831437d48c2521f0db2860661c9c67 100644 (file)
--- a/src/backend/storage/lmgr/predicate.c
+++ b/src/backend/storage/lmgr/predicate.c
@@ -1830,7 +1830,7 @@ GetSerializableTransactionSnapshotInt(Snapshot snapshot,
        sxact->finishedBefore = InvalidTransactionId;
        sxact->xmin = snapshot->xmin;
        sxact->pid = MyProcPid;
-       sxact->pgprocno = MyProc->pgprocno;
+       sxact->pgprocno = MyProcNumber;
        dlist_init(&sxact->predicateLocks);
        dlist_node_init(&sxact->finishedLink);
        sxact->flags = 0;

diff --git a/src/backend/storage/lmgr/proc.c b/src/backend/storage/lmgr/proc.c
index 1afcbfc052cefa4e2555c3c49d66f6ac9f8a6df6..4aec4a3c5f426c9a08c3b8203d8c3b156c4ea29b 100644 (file)
--- a/src/backend/storage/lmgr/proc.c
+++ b/src/backend/storage/lmgr/proc.c
@@ -65,6 +65,7 @@ bool          log_lock_waits = false;
 
 /* Pointer to this process's PGPROC struct, if any */
 PGPROC    *MyProc = NULL;
+int                    MyProcNumber = INVALID_PGPROCNO;
 
 /*
  * This spinlock protects the freelist of recycled PGPROC structures.
@@ -228,7 +229,6 @@ InitProcGlobal(void)
                        InitSharedLatch(&(proc->procLatch));
                        LWLockInitialize(&(proc->fpInfoLock), LWTRANCHE_LOCK_FASTPATH);
                }
-               proc->pgprocno = i;
 
                /*
                 * Newly created PGPROCs for normal backends, autovacuum and bgworkers
@@ -353,6 +353,7 @@ InitProcess(void)
                                (errcode(ERRCODE_TOO_MANY_CONNECTIONS),
                                 errmsg("sorry, too many clients already")));
        }
+       MyProcNumber = GetNumberFromPGProc(MyProc);
 
        /*
         * Cross-check that the PGPROC is of the type we expect; if this were not
@@ -566,6 +567,8 @@ InitAuxiliaryProcess(void)
 
        SpinLockRelease(ProcStructLock);
 
+       MyProcNumber = GetNumberFromPGProc(MyProc);
+
        /*
         * Initialize all fields of MyProc, except for those previously
         * initialized by InitProcGlobal.
@@ -907,6 +910,7 @@ ProcKill(int code, Datum arg)
 
        proc = MyProc;
        MyProc = NULL;
+       MyProcNumber = INVALID_PGPROCNO;
        DisownLatch(&proc->procLatch);
 
        procgloballist = proc->procgloballist;
@@ -978,6 +982,7 @@ AuxiliaryProcKill(int code, Datum arg)
 
        proc = MyProc;
        MyProc = NULL;
+       MyProcNumber = INVALID_PGPROCNO;
        DisownLatch(&proc->procLatch);
 
        SpinLockAcquire(ProcStructLock);
@@ -1903,10 +1908,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.
+        * calculates the proc number based on the PGPROC slot without looking at
+        * its contents, so we will acquire the correct lock even if the leader
+        * PGPROC is in process of being recycled.
         */
        leader_lwlock = LockHashPartitionLockByProc(leader);
        LWLockAcquire(leader_lwlock, LW_EXCLUSIVE);

diff --git a/src/include/storage/lock.h b/src/include/storage/lock.h
index 00679624f7d8729a1f0c1185ac5776b7f2bbe4cd..ed6071f328669a9ffb9b523a75c89e717a3124f3 100644 (file)
--- a/src/include/storage/lock.h
+++ b/src/include/storage/lock.h
@@ -540,7 +540,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(GetNumberFromPGProc(leader_pgproc))
 
 /*
  * function prototypes

diff --git a/src/include/storage/proc.h b/src/include/storage/proc.h
index 20d6fa652dc690a3770e57abdfcffe6f0713dbf1..4453c6df87716bd169d084a783ff1d752d7db783 100644 (file)
--- a/src/include/storage/proc.h
+++ b/src/include/storage/proc.h
@@ -194,11 +194,6 @@ struct PGPROC
        int                     pgxactoff;              /* offset into various ProcGlobal->arrays with
                                                                 * data mirrored from this PGPROC */
 
-       int                     pgprocno;               /* Number of this PGPROC in
-                                                                * ProcGlobal->allProcs array. This is set
-                                                                * once by InitProcGlobal().
-                                                                * ProcGlobal->allProcs[n].pgprocno == n */
-
        /* These fields are zero while a backend is still starting up: */
        BackendId       backendId;              /* This backend's backend ID (if assigned) */
        Oid                     databaseId;             /* OID of database this backend is using */
@@ -307,6 +302,7 @@ struct PGPROC
 
 
 extern PGDLLIMPORT PGPROC *MyProc;
+extern PGDLLIMPORT int MyProcNumber;   /* same as GetNumberFromPGProc(MyProc) */
 
 /*
  * There is one ProcGlobal struct for the whole database cluster.
@@ -410,8 +406,9 @@ extern PGDLLIMPORT PROC_HDR *ProcGlobal;
 
 extern PGDLLIMPORT PGPROC *PreparedXactProcs;
 
-/* Accessor for PGPROC given a pgprocno. */
+/* Accessor for PGPROC given a pgprocno, and vice versa. */
 #define GetPGProcByNumber(n) (&ProcGlobal->allProcs[(n)])
+#define GetNumberFromPGProc(proc) ((proc) - &ProcGlobal->allProcs[0])
 
 /*
  * We set aside some extra PGPROC structures for auxiliary processes,