On Mon, Dec 5, 2022 at 9:33 PM Tom Lane <
tgl@sss.pgh.pa.us> wrote:
>
> Alvaro Herrera <
alvherre@alvh.no-ip.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.
