|From:||Aleksander Alekseev <a(dot)alekseev(at)postgrespro(dot)ru>|
|To:||Robert Haas <robertmhaas(at)gmail(dot)com>|
|Cc:||pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>, Anastasia Lubennikova <a(dot)lubennikova(at)postgrespro(dot)ru>|
|Subject:||Re: Patch: fix lock contention for HASHHDR.mutex|
|Views:||Raw Message | Whole Thread | Download mbox | Resend email|
> 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
> - 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.
|Next Message||Andres Freund||2016-02-11 14:15:55||Re: GinPageIs* don't actually return a boolean|
|Previous Message||Robert Haas||2016-02-11 14:04:54||Re: Speed up Clog Access by increasing CLOG buffers|