Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/16777 )
Change subject: KUDU-3108: fix invalid memory accesses in merge iterator ...................................................................... Patch Set 6: (1 comment) http://gerrit.cloudera.org:8080/#/c/16777/6/src/kudu/common/generic_iterators.cc File src/kudu/common/generic_iterators.cc: http://gerrit.cloudera.org:8080/#/c/16777/6/src/kudu/common/generic_iterators.cc@556 PS6, Line 556: typedef std::set<MergeIterState*, MergeIterStateSmallestMaxComparator> MergeStateOrderedSet; > Algorithmically an std::set should be more or less equivalent to a heap for Another factor here is that we're now eagerly calling set::find() to get a reference to the set iterator. Before, this was the pop() call in the main loop, but it was conditioned on the merge window changing. Now the find() call necessarily happens unconditionally, since the merge window changing could impair our ability to do an erase(). I'll explore these suggestions bit more. Plus, seems like absl support might be just around the corner. -- To view, visit http://gerrit.cloudera.org:8080/16777 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8ec1cd3fd67ec4ea92a55b5b0ce555123748824d Gerrit-Change-Number: 16777 Gerrit-PatchSet: 6 Gerrit-Owner: Andrew Wong <aw...@cloudera.com> Gerrit-Reviewer: Adar Lieber-Dembo <a...@apache.org> Gerrit-Reviewer: Alexey Serbin <aser...@cloudera.com> Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Todd Lipcon <t...@apache.org> Gerrit-Comment-Date: Wed, 25 Nov 2020 19:35:58 +0000 Gerrit-HasComments: Yes
