On Fri, Nov 28, 2014 at 8:20 AM, Tom Lane <t...@sss.pgh.pa.us> wrote:
>
> The attached proposed patch adds bms_next_member() and replaces
> bms_first_member() calls where it seemed to make sense. I've had a
> hard time measuring much speed difference for this patch in isolation,
> but in principle it should be less code and less cycles. It also seems
> safer and more natural to not use destructive looping techniques.
>
>
I've had a quick read of the patch and it seems like a good idea.
I have to say I don't really like the modifying of the loop iterator that's
going on here:
col = -1;
while ((col = bms_next_member(rte->modifiedCols, col)) >= 0)
{
col += FirstLowInvalidHeapAttributeNumber;
/* do stuff */
col -= FirstLowInvalidHeapAttributeNumber;
}
Some other code is doing this:
col = -1;
while ((col = bms_next_member(cols, col)) >= 0)
{
/* bit numbers are offset by FirstLowInvalidHeapAttributeNumber */
AttrNumber attno = col + FirstLowInvalidHeapAttributeNumber;
Which seems less prone to future breakage and possibly slightly less cycles.
A while back when I was benchmarking the planner time during my trials with
anti/semi join removals, I wrote a patch to change the usage pattern for
cases
such as:
if (bms_membership(a) != BMS_SINGLETON)
return; /* nothing to do */
singleton = bms_singleton_member(a);
...
Into:
if (!bms_get_singleton(a, &singleton))
return; /* nothing to do */
...
Which means 1 function call and loop over the bitmapset, rather than 2
function
calls and 2 loops over the set when the set is a singleton.
This knocked between 4 and 22% off of the time the planner spent in the join
removals path.
The patch to implement this and change all suitable calls sites is attached.
Regards
David Rowley
bms_get_singleton_v1.patch
Description: Binary data
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
