Re: lazy vxid locks, v1 - Mailing list pgsql-hackers

From Stefan Kaltenbrunner
Subject Re: lazy vxid locks, v1
Date
Msg-id 4DF615DE.3070907@kaltenbrunner.cc
Whole thread Raw
In response to lazy vxid locks, v1  (Robert Haas <robertmhaas@gmail.com>)
Responses Re: lazy vxid locks, v1
List pgsql-hackers
On 06/12/2011 11:39 PM, Robert Haas wrote:
> Here is a patch that applies over the "reducing the overhead of
> frequent table locks" (fastlock-v3) patch and allows heavyweight VXID
> locks to spring into existence only when someone wants to wait on
> them.  I believe there is a large benefit to be had from this
> optimization, because the combination of these two patches virtually
> eliminates lock manager traffic on "pgbench -S" workloads.  However,
> there are several flies in the ointment.
> 
> 1. It's a bit of a kludge.  I leave it to readers of the patch to
> determine exactly what about this patch they think is kludgey, but
> it's likely not the empty set.  I suspect that MyProc->fpLWLock needs
> to be renamed to something a bit more generic if we're going to use it
> like this, but I don't immediately know what to call it.  Also, the
> mechanism whereby we take SInvalWriteLock to work out the mapping from
> BackendId to PGPROC * is not exactly awesome.  I don't think it
> matters from a performance point of view, because operations that need
> VXID locks are sufficiently rare that the additional lwlock traffic
> won't matter a bit.  However, we could avoid this altogether if we
> rejiggered the mechanism for allocating PGPROCs and backend IDs.
> Right now, we allocate PGPROCs off of linked lists, except for
> auxiliary procs which allocate them by scanning a three-element array
> for an empty slot.  Then, when the PGPROC subscribes to sinval, the
> sinval mechanism allocates a backend ID by scanning for the lowest
> unused backend ID in the ProcState array.  If we changed the logic for
> allocating PGPROCs to mimic what the sinval queue currently does, then
> the backend ID could be defined as the offset into the PGPROC array.
> Translating between a backend ID and a PGPROC * now becomes a matter
> of pointer arithmetic.  Not sure if this is worth doing.
> 
> 2. Bad thing happen with large numbers of connections.  This patch
> increases peak performance, but as you increase the number of
> concurrent connections beyond the number of CPU cores, performance
> drops off faster with the patch than without it.  For example, on the
> 32-core loaner from Nate Boley, using 80 pgbench -S clients, unpatched
> HEAD runs at ~36K TPS; with fastlock, it jumps up to about ~99K TPS;
> with this patch also applied, it drops down to about ~64K TPS, despite
> the fact that nearly all the contention on the lock manager locks has
> been eliminated.    On Stefan Kaltenbrunner's 40-core box, he was
> actually able to see performance drop down below unpatched HEAD with
> this applied!  This is immensely counterintuitive.  What is going on?
> 
> Profiling reveals that the system spends enormous amounts of CPU time
> in s_lock.  

just to reiterate that with numbers - at 160 threads with both patches
applied the profile looks like:

samples  %        image name               symbol name
828794   75.8662  postgres                 s_lock
51672     4.7300  postgres                 LWLockAcquire
51145     4.6817  postgres                 LWLockRelease
17636     1.6144  postgres                 GetSnapshotData
7521      0.6885  postgres                 hash_search_with_hash_value
6193      0.5669  postgres                 AllocSetAlloc
4527      0.4144  postgres                 SearchCatCache
4521      0.4138  postgres                 PinBuffer
3385      0.3099  postgres                 SIGetDataEntries
3160      0.2893  postgres                 PostgresMain
2706      0.2477  postgres                 _bt_compare
2687      0.2460  postgres                 fmgr_info_cxt_security
1963      0.1797  postgres                 UnpinBuffer
1846      0.1690  postgres                 LockAcquireExtended
1770      0.1620  postgres                 exec_bind_message
1730      0.1584  postgres                 hash_any
1644      0.1505  postgres                 ExecInitExpr

even at the peak performance spot of the combined patch-set (-c40) the
contention is noticable in the profile:

samples  %        image name               symbol name
1497826  22.0231  postgres                 s_lock
592104    8.7059  postgres                 LWLockAcquire
512213    7.5313  postgres                 LWLockRelease
230050    3.3825  postgres                 GetSnapshotData
176252    2.5915  postgres                 AllocSetAlloc
155122    2.2808  postgres                 hash_search_with_hash_value
116235    1.7091  postgres                 SearchCatCache
110197    1.6203  postgres                 _bt_compare
94101     1.3836  postgres                 PinBuffer
80119     1.1780  postgres                 PostgresMain
65584     0.9643  postgres                 fmgr_info_cxt_security
55198     0.8116  postgres                 hash_any
52872     0.7774  postgres                 exec_bind_message
48438     0.7122  postgres                 LockReleaseAll
46631     0.6856  postgres                 MemoryContextAlloc
45909     0.6750  postgres                 ExecInitExpr
42293     0.6219  postgres                 AllocSetFree



Stefan


pgsql-hackers by date:

Previous
From: Tom Lane
Date:
Subject: Re: Boolean operators without commutators vs. ALL/ANY
Next
From: Tom Lane
Date:
Subject: Re: wrong message on REASSIGN OWNED