On Tue, 15 Dec 2015 13:56:30 -0500
Robert Haas <robertmh...@gmail.com> wrote:
> On Sun, Dec 13, 2015 at 6:35 AM, and...@anarazel.de
> <and...@anarazel.de> wrote:
> > On 2015-12-12 21:15:52 -0500, Robert Haas wrote:
> >> On Sat, Dec 12, 2015 at 1:17 PM, and...@anarazel.de
> >> <and...@anarazel.de> wrote:
> >> > Here's two patches doing that. The first is an adaption of your
> >> > constants patch, using an enum and also converting xlog.c's
> >> > locks. The second is the separation into distinct tranches.
> >>
> >> Personally, I prefer the #define approach to the enum, but I can
> >> live with doing it this way.
> >
> > I think the lack needing to adjust the 'last defined' var is worth
> > it...
> >> Other than that, I think these patches look
> >> good, although if it's OK with you I would like to make a pass over
> >> the comments and the commit messages which seem to me that they
> >> could benefit from a bit of editing (but not much substantive
> >> change).
> >
> > Sounds good to me. You'll then commit that?
>
> Yes. Done!
>
> In terms of this project overall, NumLWLocks() now knows about only
> four categories of stuff: fixed lwlocks, backend locks (proc.c),
> replication slot locks, and locks needed by extensions. I think it'd
> probably be fine to move the backend locks into PGPROC directly, and
> the replication slot locks into ReplicationSlot. I don't know if that
> will improve performance but it doesn't seem like it should regress
> anything, though we should probably test that. I'm not sure what to
> do about extension-requested locks - maybe give those their own
> tranche somehow?
>
> I think we should also look at tranche-ifying the locks counted in
> NUM_FIXED_LWLOCKS but not NUM_INDIVIDUAL_LWLOCKS. That's basically
> just the lock manager locks and the predicate lock manager locks.
> That would get us to a place where every lock in the system has a
> descriptive name, either via the tranche or because it's an
> individually named lock, which sounds excellent.
>
There is a patch that moves backend LWLocks into PGPROC and to a
separate tranche. I did tests, and it doesn't regress and the same time
doesn't improve performance on my computer.
--
Ildus Kurbangaliev
Postgres Professional: http://www.postgrespro.com
Russian Postgres Company
diff --git a/src/backend/storage/lmgr/lock.c b/src/backend/storage/lmgr/lock.c
index 76fc615..698da5c 100644
--- a/src/backend/storage/lmgr/lock.c
+++ b/src/backend/storage/lmgr/lock.c
@@ -825,13 +825,13 @@ LockAcquireExtended(const LOCKTAG *locktag,
* FastPathStrongRelationLocks->counts becomes visible after we test
* it has yet to begin to transfer fast-path locks.
*/
- LWLockAcquire(MyProc->backendLock, LW_EXCLUSIVE);
+ LWLockAcquire(&MyProc->backendLock, LW_EXCLUSIVE);
if (FastPathStrongRelationLocks->count[fasthashcode] != 0)
acquired = false;
else
acquired = FastPathGrantRelationLock(locktag->locktag_field2,
lockmode);
- LWLockRelease(MyProc->backendLock);
+ LWLockRelease(&MyProc->backendLock);
if (acquired)
{
/*
@@ -1838,10 +1838,10 @@ LockRelease(const LOCKTAG *locktag, LOCKMODE lockmode, bool sessionLock)
* We might not find the lock here, even if we originally entered it
* here. Another backend may have moved it to the main table.
*/
- LWLockAcquire(MyProc->backendLock, LW_EXCLUSIVE);
+ LWLockAcquire(&MyProc->backendLock, LW_EXCLUSIVE);
released = FastPathUnGrantRelationLock(locktag->locktag_field2,
lockmode);
- LWLockRelease(MyProc->backendLock);
+ LWLockRelease(&MyProc->backendLock);
if (released)
{
RemoveLocalLock(locallock);
@@ -2044,7 +2044,7 @@ LockReleaseAll(LOCKMETHODID lockmethodid, bool allLocks)
*/
if (!have_fast_path_lwlock)
{
- LWLockAcquire(MyProc->backendLock, LW_EXCLUSIVE);
+ LWLockAcquire(&MyProc->backendLock, LW_EXCLUSIVE);
have_fast_path_lwlock = true;
}
@@ -2061,7 +2061,7 @@ LockReleaseAll(LOCKMETHODID lockmethodid, bool allLocks)
* transferred to the main lock table. That's going to require
* some extra work, so release our fast-path lock before starting.
*/
- LWLockRelease(MyProc->backendLock);
+ LWLockRelease(&MyProc->backendLock);
have_fast_path_lwlock = false;
/*
@@ -2087,7 +2087,7 @@ LockReleaseAll(LOCKMETHODID lockmethodid, bool allLocks)
/* Done with the fast-path data structures */
if (have_fast_path_lwlock)
- LWLockRelease(MyProc->backendLock);
+ LWLockRelease(&MyProc->backendLock);
/*
* Now, scan each lock partition separately.
@@ -2490,7 +2490,7 @@ FastPathTransferRelationLocks(LockMethod lockMethodTable, const LOCKTAG *locktag
PGPROC *proc = &ProcGlobal->allProcs[i];
uint32 f;
- LWLockAcquire(proc->backendLock, LW_EXCLUSIVE);
+ LWLockAcquire(&proc->backendLock, LW_EXCLUSIVE);
/*
* If the target backend isn't referencing the same database as the
@@ -2499,7 +2499,7 @@ FastPathTransferRelationLocks(LockMethod lockMethodTable, const LOCKTAG *locktag
*
* proc->databaseId is set at backend startup time and never changes
* thereafter, so it might be safe to perform this test before
- * acquiring proc->backendLock. In particular, it's certainly safe to
+ * acquiring &proc->backendLock. In particular, it's certainly safe to
* assume that if the target backend holds any fast-path locks, it
* must have performed a memory-fencing operation (in particular, an
* LWLock acquisition) since setting proc->databaseId. However, it's
@@ -2509,7 +2509,7 @@ FastPathTransferRelationLocks(LockMethod lockMethodTable, const LOCKTAG *locktag
*/
if (proc->databaseId != locktag->locktag_field1)
{
- LWLockRelease(proc->backendLock);
+ LWLockRelease(&proc->backendLock);
continue;
}
@@ -2536,7 +2536,7 @@ FastPathTransferRelationLocks(LockMethod lockMethodTable, const LOCKTAG *locktag
if (!proclock)
{
LWLockRelease(partitionLock);
- LWLockRelease(proc->backendLock);
+ LWLockRelease(&proc->backendLock);
return false;
}
GrantLock(proclock->tag.myLock, proclock, lockmode);
@@ -2547,7 +2547,7 @@ FastPathTransferRelationLocks(LockMethod lockMethodTable, const LOCKTAG *locktag
/* No need to examine remaining slots. */
break;
}
- LWLockRelease(proc->backendLock);
+ LWLockRelease(&proc->backendLock);
}
return true;
}
@@ -2569,7 +2569,7 @@ FastPathGetRelationLockEntry(LOCALLOCK *locallock)
Oid relid = locktag->locktag_field2;
uint32 f;
- LWLockAcquire(MyProc->backendLock, LW_EXCLUSIVE);
+ LWLockAcquire(&MyProc->backendLock, LW_EXCLUSIVE);
for (f = 0; f < FP_LOCK_SLOTS_PER_BACKEND; f++)
{
@@ -2592,7 +2592,7 @@ FastPathGetRelationLockEntry(LOCALLOCK *locallock)
if (!proclock)
{
LWLockRelease(partitionLock);
- LWLockRelease(MyProc->backendLock);
+ LWLockRelease(&MyProc->backendLock);
ereport(ERROR,
(errcode(ERRCODE_OUT_OF_MEMORY),
errmsg("out of shared memory"),
@@ -2607,7 +2607,7 @@ FastPathGetRelationLockEntry(LOCALLOCK *locallock)
break;
}
- LWLockRelease(MyProc->backendLock);
+ LWLockRelease(&MyProc->backendLock);
/* Lock may have already been transferred by some other backend. */
if (proclock == NULL)
@@ -2732,7 +2732,7 @@ GetLockConflicts(const LOCKTAG *locktag, LOCKMODE lockmode)
if (proc == MyProc)
continue;
- LWLockAcquire(proc->backendLock, LW_SHARED);
+ LWLockAcquire(&proc->backendLock, LW_SHARED);
/*
* If the target backend isn't referencing the same database as
@@ -2744,7 +2744,7 @@ GetLockConflicts(const LOCKTAG *locktag, LOCKMODE lockmode)
*/
if (proc->databaseId != locktag->locktag_field1)
{
- LWLockRelease(proc->backendLock);
+ LWLockRelease(&proc->backendLock);
continue;
}
@@ -2782,7 +2782,7 @@ GetLockConflicts(const LOCKTAG *locktag, LOCKMODE lockmode)
break;
}
- LWLockRelease(proc->backendLock);
+ LWLockRelease(&proc->backendLock);
}
}
@@ -3332,7 +3332,7 @@ GetLockStatusData(void)
PGPROC *proc = &ProcGlobal->allProcs[i];
uint32 f;
- LWLockAcquire(proc->backendLock, LW_SHARED);
+ LWLockAcquire(&proc->backendLock, LW_SHARED);
for (f = 0; f < FP_LOCK_SLOTS_PER_BACKEND; ++f)
{
@@ -3390,7 +3390,7 @@ GetLockStatusData(void)
el++;
}
- LWLockRelease(proc->backendLock);
+ LWLockRelease(&proc->backendLock);
}
/*
@@ -3930,7 +3930,7 @@ VirtualXactLockTableInsert(VirtualTransactionId vxid)
{
Assert(VirtualTransactionIdIsValid(vxid));
- LWLockAcquire(MyProc->backendLock, LW_EXCLUSIVE);
+ LWLockAcquire(&MyProc->backendLock, LW_EXCLUSIVE);
Assert(MyProc->backendId == vxid.backendId);
Assert(MyProc->fpLocalTransactionId == InvalidLocalTransactionId);
@@ -3939,7 +3939,7 @@ VirtualXactLockTableInsert(VirtualTransactionId vxid)
MyProc->fpVXIDLock = true;
MyProc->fpLocalTransactionId = vxid.localTransactionId;
- LWLockRelease(MyProc->backendLock);
+ LWLockRelease(&MyProc->backendLock);
}
/*
@@ -3959,14 +3959,14 @@ VirtualXactLockTableCleanup(void)
/*
* Clean up shared memory state.
*/
- LWLockAcquire(MyProc->backendLock, LW_EXCLUSIVE);
+ LWLockAcquire(&MyProc->backendLock, LW_EXCLUSIVE);
fastpath = MyProc->fpVXIDLock;
lxid = MyProc->fpLocalTransactionId;
MyProc->fpVXIDLock = false;
MyProc->fpLocalTransactionId = InvalidLocalTransactionId;
- LWLockRelease(MyProc->backendLock);
+ LWLockRelease(&MyProc->backendLock);
/*
* If fpVXIDLock has been cleared without touching fpLocalTransactionId,
@@ -4022,13 +4022,13 @@ VirtualXactLock(VirtualTransactionId vxid, bool wait)
* against the ones we're waiting for. The target backend will only set
* or clear lxid while holding this lock.
*/
- LWLockAcquire(proc->backendLock, LW_EXCLUSIVE);
+ LWLockAcquire(&proc->backendLock, LW_EXCLUSIVE);
/* If the transaction has ended, our work here is done. */
if (proc->backendId != vxid.backendId
|| proc->fpLocalTransactionId != vxid.localTransactionId)
{
- LWLockRelease(proc->backendLock);
+ LWLockRelease(&proc->backendLock);
return true;
}
@@ -4038,7 +4038,7 @@ VirtualXactLock(VirtualTransactionId vxid, bool wait)
*/
if (!wait)
{
- LWLockRelease(proc->backendLock);
+ LWLockRelease(&proc->backendLock);
return false;
}
@@ -4063,7 +4063,7 @@ VirtualXactLock(VirtualTransactionId vxid, bool wait)
if (!proclock)
{
LWLockRelease(partitionLock);
- LWLockRelease(proc->backendLock);
+ LWLockRelease(&proc->backendLock);
ereport(ERROR,
(errcode(ERRCODE_OUT_OF_MEMORY),
errmsg("out of shared memory"),
@@ -4077,7 +4077,7 @@ VirtualXactLock(VirtualTransactionId vxid, bool wait)
}
/* Done with proc->fpLockBits */
- LWLockRelease(proc->backendLock);
+ LWLockRelease(&proc->backendLock);
/* Time to wait. */
(void) LockAcquire(&tag, ShareLock, false, false);
diff --git a/src/backend/storage/lmgr/lwlock.c b/src/backend/storage/lmgr/lwlock.c
index d43fb61..36fd5b4 100644
--- a/src/backend/storage/lmgr/lwlock.c
+++ b/src/backend/storage/lmgr/lwlock.c
@@ -353,9 +353,6 @@ NumLWLocks(void)
/* Predefined LWLocks */
numLocks = NUM_FIXED_LWLOCKS;
- /* proc.c needs one for each backend or auxiliary process */
- numLocks += MaxBackends + NUM_AUXILIARY_PROCS;
-
/* slot.c needs one for each slot */
numLocks += max_replication_slots;
diff --git a/src/backend/storage/lmgr/proc.c b/src/backend/storage/lmgr/proc.c
index bb10c1b..ff86743 100644
--- a/src/backend/storage/lmgr/proc.c
+++ b/src/backend/storage/lmgr/proc.c
@@ -63,6 +63,9 @@ bool log_lock_waits = false;
PGPROC *MyProc = NULL;
PGXACT *MyPgXact = NULL;
+/* LWLock tranche for backend locks */
+LWLockTranche ProcLWLockTranche;
+
/*
* This spinlock protects the freelist of recycled PGPROC structures.
* We cannot use an LWLock because the LWLock manager depends on already
@@ -226,7 +229,7 @@ InitProcGlobal(void)
{
PGSemaphoreCreate(&(procs[i].sem));
InitSharedLatch(&(procs[i].procLatch));
- procs[i].backendLock = LWLockAssign();
+ LWLockInitialize(&(procs[i].backendLock), LWTRANCHE_PROC);
}
procs[i].pgprocno = i;
@@ -437,6 +440,13 @@ InitProcessPhase2(void)
{
Assert(MyProc != NULL);
+ /* Register and initialize fields of ProcLWLockTranche */
+ ProcLWLockTranche.name = "proc";
+ ProcLWLockTranche.array_base = (char *) (ProcGlobal->allProcs) +
+ offsetof(PGPROC, backendLock);
+ ProcLWLockTranche.array_stride = sizeof(PGPROC);
+ LWLockRegisterTranche(LWTRANCHE_PROC, &ProcLWLockTranche);
+
/*
* Add our PGPROC to the PGPROC array in shared memory.
*/
diff --git a/src/include/storage/lwlock.h b/src/include/storage/lwlock.h
index ff34529..cf90329 100644
--- a/src/include/storage/lwlock.h
+++ b/src/include/storage/lwlock.h
@@ -213,6 +213,7 @@ typedef enum BuiltinTrancheIds
LWTRANCHE_WAL_INSERT,
LWTRANCHE_BUFFER_CONTENT,
LWTRANCHE_BUFFER_IO_IN_PROGRESS,
+ LWTRANCHE_PROC,
LWTRANCHE_FIRST_USER_DEFINED
} BuiltinTrancheIds;
diff --git a/src/include/storage/proc.h b/src/include/storage/proc.h
index 3d68017..40fd93a 100644
--- a/src/include/storage/proc.h
+++ b/src/include/storage/proc.h
@@ -147,7 +147,7 @@ struct PGPROC
TransactionId backendLatestXid;
/* Per-backend LWLock. Protects fields below. */
- LWLock *backendLock; /* protects the fields below */
+ LWLock backendLock;
/* Lock manager data, recording fast-path locks taken by this backend. */
uint64 fpLockBits; /* lock modes held for each fast-path slot */
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
