Re: Microoptimization of Bitmapset usage in postgres_fdw

> On 17 Jun 2018, at 14:47, Michael Paquier <michael(at)paquier(dot)xyz> wrote:
>
> On Thu, Jun 14, 2018 at 08:14:54PM +0000, Bossart, Nathan wrote:
>> I'll go ahead and mark this as Ready for Committer.
>
> Another thing not mentioned on this thread is that bms_membership is
> faster than bms_num_members by design with many members, so this change
> makes sense to shave a couple of cycles.
>
> /*
> * bms_num_members - count members of set
> + *
> + * In situations where the exact count isn't required, bms_membership can be
> + * used to test if the set has 0, 1 or multiple members.
> */
> bms_membership is a couple of lines down, I am not sure I see much point
> in duplicating what's already present.

It is indeed a bit of a duplication, but the only reason this patch came to be
was that the original author of the instances in postgres_fdw had missed said
comment on bms_membership a few lines down.

> - if (bms_num_members(clauses_attnums) < 2)
> + if (bms_membership(clauses_attnums) != BMS_MULTIPLE)
> For this one, the comment above directly mentions that at least two
> attnums need to be present, so it seems to me that the current coding is
> easier to understand and intentional... So I would be incline to not
> change it.

I don’t have any strong feelings either way, and will leave that call to the
committer who picks this up. I agree that the current coding is easy to
understand but I don’t see this being much harder.

> I think that this should not go into the tree until REL_11_STABLE gets
> created though, so this will have to wait a bit.

100% agree.

cheers ./daniel