Thread: pgsql: Improve LWLock scalability.
Improve LWLock scalability. The old LWLock implementation had the problem that concurrent lock acquisitions required exclusively acquiring a spinlock. Often that could lead to acquirers waiting behind the spinlock, even if the actual LWLock was free. The new implementation doesn't acquire the spinlock when acquiring the lock itself. Instead the new atomic operations are used to atomically manipulate the state. Only the waitqueue, used solely in the slow path, is still protected by the spinlock. Check lwlock.c's header for an explanation about the used algorithm. For some common workloads on larger machines this can yield significant performance improvements. Particularly in read mostly workloads. Reviewed-By: Amit Kapila and Robert Haas Author: Andres Freund Discussion: 20130926225545.GB26663@awork2.anarazel.de Branch ------ master Details ------- http://git.postgresql.org/pg/commitdiff/ab5194e6f617a9a9e7aadb3dd1cee948a42d0755 Modified Files -------------- src/backend/storage/lmgr/lwlock.c | 931 +++++++++++++++++++++++++------------ src/include/storage/lwlock.h | 24 +- 2 files changed, 653 insertions(+), 302 deletions(-)
> Improve LWLock scalability. > src/backend/storage/lmgr/lwlock.c | 931 +++++++++++++++++++++++++------------ > src/include/storage/lwlock.h | 24 +- > 2 files changed, 653 insertions(+), 302 deletions(-) This broke dtrace probes in a trivial way. Looking at the log made me notice though that the probes in lwlock.c generate warnings since the tranche changes went in. Check http://pgbuildfarm.org/cgi-bin/show_stage_log.pl?nm=locust&dt=2014-12-25%2014%3A30%3A40&stg=make lwlock.c: In function 'LWLockAcquireCommon': lwlock.c:650: warning: passing argument 1 of '__dtrace_probe$postgresql$lwlock__wait__start$v1$63686172202a$696e74$696e74'discards qualifiers from pointer target type lwlock.c:661: warning: passing argument 1 of '__dtrace_probe$postgresql$lwlock__wait__done$v1$63686172202a$696e74$696e74'discards qualifiers from pointer target type lwlock.c:677: warning: passing argument 1 of '__dtrace_probe$postgresql$lwlock__acquire$v1$63686172202a$696e74$696e74' discardsqualifiers from pointer target type lwlock.c: In function 'LWLockConditionalAcquire': lwlock.c:750: warning: passing argument 1 of '__dtrace_probe$postgresql$lwlock__condacquire__fail$v1$63686172202a$696e74$696e74'discards qualifiers from pointer targettype lwlock.c:757: warning: passing argument 1 of '__dtrace_probe$postgresql$lwlock__condacquire$v1$63686172202a$696e74$696e74'discards qualifiers from pointer target type lwlock.c: In function 'LWLockAcquireOrWait': lwlock.c:864: warning: passing argument 1 of '__dtrace_probe$postgresql$lwlock__wait__start$v1$63686172202a$696e74$696e74'discards qualifiers from pointer target type lwlock.c:875: warning: passing argument 1 of '__dtrace_probe$postgresql$lwlock__wait__done$v1$63686172202a$696e74$696e74'discards qualifiers from pointer target type lwlock.c:897: warning: passing argument 1 of '__dtrace_probe$postgresql$lwlock__acquire__or__wait__fail$v1$63686172202a$696e74$696e74'discards qualifiers from pointertarget type lwlock.c:904: warning: passing argument 1 of '__dtrace_probe$postgresql$lwlock__acquire__or__wait$v1$63686172202a$696e74$696e74'discards qualifiers from pointer targettype lwlock.c: In function 'LWLockWaitForVar': lwlock.c:1037: warning: passing argument 1 of '__dtrace_probe$postgresql$lwlock__wait__start$v1$63686172202a$696e74$696e74'discards qualifiers from pointer target type lwlock.c:1049: warning: passing argument 1 of '__dtrace_probe$postgresql$lwlock__wait__done$v1$63686172202a$696e74$696e74'discards qualifiers from pointer target type lwlock.c:1060: warning: passing argument 1 of '__dtrace_probe$postgresql$lwlock__acquire$v1$63686172202a$696e74$696e74' discardsqualifiers from pointer target type lwlock.c: In function 'LWLockRelease': lwlock.c:1247: warning: passing argument 1 of '__dtrace_probe$postgresql$lwlock__release$v1$63686172202a$696e74' discardsqualifiers from pointer target type That's because LWLockTranche->name is const. As I've never used the dtrace probes and apparently no other developer does either I'm regarding the value of the const higher than those of not generating warnings in dtrace builds. Anyone sees that differently? Personally I think at this point we could just rip the probe support out, but I'm not going to fight for that in earnest. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
Andres Freund <andres@anarazel.de> writes: > This broke dtrace probes in a trivial way. Looking at the log made me > notice though that the probes in lwlock.c generate warnings since the > tranche changes went in. Yeah, dtrace has never behaved very nicely with "const foo *" arguments. This is a bug in dtrace, not in our usage of it, and I don't think we should de-constify our code to work around the warnings. > Personally I think at this point we could just rip the probe support > out, but I'm not going to fight for that in earnest. Not without a replacement for the functionality, IMO. regards, tom lane
On 2014-12-25 14:13:08 -0500, Tom Lane wrote: > Andres Freund <andres@anarazel.de> writes: > > This broke dtrace probes in a trivial way. Looking at the log made me > > notice though that the probes in lwlock.c generate warnings since the > > tranche changes went in. > > Yeah, dtrace has never behaved very nicely with "const foo *" arguments. > This is a bug in dtrace, not in our usage of it, and I don't think we > should de-constify our code to work around the warnings. Great, agreed. > > Personally I think at this point we could just rip the probe support > > out, but I'm not going to fight for that in earnest. > > Not without a replacement for the functionality, IMO. Fine with me, I just always had the impression that nobody uses them - I very well might be wrong. Personally I just use 'perf probe' to add tracepoints dynamically, where I need them. I assume others are doing similar things. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
On Thu, Dec 25, 2014 at 11:49 AM, Andres Freund <andres@anarazel.de> wrote: > Improve LWLock scalability. + * do so. Returns false if somebody else already has woken us up, otherwise + * returns true. + */ +static void +LWLockDequeueSelf(LWLock *lock) The comment about the return type is wrong, because the function returns void. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company