Re: compute_query_id and pg_stat_statements

Tom Lane <tgl@sss.pgh.pa.us>

From: Tom Lane <tgl@sss.pgh.pa.us>
To: Julien Rouhaud <rjuju123@gmail.com>
Cc: Stephen Frost <sfrost@snowman.net>, Bruce Momjian <bruce@momjian.us>, Christoph Berg <myon@debian.org>, Pavel Stehule <pavel.stehule@gmail.com>, Kyotaro Horiguchi <horikyota.ntt@gmail.com>, Magnus Hagander <magnus@hagander.net>, Fujii Masao <masao.fujii@oss.nttdata.com>, Michael Paquier <michael@paquier.xyz>, Andres Freund <andres@anarazel.de>, Alvaro Herrera <alvherre@alvh.no-ip.org>, Peter Eisentraut <peter.eisentraut@enterprisedb.com>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Date: 2021-05-14T04:26:23Z
Lists: pgsql-hackers

Attachments

I wrote:
> Maybe we should revert this thing pending somebody doing the work to
> make a version of queryid labeling that actually is negligibly cheap.
> It certainly seems like that could be done; one more traversal of the
> parse tree can't be that expensive in itself.  I suspect that the
> performance problem is with the particular hashing mechanism that
> was used, which looks mighty ad-hoc anyway.

To put a little bit of meat on that idea, I experimented with jacking
up the "jumble" calculation and driving some other implementations
underneath.

I thought that Julien's "worst case" scenario was pretty far from
worst case, since it involved a join which a lot of simple queries
don't.  I tested this scenario instead:

$ cat naive.sql
SELECT * FROM pg_class c ORDER BY oid DESC LIMIT 1;
$ pgbench -n -f naive.sql -T 60 postgres

which is still complicated enough that there's work for the
query fingerprinter to do, but not so much for planning and
execution.

I confirm that on HEAD, there's a noticeable TPS penalty from
turning on compute_query_id: about 3.2% on my machine.

The first patch attached replaces the "jumble" calculation
with two CRC32s (two so that we still get 64 bits out at
the end).  I see 2.7% penalty with this version.  Now,
I'm using an Intel machine with
#define USE_SSE42_CRC32C_WITH_RUNTIME_CHECK 1
so on machines without any hardware CRC support, this'd
likely be a loss.  But it still proves the point that the
existing implementation is just not very speedy.

I then tried a really dumb xor'ing implementation, and
that gives me a slowdown of 2.2%.  This could undoubtedly
be improved on further, say by unrolling the loop or
processing multiple bytes at once.  One problem with it
is that I suspect it will tend to concentrate the entropy
into the third/fourth and seventh/eighth bytes of the
accumulator, since so many of the fields being jumbled
are 4-byte or 8-byte fields with most of the entropy in
their low-order bits.  Probably that could be improved
with a bit more thought -- say, an extra bump of the
nextbyte pointer after each field.

Anyway, I think that what we have here is quite an inferior
implementation, and we can do better with some more thought.

			regards, tom lane

Commits

  1. Allow compute_query_id to be set to 'auto' and make it default

  2. Move pg_stat_statements query jumbling to core.