Repository: incubator-impala Updated Branches: refs/heads/master faebfebdf -> 37a2ba255
IMPALA-3815: clean up cross-compiled comparator This avoids including a big block of interpreted code in the cross-compiled IR and will make it easier to inline the Compare() function into other codegened functions in a later change. Perf: Ran top-n targeted perf, didn't see any significant change. Change-Id: I058917da2c13ba41d6ff7fefbb761606344312ab Reviewed-on: http://gerrit.cloudera.org:8080/4307 Reviewed-by: Dan Hecht <[email protected]> Tested-by: Internal Jenkins Project: http://git-wip-us.apache.org/repos/asf/incubator-impala/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-impala/commit/37a2ba25 Tree: http://git-wip-us.apache.org/repos/asf/incubator-impala/tree/37a2ba25 Diff: http://git-wip-us.apache.org/repos/asf/incubator-impala/diff/37a2ba25 Branch: refs/heads/master Commit: 37a2ba2553dae5bf647346cbd5d2cc286b1bdefe Parents: faebfeb Author: Tim Armstrong <[email protected]> Authored: Fri Sep 2 00:04:00 2016 -0700 Committer: Internal Jenkins <[email protected]> Committed: Fri Sep 9 02:40:47 2016 +0000 ---------------------------------------------------------------------- be/src/util/tuple-row-compare.cc | 21 +++++++++++++++++ be/src/util/tuple-row-compare.h | 44 ++++++++++++++--------------------- 2 files changed, 38 insertions(+), 27 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/37a2ba25/be/src/util/tuple-row-compare.cc ---------------------------------------------------------------------- diff --git a/be/src/util/tuple-row-compare.cc b/be/src/util/tuple-row-compare.cc index 4097801..6f68b9e 100644 --- a/be/src/util/tuple-row-compare.cc +++ b/be/src/util/tuple-row-compare.cc @@ -28,6 +28,27 @@ using namespace impala; using namespace llvm; using namespace strings; +int TupleRowComparator::CompareInterpreted( + const TupleRow* lhs, const TupleRow* rhs) const { + DCHECK_EQ(key_expr_ctxs_lhs_.size(), key_expr_ctxs_rhs_.size()); + for (int i = 0; i < key_expr_ctxs_lhs_.size(); ++i) { + void* lhs_value = key_expr_ctxs_lhs_[i]->GetValue(lhs); + void* rhs_value = key_expr_ctxs_rhs_[i]->GetValue(rhs); + + // The sort order of NULLs is independent of asc/desc. + if (lhs_value == NULL && rhs_value == NULL) continue; + if (lhs_value == NULL && rhs_value != NULL) return nulls_first_[i]; + if (lhs_value != NULL && rhs_value == NULL) return -nulls_first_[i]; + + int result = + RawValue::Compare(lhs_value, rhs_value, key_expr_ctxs_lhs_[i]->root()->type()); + if (!is_asc_[i]) result = -result; + if (result != 0) return result; + // Otherwise, try the next Expr + } + return 0; // fully equivalent key +} + Status TupleRowComparator::Codegen(RuntimeState* state) { Function* fn; RETURN_IF_ERROR(CodegenCompare(state, &fn)); http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/37a2ba25/be/src/util/tuple-row-compare.h ---------------------------------------------------------------------- diff --git a/be/src/util/tuple-row-compare.h b/be/src/util/tuple-row-compare.h index 31a347f..a1d8d72 100644 --- a/be/src/util/tuple-row-compare.h +++ b/be/src/util/tuple-row-compare.h @@ -84,35 +84,19 @@ class TupleRowComparator { key_expr_ctxs_rhs_(sort_key_exprs.rhs_ordering_expr_ctxs()), is_asc_(key_expr_ctxs_lhs_.size(), is_asc), nulls_first_(key_expr_ctxs_lhs_.size(), nulls_first ? -1 : 1), - codegend_compare_fn_(NULL) { - } + codegend_compare_fn_(NULL) {} - /// Codegens a Compare() function for this comparator that is used in the () operator. - /// Returns Status::OK() iff the codegen was successful. + /// Codegens a Compare() function for this comparator that is used in Compare(). Status Codegen(RuntimeState* state); /// Returns a negative value if lhs is less than rhs, a positive value if lhs is greater /// than rhs, or 0 if they are equal. All exprs (key_exprs_lhs_ and key_exprs_rhs_) must /// have been prepared and opened before calling this, i.e. 'sort_key_exprs' in the /// constructor must have been opened. - int Compare(const TupleRow* lhs, const TupleRow* rhs) const { - DCHECK_EQ(key_expr_ctxs_lhs_.size(), key_expr_ctxs_rhs_.size()); - for (int i = 0; i < key_expr_ctxs_lhs_.size(); ++i) { - void* lhs_value = key_expr_ctxs_lhs_[i]->GetValue(lhs); - void* rhs_value = key_expr_ctxs_rhs_[i]->GetValue(rhs); - - // The sort order of NULLs is independent of asc/desc. - if (lhs_value == NULL && rhs_value == NULL) continue; - if (lhs_value == NULL && rhs_value != NULL) return nulls_first_[i]; - if (lhs_value != NULL && rhs_value == NULL) return -nulls_first_[i]; - - int result = RawValue::Compare(lhs_value, rhs_value, - key_expr_ctxs_lhs_[i]->root()->type()); - if (!is_asc_[i]) result = -result; - if (result != 0) return result; - // Otherwise, try the next Expr - } - return 0; // fully equivalent key + int ALWAYS_INLINE Compare(const TupleRow* lhs, const TupleRow* rhs) const { + return codegend_compare_fn_ == NULL ? + CompareInterpreted(lhs, rhs) : + (*codegend_compare_fn_)(&key_expr_ctxs_lhs_[0], &key_expr_ctxs_rhs_[0], lhs, rhs); } /// Returns true if lhs is strictly less than rhs. @@ -121,10 +105,7 @@ class TupleRowComparator { /// Force inlining because it tends not to be always inlined at callsites, even in /// hot loops. bool ALWAYS_INLINE Less(const TupleRow* lhs, const TupleRow* rhs) const { - int result = codegend_compare_fn_ == NULL ? Compare(lhs, rhs) : - (*codegend_compare_fn_)(&key_expr_ctxs_lhs_[0], &key_expr_ctxs_rhs_[0], lhs, rhs); - if (result < 0) return true; - return false; + return Compare(lhs, rhs) < 0; } bool ALWAYS_INLINE Less(const Tuple* lhs, const Tuple* rhs) const { @@ -140,12 +121,21 @@ class TupleRowComparator { } private: + /// Interpreted implementation of Compare(). + int CompareInterpreted(const TupleRow* lhs, const TupleRow* rhs) const; + /// Codegen Compare(). Returns a non-OK status if codegen is unsuccessful. - /// TODO: have codegen'd users inline this instead of calling through the () operator + /// TODO: inline this at codegen'd callsites instead of indirectly calling via function + /// pointer. Status CodegenCompare(RuntimeState* state, llvm::Function** fn); + /// References to ExprContexts managed by SortExecExprs. The lhs ExprContexts must + /// be created and prepared before the TupleRowCompator is constructed, but the rhs + /// ExprContexts are only created via cloning when SortExecExprs is Open()ed (which + /// may be after the TupleRowComparator is constructed). const std::vector<ExprContext*>& key_expr_ctxs_lhs_; const std::vector<ExprContext*>& key_expr_ctxs_rhs_; + std::vector<bool> is_asc_; std::vector<int8_t> nulls_first_;
