Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/15042 )
Change subject: cfile: change BlockBuilder API to yield a vector of Slices ...................................................................... Patch Set 1: (2 comments) http://gerrit.cloudera.org:8080/#/c/15042/1/src/kudu/cfile/binary_dict_block.cc File src/kudu/cfile/binary_dict_block.cc: http://gerrit.cloudera.org:8080/#/c/15042/1/src/kudu/cfile/binary_dict_block.cc@85 PS1, Line 85: data_slices.insert(data_slices.begin(), Slice(buffer_)); : *slices = std::move(data_slices); > I don't think reserve() can necessarily help because data_builder_->Finish( Yep, reserve() could help in the alternative approach that I suggested, but not here. I think it's not a big deal anyway, especially given the way how std::vector grows internally (i.e. reserve() might help only in certain cases on 2x size boundaries or alike). http://gerrit.cloudera.org:8080/#/c/15042/1/src/kudu/cfile/encoding-test.cc File src/kudu/cfile/encoding-test.cc: http://gerrit.cloudera.org:8080/#/c/15042/1/src/kudu/cfile/encoding-test.cc@127 PS1, Line 127: contiguous_buf_ > that's true. I think adding that CHECK will break things, though, since thi SGMT -- To view, visit http://gerrit.cloudera.org:8080/15042 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ifc7a5f148a4a43cedac2428f4c1a18d0f93a10db Gerrit-Change-Number: 15042 Gerrit-PatchSet: 1 Gerrit-Owner: Todd Lipcon <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Andrew Wong <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-Comment-Date: Thu, 16 Jan 2020 22:27:52 +0000 Gerrit-HasComments: Yes
