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

Reply via email to