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