Re: Storing hot members of PGPROC out of the band

On 04.11.2011 00:43, Simon Riggs wrote:
> On Thu, Nov 3, 2011 at 6:12 PM, Pavan Deolasee<pavan(dot)deolasee(at)gmail(dot)com> wrote:
>
>> When PGPROC array is allocated, we also allocate another array of
>> PGPROC_MINIMAL structures of the same size. While accessing the
>> ProcArray, a simple pointer mathematic can get us the corresponding
>> PGPROC_MINIMAL structure. The only exception being the dummy PGPROC
>> for prepared transaction. A first cut version of the patch is
>> attached. It looks big, but most of the changes are cosmetic because
>> of added indirection. The patch also contains another change to keep
>> the ProcArray sorted by (PGPROC *) to preserve locality of references
>> while traversing the array.
>
> This is very good.
>
> If you look at your PGPROC_MINIMAL, its mostly transaction related
> stuff, so I would rename it PGXACT or similar. Not sure why you talk
> about pointer math, seems easy enough just to have two arrays
> protected by one lock, and have each proc use the same offset in both
> arrays.

Agreed, that seems more clean. The PGPROCs for prepared transactions are
currently allocated separately, so they're not currently in the same
array as all other PGPROCs, but that could be changed. Here's a WIP
patch for that. I kept the PGPROC_MINIMAL nomenclature for now, but I
agree with Simon's suggestion to rename it.

On 03.11.2011 20:12, Pavan Deolasee wrote:
> Its quite possible that the effect of the patch is more evident on the
> particular hardware that I am testing. But the approach nevertheless
> seems reasonable. It will very useful if someone else having access to
> a large box can test the effect of the patch.

I tested this on an 8-core x64 box, but couldn't see any measurable
difference in pgbench performance. I tried with and without -N and -S,
and --unlogged-tables, but no difference.

While looking at GetSnapshotData(), it occurred to me that when a PGPROC
entry does not have its xid set, ie. xid == InvalidTransactionId, it's
pointless to check the subxid array for that proc. If a transaction has
no xid, none of its subtransactions can have an xid either. That's a
trivial optimization, see attached. I tested this with "pgbench -S -M
prepared -c 500" on the 8-core box, and it seemed to make a 1-2%
difference (without the other patch). So, almost in the noise, but it
also doesn't cost us anything in terms of readability or otherwise.

--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com