Re: Improving connection scalability (src/backend/storage/ipc/procarray.c)

Em qua., 25 de mai. de 2022 às 00:56, Andres Freund <andres(at)anarazel(dot)de>
escreveu:

> Hi,
>
> On 2022-05-24 13:23:43 -0300, Ranier Vilela wrote:
> > It certainly helps, but I trust that's not the only reason, in all the
> > tests I did, there was an improvement in performance, even before using
> > this feature.
> > If you look closely at GetSnapShotData() you will see that
> > GetSnapshotDataReuse is called for all snapshots, even the new ones,
> which
> > is unnecessary.
>
> That only happens a handful of times as snapshots are persistently
> allocated.

Yes, but now this does not happen with new snapshots.

Doing an extra GetSnapshotDataReuse() in those cases doesn't matter
> for performance. If anything this increases the number of jumps for the
> common
> case.
>
IMHO with GetSnapShotData(), any gain makes a difference.

>
> It'd be a huge win to avoid needing ProcArrayLock when reusing a snapshot,
> but
> it's not at all easy to guarantee that it's correct / see how to make it
> correct. I'm fairly sure it can be made correct, but ...
>
I believe it's worth the effort to make sure everything goes well and use
this feature.

> > Another example NormalTransactionIdPrecedes is more expensive than
> testing
> > statusFlags.
>
> That may be true when you count instructions, but isn't at all true when
> you
> take into account that the cachelines containing status flags are hotly
> contended.
>

> Also, the likelihood of filtering out a proc due to
> NormalTransactionIdPrecedes(xid, xmax) is *vastly* higher than the due to
> the
> statusFlags check. There may be a lot of procs failing that test, but
> typically there will be far fewer backends in vacuum or logical decoding.
>
I believe that keeping the instructions in the cache together works better
than having the status flags test in the middle.
But I will test this to be sure.

regards,
Ranier Vilela