Re: Making empty Bitmapsets always be NULL

On Thu, 22 Jun 2023 at 00:16, Ranier Vilela <ranier.vf@gmail.com> wrote:
> 2. Only compute BITNUM when necessary.

I doubt this will help.  The % 64 done by BITNUM will be transformed
to an AND operation by the compiler which is likely going to be single
instruction latency on most CPUs which probably amounts to it being
"free".  There's maybe a bit of reading for you in [1] and [2] if
you're wondering how any operation could be free.

(The compiler is able to transform the % into what is effectively &
because 64 is a power of 2.  uintvar % 64 is the same as uintvar & 63.
Play around with [3] to see what I mean)

> 3. Avoid enlargement when nwords is equal wordnum.
>     Can save cycles when in corner cases?

No, you're just introducing a bug here.  Arrays in C are zero-based,
so "wordnum >=  a->nwords" is exactly the correct way to check if
wordnum falls outside the bounds of the existing allocated memory. By
changing that to "wordnum > a->nwords" we'll fail to enlarge the words
array when it needs to be enlarged by 1 element.

It looks like you've introduced a bunch of random white space and
changed around a load of other random things in the patch too. I'm not
sure why you think that's a good idea.

FWIW, we normally only write "if (somevar)" as a shortcut when somevar
is boolean and we want to know that it's true.   The word value is not
a boolean type, so although "if (someint)" and "if (someint != 0)"
will compile to the same machine code, we don't normally write our C
code that way in PostgreSQL.  We also tend to write "if (someptr !=
NULL)" rather than "if (someptr)". The compiler will produce the same
code for each, but we write the former to assist people reading the
code so they know we're checking for NULL rather than checking if some
boolean variable is true.

Overall, I'm not really interested in sneaking any additional changes
that are unrelated to adjusting Bitmapsets so that don't carry
trailing zero words. If have other optimisations you think are
worthwhile, please include them in another thread along with
benchmarks to show the performance increase.  For learning, I'd
encourage you to do some micro benchmarks outside of PostgreSQL and
mock up some Bitmapset code in a single .c file and try out with any
without your changes after calling the function in a tight loop to see
if you can measure any performance gains. Just remember you'll never
see any gains in performance when your change compiles into the exact
same code as without your change.  Between [1] and [2], you still
might not see performance changes even when the compiled code is
changed (I'm thinking of your #2 change here).