While debugging the recent fastpath lock unpleasantness, I noticed that
the code tends to use only the last few entries in the fpRelId[] arrays,
which seemed a bit surprising. The reason of course is the way that
FastPathGrantRelationLock() is written: it remembers the *last* unused
slot while scanning the array. This ends up wasting cycles in
FastPathUnGrantRelationLock(), as well as other places where we search
for an existing entry, since they'll generally have to iterate to the end
of the array to find it. We should prefer to put entries near the front
of the array, not the back. (Of course, if the array is about full then
it's going to be a wash, but in simple transactions we might only have a
few relations with fast-path locks.)
We could add an extra test in FastPathGrantRelationLock's loop to make
it remember the first unused slot rather than the last one, but that
would add some cycles there, partially negating any benefit. Instead
I propose that we reverse the direction of the search loop, as attached.
I doubt this will actually make any easily-measurable difference, so I've
not attempted to benchmark it, but in principle we should be able to save
a few loop iterations in places where we're holding locks. Comments?
regards, tom lane
diff --git a/src/backend/storage/lmgr/lock.c b/src/backend/storage/lmgr/lock.c
index fde2042..c1b2b42 100644
*** a/src/backend/storage/lmgr/lock.c
--- b/src/backend/storage/lmgr/lock.c
*************** LockReassignOwner(LOCALLOCK *locallock,
*** 2407,2417 ****
static bool
FastPathGrantRelationLock(Oid relid, LOCKMODE lockmode)
{
! uint32 f;
! uint32 unused_slot = FP_LOCK_SLOTS_PER_BACKEND;
! /* Scan for existing entry for this relid, remembering empty slot. */
! for (f = 0; f < FP_LOCK_SLOTS_PER_BACKEND; f++)
{
if (FAST_PATH_GET_BITS(MyProc, f) == 0)
unused_slot = f;
--- 2407,2421 ----
static bool
FastPathGrantRelationLock(Oid relid, LOCKMODE lockmode)
{
! int f;
! int unused_slot = -1;
! /*
! * Scan for existing entry for this relid, remembering first empty slot.
! * We prefer to assign a low-numbered slot so that later search loops,
! * which search the array forwards, will find entries more quickly.
! */
! for (f = FP_LOCK_SLOTS_PER_BACKEND - 1; f >= 0; f--)
{
if (FAST_PATH_GET_BITS(MyProc, f) == 0)
unused_slot = f;
*************** FastPathGrantRelationLock(Oid relid, LOC
*** 2423,2430 ****
}
}
! /* If no existing entry, use any empty slot. */
! if (unused_slot < FP_LOCK_SLOTS_PER_BACKEND)
{
MyProc->fpRelId[unused_slot] = relid;
FAST_PATH_SET_LOCKMODE(MyProc, unused_slot, lockmode);
--- 2427,2434 ----
}
}
! /* If no existing entry, use first empty slot. */
! if (unused_slot >= 0)
{
MyProc->fpRelId[unused_slot] = relid;
FAST_PATH_SET_LOCKMODE(MyProc, unused_slot, lockmode);
--
Sent via pgsql-hackers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
