Re: Optimizing TransactionIdIsCurrentTransactionId()

On Fri, 20 Dec 2019 at 13:07, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:

>
> > Read only transactions should have a fast path thru this function since
> they frequently read more data than write transactions.
>
> With regard to this point, I second Tomas's comments.
>

I also agree with Tomas' comments. I am explaining *why* it will be an
improvement, expanding on my earlier notes.

This function is called extremely frequently in query processing and is
fairly efficient. I'm pointing out cases where making it even quicker makes
sense.

The TopXid is assigned in very few calls. Write transactions perform
searching before the xid is assigned, so UPDATE and DELETE transactions
will call this with TopXid unassigned in many small transactions, e.g.
simple pgbench. In almost all read-only cases and especially on standby
nodes there will be no TopXid assigned, so I estimate that 90-99% of calls
will be made with TopXid invalid. In this case it makes a great deal of
sense to have a fastpath out of this function, by testing
TransactionIdIsNormal(topxid).

I also now notice that on entry the xid provided is hardly ever
InvalidTransactionId. Once, it might have been called repeatedly with
FrozenTransactionId, but that is no longer the case since we no longer
reset the xid on freezing. So the test for TransactionIdIsNormal(xid)
appears to need rethinking since it is now mostly redundant.

So if adding a test is considered heavy, I would swap the test for
TransactionIdIsNormal(xid) and replace with a test for
TransactionIdIsNormal(topxid).

Such a frequently used function is worth discussing, just as we previously
optimised TransactionIdIsInProgress() and MVCC visibility routines, where
we discussed what the most common routes through the functions were before
deciding how to optimize them.

--
Simon Riggs http://www.2ndQuadrant.com/
<http://www.2ndquadrant.com/>
PostgreSQL Solutions for the Enterprise