Re: [HACKERS] Fix performance degradation of contended LWLock on NUMA

From: Dmitry Dolgov <9erthalion6(at)gmail(dot)com>
To: Andres Freund <andres(at)anarazel(dot)de>
Cc: Jesper Pedersen <jesper(dot)pedersen(at)redhat(dot)com>, Юрий Соколов <funny(dot)falcon(at)gmail(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [HACKERS] Fix performance degradation of contended LWLock on NUMA
Date: 2018-11-30 16:21:51
Message-ID: CA+q6zcVvKr1N19HWzVQZJDAkjZSBB6qjyZGMW9Sfcvba-fSqeA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

> On Sat, Nov 10, 2018 at 8:37 PM Andres Freund <andres(at)anarazel(dot)de> wrote:
>
> On 2018-11-10 20:18:33 +0100, Dmitry Dolgov wrote:
> > > On Mon, 2 Jul 2018 at 15:54, Jesper Pedersen <jesper(dot)pedersen(at)redhat(dot)com> wrote:
> > >
> > > The patch from November 27, 2017 still applies (with hunks),
> > >
> > > https://commitfest.postgresql.org/18/1166/
> > >
> > > passes "make check-world" and shows performance improvements.
> > >
> > > Keeping it in "Ready for Committer".
> >
> > Looks like for some reason this patch is failing to attract committers, any
> > idea why? One of the plausible explanations for me is that the patch requires
> > some more intensive benchmarking of different workloads and types of lock
> > contention to make everyone more confident about it.
>
> Personally it's twofold:
>
> 1) It changes a lot of things, more than I think are strictly
> necessary to achieve the goal.
>
> 2) While clearly the retry logic is not necessary anymore (it was
> introduced when wait-queue was protected by a separate spinlock, which
> could not atomically manipulated together with the lock's state),
> there's reasons why it would be advantageous to keep: My original
> patch for changing lwlocks to atomics, used lock xadd / fetch_add
> to acquire shared locks (just incrementing the #shared bits after an
> unlocked check) - obviously that can cause superfluous failures for
> concurrent lock releases. Having the retry logic around can make
> that safe.
>
> Using lock xadd to acquire shared locks turns out to be considerably
> more efficient - most locks where the lock state is contended (rather
> than just having processes wait), tend to have a very large fraction
> of shared lockers. And being able to do such a lock acquisition on a
> conteded cacheline with just a single locked operation, commonly
> without retries, is quite beneficial.

Due to lack of response and taking into account this commentary, I'm marking
this patch as "Returned with feedback", but hopefully I can pick it up later to
improve.

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Dmitry Dolgov 2018-11-30 16:35:21 Re: [HACKERS] [PATCH v2] Add and report the new "session_read_only" GUC pseudo-variable.
Previous Message Dmitry Dolgov 2018-11-30 16:16:51 Re: pg_dump multi VALUES INSERT