On Thu, Feb 11, 2016 at 9:11 AM, Aleksander Alekseev <a.aleks...@postgrespro.ru> wrote: >> 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. > > Here is a corrected patch. I decided to keep a small change in > InitLocks procedure (lock.c) since ShmemInitHash description clearly > states "For a table whose maximum size is certain, [init_size] should > be equal to max_size". Also I added two items to my TODO list to send > more patches as soon (and if) this patch will be applied. > > Thanks for your contribution to this patch.
The fact that InitLocks() doesn't do this has been discussed before and there's no consensus on changing it. It is, at any rate, a separate issue. I'll go through the rest of this patch again now. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (email@example.com) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers