Re: Buffer locking is special (hints, checksums, AIO writes)

Hi,

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)

- Tried to address Melanie's feedback, with some exceptions (some noted below,
but I also need to make another pass through the reviews)

- re-implemented AssertNotCatalogBufferLock() in the new world

- Substantially expanded comments around setting hint bits (in buffer/README,
heapam_visibility.c and bufmgr.c)

- split out the change to fsm_vacuum_page() to start to lock the page into is
own commit

- reordered patch series so that smaller changes are before the 64bit-state
and "Implement buffer content locks independently of" commits, so they can
be committed while we finish cleaning the later changes

- I didn't invest much in cleaning up the later patches ("Don't copy pages
while writing out" and "Make UnlockReleaseBuffer() more efficient") yet,
wanted to focus on the earlier patches first

Todo:

- still need to rename ResOwnerReleaseBufferPin(). Wondering about what to
rename ResourceOwnerDesc.name to. "buffer ownership" maybe? Not great...

- gistkillitems() complaint by Melanie

- amortize vs batch vs SetHintBits comment + SHB_* names

- for the next version I'll remove the BATCHMVCC_FEWER_ARGS conditionals from
0010. I don't love needing BatchMVCCState but I don't really see an
alternative, the performance difference is pretty persistent.

Questions:
- ForEachLWLockHeldByMe() and LWLockDisown() aren't used anymore, should we
remove them?

Greetings,

Andres Freund