Re: Hash support for grouping sets

>>>>> "Andres" == Andres Freund <andres(at)anarazel(dot)de> writes:

Andres> Changes to advance_aggregates() are, in my experience, quite
Andres> likely to have performance effects. This needs some
Andres> performance tests.
[...]
Andres> Looks like it could all be noise, but it seems worthwhile to
Andres> look into some.

Trying to sort out signal from noise when dealing with performance
impacts of no more than a few percent is _extremely hard_ these days.

Remember this, from a couple of years back? http://tinyurl.com/op9qg8a

That's a graph of performance results from tests where the only change
being made was adding padding bytes to a function that was never called
during the test. Performance differences on the order of 3% were being
introduced by doing nothing more than changing the actual memory
locations of functions.

My latest round of tests on this patch shows a similar effect. Here's
one representative test timing (a select count(*) with no grouping sets
involved):

master: 5727ms
gsets_hash: 5949ms
gsets_hash + 500 padding bytes: 5680ms

Since the effect of padding is going to vary over time as other,
unrelated, patches happen, I think I can safely claim that the
performance of the patch at least overlaps the noise range of the
performance of current code. To get a more definitive result, it would
be necessary to run at least some dozens of tests, with different
padding sizes, and determine whether the average changes detectably
between the two versions. I will go ahead and do this, out of sheer
curiosity if nothing else, but the preliminary results suggest there's
probably nothing worth holding up the patch for.

--
Andrew (irc:RhodiumToad)