From: | Nathan Bossart <nathandbossart(at)gmail(dot)com> |
---|---|
To: | Ranier Vilela <ranier(dot)vf(at)gmail(dot)com> |
Cc: | Pg Hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Optmize bitmapword macros calc (src/backend/nodes/bitmapset.c) |
Date: | 2024-01-29 19:32:03 |
Message-ID: | 20240129193203.GA3671663@nathanxps13 |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Mon, Jan 29, 2024 at 01:30:47PM -0300, Ranier Vilela wrote:
> IMO I believe that bitmapset can obtain an optimization in the calculation
> of the WORDNUM and BITNUM macros.
>
> As you know, in bitmapset, negative members are not allowed.
>
> if (x < 0)
> elog(ERROR, "negative bitmapset member not allowed");
>
> Then, allow the compiler to optimize and do the calculations in unsigned.
I'm currently +0.1 for this change. I don't see any huge problem with
trimming a few instructions, but I'm dubious there's any measurable impact.
However, a cycle saved is a cycle earned...
-#define WORDNUM(x) ((x) / BITS_PER_BITMAPWORD)
-#define BITNUM(x) ((x) % BITS_PER_BITMAPWORD)
+#define WORDNUM(x) ((bitmapword)(x) / BITS_PER_BITMAPWORD)
+#define BITNUM(x) ((bitmapword)(x) % BITS_PER_BITMAPWORD)
I'm curious why we'd cast to bitmapword and not straight to uint32. I
don't think the intent is that callers will provide a bitmapword to these
macros. I also wonder if it's worth asserting that x is >= 0 before
casting here.
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
From | Date | Subject | |
---|---|---|---|
Next Message | Ranier Vilela | 2024-01-29 19:43:32 | Re: Optmize bitmapword macros calc (src/backend/nodes/bitmapset.c) |
Previous Message | Euler Taveira | 2024-01-29 19:30:59 | Re: Should we remove -Wdeclaration-after-statement? |