Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/16223 )
Change subject: IMPALA-9979: part 1: factor out Top-N heap. ...................................................................... Patch Set 3: (1 comment) http://gerrit.cloudera.org:8080/#/c/16223/3/be/src/exec/topn-node-ir.cc File be/src/exec/topn-node-ir.cc: http://gerrit.cloudera.org:8080/#/c/16223/3/be/src/exec/topn-node-ir.cc@37 PS3, Line 37: priority_queue_.size() < heap_capacity() > just thinking out loud, do you think generally the else part will be more c The branch should be predictable at least - you're right that we'd want to optimise for the case when there are many rows. Probably not worth investing too much into tuning until we do codegen of the comparator, cause that will completely change the performance profile anyway. -- To view, visit http://gerrit.cloudera.org:8080/16223 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1f585216b547af7a470e02f75458b1901dc44a31 Gerrit-Change-Number: 16223 Gerrit-PatchSet: 3 Gerrit-Owner: Tim Armstrong <[email protected]> Gerrit-Reviewer: Aman Sinha <[email protected]> Gerrit-Reviewer: Bikramjeet Vig <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-Comment-Date: Wed, 22 Jul 2020 22:18:46 +0000 Gerrit-HasComments: Yes
