Marcel Kornacker has posted comments on this change. Change subject: IMPALA-2548: Codegen Tuple::MaterializeExprs() and use in TopN node ......................................................................
Patch Set 10: (1 comment) http://gerrit.cloudera.org:8080/#/c/1901/10/be/src/exec/topn-node.cc File be/src/exec/topn-node.cc: Line 79: sort_exec_exprs_.sort_tuple_slot_expr_ctxs(), NULL, &materialize_exprs_no_pool_fn)); > You mean a single symbol for the pool and no pool case? I wanted to remove i understand that you wanted that as a template parameter in order to have the compiler remove the branch, but since this is now optimized at runtime, and you're passing a constant null as a parameter in the no-pool case (at least in InsertTupleRow), wouldn't llvm be able to get rid of the branch anyway? why do you still need to have multiple symbols for the same thing? regarding your question: regardless of what we end up doing here, i think that would be worth having. -- To view, visit http://gerrit.cloudera.org:8080/1901 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib422a8d50303c21c6a228675157bf867e8619444 Gerrit-PatchSet: 10 Gerrit-Project: Impala Gerrit-Branch: cdh5-trunk Gerrit-Owner: Skye Wanderman-Milne <[email protected]> Gerrit-Reviewer: Marcel Kornacker <[email protected]> Gerrit-Reviewer: Michael Ho <[email protected]> Gerrit-Reviewer: Skye Wanderman-Milne <[email protected]> Gerrit-HasComments: Yes
