Re: LogwrtResult contended spinlock

On Thu, Feb 22, 2024 at 1:54 AM Jeff Davis <pgsql(at)j-davis(dot)com> wrote:
> On Wed, 2024-02-21 at 20:00 +0530, Bharath Rupireddy wrote:
> > The local copy of LogwrtResult is so frequently used in the backends,
> > if we were to replace it with atomic accesses, won't the atomic reads
> > be costly and start showing up in perf profiles?
>
> I don't see exactly where the extra cost would come from. What is your
> concern?
>
> In functions that access it several times, it may make sense to copy it
> to a function-local variable, of course. But having a global non-shared
> LogwrtResult doesn't seem particularly useful to me.

I would love to get rid of the global variable; all of our code relies
too much on global variables, but the xlog code is some of the worst.
The logic is complex and difficult to reason about.

But I do think that there's room for concern about performance, too. I
don't know if there is an actual problem, but there could be a
problem. My impression is that accessing shared memory isn't
intrinsically more costly than accessing backend-private memory, but
accessing heavily contended memory can definitely be more expensive
than accessing uncontended memory -- and not just slightly more
expensive, but WAY more expensive.

I think the problems tend to be worst when you have some bit of data
that's being frequently modified by multiple backends. Every backend
that wants to modify the value needs to steal the cache line, and
eventually you spend most of your CPU time stealing cache lines from
other sockets and not much of it doing any actual work. If you have a
value that's just being read by a lot of backends without
modification, I think the cache line can be shared in read only mode
by all the CPUs and it's not too bad.

--
Robert Haas
EDB: http://www.enterprisedb.com