From f83d7b21ff2eac6d2ebc582185956963375730e7 Mon Sep 17 00:00:00 2001 From: Michail Nikolaev Date: Sun, 7 Aug 2022 15:30:14 +0300 Subject: [PATCH v1] TODO --- src/backend/storage/ipc/procarray.c | 82 ++++++++++++++++++++--------- 1 file changed, 57 insertions(+), 25 deletions(-) diff --git a/src/backend/storage/ipc/procarray.c b/src/backend/storage/ipc/procarray.c index 0555b02a8d..86595a428d 100644 --- a/src/backend/storage/ipc/procarray.c +++ b/src/backend/storage/ipc/procarray.c @@ -338,9 +338,9 @@ static void DisplayXidCache(void); static void KnownAssignedXidsCompress(bool force); static void KnownAssignedXidsAdd(TransactionId from_xid, TransactionId to_xid, bool exclusive_lock); -static bool KnownAssignedXidsSearch(TransactionId xid, bool remove); +static int KnownAssignedXidsSearch(TransactionId xid, bool remove, int start); static bool KnownAssignedXidExists(TransactionId xid); -static void KnownAssignedXidsRemove(TransactionId xid); +static int KnownAssignedXidsRemove(TransactionId xid, int start); static void KnownAssignedXidsRemoveTree(TransactionId xid, int nsubxids, TransactionId *subxids); static void KnownAssignedXidsRemovePreceding(TransactionId xid); @@ -4617,18 +4617,24 @@ KnownAssignedXidsCompress(bool force) { /* * If we can choose how much to compress, use a heuristic to avoid - * compressing too often or not often enough. + * compressing too often or not often enough. "Compress" here means + * simply moving the values to the beginning of the array, so is + * not as complex or costly as typical data compression algorithms. * - * Heuristic is if we have a large enough current spread and less than - * 50% of the elements are currently in use, then compress. This - * should ensure we compress fairly infrequently. We could compress - * less often though the virtual array would spread out more and - * snapshots would become more expensive. + * Heuristic is if less than 50% of the elements are currently in + * use, then compress. This ensures time to take a snapshot is + * bounded at S=2N, using the same notation from earlier comments, + * which is essential to avoid limiting scalability with high N. + * Previously, we prevented compression until S=4M, ensuring that + * snapshot speed would be slow and scale poorly with many CPUs. + * + * As noted earlier, compression is O(S), so now O(2N), while frequency + * of compression is now O(1/N) so that as N varies, the algorithm + * balances nicely the frequency and cost of compression. */ int nelements = head - tail; - if (nelements < 4 * PROCARRAY_MAXPROCS || - nelements < 2 * pArray->numKnownAssignedXids) + if (nelements < 2 * pArray->numKnownAssignedXids) return; } @@ -4771,30 +4777,46 @@ KnownAssignedXidsAdd(TransactionId from_xid, TransactionId to_xid, } } +#define KAX_INVALID (-1) +#define KAX_START (-1) + /* * KnownAssignedXidsSearch * * Searches KnownAssignedXids for a specific xid and optionally removes it. * Returns true if it was found, false if not. * + * `start` is tail position to use instead. Use KAX_START as default (search + * in the whole array). Applied only if `remove` is true. + * * Caller must hold ProcArrayLock in shared or exclusive mode. * Exclusive lock must be held for remove = true. */ -static bool -KnownAssignedXidsSearch(TransactionId xid, bool remove) +static int +KnownAssignedXidsSearch(TransactionId xid, bool remove, int start) { ProcArrayStruct *pArray = procArray; int first, last; int head; int tail; - int result_index = -1; + int result_index = KAX_INVALID; if (remove) { /* we hold ProcArrayLock exclusively, so no need for spinlock */ - tail = pArray->tailKnownAssignedXids; head = pArray->headKnownAssignedXids; + + /* + * Start at the tail, or if we are removing a set of sorted + * xids, we can set the tail to the location of previous removal, + * reducing search time when N is large (note: KAX_START is -1, + * so Max works as `if` too). + * + * Also, special check to make sure start is not before the tail + * - such a case is possible because optimisation in tail removal. + */ + tail = Max(start, pArray->tailKnownAssignedXids); } else { @@ -4831,10 +4853,10 @@ KnownAssignedXidsSearch(TransactionId xid, bool remove) } if (result_index < 0) - return false; /* not in array */ + return KAX_INVALID; /* not in array */ if (!KnownAssignedXidsValid[result_index]) - return false; /* in array, but invalid */ + return KAX_INVALID; /* in array, but invalid */ if (remove) { @@ -4865,7 +4887,7 @@ KnownAssignedXidsSearch(TransactionId xid, bool remove) } } - return true; + return result_index; } /* @@ -4878,16 +4900,19 @@ KnownAssignedXidExists(TransactionId xid) { Assert(TransactionIdIsValid(xid)); - return KnownAssignedXidsSearch(xid, false); + return KnownAssignedXidsSearch(xid, false, KAX_START) == KAX_INVALID + ? false : true; } /* * Remove the specified XID from KnownAssignedXids[]. * * Caller must hold ProcArrayLock in exclusive mode. + * + * Returns the array offset of the removed element, or KAX_INVALID if not present. */ -static void -KnownAssignedXidsRemove(TransactionId xid) +static int +KnownAssignedXidsRemove(TransactionId xid, int start) { Assert(TransactionIdIsValid(xid)); @@ -4903,7 +4928,7 @@ KnownAssignedXidsRemove(TransactionId xid) * actual errors, but it would be complicated and probably not worth it. * So, just ignore the search result. */ - (void) KnownAssignedXidsSearch(xid, true); + return KnownAssignedXidsSearch(xid, true, start); } /* @@ -4911,21 +4936,28 @@ KnownAssignedXidsRemove(TransactionId xid) * Remove xid (if it's not InvalidTransactionId) and all the subxids. * * Caller must hold ProcArrayLock in exclusive mode. + * + * Since the xid and subxids are sorted, we can chain them together so + * we start searching for next xid at the point where we removed the previous + * xid in the tree. First search starts at KAX_START. */ static void KnownAssignedXidsRemoveTree(TransactionId xid, int nsubxids, TransactionId *subxids) { int i; + int start = KAX_START; if (TransactionIdIsValid(xid)) - KnownAssignedXidsRemove(xid); + start = KnownAssignedXidsRemove(xid, start); for (i = 0; i < nsubxids; i++) - KnownAssignedXidsRemove(subxids[i]); + start = KnownAssignedXidsRemove(subxids[i], start); - /* Opportunistically compress the array */ - KnownAssignedXidsCompress(false); + /* Opportunistically compress the array, every N commits */ + if (TransactionIdIsValid(xid) && + ((int) xid) % 8 == 0) + KnownAssignedXidsCompress(false); } /* -- 2.25.1