Re: remove adaptive spins_per_delay code

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