Re: Patch: fix lock contention for HASHHDR.mutex - Mailing list pgsql-hackers

From Robert Haas
Subject Re: Patch: fix lock contention for HASHHDR.mutex
Date
Msg-id CA+TgmoZm=Uowt8a_XaSfooGwufeeLJ861NTADiCEOpyFehV8Wg@mail.gmail.com
Whole thread Raw
In response to Re: Patch: fix lock contention for HASHHDR.mutex  (Aleksander Alekseev <a.alekseev@postgrespro.ru>)
Responses Re: Patch: fix lock contention for HASHHDR.mutex
List pgsql-hackers
On Wed, Feb 10, 2016 at 3:24 AM, Aleksander Alekseev
<a.alekseev@postgrespro.ru> wrote:
>> Basically, the burden for you to impose a new coding rule on everybody
>> who uses shared hash tables in the future is very high.
>
> I fixed an issue you described. Number of spinlocks doesn't depend of
> NUM_LOCK_PARTITIONS anymore and could be specified for each hash table
> on a calling side.

Thanks, this looks way better.  Some more comments:

- I don't think there's any good reason for this patch to change the
calling convention for ShmemInitHash().  Maybe that's a good idea and
maybe it isn't, but it's a separate issue from what this patch is
doing, and if we're going to do it at all, it should be discussed
separately.  Let's leave it out of this patch.

- I would not provide an option to change the number of freelist
mutexes.  Let's dump DEFAULT_MUTEXES_NUM and MAX_MUTEXES_NUM and have
FREELIST_MUTEXES_NUM.  The value of 32 which you selected is fine with
me.

- The change to LOG2_NUM_LOCK_PARTITIONS in lwlock.h looks like an
independent change.  Again, leave it out of this patch and submit it
separately with its own benchmarking data if you want to argue for it.

I think if you make these changes this patch will be quite a bit
smaller and in pretty good shape.

Thanks for working on this.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company



pgsql-hackers by date:

Previous
From: David Steele
Date:
Subject: Re: Updated backup APIs for non-exclusive backups
Next
From: Tom Lane
Date:
Subject: Re: [COMMITTERS] pgsql: Code cleanup in the wake of recent LWLock refactoring.