Improve GetSnapshotData() performance by avoiding indirection for subxid access.
authorAndres Freund <[email protected]>
Tue, 7 Apr 2020 04:28:55 +0000 (21:28 -0700)
committerAndres Freund <[email protected]>
Tue, 7 Apr 2020 04:28:55 +0000 (21:28 -0700)
src/backend/access/transam/clog.c
src/backend/access/transam/twophase.c
src/backend/access/transam/varsup.c
src/backend/storage/ipc/procarray.c
src/backend/storage/lmgr/proc.c
src/include/storage/proc.h

index f7d5e1aea77542cd4df737845be14b9fbd6b6777..e7126656b30f0422c93b2587a129d3e09bede1d5 100644 (file)
@@ -295,7 +295,7 @@ TransactionIdSetPageStatus(TransactionId xid, int nsubxids,
     */
    if (all_xact_same_page && xid == MyProc->xidCopy &&
        nsubxids <= THRESHOLD_SUBTRANS_CLOG_OPT &&
-       nsubxids == MyPgXact->nxids &&
+       nsubxids == MyProc->nsubxidsCopy &&
        memcmp(subxids, MyProc->subxids.xids,
               nsubxids * sizeof(TransactionId)) == 0)
    {
@@ -510,16 +510,15 @@ TransactionGroupUpdateXidStatus(TransactionId xid, XidStatus status,
    while (nextidx != INVALID_PGPROCNO)
    {
        PGPROC     *proc = &ProcGlobal->allProcs[nextidx];
-       PGXACT     *pgxact = &ProcGlobal->allPgXact[nextidx];
 
        /*
         * Transactions with more than THRESHOLD_SUBTRANS_CLOG_OPT sub-XIDs
         * should not use group XID status update mechanism.
         */
-       Assert(pgxact->nxids <= THRESHOLD_SUBTRANS_CLOG_OPT);
+       Assert(proc->nsubxidsCopy <= THRESHOLD_SUBTRANS_CLOG_OPT);
 
        TransactionIdSetPageStatusInternal(proc->clogGroupMemberXid,
-                                          pgxact->nxids,
+                                          proc->nsubxidsCopy,
                                           proc->subxids.xids,
                                           proc->clogGroupMemberXidStatus,
                                           proc->clogGroupMemberLsn,
index 06b61605649a06fbb0cde07b0401dbabd7e03ad7..ebbd21186b2ece08e5815866c8c3b045609aa29f 100644 (file)
@@ -447,14 +447,12 @@ MarkAsPreparingGuts(GlobalTransaction gxact, TransactionId xid, const char *gid,
                    TimestampTz prepared_at, Oid owner, Oid databaseid)
 {
    PGPROC     *proc;
-   PGXACT     *pgxact;
    int         i;
 
    Assert(LWLockHeldByMeInMode(TwoPhaseStateLock, LW_EXCLUSIVE));
 
    Assert(gxact != NULL);
    proc = &ProcGlobal->allProcs[gxact->pgprocno];
-   pgxact = &ProcGlobal->allPgXact[gxact->pgprocno];
 
    /* Initialize the PGPROC entry */
    MemSet(proc, 0, sizeof(PGPROC));
@@ -480,8 +478,7 @@ MarkAsPreparingGuts(GlobalTransaction gxact, TransactionId xid, const char *gid,
    for (i = 0; i < NUM_LOCK_PARTITIONS; i++)
        SHMQueueInit(&(proc->myProcLocks[i]));
    /* subxid data must be filled later by GXactLoadSubxactData */
-   pgxact->overflowed = false;
-   pgxact->nxids = 0;
+   proc->nsubxidsCopy = 0;
 
    gxact->prepared_at = prepared_at;
    gxact->xid = xid;
@@ -510,20 +507,18 @@ GXactLoadSubxactData(GlobalTransaction gxact, int nsubxacts,
                     TransactionId *children)
 {
    PGPROC     *proc = &ProcGlobal->allProcs[gxact->pgprocno];
-   PGXACT     *pgxact = &ProcGlobal->allPgXact[gxact->pgprocno];
 
    /* We need no extra lock since the GXACT isn't valid yet */
    if (nsubxacts > PGPROC_MAX_CACHED_SUBXIDS)
    {
-       pgxact->overflowed = true;
-       nsubxacts = PGPROC_MAX_CACHED_SUBXIDS;
+       nsubxacts = -1;
    }
    if (nsubxacts > 0)
    {
        memcpy(proc->subxids.xids, children,
               nsubxacts * sizeof(TransactionId));
-       pgxact->nxids = nsubxacts;
    }
+   proc->nsubxidsCopy = nsubxacts;
 }
 
 /*
index 35b00eaefc3b7f078e1dbd42a20c8757bb04401f..d8fedb4478285bc9c1d1c122e199b692fcd48b7a 100644 (file)
@@ -224,19 +224,26 @@ GetNewTransactionId(bool isSubXact)
        /* LWLockRelease acts as barrier */
        ProcGlobal->xids[MyProc->pgxactoff] = xid;
        MyProc->xidCopy = xid;
+
+       Assert(ProcGlobal->nsubxids[MyProc->pgxactoff] == 0);
    }
    else
    {
-       int         nxids = MyPgXact->nxids;
+       int8       *pnxids = &ProcGlobal->nsubxids[MyProc->pgxactoff];
+       int8        nxids = *pnxids;
 
-       if (nxids < PGPROC_MAX_CACHED_SUBXIDS)
+       if (nxids != -1 && nxids < PGPROC_MAX_CACHED_SUBXIDS)
        {
-           MyProc->subxids.xids[nxids] = xid;
+           MyProc->subxids.xids[*pnxids] = xid;
            pg_write_barrier();
-           MyPgXact->nxids = nxids + 1;
+           *pnxids = nxids + 1;
        }
        else
-           MyPgXact->overflowed = true;
+       {
+           /* mark as overflowed */
+           *pnxids = -1;
+       }
+       MyProc->nsubxidsCopy = *pnxids;
    }
 
    LWLockRelease(XidGenLock);
index ee571459c1a055653b1fea28473f8dae9a9571c7..5c22404956e530e20902caae4fd487fea73ad509 100644 (file)
@@ -431,11 +431,14 @@ ProcArrayAdd(PGPROC *proc)
            (arrayP->numProcs - index) * sizeof(*arrayP->pgprocnos));
    memmove(&ProcGlobal->xids[index + 1], &ProcGlobal->xids[index],
            (arrayP->numProcs - index) * sizeof(*ProcGlobal->xids));
+   memmove(&ProcGlobal->nsubxids[index + 1], &ProcGlobal->nsubxids[index],
+           (arrayP->numProcs - index) * sizeof(*ProcGlobal->nsubxids));
    memmove(&ProcGlobal->vacuumFlags[index + 1], &ProcGlobal->vacuumFlags[index],
            (arrayP->numProcs - index) * sizeof(*ProcGlobal->vacuumFlags));
 
    arrayP->pgprocnos[index] = proc->pgprocno;
    ProcGlobal->xids[index] = proc->xidCopy;
+   ProcGlobal->nsubxids[index] = proc->nsubxidsCopy;
    ProcGlobal->vacuumFlags[index] = proc->vacuumFlagsCopy;
 
    arrayP->numProcs++;
@@ -492,6 +495,7 @@ ProcArrayRemove(PGPROC *proc, TransactionId latestXid)
        MaintainLatestCompletedXid(latestXid);
 
        ProcGlobal->xids[proc->pgxactoff] = 0;
+       ProcGlobal->nsubxids[proc->pgxactoff] = 0;
    }
    else
    {
@@ -500,6 +504,7 @@ ProcArrayRemove(PGPROC *proc, TransactionId latestXid)
    }
 
    Assert(TransactionIdIsValid(ProcGlobal->xids[proc->pgxactoff] == 0));
+   Assert(TransactionIdIsValid(ProcGlobal->nsubxids[proc->pgxactoff] == 0));
    ProcGlobal->vacuumFlags[proc->pgxactoff] = 0;
 
    for (index = 0; index < arrayP->numProcs; index++)
@@ -511,6 +516,8 @@ ProcArrayRemove(PGPROC *proc, TransactionId latestXid)
                    (arrayP->numProcs - index - 1) * sizeof(*arrayP->pgprocnos));
            memmove(&ProcGlobal->xids[index], &ProcGlobal->xids[index + 1],
                    (arrayP->numProcs - index - 1) * sizeof(*ProcGlobal->xids));
+           memmove(&ProcGlobal->nsubxids[index], &ProcGlobal->nsubxids[index + 1],
+                   (arrayP->numProcs - index - 1) * sizeof(*ProcGlobal->nsubxids));
            memmove(&ProcGlobal->vacuumFlags[index], &ProcGlobal->vacuumFlags[index + 1],
                    (arrayP->numProcs - index - 1) * sizeof(*ProcGlobal->vacuumFlags));
 
@@ -589,15 +596,13 @@ ProcArrayEndTransaction(PGPROC *proc, TransactionId latestXid)
         * estimate of global xmin, but that's OK.
         */
        Assert(!TransactionIdIsValid(proc->xidCopy));
+       Assert(proc->nsubxidsCopy == 0);
 
        proc->lxid = InvalidLocalTransactionId;
        proc->xmin = InvalidTransactionId;
        proc->delayChkpt = false; /* be sure this is cleared in abort */
        proc->recoveryConflictPending = false;
 
-       Assert(pgxact->nxids == 0);
-       Assert(pgxact->overflowed == false);
-
        /* must be cleared with xid/xmin: */
        /* avoid unnecessarily dirtying shared cachelines */
        if (proc->vacuumFlagsCopy & PROC_VACUUM_STATE_MASK)
@@ -641,8 +646,12 @@ ProcArrayEndTransactionInternal(PGPROC *proc, PGXACT *pgxact,
    }
 
    /* Clear the subtransaction-XID cache too while holding the lock */
-   pgxact->nxids = 0;
-   pgxact->overflowed = false;
+   Assert(ProcGlobal->nsubxids[pgxactoff] == proc->nsubxidsCopy);
+   if (proc->nsubxidsCopy != 0)
+   {
+       ProcGlobal->nsubxids[pgxactoff] = 0;
+       proc->nsubxidsCopy = 0;
+   }
 
    /* Also advance global latestCompletedXid while holding the lock */
    MaintainLatestCompletedXid(latestXid);
@@ -778,7 +787,6 @@ ProcArrayGroupClearXid(PGPROC *proc, TransactionId latestXid)
 void
 ProcArrayClearTransaction(PGPROC *proc)
 {
-   PGXACT     *pgxact = &allPgXact[proc->pgprocno];
    size_t      pgxactoff;
 
    /*
@@ -803,8 +811,12 @@ ProcArrayClearTransaction(PGPROC *proc)
    Assert(!proc->delayChkpt);
 
    /* Clear the subtransaction-XID cache too */
-   pgxact->nxids = 0;
-   pgxact->overflowed = false;
+   Assert(ProcGlobal->nsubxids[pgxactoff] == proc->nsubxidsCopy);
+   if (proc->nsubxidsCopy != 0)
+   {
+       ProcGlobal->nsubxids[pgxactoff] = 0;
+       proc->nsubxidsCopy = 0;
+   }
 
    LWLockRelease(ProcArrayLock);
 }
@@ -1201,6 +1213,7 @@ TransactionIdIsInProgress(TransactionId xid)
 {
    static TransactionId *xids = NULL;
    static TransactionId *other_xids;
+   static int8 *other_nsubxids;
    int         nxids = 0;
    ProcArrayStruct *arrayP = procArray;
    TransactionId topxid;
@@ -1276,11 +1289,11 @@ TransactionIdIsInProgress(TransactionId xid)
 
    /* No shortcuts, gotta grovel through the array */
    other_xids = ProcGlobal->xids;
+   other_nsubxids = ProcGlobal->nsubxids;
    for (i = 0; i < arrayP->numProcs; i++)
    {
        int         pgprocno = arrayP->pgprocnos[i];
        PGPROC     *proc = &allProcs[pgprocno];
-       PGXACT     *pgxact = &allPgXact[pgprocno];
        TransactionId pxid;
        int         pxids;
 
@@ -1314,7 +1327,7 @@ TransactionIdIsInProgress(TransactionId xid)
        /*
         * Step 2: check the cached child-Xids arrays
         */
-       pxids = pgxact->nxids;
+       pxids = other_nsubxids[i];
        pg_read_barrier();      /* pairs with barrier in GetNewTransactionId() */
        for (j = pxids - 1; j >= 0; j--)
        {
@@ -1336,7 +1349,7 @@ TransactionIdIsInProgress(TransactionId xid)
         * we hold ProcArrayLock.  So we can't miss an Xid that we need to
         * worry about.)
         */
-       if (pgxact->overflowed)
+       if (proc->nsubxidsCopy == -1)
            xids[nxids++] = pxid;
    }
 
@@ -1892,6 +1905,7 @@ GetSnapshotData(Snapshot snapshot)
    {
        size_t      numProcs = arrayP->numProcs;
        TransactionId *xip = snapshot->xip;
+       int8       *allnsubxids = ProcGlobal->nsubxids;
        int         workspace_count = 0;
        uint8      *allVacuumFlags = ProcGlobal->vacuumFlags;
 
@@ -1940,9 +1954,7 @@ GetSnapshotData(Snapshot snapshot)
        {
            int pgxactoff = snapshot_workspace_off[i];
            TransactionId xid = snapshot_workspace_xid[i];
-           int         pgprocno = arrayP->pgprocnos[pgxactoff];
-           PGXACT     *pgxact = &allPgXact[pgprocno];
-           uint8       vacuumFlags = allVacuumFlags[pgxactoff];
+           uint8       vacuumFlags = allVacuumFlags[pgxactoff];
            int         nsubxids;
 
            Assert(allProcs[arrayP->pgprocnos[pgxactoff]].pgxactoff == pgxactoff);
@@ -1978,9 +1990,9 @@ GetSnapshotData(Snapshot snapshot)
            if (suboverflowed)
                continue;
 
-           nsubxids = pgxact->nxids;
+           nsubxids = allnsubxids[pgxactoff];
 
-           if (pgxact->overflowed)
+           if (nsubxids == -1)
            {
                suboverflowed = true;
                continue;
@@ -2381,8 +2393,6 @@ GetRunningTransactionData(void)
     */
    for (index = 0; index < arrayP->numProcs; index++)
    {
-       int         pgprocno = arrayP->pgprocnos[index];
-       PGXACT     *pgxact = &allPgXact[pgprocno];
        TransactionId xid;
 
        /* Fetch xid just once - see GetNewTransactionId */
@@ -2403,7 +2413,7 @@ GetRunningTransactionData(void)
        if (TransactionIdPrecedes(xid, oldestRunningXid))
            oldestRunningXid = xid;
 
-       if (pgxact->overflowed)
+       if (ProcGlobal->nsubxids[index] == -1)
            suboverflowed = true;
 
        /*
@@ -2423,27 +2433,28 @@ GetRunningTransactionData(void)
     */
    if (!suboverflowed)
    {
+       int8       *other_nsubxids = ProcGlobal->nsubxids;
+
        for (index = 0; index < arrayP->numProcs; index++)
        {
            int         pgprocno = arrayP->pgprocnos[index];
            PGPROC     *proc = &allProcs[pgprocno];
-           PGXACT     *pgxact = &allPgXact[pgprocno];
-           int         nxids;
+           int         nsubxids;
 
            /*
             * Save subtransaction XIDs. Other backends can't add or remove
             * entries while we're holding XidGenLock.
             */
-           nxids = pgxact->nxids;
-           if (nxids > 0)
+           nsubxids = other_nsubxids[index];
+           if (nsubxids > 0)
            {
                /* barrier not really required, as XidGenLock is held, but ... */
                pg_read_barrier();  /* pairs with GetNewTransactionId */
 
                memcpy(&xids[count], (void *) proc->subxids.xids,
-                      nxids * sizeof(TransactionId));
-               count += nxids;
-               subcount += nxids;
+                      nsubxids * sizeof(TransactionId));
+               count += nsubxids;
+               subcount += nsubxids;
 
                /*
                 * Top-level XID of a transaction is always less than any of
@@ -3511,13 +3522,6 @@ ProcArrayGetReplicationSlotXmin(TransactionId *xmin,
 }
 
 
-#define XidCacheRemove(i) \
-   do { \
-       MyProc->subxids.xids[i] = MyProc->subxids.xids[MyPgXact->nxids - 1]; \
-       pg_write_barrier(); \
-       MyPgXact->nxids--; \
-   } while (0)
-
 /*
  * XidCacheRemoveRunningXids
  *
@@ -3533,6 +3537,7 @@ XidCacheRemoveRunningXids(TransactionId xid,
 {
    int         i,
                j;
+   int8       *pnsubxids;
 
    Assert(TransactionIdIsValid(xid));
 
@@ -3550,6 +3555,8 @@ XidCacheRemoveRunningXids(TransactionId xid,
     */
    LWLockAcquire(ProcArrayLock, LW_EXCLUSIVE);
 
+   pnsubxids = &ProcGlobal->nsubxids[MyProc->pgxactoff];
+
    /*
     * Under normal circumstances xid and xids[] will be in increasing order,
     * as will be the entries in subxids.  Scan backwards to avoid O(N^2)
@@ -3559,11 +3566,13 @@ XidCacheRemoveRunningXids(TransactionId xid,
    {
        TransactionId anxid = xids[i];
 
-       for (j = MyPgXact->nxids - 1; j >= 0; j--)
+       for (j = *pnsubxids - 1; j >= 0; j--)
        {
            if (TransactionIdEquals(MyProc->subxids.xids[j], anxid))
            {
-               XidCacheRemove(j);
+               MyProc->subxids.xids[j] = MyProc->subxids.xids[*pnsubxids - 1];
+               pg_write_barrier();
+               (*pnsubxids)--;
                break;
            }
        }
@@ -3575,22 +3584,26 @@ XidCacheRemoveRunningXids(TransactionId xid,
         * error during AbortSubTransaction.  So instead of Assert, emit a
         * debug warning.
         */
-       if (j < 0 && !MyPgXact->overflowed)
+       if (j < 0 && MyProc->nsubxidsCopy != -1)
            elog(WARNING, "did not find subXID %u in MyProc", anxid);
    }
 
-   for (j = MyPgXact->nxids - 1; j >= 0; j--)
+   for (j = *pnsubxids - 1; j >= 0; j--)
    {
        if (TransactionIdEquals(MyProc->subxids.xids[j], xid))
        {
-           XidCacheRemove(j);
+           MyProc->subxids.xids[j] = MyProc->subxids.xids[*pnsubxids - 1];
+           pg_write_barrier();
+           (*pnsubxids)--;
            break;
        }
    }
    /* Ordinarily we should have found it, unless the cache has overflowed */
-   if (j < 0 && !MyPgXact->overflowed)
+   if (j < 0 && MyProc->nsubxidsCopy != -1)
        elog(WARNING, "did not find subXID %u in MyProc", xid);
 
+   MyProc->nsubxidsCopy = *pnsubxids;
+
    /* Also advance global latestCompletedXid while holding the lock */
    if (TransactionIdPrecedes(XidFromFullTransactionId(ShmemVariableCache->latestCompletedFullXid),
                              latestXid))
index 00f26d93c1a45e0276d82dc97e94e2fca91600e6..650543bdc52d928d12300f5ea3105bf38b01feac 100644 (file)
@@ -113,6 +113,7 @@ ProcGlobalShmemSize(void)
    size = add_size(size, mul_size(NUM_AUXILIARY_PROCS, sizeof(PGXACT)));
    size = add_size(size, mul_size(max_prepared_xacts, sizeof(PGXACT)));
    size = add_size(size, mul_size(TotalProcs, sizeof(*ProcGlobal->xids)));
+   size = add_size(size, mul_size(TotalProcs, sizeof(*ProcGlobal->nsubxids)));
    size = add_size(size, mul_size(TotalProcs, sizeof(*ProcGlobal->vacuumFlags)));
 
    return size;
@@ -231,6 +232,8 @@ InitProcGlobal(void)
    // XXX: Pad to cacheline (or even page?)!
    ProcGlobal->xids = (TransactionId *) ShmemAlloc(TotalProcs * sizeof(*ProcGlobal->xids));
    MemSet(ProcGlobal->xids, 0, TotalProcs * sizeof(*ProcGlobal->xids));
+   ProcGlobal->nsubxids = (int8 *) ShmemAlloc(TotalProcs * sizeof(*ProcGlobal->nsubxids));
+   MemSet(ProcGlobal->nsubxids, 0, TotalProcs * sizeof(*ProcGlobal->nsubxids));
    ProcGlobal->vacuumFlags = (uint8 *) ShmemAlloc(TotalProcs * sizeof(*ProcGlobal->vacuumFlags));
    MemSet(ProcGlobal->vacuumFlags, 0, TotalProcs * sizeof(*ProcGlobal->vacuumFlags));
 
index e765eeec92a073f300daa06e3b80bbd4482edd96..d721834ecea25feda97c21003953a88ea7c00fd6 100644 (file)
@@ -105,6 +105,7 @@ struct PGPROC
     * accessed without holding ProcArrayLock.
     */
    TransactionId xidCopy;
+   int8        nsubxidsCopy;
    uint8       vacuumFlagsCopy;
 
    LocalTransactionId lxid;    /* local id of top-level transaction currently
@@ -229,9 +230,6 @@ extern PGDLLIMPORT struct PGXACT *MyPgXact;
  */
 typedef struct PGXACT
 {
-   bool        overflowed;
-
-   uint8       nxids;
 } PGXACT;
 
 /*
@@ -266,6 +264,14 @@ typedef struct PROC_HDR
     */
    TransactionId *xids;
 
+   /*
+    * Number of subtransactions in proc, for PGPROC.subxids. If subxids
+    * overflows, -1 is stored.
+    *
+    * Each PGPROC has a copy of its value in PGPROC.nsubxidsCopy.
+    */
+   int8       *nsubxids;
+
    /*
     * Vacuum flags. See PROC_* above.
     */