"Kevin Grittner" <kevin.gritt...@wicourts.gov> wrote:
> Robert Haas <robertmh...@gmail.com> wrote:
>> Kevin Grittner <kevin.gritt...@wicourts.gov> wrote:
>>> The extraWaits code still looks like black magic to me
>> [explanation of the extraWaits behavior]
>
> Thanks. I'll spend some time reviewing this part. There is some
> rearrangement of related code, and this should arm me with enough
> of a grasp to review that.
I got through that without spotting any significant problems,
although I offer the attached micro-optimizations for your
consideration. (Applies over the top of your patches.)
As far as I'm concerned it looks great and "Ready for Committer"
except for the modularity/pluggability question. Perhaps that could
be done as a follow-on patch (if deemed a good idea)?
-Kevin
diff --git a/src/backend/storage/lmgr/procarraylock.c
b/src/backend/storage/lmgr/procarraylock.c
index 7cd4b6b..13b51cb 100644
--- a/src/backend/storage/lmgr/procarraylock.c
+++ b/src/backend/storage/lmgr/procarraylock.c
@@ -153,9 +153,10 @@ ProcArrayLockClearTransaction(TransactionId latestXid)
{
volatile ProcArrayLockStruct *lock = ProcArrayLockPointer();
PGPROC *proc = MyProc;
- int extraWaits = 0;
bool mustwait;
+ Assert(TransactionIdIsValid(latestXid));
+
HOLD_INTERRUPTS();
/* Acquire mutex. Time spent holding mutex should be short! */
@@ -186,8 +187,11 @@ ProcArrayLockClearTransaction(TransactionId latestXid)
/* Rats, must wait. */
proc->flWaitLink = lock->ending;
lock->ending = proc;
- if (!TransactionIdIsValid(lock->latest_ending_xid) ||
- TransactionIdPrecedes(lock->latest_ending_xid,
latestXid))
+ /*
+ * lock->latest_ending_xid may be invalid, but invalid
transaction
+ * IDs always precede valid ones.
+ */
+ if (TransactionIdPrecedes(lock->latest_ending_xid, latestXid))
lock->latest_ending_xid = latestXid;
mustwait = true;
}
@@ -202,7 +206,9 @@ ProcArrayLockClearTransaction(TransactionId latestXid)
*/
if (mustwait)
{
- extraWaits += FlexLockWait(ProcArrayLock, 2);
+ int extraWaits;
+
+ extraWaits = FlexLockWait(ProcArrayLock, 2);
while (extraWaits-- > 0)
PGSemaphoreUnlock(&proc->sem);
}
@@ -247,7 +253,7 @@ ProcArrayLockRelease(void)
ending = lock->ending;
vproc = ending;
- while (vproc != NULL)
+ do
{
volatile PGXACT *pgxact =
&ProcGlobal->allPgXact[vproc->pgprocno];
@@ -258,7 +264,7 @@ ProcArrayLockRelease(void)
pgxact->nxids = 0;
pgxact->overflowed = false;
vproc = vproc->flWaitLink;
- }
+ } while (vproc != NULL);
/* Also advance global latestCompletedXid */
if
(TransactionIdPrecedes(ShmemVariableCache->latestCompletedXid,
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
