Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/14445 )
Change subject: [tablet] TabletMetadata::CollectBlockIds use list instead of vector ...................................................................... Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/14445/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/14445/1//COMMIT_MSG@9 PS1, Line 9: Container size may reallocate and elements will be copied several times : if use std::vector in TabletMetadata::CollectBlockIds, std::list is more : efficient in this case. Yeah, but there are downsides to using std::list too: - More internal heap fragmentation due to per-block allocations. - Allocation/deallocation overhead for the addition/removal of one entry. - Iterating the list means traversing a bunch of different heap allocations, which is bad for CPU cache locality. I'd expect a std::deque to be a better choice. https://stackoverflow.com/questions/18449038/is-there-ever-a-reason-to-use-stdlist (and the linked questions) has some more information. Either way, you need to demonstrate that this change yields an improvement. The trade-offs are subtle and may fluctuate depending on number of blocks in question. Is there a benchmark you could run? -- 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: 1 Gerrit-Owner: Yingchun Lai <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Tue, 15 Oct 2019 20:44:57 +0000 Gerrit-HasComments: Yes
