Low hanging fruit in lazy-XID-assignment patch?

Simon was complaining a bit ago that we still have problems with
excessive contention for the ProcArrayLock, and that much of this stems
from the need for transaction exit to take that lock exclusively.
The lazy-XID patch, as committed, doesn't help that situation at all,
saying

/*
* Lock ProcArrayLock because that's what GetSnapshotData uses.
* You might assume that we can skip this step if we had no
* transaction id assigned, because the failure case outlined
* in GetSnapshotData cannot happen in that case. This is true,
* but we *still* need the lock guarantee that two concurrent
* computations of the *oldest* xmin will get the same result.
*/

On reflection though this seems wrong: I believe that we could skip
taking the lock when exiting a transaction with no XID. The actions
being guarded with the lock are

MyProc->xid = InvalidTransactionId;
MyProc->lxid = InvalidLocalTransactionId;
MyProc->xmin = InvalidTransactionId;
MyProc->inVacuum = false; /* must be cleared with xid/xmin */

/* Clear the subtransaction-XID cache too while holding the lock */
MyProc->subxids.nxids = 0;
MyProc->subxids.overflowed = false;

Clearing xid is obviously a no-op if we had no xid, and if we had no xid
we have no subxids either, so the last 2 lines are also no-ops. I
cannot see any reason why we need a guard on clearing lxid, either.
inVacuum is only interesting if xmin is, since if there's no xid
assigned then it's effectively just a filter on whether other backends
pay attention to this one's xmin. That leaves xmin, which AFAICS is
only interesting for the computations of GetOldestXmin() and
RecentGlobalXmin. And I assert it doesn't matter if those numbers
advance asynchronously, so long as they never go backward.

Comments?

regards, tom lane