On Mon, Feb 20, 2017 at 10:49 PM, Simon Riggs <simon@2ndquadrant.com> wrote:
>> Regarding reduce_pgxact_access_AtEOXact.v1.patch, it took me a few
>> minutes to figure out that the comment was referring to
>> ProcArrayEndTransaction(), so it might be good to be more explicit
>> about that if we go forward with this.
>
> Sure, attached.
Looks better, thanks.
>> Have you checked whether this
>> patch makes any noticeable performance difference?
>
> No, but then we're reducing the number of calls to PgXact directly;
> there is no heuristic involved, its just a pure saving.
Well, it's adding a branch where there wasn't one. Maybe that costs
essentially nothing and the saved write to shared memory saves
something noticeable, but for all I know it's the reverse. If I had
to guess, it would be that neither the costs nor the savings from this
are in the slightest way noticeable on a macrobenchmark, and therefore
there's not much point in changing it, but that could be 100% wrong.
>> It's sure
>> surprising that we go to all of this trouble to clean things up in
>> AtEOXact_Snapshot() when we've already nuked MyPgXact->xmin from
>> orbit. (Instead of changing AtEOXact_Snapshot, should we think about
>> removing the xid clear logic from ProcArrayEndTransaction and only
>> doing it here, or would that be wrong-headed?)
>
> If anything, I'd move the call to PgXact->xmin = InvalidTransactionId
> into a function inside procarray.c, so we only touch snapshots in
> snapmgr.c and all procarray stuff is isolated. (Not done here, yet).
I'm not convinced that really buys us anything except more
function-call overhead.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
