Re: Hash support for grouping sets
Andrew Gierth <andrew@tao11.riddles.org.uk>
From: Andrew Gierth <andrew@tao11.riddles.org.uk>
To: Andres Freund <andres@anarazel.de>
Cc: Mark Dilger <hornschnorter@gmail.com>, pgsql-hackers@postgresql.org
Date: 2017-03-23T03:43:57Z
Lists: pgsql-hackers
>>>>> "Andres" == Andres Freund <andres@anarazel.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)
Commits
-
Support hashed aggregation with grouping sets.
- b5635948ab16 10.0 landed