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) { > Another idea is to use inlinehint attribute instead of (alwaysinline vs noi I don't think LLVM's inliner avoids overinlining here since it doesn't factor in the size of the caller, only the callee: https://github.com/llvm-mirror/llvm/blob/master/lib/Analysis/InlineCost.cpp#L1197 . It's also very aggressive about inlining if the called function has local linkage and only one callsite, which is the case for UpdateSlot(). It may be a good idea to avoid overinlining in other cases. It also doesn't actually work on our IR in many cases because of IMPALA-4164. -- 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