Re: Don't use bms_membership in places where it's not needed

Hi,

On 2023-11-24 17:06:25 +1300, David Rowley wrote:
> While working on the patch in [1], I noticed that ever since
> 00b41463c, it's now suboptimal to do the following:
>
> switch (bms_membership(relids))
> {
> case BMS_EMPTY_SET:
> /* handle empty set */
> break;
> case BMS_SINGLETON:
> /* call bms_singleton_member() and handle singleton set */
> break;
> case BMS_MULTIPLE:
> /* handle multi-member set */
> break;
> }
>
> The following is cheaper as we don't need to call bms_membership() and
> bms_singleton_member() for singleton sets. It also saves function call
> overhead for empty sets.
>
> if (relids == NULL)
> /* handle empty set */
> else
> {
> int relid;
>
> if (bms_get_singleton(relids, &relid))
> /* handle singleton set */
> else
> /* handle multi-member set */
> }

Hm, does this ever matter from a performance POV? The current code does look
simpler to read to me. If the overhead is relevant, I'd instead just move the
code into a static inline?

Greetings,

Andres Freund