Re: Add min and max execute statement time in pg_stat_statement

Peter Geoghegan <pg(at)heroku(dot)com> writes:
> On Wed, Jan 29, 2014 at 8:48 PM, KONDO Mitsumasa
> <kondo(dot)mitsumasa(at)lab(dot)ntt(dot)co(dot)jp> wrote:
>> method | try1 | try2 | try3 | degrade performance ratio
>> -----------------------------------------------------------------
>> method 1 | 6.546 | 6.558 | 6.638 | 0% (reference score)
>> method 2 | 6.527 | 6.556 | 6.574 | 1%
>> peter 1 | 5.204 | 5.203 | 5.216 |20%
>> peter 2 | 4.241 | 4.236 | 4.262 |35%
>>
>> method 5 | 5.931 | 5.848 | 5.872 |11%
>> method 6 | 5.794 | 5.792 | 5.776 |12%

> So, if we compare the old pg_stat_statements and old default with the
> new pg_stat_statements and the new default (why not? The latter still
> uses less shared memory than the former), by the standard of this
> benchmark there is a regression of about 20%. But you also note that
> there is an 11% "regression" in the old pg_stat_statements against
> *itself* when used with a higher pg_stat_statements.max setting of
> 5,000. This is completely contrary to everyone's prior understanding
> that increasing the size of the hash table had a very *positive*
> effect on performance by relieving cache pressure and thereby causing
> less exclusive locking for an entry_dealloc().

> Do you suppose that this very surprising result might just be because
> this isn't a very representative benchmark? Nothing ever has the
> benefit of *not* having to exclusive lock.

If I understand this test scenario properly, there are no duplicate
queries, so that each iteration creates a new hashtable entry (possibly
evicting an old one). And it's a single-threaded test, so that there
can be no benefit from reduced locking.

I don't find the results particularly surprising given that. We knew
going in that the external-texts patch would slow down creation of a new
hashtable entry: there's no way that writing a query text to a file isn't
slower than memcpy'ing it into shared memory. The performance argument
for doing it anyway is that by greatly reducing the size of hashtable
entries, it becomes more feasible to size the hashtable large enough so
that it's seldom necessary to evict hashtable entries. It's always going
to be possible to make the patch look bad by testing cases where no
savings in hashtable evictions is possible; which is exactly what this
test case does. But I don't think that's relevant to real-world usage.
pg_stat_statements isn't going to be useful unless there's a great deal of
repeated statement execution, so what we should be worrying about is the
case where a table entry exists not the case where it doesn't. At the
very least, test cases where there are *no* repeats are not representative
of cases people would be using pg_stat_statements with.

As for the measured slowdown with larger hashtable size (but still smaller
than the number of distinct queries in the test), my money is on the
larger table not fitting in L3 cache or something like that.

regards, tom lane