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/04/01 23:05:09 UTC

[kudu-CR] remove schema copies from RowBlock and RowBuilder 

Hello Mike Percy,

I'd like you to do a code review. Please visit

    http://gerrit.cloudera.org:8080/12907

to review the following change.


Change subject: remove schema copies from RowBlock and RowBuilder
......................................................................

remove schema copies from RowBlock and RowBuilder

AFAICT the schemas in these two always outlive the classes themselves, so
there's no reason to make schema copies. This is especially painful for the
MergeIterator where each sub-iterator has a RowBlock (and thus a full copy
of the schema).

Change-Id: Ie4d60640b2bac4f2b3077ddd696966e1f6658740
---
M src/kudu/codegen/codegen-test.cc
M src/kudu/common/generic_iterators-test.cc
M src/kudu/common/generic_iterators.cc
M src/kudu/common/row.h
M src/kudu/common/row_changelist-test.cc
M src/kudu/common/rowblock.cc
M src/kudu/common/rowblock.h
M src/kudu/common/schema-test.cc
M src/kudu/common/wire_protocol-test.cc
M src/kudu/common/wire_protocol.cc
M src/kudu/integration-tests/linked_list-test-util.h
M src/kudu/master/sys_catalog.cc
M src/kudu/tablet/cfile_set-test.cc
M src/kudu/tablet/compaction-test.cc
M src/kudu/tablet/compaction.cc
M src/kudu/tablet/delta_compaction.cc
M src/kudu/tablet/deltafile-test.cc
M src/kudu/tablet/diskrowset-test-base.h
M src/kudu/tablet/diskrowset-test.cc
M src/kudu/tablet/diskrowset.cc
M src/kudu/tablet/memrowset-test.cc
M src/kudu/tablet/mt-rowset_delta_compaction-test.cc
M src/kudu/tablet/mt-tablet-test.cc
M src/kudu/tablet/tablet-decoder-eval-test.cc
M src/kudu/tablet/tablet-pushdown-test.cc
M src/kudu/tablet/tablet-test-base.h
M src/kudu/tablet/tablet-test-util.h
M src/kudu/tablet/tablet-test.cc
M src/kudu/tablet/tablet_random_access-test.cc
M src/kudu/tserver/tablet_server-test-base.cc
M src/kudu/tserver/tablet_service.cc
31 files changed, 152 insertions(+), 139 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/07/12907/1
-- 
To view, visit http://gerrit.cloudera.org:8080/12907
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ie4d60640b2bac4f2b3077ddd696966e1f6658740
Gerrit-Change-Number: 12907
Gerrit-PatchSet: 1
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Mike Percy <mp...@apache.org>

[kudu-CR] remove schema copies from RowBlock and RowBuilder

Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Mike Percy, Kudu Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/12907

