Re: Buffer locking is special (hints, checksums, AIO writes) - Mailing list pgsql-hackers

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