Qifan Chen has posted comments on this change. ( http://gerrit.cloudera.org:8080/16621 )
Change subject: IMPALA-3816: Codegen perf critical loops in Sorter ...................................................................... Patch Set 8: (1 comment) http://gerrit.cloudera.org:8080/#/c/16621/8/be/src/runtime/sorter.cc File be/src/runtime/sorter.cc: http://gerrit.cloudera.org:8080/#/c/16621/8/be/src/runtime/sorter.cc@1215 PS8, Line 1215: llvm::Function* fn = codegen->GetFunction(IRFunction::TUPLE_SORTER_SORT_HELPER, true); > We don't anywhere else. https://gerrit.cloudera.org/#/c/12828/ have a compl Looks like there is only on call to SortHelper() which is from Sorter::TupleSorter::Sort() shown below. 752 Status Sorter::TupleSorter::Sort(Run* run) { 753 DCHECK(run->is_finalized()); 754 DCHECK(!run->is_sorted()); 755 run_ = run; 756 const SortHelperFn sort_helper_fn = parent_->codegend_sort_helper_fn_.load(); 757 if (sort_helper_fn != nullptr) { 758 RETURN_IF_ERROR( 759 sort_helper_fn(this, TupleIterator::Begin(run_), TupleIterator::End(run_))); 760 } else { 761 RETURN_IF_ERROR(SortHelper(TupleIterator::Begin(run_), TupleIterator::End(run_))); 762 } 763 run_->set_sorted(); 764 return Status::OK(); 765 } The speedup may also come from the inlining of several small functions before LLVM optimizing "the big chunk of code" :-). I have collected the llvm code and uploaded it to the case here IMPALA-3816" target="_blank" rel="nofollow">https://issues.apache.org/jira/browse/IMPALA-3816. Interestingly, there are 2 definitions (one fast cc and one regular) for the SortHelp method and 4 calls to the fast cc version. 803 define internal fastcc void @_ZN6impala6Sorter11TupleSorter10SortHelperENS0_13TupleIteratorES2_(%"class.impala::Status"* noalias sr et, %"class.impala::Sorter::TupleSorter"*, %"class.impala::Sorter::TupleIterator"* byval nocapture align 8, %"class.impala::Sorter: :TupleIterator"* byval nocapture align 8) unnamed_addr #3 align 2 personality i8* bitcast (i32 (...)* @__gxx_personality_v0 to i8*) 1724 call fastcc void @_ZN6impala6Sorter11TupleSorter10SortHelperENS0_13TupleIteratorES2_(%"class.impala::Status"* nonnull sret %19, % "class.impala::Sorter::TupleSorter"* nonnull %1, %"class.impala::Sorter::TupleIterator"* byval nonnull align 8 %2, %"class.impala:: Sorter::TupleIterator"* byval nonnull align 8 %17) 1753 call fastcc void @_ZN6impala6Sorter11TupleSorter10SortHelperENS0_13TupleIteratorES2_(%"class.impala::Status"* nonnull sret %20, % "class.impala::Sorter::TupleSorter"* nonnull %1, %"class.impala::Sorter::TupleIterator"* byval nonnull align 8 %17, %"class.impala: :Sorter::TupleIterator"* byval nonnull align 8 %3) 2756 define void @_ZN6impala6Sorter11TupleSorter10SortHelperENS0_13TupleIteratorES2_.3(%"class.impala::Status"* noalias sret, %"class.im pala::Sorter::TupleSorter"*, %"class.impala::Sorter::TupleIterator"* byval nocapture align 8, %"class.impala::Sorter::TupleIterator "* byval nocapture align 8) local_unnamed_addr #3 align 2 personality i8* bitcast (i32 (...)* @__gxx_personality_v0 to i8*) { 3877 call fastcc void @_ZN6impala6Sorter11TupleSorter10SortHelperENS0_13TupleIteratorES2_(%"class.impala::Status"* nonnull sret %11, % "class.impala::Sorter::TupleSorter"* nonnull %1, %"class.impala::Sorter::TupleIterator"* byval nonnull align 8 %2, %"class.impala:: Sorter::TupleIterator"* byval nonnull align 8 %9) 3905 call fastcc void @_ZN6impala6Sorter11TupleSorter10SortHelperENS0_13TupleIteratorES2_(%"class.impala::Status"* nonnull sret %12, % "class.impala::Sorter::TupleSorter"* nonnull %1, %"class.impala::Sorter::TupleIterator"* byval nonnull align 8 %9, %"class.impala:: Sorter::TupleIterator"* byval nonnull align 8 %3) Edit -- To view, visit http://gerrit.cloudera.org:8080/16621 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie08137449d4a7b554ca8b8650260f8bd72e0a81b Gerrit-Change-Number: 16621 Gerrit-PatchSet: 8 Gerrit-Owner: Qifan Chen <qc...@cloudera.com> Gerrit-Reviewer: Csaba Ringhofer <csringho...@cloudera.com> Gerrit-Reviewer: Daniel Becker <daniel.bec...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Qifan Chen <qc...@cloudera.com> Gerrit-Reviewer: Sahil Takiar <stak...@cloudera.com> Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com> Gerrit-Comment-Date: Tue, 27 Oct 2020 19:30:57 +0000 Gerrit-HasComments: Yes
