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

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.

Greetings,

Andres Freund