Andrew Wong has posted comments on this change. Change subject: generic_iterators: avoid holding completed iters in merge/union ......................................................................
Patch Set 1: (3 comments) http://gerrit.cloudera.org:8080/#/c/6460/1//COMMIT_MSG Commit Message: PS1, Line 15: ntl nit: until http://gerrit.cloudera.org:8080/#/c/6460/1/src/kudu/common/generic_iterators.cc File src/kudu/common/generic_iterators.cc: Line 325: iters_.assign(iters.begin(), iters.end()); Would it make sense to std::move(iters) here as well? Do we not do this to preserve ToString, or is memory consumption not as prominent here? http://gerrit.cloudera.org:8080/#/c/6460/1/src/kudu/common/generic_iterators.h File src/kudu/common/generic_iterators.h: Line 43: // a subset of the columns in 'iters'. Not sure if this is the right place, but it might be nice to note the reason for not treating these like "normal" (const reference) inputs. Might not be immediately obvious why we're breaking convention. Something like: "Initialization of this class transfers ownership of the schema and iterators so memory allocated for individual subiterators can be freed once used." -- To view, visit http://gerrit.cloudera.org:8080/6460 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I8e068c256c5855efeff5c6325dbb5fbf770eb900 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon <[email protected]> Gerrit-Reviewer: Andrew Wong <[email protected]> Gerrit-Reviewer: Andrew Wong <[email protected]> Gerrit-Reviewer: David Ribeiro Alves <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-HasComments: Yes
