From 7fad990773a868600c42786f7b6c8156adfc4a9c Mon Sep 17 00:00:00 2001 From: Andres Freund Date: Wed, 19 Feb 2020 18:04:56 -0800 Subject: [PATCH v1 6/6] Remove PROC_QUEUE.size. It's not really needed, and it adds a bit of overhead to much more common paths than where it reduces costs. Perhaps we should just remove the PROC_QUEUE type now? --- src/include/storage/lock.h | 1 - src/backend/storage/lmgr/deadlock.c | 37 +++++++++++++---------------- src/backend/storage/lmgr/lock.c | 8 ++++--- src/backend/storage/lmgr/proc.c | 11 +-------- 4 files changed, 23 insertions(+), 34 deletions(-) diff --git a/src/include/storage/lock.h b/src/include/storage/lock.h index 3569f145092..a1356b50287 100644 --- a/src/include/storage/lock.h +++ b/src/include/storage/lock.h @@ -30,7 +30,6 @@ typedef struct PGPROC PGPROC; typedef struct PROC_QUEUE { dlist_head links; /* list of PGPROC objects */ - int size; /* number of entries in list */ } PROC_QUEUE; /* GUC variables */ diff --git a/src/backend/storage/lmgr/deadlock.c b/src/backend/storage/lmgr/deadlock.c index ca2abea07f1..0fde8a95e40 100644 --- a/src/backend/storage/lmgr/deadlock.c +++ b/src/backend/storage/lmgr/deadlock.c @@ -87,7 +87,7 @@ static bool FindLockCycleRecurseMember(PGPROC *checkProc, int depth, EDGE *softEdges, int *nSoftEdges); static bool ExpandConstraints(EDGE *constraints, int nConstraints); static bool TopoSort(LOCK *lock, EDGE *constraints, int nConstraints, - PGPROC **ordering); + int queue_size, PGPROC **ordering); #ifdef DEBUG_DEADLOCK static void PrintLockQueue(LOCK *lock, const char *info); @@ -250,8 +250,6 @@ DeadLockCheck(PGPROC *proc) int nProcs = waitOrders[i].nProcs; PROC_QUEUE *waitQueue = &(lock->waitProcs); - Assert(nProcs == waitQueue->size); - #ifdef DEBUG_DEADLOCK PrintLockQueue(lock, "DeadLockCheck:"); #endif @@ -261,7 +259,6 @@ DeadLockCheck(PGPROC *proc) for (int j = 0; j < nProcs; j++) { dlist_push_tail(&waitQueue->links, &(procs[j]->links)); - waitQueue->size++; } #ifdef DEBUG_DEADLOCK @@ -802,7 +799,8 @@ ExpandConstraints(EDGE *constraints, for (i = nConstraints; --i >= 0;) { LOCK *lock = constraints[i].lock; - + dlist_iter proc_iter; + int queue_size = 0; /* Did we already make a list for this lock? */ for (j = nWaitOrders; --j >= 0;) { @@ -814,8 +812,17 @@ ExpandConstraints(EDGE *constraints, /* No, so allocate a new list */ waitOrders[nWaitOrders].lock = lock; waitOrders[nWaitOrders].procs = waitOrderProcs + nWaitOrderProcs; - waitOrders[nWaitOrders].nProcs = lock->waitProcs.size; - nWaitOrderProcs += lock->waitProcs.size; + + /* Fill topoProcs[] array with the procs in their current order */ + dlist_foreach(proc_iter, &lock->waitProcs.links) + { + PGPROC *proc; + + proc = dlist_container(PGPROC, links, proc_iter.cur); + topoProcs[queue_size++] = proc; + } + waitOrders[nWaitOrders].nProcs = queue_size; + nWaitOrderProcs += waitOrders[nWaitOrders].nProcs; Assert(nWaitOrderProcs <= MaxBackends); /* @@ -823,8 +830,9 @@ ExpandConstraints(EDGE *constraints, * one, since they must be for different locks. */ if (!TopoSort(lock, constraints, i + 1, - waitOrders[nWaitOrders].procs)) + queue_size, waitOrders[nWaitOrders].procs)) return false; + nWaitOrders++; } return true; @@ -860,10 +868,9 @@ static bool TopoSort(LOCK *lock, EDGE *constraints, int nConstraints, + int queue_size, PGPROC **ordering) /* output argument */ { - PROC_QUEUE *waitQueue = &(lock->waitProcs); - int queue_size = waitQueue->size; PGPROC *proc; int i, j, @@ -871,16 +878,6 @@ TopoSort(LOCK *lock, k, kk, last; - dlist_iter proc_iter; - - /* First, fill topoProcs[] array with the procs in their current order */ - i = 0; - dlist_foreach(proc_iter, &waitQueue->links) - { - proc = dlist_container(PGPROC, links, proc_iter.cur); - topoProcs[i++] = proc; - } - Assert(i == queue_size); /* * Scan the constraints, and for each proc in the array, generate a count diff --git a/src/backend/storage/lmgr/lock.c b/src/backend/storage/lmgr/lock.c index 01ac3c06c5e..ad8f7bca40a 100644 --- a/src/backend/storage/lmgr/lock.c +++ b/src/backend/storage/lmgr/lock.c @@ -1828,12 +1828,11 @@ RemoveFromWaitQueue(PGPROC *proc, uint32 hashcode) Assert(proc->waitStatus == STATUS_WAITING); Assert(proc->links.next != NULL); Assert(waitLock); - Assert(waitLock->waitProcs.size > 0); + Assert(!dlist_is_empty(&waitLock->waitProcs.links)); Assert(0 < lockmethodid && lockmethodid < lengthof(LockMethods)); /* Remove proc from lock's wait queue */ dlist_delete(&proc->links); - waitLock->waitProcs.size--; /* Undo increments of request counts by waiting process */ Assert(waitLock->nRequested > 0); @@ -3765,7 +3764,10 @@ GetSingleProcBlockerStatusData(PGPROC *blocked_proc, BlockedProcsData *data) /* Enlarge waiter_pids[] if it's too small to hold all wait queue PIDs */ waitQueue = &(theLock->waitProcs); - queue_size = waitQueue->size; + + queue_size = 0; + dlist_foreach(proc_iter, &waitQueue->links) + queue_size++; if (queue_size > data->maxpids - data->npids) { diff --git a/src/backend/storage/lmgr/proc.c b/src/backend/storage/lmgr/proc.c index a4c338d7aff..33d2d358567 100644 --- a/src/backend/storage/lmgr/proc.c +++ b/src/backend/storage/lmgr/proc.c @@ -1031,7 +1031,6 @@ void ProcQueueInit(PROC_QUEUE *queue) { dlist_init(&queue->links); - queue->size = 0; } @@ -1168,9 +1167,6 @@ ProcSleep(LOCALLOCK *locallock, LockMethod lockMethodTable) else dlist_push_tail(&waitQueue->links, &MyProc->links); - - waitQueue->size++; - lock->waitMask |= LOCKBIT_ON(lockmode); /* Set up wait information in PGPROC object, too */ @@ -1581,7 +1577,6 @@ ProcWakeup(PGPROC *proc, int waitStatus) /* Remove process from wait queue */ dlist_delete_thoroughly(&proc->links); - (proc->waitLock->waitProcs.size)--; /* Clean up process' state and pass it the ok/fail signal */ proc->waitLock = NULL; @@ -1606,9 +1601,7 @@ ProcLockWakeup(LockMethod lockMethodTable, LOCK *lock) LOCKMASK aheadRequests = 0; dlist_mutable_iter miter; - Assert(waitQueue->size >= 0); - - if (waitQueue->size == 0) + if (dlist_is_empty(&waitQueue->links)) return; dlist_foreach_modify(miter, &waitQueue->links) @@ -1640,8 +1633,6 @@ ProcLockWakeup(LockMethod lockMethodTable, LOCK *lock) aheadRequests |= LOCKBIT_ON(lockmode); } } - - Assert(waitQueue->size >= 0); } /* -- 2.25.0.114.g5b0ca878e0