Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/8041 )
Change subject: KUDU-2055 [part 2]: Add util to construct sorted disjoint interval list ...................................................................... Patch Set 6: (1 comment) http://gerrit.cloudera.org:8080/#/c/8041/6/src/kudu/util/sorted_disjoint_interval_list.h File src/kudu/util/sorted_disjoint_interval_list.h: http://gerrit.cloudera.org:8080/#/c/8041/6/src/kudu/util/sorted_disjoint_interval_list.h@88 PS6, Line 88: if (tail != cur_tail) { > I believe this is supposed to be 'head != cur_tail'. 'tail != cur_tail' is I don't know what to say that I put the opposite one here when renaming...But thanks a lot for catching that! The reason that the current test cases did not discover it is 'tail != cur_tail' is always true here, that result in '*head = std::move(*cur_tail);' always gets executed. This should not affect the correctness of the output but will affect the performance (there could be a extra move operation which is not needed). We may want to check how many times the move operations are actually performed in the test case, but not sure what is the best way to do that? -- To view, visit http://gerrit.cloudera.org:8080/8041 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I61a813c047be4882f246eaf404598e7e18fcac87 Gerrit-Change-Number: 8041 Gerrit-PatchSet: 6 Gerrit-Owner: Hao Hao <hao....@cloudera.com> Gerrit-Reviewer: Adar Dembo <a...@cloudera.com> Gerrit-Reviewer: Alexey Serbin <aser...@cloudera.com> Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com> Gerrit-Reviewer: Dan Burkert <danburk...@apache.org> Gerrit-Reviewer: David Ribeiro Alves <davidral...@gmail.com> Gerrit-Reviewer: Hao Hao <hao....@cloudera.com> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon <t...@apache.org> Gerrit-Comment-Date: Mon, 25 Sep 2017 19:41:10 +0000 Gerrit-HasComments: Yes
