Re: Marginal performance improvement: replace bms_first_member loops

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: David Rowley <dgrowleyml(at)gmail(dot)com>
Cc: PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Marginal performance improvement: replace bms_first_member loops
Date: 2014-11-28 15:11:40
Message-ID: 9142.1417187500@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

David Rowley <dgrowleyml(at)gmail(dot)com> writes:
> 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.

Yeah, I'd come to the same conclusion while thinking about it in the
shower this morning ...

> 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:
> ...
> This knocked between 4 and 22% off of the time the planner spent in the join
> removals path.

Really!? I've never seen either of those functions show up all that high
in profiles. Can you share the test case you were measuring?

regards, tom lane

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Peter Eisentraut 2014-11-28 16:18:46 Re: no test programs in contrib
Previous Message Tom Lane 2014-11-28 15:08:36 Re: Marginal performance improvement: replace bms_first_member loops