Adar Lieber-Dembo 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: (2 comments) 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; > Maybe use absl btree set instead? Or some other set which is not node-based Algorithmically an std::set should be more or less equivalent to a heap for the operations that we perform, which makes me think that the perf gap between the status quo and your patch may very well be due to memory allocations as Todd suggests. Maybe the Boost intrusive set class would be a good choice? http://gerrit.cloudera.org:8080/#/c/16777/6/src/kudu/common/generic_iterators.cc@690 PS6, Line 690: CHECK(hotmax_iter != hotmaxes_.end()); CHECK_NE? -- 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 <[email protected]> Gerrit-Reviewer: Adar Lieber-Dembo <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Andrew Wong <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-Comment-Date: Wed, 25 Nov 2020 17:37:41 +0000 Gerrit-HasComments: Yes
