Re: WIP patch for latestCompletedXid method of computing snapshot xmax

"Florian G. Pflug" <fgp(at)phlo(dot)org> writes:
> 1) I wonder why you made XidCacheRemoveRunningXids take the latestXid as
> an extra argument, though - it should be able to figure that out on
> it's own I'd have thought. Is that just for consistency, or did I miss
> something?

Well, we were going to have to figure the latestXid in xact.c anyway in
the main-xact path, so I thought it was better to have it responsible
in both cases. It's a judgment call though.

> 2) I don't see how either the childXids array or the XID cache in the
> proc array could ever not be in ascending xid order. We do guarantee
> that a child xid is always greater than it's parent.

It's the relationships among siblings that worry me. It's likely that
we could guarantee this with a bit of care and some Asserts in xact.c's
xid-list manipulations, but it's not quite obvious that it must be so
--- for instance using an lcons instead of lappend might break it.

> This guarantee
> enables a few optimizations. First, as you say in the comments,
> finding the largest xid when committing becomes trivial. But more
> important, if we can assume that the proc array xid cache is always
> sorted, we can get ride of the exclusive lock during subxact abort.

> We will *always* remove the last n xids from the cache, so we just
> need to reset subxids.nxids, and not touch the actual array at all.
> (We might later set the now-unused entries to InvalidTransactionId,
> but thats only cosmetic).

I'm not convinced that that works when the subxids cache has overflowed
earlier in the transaction.

In any case, the bottom line is that it's very unlikely that that code
costs enough to be worth optimizing. When you think about the total
number of cycles that have been expended on each subtransaction
(especially one that has an xid), another comparison or two is lost in
the noise. As long as the code isn't O(N^2) ... which it isn't AFAICS
... I would rather keep it simple and robust.

regards, tom lane