Re: Microoptimization of Bitmapset usage in postgres_fdw

From: Daniel Gustafsson <daniel(at)yesql(dot)se>
To: Nathan Bossart <bossartn(at)amazon(dot)com>
Cc: pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Re: Microoptimization of Bitmapset usage in postgres_fdw
Date: 2018-06-14 19:32:44
Message-ID: C9E798C6-B288-4789-B482-CEA901FA47C2@yesql.se
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

> On 14 Jun 2018, at 16:56, Nathan Bossart <bossartn(at)amazon(dot)com> wrote:

> The v2 patches look good to me. However, I found a couple other
> places where we might be able to use this micro-optimization.

Thanks a lot for your review!

> 1) dependencies_clauselist_selectivity() in dependencies.c
>
> /*
> * If there's not at least two distinct attnums then reject the whole list
> * of clauses. We must return 1.0 so the calling function's selectivity is
> * unaffected.
> */
> if (bms_num_members(clauses_attnums) < 2)
> {
> pfree(list_attnums);
> return 1.0;
> }

I agree with this one, not sure why I missed that when grep’ing around while
writing the patch. Fixed in the attached v3 patchset (which are now awkwardly
named but I didn’t change that).

> 2) BuildRelationExtStatistics() in extended_stats.c.
>
> /* check allowed number of dimensions */
> Assert(bms_num_members(stat->columns) >= 2 &&
> bms_num_members(stat->columns) <= STATS_MAX_DIMENSIONS);

Since this usage is in an assertion I don’t see the value in changing it as the
current programming is more optimized for readability.

cheers ./daniel

Attachment Content-Type Size
bms_num_members_comment-v3.patch application/octet-stream 831 bytes
postgres_fdw_bms_multiple-v3.patch application/octet-stream 2.3 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Alvaro Herrera 2018-06-14 19:51:23 Re: Portability concerns over pq_sendbyte?
Previous Message Andrew Dunstan 2018-06-14 19:24:31 Re: Few cosmetic suggestions for commit 16828d5c (Fast Alter Table Add Column...)