On Thu, Nov 28, 2013 at 1:46 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> While debugging the recent fastpath lock unpleasantness, I noticed that
> the code tends to use only the last few entries in the fpRelId[] arrays,
> which seemed a bit surprising. The reason of course is the way that
> FastPathGrantRelationLock() is written: it remembers the *last* unused
> slot while scanning the array. This ends up wasting cycles in
> FastPathUnGrantRelationLock(), as well as other places where we search
> for an existing entry, since they'll generally have to iterate to the end
> of the array to find it. We should prefer to put entries near the front
> of the array, not the back. (Of course, if the array is about full then
> it's going to be a wash, but in simple transactions we might only have a
> few relations with fast-path locks.)
>
> We could add an extra test in FastPathGrantRelationLock's loop to make
> it remember the first unused slot rather than the last one, but that
> would add some cycles there, partially negating any benefit. Instead
> I propose that we reverse the direction of the search loop, as attached.
>
> I doubt this will actually make any easily-measurable difference, so I've
> not attempted to benchmark it, but in principle we should be able to save
> a few loop iterations in places where we're holding locks. Comments?
Well, the reason why the array is only 64 bytes in size is to make
sure that searching the whole thing is really fast. We figure we're
going to have to do that often, so it needs to be cheap. If it's not,
we're hosed already, I think.
Moreover, the whole point of the backendLock is that holding your own
costs very little, because other backends should acquire it only
rarely. Shaving cycles off code that runs while holding the partition
locks is doubtless valuable; shaving them here is much less so.
But I don't think it'll hurt anything. If you can prove that it
actually helps something, I'll buy you a glass of wine. :-)
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
