Re: Should we cacheline align PGXACT?

From: Robert Haas <robertmhaas(at)gmail(dot)com>
To: Simon Riggs <simon(at)2ndquadrant(dot)com>
Cc: Andres Freund <andres(at)anarazel(dot)de>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, Alexander Korotkov <a(dot)korotkov(at)postgrespro(dot)ru>, Bernd Helmle <mailings(at)oopsware(dot)de>, Tomas Vondra <tomas(dot)vondra(at)2ndquadrant(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Should we cacheline align PGXACT?
Date: 2017-02-20 17:32:26
Message-ID: CA+TgmoZt9V4f+-5aTe37Wohv97-X_p68CV3+T1P2ePdSa67OAw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Mon, Feb 20, 2017 at 10:49 PM, Simon Riggs <simon(at)2ndquadrant(dot)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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Robert Haas 2017-02-20 17:48:34 Re: Parallel bitmap heap scan
Previous Message David Christensen 2017-02-20 17:22:48 Re: [PATCH] Add pg_disable_checksums() and supporting infrastructure