Обсуждение: Remove header lock BufferGetLSNAtomic() on architectures with 64 bit atomic operations

Hi,

Andres pointed out this possible optimization on Discord so I hacked up 
a quick patch which avoids taking a lock when reading the LSN from a 
page on architectures where we can be sure to not get a torn value. It 
is always nice to remove a lock from a reasonably hot code path.

I thought about using our functions for atomics but did not do so since 
I did not want to introduce any extra overhead on platforms which do not 
support 64-bit atomic operations.

I decided to just remove the struct to make the code simpler and more 
consistent but I can also see an argument for keeping it to get some 
degree of type safety.

I have not properly benchmarked it yet but plan to do so when I am back 
from my vacation.

I have also included a cleanup patch where I change a macro into an 
inline function which I think improves code readability. Feel free to 
ignore that one if you want.

--
Andreas
Percona

On Sun, Nov 23, 2025 at 6:10 PM Andreas Karlsson <andreas@proxel.se> wrote:
> Andres pointed out this possible optimization on Discord so I hacked up
> a quick patch which avoids taking a lock when reading the LSN from a
> page on architectures where we can be sure to not get a torn value. It
> is always nice to remove a lock from a reasonably hot code path.

This seems very useful.

One reason why I'm interested in this work is that it will facilitate
other work (from Tomas Vondra and myself) on the proposed new
amgetbatch interface, which enables optimizations such as index
prefetching. That work will replace the use of the amgettuple
interface with a index page/batch oriented amgetbatch interface. It
generalizes nbtree's dropPin optimization (originally added under a
different name by commit 2ed5b87f) to work with any index AM that uses
the new amgetbatch interface -- which will include both nbtree and
hash in the initial version (and possibly other index AMs in the
future).

This means that there'll be quite a few new BufferGetLSNAtomic calls
during scans of hash indexes, where before there were none -- we need
to stash an LSN so that we have an alternative way of detecting unsafe
concurrent TID recycling when LP_DEAD-marking index tuples as a batch
is released (since there'll have been no buffer pin held on the index
leaf page while we accessed the heap when the dropPin optimization is
applied).

With page-level checksums enabled, on the recently posted v4 of our
patch set, I find that there's a regression of about 5% of transaction
throughput with a variant of pgbench SELECT that uses hash indexes.
But with your patch applied on top of our own, that regression
completely goes away.

FWIW the improvement I see with regular nbtree index scans is still
visible, but not quite as good as with hash indexes + the amgetbatch
patch set. It should be easy enough to see this for yourself. Standard
pgbench SELECT should work well enough -- just make sure that you test
with page-level checksums enabled.

--
Peter Geoghegan