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 <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Andrew Wong <[email protected]> Gerrit-Reviewer: Dan Burkert <[email protected]> Gerrit-Reviewer: David Ribeiro Alves <[email protected]> Gerrit-Reviewer: Hao Hao <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-Comment-Date: Mon, 25 Sep 2017 19:41:10 +0000 Gerrit-HasComments: Yes
