> 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.
