On Thu, Dec 19, 2019 at 02:27:01PM -0500, Robert Haas wrote:
On Wed, Dec 18, 2019 at 5:07 AM Simon Riggs <si...@2ndquadrant.com> wrote:
TransactionIdIsCurrentTransactionId() doesn't seem to be well optimized for the
case when an xid has not yet been assigned, so for read only transactions.
A patch for this is attached.
It might be an idea to first call TransactionIdIsNormal(xid), then
GetTopTransactionIdIfAny(), then TransactionIdIsNormal(topxid), so
that we don't bother with GetTopTransactionIdIfAny() when
!TransactionIdIsNormal(xid).
But it's also not clear to me whether this is actually a win. You're
dong an extra TransactionIdIsNormal() test to sometimes avoid a
GetTopTransactionIdIfAny() test. TransactionIdIsNormal() is pretty
cheap, but GetTopTransactionIdIfAny() isn't all that expensive either,
and adding more branches costs something.
I think "optimization" patches should generally come with some sort of
quantification of the gains - e.g. a benchmark with somewhat realistic
workload (but even synthetic is better than nothing). Or at least some
explanation *why* it's going to be an improvement.
regards
--
Tomas Vondra http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
