Re: strange perf regression with data checksums

Hi,

On 2025-05-19 18:13:02 +0200, Tomas Vondra wrote:
> I believe enabling data checksums simply makes it more severe, because
> the BufferGetLSNAtomic() has to obtain header lock, which uses the same
> "state" field, with exactly the same retry logic. It can probably happen
> even without it, but as the lock is exclusive, it also "serializes" the
> access, making the conflicts more likely.
>
> BufferGetLSNAtomic does this:
>
>     bufHdr = GetBufferDescriptor(buffer - 1);
>     buf_state = LockBufHdr(bufHdr);
>     lsn = PageGetLSN(page);
>     UnlockBufHdr(bufHdr, buf_state);
>
> AFAICS the lock is needed simply to read a consistent value from the
> page header, but maybe we could have an atomic variable with a copy of
> the LSN in the buffer descriptor?

I think we can do better - something like

#ifdef PG_HAVE_8BYTE_SINGLE_COPY_ATOMICITY
    lsn = PageGetLSN(page);
#else
    buf_state = LockBufHdr(bufHdr);
    lsn = PageGetLSN(page);
    UnlockBufHdr(bufHdr, buf_state);
#endif

All perf relevant systems support reading 8 bytes without a chance of
tearing...

Greetings,

Andres Freund