Tom Lane wrote:
> "Heikki Linnakangas" <[EMAIL PROTECTED]> writes:
>> If you look at the callgraph, you'll see that those
>> LWLockAcquire/Release calls are coming from HeapTupleSatisfiesVacuum ->
>> TransactionIdIsInProgress, which keeps trashing the ProcArrayLock. A
>> "if(TransactionIdIsCurrentTransactionId(xid)) return true;" check in
>> TransactionIdIsInProgress would speed that up, but I wonder if there's a
>> more general solution to make HeapTupleSatisfiesVacuum cheaper. For
>> example, we could cache the in-progress status of tuples.
> 
> Dunno about "more general", but your idea reduces the runtime of this
> example by about 50% (22.2s to 10.5s) for me.  I'm worried though that
> it would be a net negative in more typical situations, especially if
> you've got a lot of open subtransactions.

Yeah. I played with this a bit more, and came up with a couple of other
micro-optimizations:

1. Instead of pallocing and pfreeing a new array in
TransactionIdIsInProgress, we could just malloc the array once and reuse
it. That palloc/pfree alone was consuming around 8% of CPU time in the
test case.

2. About ~30% of CPU time is spent in HeapTupleSatisfiesMVCC ->
TransactionIdDidAbort. This is in fact not related to HOT, but it caught
my eye while looking at the profile. We have this piece of code in
HeapTupleSatisfiesMVCC:

>       /* deleting subtransaction aborted? */
>       /* FIXME -- is this correct w.r.t. the cmax of the tuple? */
>       if (TransactionIdDidAbort(HeapTupleHeaderGetXmax(tuple)))
>       {
>           SetHintBits(tuple, buffer, HEAP_XMAX_INVALID,
>                       InvalidTransactionId);
>           return true;
>       }
> 
>       
> Assert(TransactionIdIsCurrentTransactionId(HeapTupleHeaderGetXmax(tuple)));

We've already checked that the xmin is our own transaction id, so we
check if the xmax is an aborted subtransaction of our own transaction. A
TransactionIdDidAbort call seems like an awfully expensive way to check
that. We could call TransactionIdIsCurrentTransactionId instead, which
doesn't need to access any shared memory structures (but might be
expensive if you have a lot of active subxacts, as you pointed out). Or
we could keep a list of aborted subtransactions in backend private
memory, and just check against that. In this particular case, a simple
"if(xmin==xmax)" check would be effective as well, since we know that
the xmin is not aborted at that point.

3. One more general alternative to the little patch I sent earlier would
be to build an array of in-progress-xids in TransactionIdInProgress,
like we do in GetSnapshotData, and use that for the in-progress checks
in HeapTupleSatisfiesVacuum. That should work, if we build the cached
array when we lock the page for pruning, and use it until we unlock. If
a transaction commits/abort after we build the cached array, we might
return DELETE/INSERT_IN_PROGRESS for a tuple that is in fact already
DEAD, but that's ok. Since we're holding a lock on the page, it's not
possible for new transaction to start and subsequently put it's xid on
the page where we would bump into it. I believe this would have roughly
the same performance effect in this test case as calling
TransactionIdIsCurrentTransactionId from TransactionIdIsInProgress.

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

---------------------------(end of broadcast)---------------------------
TIP 6: explain analyze is your friend

Reply via email to