Re: Small and unlikely overflow hazard in bms_next_member()

On Fri, 3 Apr 2026 at 11:12, David Rowley <dgrowleyml@gmail.com> wrote:
>
> On Thu, 2 Apr 2026 at 17:22, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> > I don't think we should add cycles here for this purpose.
>
> I'm not keen on slowing things down for this either. I did do some
> experiments in [1] that sees fewer instructions from using 64-bit
> maths. I might go off and see if there are any wins there that also
> give us the INT_MAX fix. It's not great effort to reward ratio
> though...

The reduction in instructions with the patched version got me curious
to see if it would translate into a performance increase.  I tested on
an AMD Zen2 machine, and it's a decent amount faster than master. I
tested with gcc and clang.

I also scanned over the remaining parts of bitmapset.c and didn't find
anywhere else that has overflow risk aside from what you pointed out
in bms_prev_member().

The attached patch contains the benchmark function I added to the
test_bitmapset module. It should apply to master with a bit of noise.

CREATE EXTENSION test_bitmapset;
SELECT
    generate_series(1,3) AS run,
    bench_bms_next_member('(b 1 2 3 4 5 6 7 8 64)', 1000000)/1000 AS
bms_next_member_us,
    bench_bms_prev_member('(b 1 2 3 4 5 6 7 8 64)', 1000000)/1000 AS
bms_prev_member_us;

master (gcc)

 run | bms_next_member_us | bms_prev_member_us
-----+--------------------+--------------------
   1 |              26473 |              40404
   2 |              26218 |              40413
   3 |              26209 |              40387

patched (gcc)

 run | bms_next_member_us | bms_prev_member_us
-----+--------------------+--------------------
   1 |              25409 |              29705
   2 |              24905 |              29693
   3 |              24870 |              29707

Times are in microseconds to do 1 million bms_*_member() loops over
the entire set.

I've also attached the full results I got. I've also included the
results from Chao's version, which does slow things down decently on
clang.

IMO, if we can make bitmapset.c work with INT_MAX members and get a
performance increase, then we should do it.

David

> [1] https://godbolt.org/z/Eh1vzssq7