Heikki Linnakangas <hlinnakangas@vmware.com> wrote:
> On 10/02/2014 03:20 AM, Kevin Grittner wrote:
>> My only concern from the benchmarks is that it seemed like there
>> was a statistically significant increase in planning time:
>>
>> unpatched plan time average: 0.450 ms
>> patched plan time average: 0.536 ms
>>
>> That *might* just be noise, but it seems likely to be real. For
>> the improvement in run time, I'd put up with an extra 86us in
>> planning time per hash join; but if there's any way to shave some
>> of that off, all the better.
>
> The patch doesn't modify the planner at all, so it would be rather
> surprising if it increased planning time. I'm willing to just write that
> off as noise.
Fair enough. I have seen much larger variations caused by how
the executable code happened to line up relative to cacheline
boundaries; and we've never made any effort to manage that.
I've tried various other tests using \timing rather than EXPLAIN,
and the patched version looks even better in those cases. I have
seen up to 4x the performance for a query using the patched
version, higher variability in run time without the patch, and have
yet to devise a benchmark where the patched version came out slower
(although I admit to not being as good at creating such cases as
some here).
When given a generous work_mem setting the patched version often
uses more of what it is allowed than the unpatched version (which
is probably one of the reasons it tends to do better). If someone
has set a high work_mem and has gotten by only because the
configured amount is not all being used when it would benefit
performance, they may need to adjust work_mem down to avoid memory
problems. That doesn't seem to me to be a reason to reject the
patch.
This is in "Waiting on Author" status only because I never got an
answer about why the debug code used printf() rather the elog() at
a DEBUG level. Other than that, I would say this patch is Ready
for Committer. Tomas? You there?
--
Kevin Grittner
EDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
