Re: Optmize bitmapword macros calc (src/backend/nodes/bitmapset.c)

Em seg., 29 de jan. de 2024 às 16:32, Nathan Bossart <
nathandbossart(at)gmail(dot)com> escreveu:

> 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...
>
Bitmapset is extensively used.

> -#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.

bitmapword It is the most correct and prudent option, if in the future,
we decide to change the number of nwords to uint64.

> I also wonder if it's worth asserting that x is >= 0 before
> casting here.
>
I don't think this would change anything.

best regards,
Ranier Vilela