Re: remove adaptive spins_per_delay code - Mailing list pgsql-hackers

From Nathan Bossart
Subject Re: remove adaptive spins_per_delay code
Date
Msg-id Zs4hJ6-Fg8DMgU_P@nathan
Whole thread Raw
In response to Re: remove adaptive spins_per_delay code  (Andres Freund <andres@anarazel.de>)
List pgsql-hackers
On Tue, Aug 27, 2024 at 02:27:00PM -0400, Andres Freund wrote:
> FWIW, I've seen cases on multi-socket machines where performance was vastly
> worse under contention with some values of spins_per_delay. With good numbers
> being quite different on smaller machines. Most new-ish server CPUs these days
> basically behave like a multi-socket machine internally, due to being
> internally partitioned into multiple chiplets. And it's pretty clear that that
> trend isn't going to go away.  So finding a good value probably isn't easy.

Yeah.

> We don't have a whole lot of contended spin.h spinlocks left, except that we
> have one very critical one, XLogCtl->Insert.insertpos_lck. And of course we
> use the same spinning logic for buffer header locks - which can be heavily
> contended.

Another one I've been looking into is pgssEntry->mutex, which shows up
prominently when pg_stat_statements.track_planning is on.  There was some
previous discussion about this [0], which resulted in that parameter
getting turned off by default (commit d1763ea).  I tried converting those
locks to LWLocks, but that actually hurt performance.  I also tried
changing the counters to atomics, which AFAICT is mostly doable except for
"usage".  That one would require some more thought to be able to convert it
away from a double.

> I suspect that eventually we ought to replace all our userspace
> spinlock-like-things with a framework for writing properly "waiting" locks
> with some spinning. We can't just use lwlocks because support for
> reader-writer locks makes them much more heavyweight (mainly because it
> implies having to use an atomic operation for lock release, which shows up
> substantially).

Another approach I'm investigating is adding exponential backoff via extra
spins in perform_spin_delay().  I'm doubtful this will be a popular
suggestion, as appropriate settings seem to be hardware/workload dependent
and therefore will require a GUC or two, but it does seem to help
substantially on machines with many cores.  In any case, I think we ought
to do _something_ in this area for v18.

[0] https://postgr.es/m/2895b53b033c47ccb22972b589050dd9%40EX13D05UWC001.ant.amazon.com

-- 
nathan



pgsql-hackers by date:

Previous
From: Peter Geoghegan
Date:
Subject: Re: PoC: prefetching data between executor nodes (e.g. nestloop + indexscan)
Next
From: Tom Lane
Date:
Subject: Re: Showing primitive index scan count in EXPLAIN ANALYZE (for skip scan and SAOP scans)