Em qua., 25 de mai. de 2022 às 00:56, Andres Freund <andres@anarazel.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.
