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 <[email protected]> Gerrit-Reviewer: David Ribeiro Alves <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-Comment-Date: Thu, 22 Feb 2018 21:50:01 +0000 Gerrit-HasComments: Yes
