From: | Tomas Vondra <tomas(dot)vondra(at)2ndquadrant(dot)com> |
---|---|
To: | Robert Haas <robertmhaas(at)gmail(dot)com> |
Cc: | Simon Riggs <simon(at)2ndquadrant(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
Subject: | Re: Optimizing TransactionIdIsCurrentTransactionId() |
Date: | 2019-12-20 00:26:08 |
Message-ID: | 20191220002608.2lbpibbmj2h6c64h@development |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Thu, Dec 19, 2019 at 02:27:01PM -0500, Robert Haas wrote:
>On Wed, Dec 18, 2019 at 5:07 AM Simon Riggs <simon(at)2ndquadrant(dot)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
From | Date | Subject | |
---|---|---|---|
Next Message | Thomas Munro | 2019-12-20 00:26:22 | Re: [HACKERS] kqueue |
Previous Message | Peter Geoghegan | 2019-12-20 00:12:22 | Re: Building infrastructure for B-Tree deduplication that recognizes when opclass equality is also equivalence |