On Thu, Jun 21, 2012 at 5:32 AM, Heikki Linnakangas
<heikki.linnakangas@enterprisedb.com> wrote:
> On 18.06.2012 13:59, Heikki Linnakangas wrote:
>>
>> On 10.06.2012 23:39, Jeff Janes wrote:
>> I found the interface between resowner.c and lock.c a bit confusing.
>> resowner.c would sometimes call LockReassignCurrentOwner() to reassign
>> all the locks, and sometimes it would call LockReassignCurrentOwner() on
>> each individual lock, with the net effect that's the same as calling
>> LockReleaseCurrentOwner(). And it doesn't seem right for
>> ResourceOwnerRemember/ForgetLock to have to accept a NULL owner.
>>
>> I rearranged that so that there's just a single
>> LockReassignCurrentOwner() function, like before this patch. But it
>> takes as an optional argument a list of locks held by the current
>> resource owner. If the caller knows it, it can pass them to make the
>> call faster, but if it doesn't it can just pass NULL and the function
>> will traverse the hash table the old-fashioned way. I think that's a
>> better API.
Thank you, that does look much cleaner.
>>
>> Please take a look to see if I broke something.
>
>
> I hear no complaints, so committed. Thanks!
>
Thanks.
Just for the record, I'd previously promised some long running
performance tests with my proposed -P option for pgbench, which are
now done and showed a 0.2% degradation with my patch. With enough
data collected, that difference is statistically significant, but
probably not practically significant. It was with my original
version, but I can't imagine your version being different in
performance. Also, this test is very pessimistic. Since the primary
key look-up in the pl/pgSQL look up runs in a portal each time, it
pushes the locks to the parent each time. If the lookup was instead
running as the inner side of a nested loop, it would not do the
reassign on each loop.
Cheers,
Jeff
