>>>>> "Andres" == Andres Freund <andres@anarazel.de> writes:
Andres> Changes to advance_aggregates() are, in my experience, quiteAndres> likely to have performance effects. This
needssomeAndres> performance tests.[...]Andres> Looks like it could all be noise, but it seems worthwhile toAndres>
lookinto 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: 5949msgsets_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)
