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
