Todd Lipcon has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9404 )

Change subject: row: optimize copying of MRS rows into the Arena
......................................................................


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9404/1/src/kudu/common/row.h
File src/kudu/common/row.h:

http://gerrit.cloudera.org:8080/#/c/9404/1/src/kudu/common/row.h@348
PS1, Line 348:   for (int i = 0; i < schema->num_columns(); i++) {
             :     typename RowType::Cell cell = row->cell(i);
             :     if (cell.typeinfo()->physical_type() == BINARY) {
             :       if (cell.is_nullable() && cell.is_null()) {
             :         continue;
             :       }
             :
             :       const Slice *slice = reinterpret_cast<const Slice 
*>(cell.ptr());
             :       size += slice->size();
             :     }
             :   }
> would it make sense to have an "GetIndirectDataSize()" in row? we might cac
the problem is Row is more of a "concept" than a specific class (ie it can be 
ConstContiguousRow or RowBlockRow or MRSRow, etc). Given that I don't think 
there's an easy way to cache it, and sharing this code across them would 
require duplicating it.

It seems outside of tests that we only call this function from MemRowSet so I 
considered moving it, but then I'd need to update the tests a bit to do 
something else, and didn't want to yak-shave too far here



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6eea882d1d9a7355fb0bbad12c388908ec399a39
Gerrit-Change-Number: 9404
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon <t...@apache.org>
Gerrit-Reviewer: David Ribeiro Alves <davidral...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <t...@apache.org>
Gerrit-Comment-Date: Thu, 22 Feb 2018 21:50:01 +0000
Gerrit-HasComments: Yes

Reply via email to