You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Adar Dembo (Code Review)" <ge...@cloudera.org> on 2019/01/11 23:52:34 UTC
[kudu-CR] generic iterators: move iterator declarations into cc file
Hello Mike Percy, Grant Henke, Todd Lipcon,
I'd like you to do a code review. Please visit
http://gerrit.cloudera.org:8080/12223
to review the following change.
Change subject: generic_iterators: move iterator declarations into cc file
......................................................................
generic_iterators: move iterator declarations into cc file
I plan on converting MergeIterator's state vector into an intrusive list,
which means MergeIterState's class definition needs to be visible to
MergeIterator's class definition.
Instead of moving MergeIterState into the header, let's go the other way and
move all of the iterator declarations into the .cc file. The code outside of
these classes doesn't care about the concrete types, and it should speed up
compilation a bit.
I had to poke a hole in for some PredicateEvaluatingIterator tests though.
Change-Id: I301f39ec0d55b73cadcf28d8104accca3219ab1b
---
M src/kudu/common/generic_iterators-test.cc
M src/kudu/common/generic_iterators.cc
M src/kudu/common/generic_iterators.h
M src/kudu/tablet/cfile_set-test.cc
M src/kudu/tablet/compaction.cc
M src/kudu/tablet/delta_compaction.cc
M src/kudu/tablet/diskrowset.cc
M src/kudu/tablet/rowset.cc
M src/kudu/tablet/tablet.cc
9 files changed, 341 insertions(+), 315 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/23/12223/1
--
To view, visit http://gerrit.cloudera.org:8080/12223
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I301f39ec0d55b73cadcf28d8104accca3219ab1b
Gerrit-Change-Number: 12223
Gerrit-PatchSet: 1
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] generic iterators: move iterator declarations into cc file
Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.
Mike Percy has posted comments on this change. ( http://gerrit.cloudera.org:8080/12223 )
Change subject: generic_iterators: move iterator declarations into cc file
......................................................................
Patch Set 4: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/12223
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I301f39ec0d55b73cadcf28d8104accca3219ab1b
Gerrit-Change-Number: 12223
Gerrit-PatchSet: 4
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Fri, 25 Jan 2019 01:00:12 +0000
Gerrit-HasComments: No
[kudu-CR] generic iterators: move iterator declarations into cc file
Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.
Mike Percy has posted comments on this change. ( http://gerrit.cloudera.org:8080/12223 )
Change subject: generic_iterators: move iterator declarations into cc file
......................................................................
Patch Set 2: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/12223
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I301f39ec0d55b73cadcf28d8104accca3219ab1b
Gerrit-Change-Number: 12223
Gerrit-PatchSet: 2
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Thu, 24 Jan 2019 00:41:08 +0000
Gerrit-HasComments: No
[kudu-CR] generic iterators: move iterator declarations into cc file
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/12223 )
Change subject: generic_iterators: move iterator declarations into cc file
......................................................................
Patch Set 3: Verified+1
Overriding Jenkins, unrelated test failure.
--
To view, visit http://gerrit.cloudera.org:8080/12223
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I301f39ec0d55b73cadcf28d8104accca3219ab1b
Gerrit-Change-Number: 12223
Gerrit-PatchSet: 3
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Thu, 24 Jan 2019 02:04:07 +0000
Gerrit-HasComments: No
[kudu-CR] generic iterators: move iterator declarations into cc file
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has removed Kudu Jenkins from this change. ( http://gerrit.cloudera.org:8080/12223 )
Change subject: generic_iterators: move iterator declarations into cc file
......................................................................
Removed reviewer Kudu Jenkins with the following votes:
* Verified-1 by Kudu Jenkins (120)
--
To view, visit http://gerrit.cloudera.org:8080/12223
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteReviewer
Gerrit-Change-Id: I301f39ec0d55b73cadcf28d8104accca3219ab1b
Gerrit-Change-Number: 12223
Gerrit-PatchSet: 3
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] generic iterators: move iterator declarations into cc file
Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.
Mike Percy has posted comments on this change. ( http://gerrit.cloudera.org:8080/12223 )
Change subject: generic_iterators: move iterator declarations into cc file
......................................................................
Patch Set 3: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/12223
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I301f39ec0d55b73cadcf28d8104accca3219ab1b
Gerrit-Change-Number: 12223
Gerrit-PatchSet: 3
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Fri, 25 Jan 2019 00:55:59 +0000
Gerrit-HasComments: No
[kudu-CR] generic iterators: move iterator declarations into cc file
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/12223 )
Change subject: generic_iterators: move iterator declarations into cc file
......................................................................
generic_iterators: move iterator declarations into cc file
I plan on converting MergeIterator's state vector into an intrusive list,
which means MergeIterState's class definition needs to be visible to
MergeIterator's class definition.
Instead of moving MergeIterState into the header, let's go the other way and
move all of the iterator declarations into the .cc file. The code outside of
these classes doesn't care about the concrete types, and it should speed up
compilation a bit.
I had to poke a hole in for some PredicateEvaluatingIterator tests though.
Change-Id: I301f39ec0d55b73cadcf28d8104accca3219ab1b
Reviewed-on: http://gerrit.cloudera.org:8080/12223
Reviewed-by: Mike Percy <mp...@apache.org>
Tested-by: Kudu Jenkins
---
M src/kudu/common/generic_iterators-test.cc
M src/kudu/common/generic_iterators.cc
M src/kudu/common/generic_iterators.h
M src/kudu/tablet/cfile_set-test.cc
M src/kudu/tablet/compaction.cc
M src/kudu/tablet/delta_compaction.cc
M src/kudu/tablet/diskrowset.cc
M src/kudu/tablet/rowset.cc
M src/kudu/tablet/tablet.cc
9 files changed, 345 insertions(+), 316 deletions(-)
Approvals:
Mike Percy: Looks good to me, approved
Kudu Jenkins: Verified
--
To view, visit http://gerrit.cloudera.org:8080/12223
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I301f39ec0d55b73cadcf28d8104accca3219ab1b
Gerrit-Change-Number: 12223
Gerrit-PatchSet: 5
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] generic iterators: move iterator declarations into cc file
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Hello Mike Percy, Kudu Jenkins, Grant Henke, Todd Lipcon,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/12223
to look at the new patch set (#2).
Change subject: generic_iterators: move iterator declarations into cc file
......................................................................
generic_iterators: move iterator declarations into cc file
I plan on converting MergeIterator's state vector into an intrusive list,
which means MergeIterState's class definition needs to be visible to
MergeIterator's class definition.
Instead of moving MergeIterState into the header, let's go the other way and
move all of the iterator declarations into the .cc file. The code outside of
these classes doesn't care about the concrete types, and it should speed up
compilation a bit.
I had to poke a hole in for some PredicateEvaluatingIterator tests though.
Change-Id: I301f39ec0d55b73cadcf28d8104accca3219ab1b
---
M src/kudu/common/generic_iterators-test.cc
M src/kudu/common/generic_iterators.cc
M src/kudu/common/generic_iterators.h
M src/kudu/tablet/cfile_set-test.cc
M src/kudu/tablet/compaction.cc
M src/kudu/tablet/delta_compaction.cc
M src/kudu/tablet/diskrowset.cc
M src/kudu/tablet/rowset.cc
M src/kudu/tablet/tablet.cc
9 files changed, 345 insertions(+), 316 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/23/12223/2
--
To view, visit http://gerrit.cloudera.org:8080/12223
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I301f39ec0d55b73cadcf28d8104accca3219ab1b
Gerrit-Change-Number: 12223
Gerrit-PatchSet: 2
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>