to look at the new patch set (#2).

Change subject: remove schema copies from RowBlock and RowBuilder
......................................................................

remove schema copies from RowBlock and RowBuilder

AFAICT the schemas in these two always outlive the classes themselves, so
there's no reason to make schema copies. This is especially painful for the
MergeIterator where each sub-iterator has a RowBlock (and thus a full copy
of the schema).

Change-Id: Ie4d60640b2bac4f2b3077ddd696966e1f6658740
---
M src/kudu/codegen/codegen-test.cc
M src/kudu/common/generic_iterators-test.cc
M src/kudu/common/generic_iterators.cc
M src/kudu/common/row.h
M src/kudu/common/row_changelist-test.cc
M src/kudu/common/rowblock.cc
M src/kudu/common/rowblock.h
M src/kudu/common/schema-test.cc
M src/kudu/common/wire_protocol-test.cc
M src/kudu/common/wire_protocol.cc
M src/kudu/integration-tests/linked_list-test-util.h
M src/kudu/master/sys_catalog.cc
M src/kudu/tablet/cfile_set-test.cc
M src/kudu/tablet/compaction-test.cc
M src/kudu/tablet/compaction.cc
M src/kudu/tablet/delta_compaction.cc
M src/kudu/tablet/deltafile-test.cc
M src/kudu/tablet/diskrowset-test-base.h
M src/kudu/tablet/diskrowset-test.cc
M src/kudu/tablet/diskrowset.cc
M src/kudu/tablet/memrowset-test.cc
M src/kudu/tablet/mt-rowset_delta_compaction-test.cc
M src/kudu/tablet/mt-tablet-test.cc
M src/kudu/tablet/tablet-decoder-eval-test.cc
M src/kudu/tablet/tablet-pushdown-test.cc
M src/kudu/tablet/tablet-test-base.h
M src/kudu/tablet/tablet-test-util.h
M src/kudu/tablet/tablet-test.cc
M src/kudu/tablet/tablet_random_access-test.cc
M src/kudu/tserver/tablet_server-test-base.cc
M src/kudu/tserver/tablet_service.cc
31 files changed, 153 insertions(+), 140 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/07/12907/2
-- 
To view, visit http://gerrit.cloudera.org:8080/12907
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie4d60640b2bac4f2b3077ddd696966e1f6658740
Gerrit-Change-Number: 12907
Gerrit-PatchSet: 2
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot (241)

[kudu-CR] remove schema copies from RowBlock and RowBuilder

Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Mike Percy, Todd Lipcon, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/12907

to look at the new patch set (#6).

Change subject: remove schema copies from RowBlock and RowBuilder
......................................................................

remove schema copies from RowBlock and RowBuilder

AFAICT the schemas in these two always outlive the classes themselves, so
there's no reason to make schema copies. This is especially painful for the
MergeIterator where each sub-iterator has a RowBlock (and thus a full copy
of the schema).

Change-Id: Ie4d60640b2bac4f2b3077ddd696966e1f6658740
---
M src/kudu/codegen/codegen-test.cc
M src/kudu/common/generic_iterators-test.cc
M src/kudu/common/generic_iterators.cc
M src/kudu/common/row.h
M src/kudu/common/row_changelist-test.cc
M src/kudu/common/rowblock.cc
M src/kudu/common/rowblock.h
M src/kudu/common/schema-test.cc
M src/kudu/common/wire_protocol-test.cc
M src/kudu/common/wire_protocol.cc
M src/kudu/integration-tests/linked_list-test-util.h
M src/kudu/master/sys_catalog.cc
M src/kudu/tablet/cfile_set-test.cc
M src/kudu/tablet/compaction-test.cc
M src/kudu/tablet/compaction.cc
M src/kudu/tablet/delta_compaction.cc
M src/kudu/tablet/deltafile-test.cc
M src/kudu/tablet/diskrowset-test-base.h
M src/kudu/tablet/diskrowset-test.cc
M src/kudu/tablet/diskrowset.cc
M src/kudu/tablet/memrowset-test.cc
M src/kudu/tablet/mt-rowset_delta_compaction-test.cc
M src/kudu/tablet/mt-tablet-test.cc
M src/kudu/tablet/tablet-decoder-eval-test.cc
M src/kudu/tablet/tablet-pushdown-test.cc
M src/kudu/tablet/tablet-test-base.h
M src/kudu/tablet/tablet-test-util.h
M src/kudu/tablet/tablet-test.cc
M src/kudu/tablet/tablet_random_access-test.cc
M src/kudu/tserver/tablet_server-test-base.cc
M src/kudu/tserver/tablet_service.cc
31 files changed, 178 insertions(+), 143 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/07/12907/6
-- 
To view, visit http://gerrit.cloudera.org:8080/12907
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie4d60640b2bac4f2b3077ddd696966e1f6658740
Gerrit-Change-Number: 12907
Gerrit-PatchSet: 6
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>

[kudu-CR] remove schema copies from RowBlock and RowBuilder

Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/12907 )

Change subject: remove schema copies from RowBlock and RowBuilder
......................................................................


Patch Set 5:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/12907/5/src/kudu/common/rowblock.h
File src/kudu/common/rowblock.h:

http://gerrit.cloudera.org:8080/#/c/12907/5/src/kudu/common/rowblock.h@195
PS5, Line 195:   RowBlock(const Schema* schema,
mind adding a comment just to indicate the lifetime requirements of 'schema' and 'arena'? It should be clear from convention but just to be extra careful.



-- 
To view, visit http://gerrit.cloudera.org:8080/12907
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie4d60640b2bac4f2b3077ddd696966e1f6658740
Gerrit-Change-Number: 12907
Gerrit-PatchSet: 5
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Tue, 09 Apr 2019 17:33:53 +0000
Gerrit-HasComments: Yes

[kudu-CR] remove schema copies from RowBlock and RowBuilder

Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Mike Percy, Kudu Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/12907

to look at the new patch set (#3).

Change subject: remove schema copies from RowBlock and RowBuilder
......................................................................

remove schema copies from RowBlock and RowBuilder

AFAICT the schemas in these two always outlive the classes themselves, so
there's no reason to make schema copies. This is especially painful for the
MergeIterator where each sub-iterator has a RowBlock (and thus a full copy
of the schema).

Change-Id: Ie4d60640b2bac4f2b3077ddd696966e1f6658740
---
M src/kudu/codegen/codegen-test.cc
M src/kudu/common/generic_iterators-test.cc
M src/kudu/common/generic_iterators.cc
M src/kudu/common/row.h
M src/kudu/common/row_changelist-test.cc
M src/kudu/common/rowblock.cc
M src/kudu/common/rowblock.h
M src/kudu/common/schema-test.cc
M src/kudu/common/wire_protocol-test.cc
M src/kudu/common/wire_protocol.cc
M src/kudu/integration-tests/linked_list-test-util.h
M src/kudu/master/sys_catalog.cc
M src/kudu/tablet/cfile_set-test.cc
M src/kudu/tablet/compaction-test.cc
M src/kudu/tablet/compaction.cc
M src/kudu/tablet/delta_compaction.cc
M src/kudu/tablet/deltafile-test.cc
M src/kudu/tablet/diskrowset-test-base.h
M src/kudu/tablet/diskrowset-test.cc
M src/kudu/tablet/diskrowset.cc
M src/kudu/tablet/memrowset-test.cc
M src/kudu/tablet/mt-rowset_delta_compaction-test.cc
M src/kudu/tablet/mt-tablet-test.cc
M src/kudu/tablet/tablet-decoder-eval-test.cc
M src/kudu/tablet/tablet-pushdown-test.cc
M src/kudu/tablet/tablet-test-base.h
M src/kudu/tablet/tablet-test-util.h
M src/kudu/tablet/tablet-test.cc
M src/kudu/tablet/tablet_random_access-test.cc
M src/kudu/tserver/tablet_server-test-base.cc
M src/kudu/tserver/tablet_service.cc
31 files changed, 154 insertions(+), 141 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/07/12907/3
-- 
To view, visit http://gerrit.cloudera.org:8080/12907
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie4d60640b2bac4f2b3077ddd696966e1f6658740
Gerrit-Change-Number: 12907
Gerrit-PatchSet: 3
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot (241)

[kudu-CR] remove schema copies from RowBlock and RowBuilder

Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has removed Kudu Jenkins from this change.  ( http://gerrit.cloudera.org:8080/12907 )

Change subject: remove schema copies from RowBlock and RowBuilder
......................................................................


Removed reviewer Kudu Jenkins with the following votes:

* Verified-1 by Kudu Jenkins (120)
-- 
To view, visit http://gerrit.cloudera.org:8080/12907
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteReviewer
Gerrit-Change-Id: Ie4d60640b2bac4f2b3077ddd696966e1f6658740
Gerrit-Change-Number: 12907
Gerrit-PatchSet: 5
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>

[kudu-CR] remove schema copies from RowBlock and RowBuilder

Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Mike Percy, Kudu Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/12907

to look at the new patch set (#4).

Change subject: remove schema copies from RowBlock and RowBuilder
......................................................................

remove schema copies from RowBlock and RowBuilder

AFAICT the schemas in these two always outlive the classes themselves, so
there's no reason to make schema copies. This is especially painful for the
MergeIterator where each sub-iterator has a RowBlock (and thus a full copy
of the schema).

Change-Id: Ie4d60640b2bac4f2b3077ddd696966e1f6658740
---
M src/kudu/codegen/codegen-test.cc
M src/kudu/common/generic_iterators-test.cc
M src/kudu/common/generic_iterators.cc
M src/kudu/common/row.h
M src/kudu/common/row_changelist-test.cc
M src/kudu/common/rowblock.cc
M src/kudu/common/rowblock.h
M src/kudu/common/schema-test.cc
M src/kudu/common/wire_protocol-test.cc
M src/kudu/common/wire_protocol.cc
M src/kudu/integration-tests/linked_list-test-util.h
M src/kudu/master/sys_catalog.cc
M src/kudu/tablet/cfile_set-test.cc
M src/kudu/tablet/compaction-test.cc
M src/kudu/tablet/compaction.cc
M src/kudu/tablet/delta_compaction.cc
M src/kudu/tablet/deltafile-test.cc
M src/kudu/tablet/diskrowset-test-base.h
M src/kudu/tablet/diskrowset-test.cc
M src/kudu/tablet/diskrowset.cc
M src/kudu/tablet/memrowset-test.cc
M src/kudu/tablet/mt-rowset_delta_compaction-test.cc
M src/kudu/tablet/mt-tablet-test.cc
M src/kudu/tablet/tablet-decoder-eval-test.cc
M src/kudu/tablet/tablet-pushdown-test.cc
M src/kudu/tablet/tablet-test-base.h
M src/kudu/tablet/tablet-test-util.h
M src/kudu/tablet/tablet-test.cc
M src/kudu/tablet/tablet_random_access-test.cc
M src/kudu/tserver/tablet_server-test-base.cc
M src/kudu/tserver/tablet_service.cc
31 files changed, 155 insertions(+), 142 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/07/12907/4
-- 
To view, visit http://gerrit.cloudera.org:8080/12907
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie4d60640b2bac4f2b3077ddd696966e1f6658740
Gerrit-Change-Number: 12907
Gerrit-PatchSet: 4
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot (241)

[kudu-CR] remove schema copies from RowBlock and RowBuilder

Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.
Mike Percy has posted comments on this change. ( http://gerrit.cloudera.org:8080/12907 )

Change subject: remove schema copies from RowBlock and RowBuilder
......................................................................


Patch Set 7: Code-Review+2


-- 
To view, visit http://gerrit.cloudera.org:8080/12907
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie4d60640b2bac4f2b3077ddd696966e1f6658740
Gerrit-Change-Number: 12907
Gerrit-PatchSet: 7
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Tue, 09 Apr 2019 23:42:29 +0000
Gerrit-HasComments: No

[kudu-CR] remove schema copies from RowBlock and RowBuilder

Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Mike Percy, Kudu Jenkins, Todd Lipcon, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/12907

to look at the new patch set (#5).

Change subject: remove schema copies from RowBlock and RowBuilder
......................................................................

remove schema copies from RowBlock and RowBuilder

AFAICT the schemas in these two always outlive the classes themselves, so
there's no reason to make schema copies. This is especially painful for the
MergeIterator where each sub-iterator has a RowBlock (and thus a full copy
of the schema).

Change-Id: Ie4d60640b2bac4f2b3077ddd696966e1f6658740
---
M src/kudu/codegen/codegen-test.cc
M src/kudu/common/generic_iterators-test.cc
M src/kudu/common/generic_iterators.cc
M src/kudu/common/row.h
M src/kudu/common/row_changelist-test.cc
M src/kudu/common/rowblock.cc
M src/kudu/common/rowblock.h
M src/kudu/common/schema-test.cc
M src/kudu/common/wire_protocol-test.cc
M src/kudu/common/wire_protocol.cc
M src/kudu/integration-tests/linked_list-test-util.h
M src/kudu/master/sys_catalog.cc
M src/kudu/tablet/cfile_set-test.cc
M src/kudu/tablet/compaction-test.cc
M src/kudu/tablet/compaction.cc
M src/kudu/tablet/delta_compaction.cc
M src/kudu/tablet/deltafile-test.cc
M src/kudu/tablet/diskrowset-test-base.h
M src/kudu/tablet/diskrowset-test.cc
M src/kudu/tablet/diskrowset.cc
M src/kudu/tablet/memrowset-test.cc
M src/kudu/tablet/mt-rowset_delta_compaction-test.cc
M src/kudu/tablet/mt-tablet-test.cc
M src/kudu/tablet/tablet-decoder-eval-test.cc
M src/kudu/tablet/tablet-pushdown-test.cc
M src/kudu/tablet/tablet-test-base.h
M src/kudu/tablet/tablet-test-util.h
M src/kudu/tablet/tablet-test.cc
M src/kudu/tablet/tablet_random_access-test.cc
M src/kudu/tserver/tablet_server-test-base.cc
M src/kudu/tserver/tablet_service.cc
31 files changed, 157 insertions(+), 143 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/07/12907/5
-- 
To view, visit http://gerrit.cloudera.org:8080/12907
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie4d60640b2bac4f2b3077ddd696966e1f6658740
Gerrit-Change-Number: 12907
Gerrit-PatchSet: 5
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>

[kudu-CR] remove schema copies from RowBlock and RowBuilder

Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/12907 )

Change subject: remove schema copies from RowBlock and RowBuilder
......................................................................


Patch Set 5: Verified+1

Overriding Jenkins, unrelated Java test failure.


-- 
To view, visit http://gerrit.cloudera.org:8080/12907
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie4d60640b2bac4f2b3077ddd696966e1f6658740
Gerrit-Change-Number: 12907
Gerrit-PatchSet: 5
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Mon, 08 Apr 2019 05:06:42 +0000
Gerrit-HasComments: No

[kudu-CR] remove schema copies from RowBlock and RowBuilder

Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/12907 )

Change subject: remove schema copies from RowBlock and RowBuilder
......................................................................


Patch Set 5:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/12907/5/src/kudu/common/rowblock.h
File src/kudu/common/rowblock.h:

http://gerrit.cloudera.org:8080/#/c/12907/5/src/kudu/common/rowblock.h@195
PS5, Line 195:   RowBlock(const Schema* schema,
> mind adding a comment just to indicate the lifetime requirements of 'schema
Done



-- 
To view, visit http://gerrit.cloudera.org:8080/12907
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie4d60640b2bac4f2b3077ddd696966e1f6658740
Gerrit-Change-Number: 12907
Gerrit-PatchSet: 5
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Tue, 09 Apr 2019 20:45:03 +0000
Gerrit-HasComments: Yes

[kudu-CR] remove schema copies from RowBlock and RowBuilder

Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/12907 )

Change subject: remove schema copies from RowBlock and RowBuilder
......................................................................

remove schema copies from RowBlock and RowBuilder

AFAICT the schemas in these two always outlive the classes themselves, so
there's no reason to make schema copies. This is especially painful for the
MergeIterator where each sub-iterator has a RowBlock (and thus a full copy
of the schema).

Change-Id: Ie4d60640b2bac4f2b3077ddd696966e1f6658740
Reviewed-on: http://gerrit.cloudera.org:8080/12907
Tested-by: Kudu Jenkins
Reviewed-by: Mike Percy <mp...@apache.org>
---
M src/kudu/codegen/codegen-test.cc
M src/kudu/common/generic_iterators-test.cc
M src/kudu/common/generic_iterators.cc
M src/kudu/common/row.h
M src/kudu/common/row_changelist-test.cc
M src/kudu/common/rowblock.cc
M src/kudu/common/rowblock.h
M src/kudu/common/schema-test.cc
M src/kudu/common/wire_protocol-test.cc
M src/kudu/common/wire_protocol.cc
M src/kudu/integration-tests/linked_list-test-util.h
M src/kudu/master/sys_catalog.cc
M src/kudu/tablet/cfile_set-test.cc
M src/kudu/tablet/compaction-test.cc
M src/kudu/tablet/compaction.cc
M src/kudu/tablet/delta_compaction.cc
M src/kudu/tablet/deltafile-test.cc
M src/kudu/tablet/diskrowset-test-base.h
M src/kudu/tablet/diskrowset-test.cc
M src/kudu/tablet/diskrowset.cc
M src/kudu/tablet/memrowset-test.cc
M src/kudu/tablet/mt-rowset_delta_compaction-test.cc
M src/kudu/tablet/mt-tablet-test.cc
M src/kudu/tablet/tablet-decoder-eval-test.cc
M src/kudu/tablet/tablet-pushdown-test.cc
M src/kudu/tablet/tablet-test-base.h
M src/kudu/tablet/tablet-test-util.h
M src/kudu/tablet/tablet-test.cc
M src/kudu/tablet/tablet_random_access-test.cc
M src/kudu/tserver/tablet_server-test-base.cc
M src/kudu/tserver/tablet_service.cc
31 files changed, 178 insertions(+), 143 deletions(-)

Approvals:
  Kudu Jenkins: Verified
  Mike Percy: Looks good to me, approved

-- 
To view, visit http://gerrit.cloudera.org:8080/12907
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ie4d60640b2bac4f2b3077ddd696966e1f6658740
Gerrit-Change-Number: 12907
Gerrit-PatchSet: 8
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>