Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/14445 )
Change subject: [tablet] Use std::deque instead of std::vector to collect blocks ...................................................................... Patch Set 4: (4 comments) http://gerrit.cloudera.org:8080/#/c/14445/4//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/14445/4//COMMIT_MSG@14 PS4, Line 14: linear time : cost Linear time cost for what operations? I think it's worth having the benchmark stats table in the commit message with the timings and the description of the operations that you posted in one of the response comments. http://gerrit.cloudera.org:8080/#/c/14445/4/src/kudu/tablet/tablet_metadata-test.cc File src/kudu/tablet/tablet_metadata-test.cc: http://gerrit.cloudera.org:8080/#/c/14445/4/src/kudu/tablet/tablet_metadata-test.cc@219 PS4, Line 219: LOG(INFO) << "block_ids size: " << block_ids.size(); It would be nice to move this out of the timing scope. http://gerrit.cloudera.org:8080/#/c/14445/4/src/kudu/tablet/tablet_metadata.cc File src/kudu/tablet/tablet_metadata.cc: http://gerrit.cloudera.org:8080/#/c/14445/4/src/kudu/tablet/tablet_metadata.cc@195 PS4, Line 195: deque<BlockId> block_ids; Did you try to see what if instead of using deques a vector of pre-allocated size is used instead here (i.e. std::vector::reserve(), where the size to reserve is reported by every element from rowsets_)? http://gerrit.cloudera.org:8080/#/c/14445/4/src/kudu/tablet/tablet_metadata.cc@198 PS4, Line 198: block_ids.begin() I think this was one of the culprits of the bad performance with std::vector: it it's not fit for inserting into the beginning, because with every insert it moves the elements around. What are the requirements here? Why to insert into the beginning of the container at all? Would it work if using block_ids.end() here instead? -- To view, visit http://gerrit.cloudera.org:8080/14445 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7ce853e35eb7dfa9f9a099e465ea8edfaa7c4aa9 Gerrit-Change-Number: 14445 Gerrit-PatchSet: 4 Gerrit-Owner: Yingchun Lai <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Yingchun Lai <[email protected]> Gerrit-Comment-Date: Thu, 14 Nov 2019 17:43:13 +0000 Gerrit-HasComments: Yes
