Re: Division in dynahash.c due to HASH_FFACTOR

From: Thomas Munro <thomas(dot)munro(at)gmail(dot)com>
To: Jakub Wartak <Jakub(dot)Wartak(at)tomtom(dot)com>
Cc: Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Division in dynahash.c due to HASH_FFACTOR
Date: 2020-09-10 02:54:17
Message-ID: CA+hUKGKHm8Ejh26W8AUhNvfh3sH9mZ5yuV9Eg15b4HtX6fcrgg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Tue, Sep 8, 2020 at 11:17 PM Jakub Wartak <Jakub(dot)Wartak(at)tomtom(dot)com> wrote:
> I agree with both, I just thought it might be interesting finding as this idiv might be (?) present in other common paths like ReadBuffer*() / PinBuffer() (some recent discussions, maybe on NUMA boxes), not just WAL recovery as it seems relatively easy to improve.

I wrote a draft commit message for Jakub's proposed change (attached),
and did a little bit of testing, but I haven't seen a case where it
wins yet; I need to work on something else now but thought I'd share
this much anyway. One observation is that the rule in init_htab had
better agree with the expansion rule in hash_search_with_hash_value,
otherwise you can size your hash table perfectly for n elements and
then it still has to expand when you insert the nth element, which is
why I changed >= to >. Does this make sense? Oh man, I don't like
the mash-up of int, long, uint32, Size dynahash uses in various places
and that are brought into relief by that comparison...

Attachment Content-Type Size
0001-Remove-custom-fill-factor-support-from-dynahash.c.patch text/x-patch 4.7 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message tsunakawa.takay@fujitsu.com 2020-09-10 02:56:37 RE: SIGQUIT handling, redux
Previous Message osumi.takamichi@fujitsu.com 2020-09-10 02:34:25 RE: extension patch of CREATE OR REPLACE TRIGGER