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
