Re: move some bitmapset.c macros to bitmapset.h

On Mon, Dec 5, 2022 at 9:33 PM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>
> Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org> writes:
> > On 2022-Dec-05, John Naylor wrote:
> >> -#define WORDNUM(x) ((x) / BITS_PER_BITMAPWORD)
> >> -#define BITNUM(x) ((x) % BITS_PER_BITMAPWORD)
>
> > In this location, nobody can complain about the naming of these macros,
> > since they're just used to implement other bitmapset.c code. However,
> > if you move them to the .h file, ISTM you should give them more
> > meaningful names.
>
> IMV these are absolutely private to bitmapset.c. I reject the idea
> that they should be exposed publicly, under these names or any others.

Well, they've already escaped to tidbitmap.c as a copy. How do you feel
about going that route?

> Maybe we need some more bitmapset primitive functions? What is it
> you actually want to accomplish in the end?

An inserter into one type of node in a tree structure must quickly find a
free position in an array. We have a bitmap of 128 bits to indicate whether
the corresponding array position is free. The proposed coding is:

/* get the first word with at least one bit not set */
for (idx = 0; idx < WORDNUM(128); idx++)
{
if (isset[idx] < ~((bitmapword) 0))
break;
}

/* To get the first unset bit in X, get the first set bit in ~X */
inverse = ~(isset[idx]);
slotpos = idx * BITS_PER_BITMAPWORD;
slotpos += bmw_rightmost_one_pos(inverse);

/* mark the slot used */
isset[idx] |= RIGHTMOST_ONE(inverse);

return slotpos;

...which, if it were reversed so that a set bit meant "available", would
essentially be bms_first_member(), so a more primitive version of that
might work.

--
John Naylor
EDB: http://www.enterprisedb.com