Hi,
On 2026-02-09 12:14:47 +0200, Heikki Linnakangas wrote:
> On 09/02/2026 03:52, Andres Freund wrote:
> > On 2026-02-07 14:59:34 +0200, Heikki Linnakangas wrote:
> > > > +/*
> > > > + * Try to set a single hint bit in a buffer.
> > > > + *
> > > > + * This is a bit faster than BufferBeginSetHintBits() /
> > > > + * BufferFinishSetHintBits() when setting a single hint bit, but slower than
> > > > + * the former when setting several hint bits.
> > > > + */
> > > > +bool
> > > > +BufferSetHintBits16(uint16 *ptr, uint16 val, Buffer buffer)
> > >
> > > This could use some more explanation. The point is that this does "*ptr =
> > > val", if it's allowed to set hint bits. That's not obvious. And "single hint
> > > bit" isn't really accurate, as you could update multiple bits in *ptr with
> > > one call.
> >
> > Agreed. I updated it to
> >
> > * Try to set hint bits on a single 16bit value in a buffer.
> > *
> > * If hint bits are allowed to be set, set *ptr = val, try mark the buffer
> > * dirty and return true. Otherwise false is returned.
> > *
> > * *ptr needs to be a pointer to memory within the buffer.
> > *
> > * This is a bit faster than BufferBeginSetHintBits() /
> > * BufferFinishSetHintBits() when setting hints once in a buffer, but slower
> > * than the former when setting hint bits multiple times in the same buffer.
>
> +1. Instead of "try mark the buffer dirty", I'd say just "mark the buffer
> dirty". The only reason it might not to mark the buffer dirty is that it was
> already marked dirty, right? I wouldn't call that a failure.
It's not quite the only reason:
/*
* If we need to protect hint bit updates from torn writes, WAL-log a
* full page image of the page. This full page image is only necessary
* if the hint bit update is the first change to the page since the
* last checkpoint.
*
* We don't check full_page_writes here because that logic is included
* when we call XLogInsert() since the value changes dynamically.
*/
if (XLogHintBitIsNeeded() && (lockstate & BM_PERMANENT))
{
/*
* If we must not write WAL, due to a relfilelocator-specific
* condition or being in recovery, don't dirty the page. We can
* set the hint, just not dirty the page as a result so the hint
* is lost when we evict the page or shutdown.
*
* See src/backend/storage/page/README for longer discussion.
*/
if (RecoveryInProgress() ||
RelFileLocatorSkippingWAL(BufTagGetRelFileLocator(&bufHdr->tag)))
return;
wal_log = true;
}
Greetings,
Andres Freund
