public inbox for [email protected]  

help / color / mirror / Atom feed

From: Andres Freund <[email protected]>
To: [email protected]
To: Melanie Plageman <[email protected]>
To: Thomas Munro <[email protected]>
To: Heikki Linnakangas <[email protected]>
To: Noah Misch <[email protected]>
To: Robert Haas <[email protected]>
To: Michael Paquier <[email protected]>
Subject: Re: Buffer locking is special (hints, checksums, AIO writes)
Date: Mon, 15 Sep 2025 19:05:37 -0400
Message-ID: <yivb2evcrj7fna5ymuunw3g5u5xxttwjbjxaa4ofkfkviystjv@4dfylftqxyxh> (raw)
In-Reply-To: <fvfmkr5kk4nyex56ejgxj3uzi63isfxovp2biecb4bspbjrze7@az2pljabhnff>
References: <fvfmkr5kk4nyex56ejgxj3uzi63isfxovp2biecb4bspbjrze7@az2pljabhnff>

Hi,

On 2025-08-22 15:44:48 -0400, Andres Freund wrote:
> The hardest part about this change is that everything kind of depends on each
> other. The changes are large enough that they clearly can't just be committed
> at once, but doing them over time risks [temporary] performance regressions.
> 
> 
> 
> 
> The order of changes I think makes the most sense is the following:
> 
> 1) Allow some modifications while holding the buffer header spinlock
> 
> 2) Reduce buffer pin with just an atomic-sub
> 
>    This needs to happen first, otherwise there are performance regressions
>    during the later steps.

Here are the first few cleaned up patches implementing the above steps, as
well as some cleanups.  I included a commit from another thread, as it
conflicts with these changes, and we really should apply it - and it's
arguably required to make the changes viable, as it removes one more use of
PinBuffer_Locked().

Another change included is to not return the buffer with the spinlock held
from StrategyGetBuffer(), and instead pin the buffer in freelist.c. The reason
for that is to reduce the most common PinBuffer_locked() call. By definition
PinBuffer_locked() will become a bit slower due to 0003. But even without 0003
it 0002 is faster than master. And the previous approach also just seems
pretty unclean.   I don't love that it requires the new TrackNewBufferPin(),
but I don't really have a better idea.

I invite particular attention to the commit message for 0003 as well as the
comment changes in buf_internals.h within.

I'm not sure I like the TODOs added in 0003 and removed in 0004, but squashing
the changes doesn't really seem better either.

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]
  Subject: Re: Buffer locking is special (hints, checksums, AIO writes)
  In-Reply-To: <yivb2evcrj7fna5ymuunw3g5u5xxttwjbjxaa4ofkfkviystjv@4dfylftqxyxh>

* 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