On 08/31/2016 06:51 PM, Andres Freund wrote:
On 2016-08-31 17:47:18 +0300, Heikki Linnakangas wrote:
We actually used to call ExecEvalExpr() directly for each argument, but that
was changed by the patch that added support for ordered set aggregates. It
looks like that was a bad idea, from a performance point of view.

I complained about that as well
http://archives.postgresql.org/message-id/20160519175727.ymv2y5tye4qgcmqx%40alap3.anarazel.de

Ah, missed that!

I propose that we go back to calling ExecEvalExpr() directly, for
non-ordered aggregates, per the attached patch. That makes that example
query about 10% faster on my laptop, which is in line with the fact that
ExecProject() accounted for about 13% of the CPU time.

My approach is a bit different.

I've first combined the projection for all the aggregates, ordered set,
or not, into one projetion. That got rid of a fair amount of overhead
when you have multiple aggregates.  I attached an, probably out of date,
WIP version of that patch.

A-ha, I also considered doing just that! I also considered a variant where we call ExecProject once for all non-ordered aggregates, and a separate ExecProject() for each ordered one. But just switching back to straight ExecEvalExprs seemed easier.

Secondly, I'm working on overhauling expression evaluation to be
faster. Even without the ExecProject overhead, the computations very
quickly become the bottleneck. During that I pretty much merged
ExecProject and ExecEvalExpr into one - they're really not that
different, and the distinction serves no purpose, except to increase the
number of function calls. The reason I'm working on getting rid of
targetlist SRFs is precisely that. A proof of concept of that is
attached to
http://archives.postgresql.org/message-id/20160714011850.bd5zhu35szle3n3c%40alap3.anarazel.de

Cool, yes, all that should help.

- Heikki



--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to