From 7dc7b09d0c57627ba9c26f3f88949a2aeba9d3b9 Mon Sep 17 00:00:00 2001 From: Andres Freund Date: Mon, 6 Apr 2020 21:28:55 -0700 Subject: [PATCH] Improve GetSnapshotData() performance by avoiding indirection for subxid access. --- src/backend/access/transam/clog.c | 7 +-- src/backend/access/transam/twophase.c | 11 +--- src/backend/access/transam/varsup.c | 17 +++-- src/backend/storage/ipc/procarray.c | 91 +++++++++++++++------------ src/backend/storage/lmgr/proc.c | 3 + src/include/storage/proc.h | 12 +++- 6 files changed, 82 insertions(+), 59 deletions(-) diff --git a/src/backend/access/transam/clog.c b/src/backend/access/transam/clog.c index f7d5e1aea7..e7126656b3 100644 --- a/src/backend/access/transam/clog.c +++ b/src/backend/access/transam/clog.c @@ -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, diff --git a/src/backend/access/transam/twophase.c b/src/backend/access/transam/twophase.c index 06b6160564..ebbd21186b 100644 --- a/src/backend/access/transam/twophase.c +++ b/src/backend/access/transam/twophase.c @@ -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; } /* diff --git a/src/backend/access/transam/varsup.c b/src/backend/access/transam/varsup.c index 35b00eaefc..d8fedb4478 100644 --- a/src/backend/access/transam/varsup.c +++ b/src/backend/access/transam/varsup.c @@ -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); diff --git a/src/backend/storage/ipc/procarray.c b/src/backend/storage/ipc/procarray.c index ee571459c1..5c22404956 100644 --- a/src/backend/storage/ipc/procarray.c +++ b/src/backend/storage/ipc/procarray.c @@ -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)) diff --git a/src/backend/storage/lmgr/proc.c b/src/backend/storage/lmgr/proc.c index 00f26d93c1..650543bdc5 100644 --- a/src/backend/storage/lmgr/proc.c +++ b/src/backend/storage/lmgr/proc.c @@ -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)); diff --git a/src/include/storage/proc.h b/src/include/storage/proc.h index e765eeec92..d721834ece 100644 --- a/src/include/storage/proc.h +++ b/src/include/storage/proc.h @@ -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. */ -- 2.39.5