Re: Use compiler intrinsics for bit ops in hash

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

In response to

Browse pgsql-hackers by date

  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 ?