From: | David Fetter <david(at)fetter(dot)org> |
---|---|
To: | John Naylor <john(dot)naylor(at)2ndquadrant(dot)com> |
Cc: | Jesse Zhang <sbjesse(at)gmail(dot)com>, PostgreSQL Development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Use compiler intrinsics for bit ops in hash |
Date: | 2020-01-19 00:00:52 |
Message-ID: | 20200119000051.GN32763@fetter.org |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Sat, Jan 18, 2020 at 11:46:24AM +0800, John Naylor wrote:
> On Wed, Jan 15, 2020 at 6:09 AM David Fetter <david(at)fetter(dot)org> wrote:
> > [v2 patch]
>
> Hi David,
>
> I have a stylistic comment on this snippet:
>
> - for (i = _hash_log2(metap->hashm_bsize); i > 0; --i)
> - {
> - if ((1 << i) <= metap->hashm_bsize)
> - break;
> - }
> + i = pg_leftmost_one_pos32(metap->hashm_bsize);
> Assert(i > 0);
> metap->hashm_bmsize = 1 << i;
> metap->hashm_bmshift = i + BYTE_TO_BIT;
>
> Naming the variable "i" made sense when it was a loop counter, but it
> seems out of place now. Same with the Assert.
Fixed by removing the variable entirely.
> Also, this
>
> + * using BSR where available */
>
> is not directly tied to anything in this function, or even in the
> function it calls, and could get out of date easily.
Removed.
Best,
David.
--
David Fetter <david(at)fetter(dot)org> http://fetter.org/
Phone: +1 415 235 3778
Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate
Attachment | Content-Type | Size |
---|---|---|
v3-0001-Use-compiler-intrinsics-for-bit-ops-in-hash.patch | text/x-diff | 8.4 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Bruce Momjian | 2020-01-19 01:47:59 | Re: our checks for read-only queries are not great |
Previous Message | Andres Freund | 2020-01-18 23:32:02 | Re: should crash recovery ignore checkpoint_flush_after ? |