public inbox for [email protected]  

help / color / mirror / Atom feed

From: Andres Freund <[email protected]>
To: Melanie Plageman <[email protected]>
Cc: Matthias van de Meent <[email protected]>
Cc: [email protected]
Cc: Thomas Munro <[email protected]>
Cc: Heikki Linnakangas <[email protected]>
Cc: Noah Misch <[email protected]>
Cc: Robert Haas <[email protected]>
Cc: Michael Paquier <[email protected]>
Subject: Re: Buffer locking is special (hints, checksums, AIO writes)
Date: Wed, 3 Dec 2025 11:03:40 -0500
Message-ID: <ev4pqektep4o5sh3acjc7hx73mhhxofupz4tywyynxekbyb7e3@r7xsusevn4pf> (raw)
In-Reply-To: <lneuyxqxamqoayd2ntau3lqjblzdckw6tjgeu4574ezwh4tzlg@noioxkquezdw>
References: <6kmid26do57ykqfpvq6iieniy4djsymhrypkjccazq5g4bbe6a@2y6owwv7qpex>
	<CAEze2WgGe8vjj3jiWqUugWuwLJ9cLryaGrnASjm-yJ=tEALX2A@mail.gmail.com>
	<pmto7djq64mei53p7r5smfync2waittilhbuzc7j7lpflf2b3y@laz7r76y5pux>
	<CAEze2WjeK9CY003S4dmCugv_H4tz9AaXgnqW+wTc=BaPDg+2xg@mail.gmail.com>
	<3je3ahgf7rrmmurxo6hnlhg5d3ffwfrtjwjxd6jm5srlv5iebp@vxqk5qtgmowr>
	<3w7v3w6a57jnssokap4k7thoekig72flnyhd4wp3yftzdd7lm7@f6lpcfen6hr7>
	<6rgb2nvhyvnszz4ul3wfzlf5rheb2kkwrglthnna7qhe24onwr@vw27225tkyar>
	<CAAKRu_ZcZCXNUWfhEg4qOYuRO8T1x3_gvaUFFrwgCfY0RnVhkg@mail.gmail.com>
	<3nce7i72ayzkunai6mkz24ckbxk74jodz4ua2chcdrwppxlxcd@w6x5kfkjrkru>
	<lneuyxqxamqoayd2ntau3lqjblzdckw6tjgeu4574ezwh4tzlg@noioxkquezdw>

Hi,

On 2025-12-02 19:47:35 -0500, 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)
> 
> - 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?

I'm planning to work on committing 0001, 0002, 0003, 0008 soon-ish, unless
somebody sees a reason to hold off on that.  After that I think 0005, 0006
would be next.  I think 0004 is a clear improvement, but nobody has looked at
it yet...

For 0007, I wished ConditionalLockBuffer() accepted the lock level, there's no
point in waiting for the lock in the use case. I'm on the fence about whether
it's worth changing the ~12 users of ConditionalLockBuffer()...

Greetings,

Andres Freund

view thread (57+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
  Subject: Re: Buffer locking is special (hints, checksums, AIO writes)
  In-Reply-To: <ev4pqektep4o5sh3acjc7hx73mhhxofupz4tywyynxekbyb7e3@r7xsusevn4pf>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox