On Mon, 17 Jun 2019 at 15:05, Tsunakawa, Takayuki
<tsunakawa.ta...@jp.fujitsu.com> wrote:
> (1)
> +#define LOCKMETHODLOCALHASH_SHRINK_SIZE 64
>
> How about LOCKMETHODLOCALHASH_SHRINK_THRESHOLD, because this determines the 
> threshold value to trigger shrinkage?  Code in PostgreSQL seems to use the 
> term threshold.

That's probably better. I've renamed it to that.

> (2)
> +/* Complain if the above are not set to something sane */
> +#if LOCKMETHODLOCALHASH_SHRINK_SIZE < LOCKMETHODLOCALHASH_INIT_SIZE
> +#error "invalid LOCKMETHODLOCALHASH_SHRINK_SIZE"
> +#endif
>
> I don't think these are necessary, because these are fixed and not 
> configurable.  FYI, src/include/utils/memutils.h doesn't have #error to test 
> these macros.

Yeah. I was thinking it was overkill when I wrote it, but somehow
couldn't bring myself to remove it. Done now.

> (3)
> +       if (hash_get_num_entries(LockMethodLocalHash) == 0 &&
> +               hash_get_max_bucket(LockMethodLocalHash) >
> +               LOCKMETHODLOCALHASH_SHRINK_SIZE)
> +               CreateLocalLockHash();
>
> I get an impression that Create just creates something where there's nothing. 
>  How about Init or Recreate?

Renamed to InitLocalLoclHash()

v5 is attached.

Thank you for the review.

-- 
 David Rowley                   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services

Attachment: shrink_bloated_locallocktable_v5.patch
Description: Binary data

Reply via email to