[kudu-CR] generic iterators: avoid holding completed iters in merge/union

["Todd Lipcon (Code Review)" <ge...@cloudera.org>]

Todd Lipcon has posted comments on this change.

Change subject: generic_iterators: avoid holding completed iters in merge/union
......................................................................


Patch Set 1:

(8 comments)

http://gerrit.cloudera.org:8080/#/c/6460/1/src/kudu/common/generic_iterators.cc
File src/kudu/common/generic_iterators.cc:

Line 54: 
> extra line
Done


Line 65: 
> nit: mind making the spacing above/below the anonymous namespace consistent
Done


Line 176:     : schema_(std::move(schema)),
> warning: passing result of std::move() as a const reference argument; no mo
tried to get too fancy here. went back to old code


Line 325:   iters_.assign(iters.begin(), iters.end());
> I see, so we're avoiding extra shared_ptr copies by not tracking all_iters,
yea, the idea is that as we pop off elements from iters_, that is hopefully the last remaining reference to the iter, so it gets to destruct as it goes.


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'.
> So we're copying the input iterators because each subiterator is a shared_p
Good catch, I'd missed the critical part of the patch for MergeIterator which actually clears orig_iters_! (I did testing with UnionIterator). Fixed in next rev.


PS1, Line 78: vector<IteratorStats> finished_iter_stats_by_col_;
> docs
Done


PS1, Line 124: void PopFront();
> docs
Done


PS1, Line 131: vector<IteratorStats> finished_iter_stats_by_col_;
> docs
Done


-- 
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 <to...@apache.org>
Gerrit-Reviewer: Andrew Wong <an...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes