Tim Armstrong has posted comments on this change. Change subject: IMPALA-4397,IMPALA-3259: reduce codegen time and memory ......................................................................
Patch Set 7: (1 comment) http://gerrit.cloudera.org:8080/#/c/4956/7/be/src/exec/partitioned-aggregation-node.cc File be/src/exec/partitioned-aggregation-node.cc: PS7, Line 1706: if (aggregate_evaluators_.size() > Expr::CODEGEN_INLINE_EXPRS_THRESHOLD) { > Do you think it's a reasonable middle ground to always inline MIN / MAX / S I don't think you can do it purely based on the aggregate operator since the input expressions trees can be arbitrarily complex. It seems like once you try to factor the size of trees of function calls into the decision you're basically writing a custom inliner (which maybe we'd want to do at some point, but I think is expanding the scope of this change a lot). -- To view, visit http://gerrit.cloudera.org:8080/4956 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id10015b49da182cb181a653ac8464b4a18b71091 Gerrit-PatchSet: 7 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Tim Armstrong <tarmstr...@cloudera.com> Gerrit-Reviewer: Marcel Kornacker <mar...@cloudera.com> Gerrit-Reviewer: Michael Ho <k...@cloudera.com> Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com> Gerrit-HasComments: Yes