Re: Buffer locking is special (hints, checksums, AIO writes)
Andres Freund <andres@anarazel.de>
From: Andres Freund <andres@anarazel.de>
To: Heikki Linnakangas <hlinnaka@iki.fi>
Cc: Melanie Plageman <melanieplageman@gmail.com>, Matthias van de Meent <boekewurm+postgres@gmail.com>,
pgsql-hackers@postgresql.org, Thomas Munro <thomas.munro@gmail.com>, Noah Misch <noah@leadboat.com>, Robert Haas <robertmhaas@gmail.com>,
Michael Paquier <michael.paquier@gmail.com>
Date: 2025-12-17T14:54:32Z
Lists: pgsql-hackers
Hi, On 2025-12-17 11:25:50 +0200, Heikki Linnakangas wrote: > On 03/12/2025 02:47, Andres Freund wrote: > > On 2025-11-25 11:54:00 -0500, Andres Freund wrote: > > > Thanks a lot for that detailed review! A few questions and comments, before I > > > try to address the comments in the next version. > > > > Here's that new new version, with the following changes > > > > - Some more micro-optimizations, most importantly adding a commit that doesn't > > initialize the delay in LockBufHdr() unless needed. With those I don't see a > > consistent slowdown anymore (slight speedup on one workstation, slight > > slowdown on another, in an absurdly adverse workload) > > +1 > > I'm comparing the patched LockBufHdr() with LWLockWaitListLock(), which does > pretty much the same thing, and LWLockWaitListLock() already did the > initialization of the delay that way. But there are some small differences: > > - LockBufHdr() uses unlikely() in the initial attempt, LWLockWaitListLock() > does not I think we probably ought to do that in LWLockWaitListLock() too. > - LWLockWaitListLock() uses pg_atomic_read_u32() after spinning, > LockBufHdr() retries directly with pg_atomic_fetch_or_u32(). I think here LWLockWaitListLock() is likely right - but it seems like a change to LockBufHdr() that I would probably make in a separate commit? > Are there reasons for the differences, or is it just that they were > developed separately and ended up looking slightly different? I think it's just the latter... Thanks for reviewing, Andres Freund