ZhangYao has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13721 )

Change subject: KUDU-2847: Optimize iteration over selection vector in 
SerializeRowBlock
......................................................................


Patch Set 3:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/13721/2/src/kudu/common/wire_protocol.cc
File src/kudu/common/wire_protocol.cc:

http://gerrit.cloudera.org:8080/#/c/13721/2/src/kudu/common/wire_protocol.cc@918
PS2, Line 918:  if (IS_NULLABLE) {
> style: this should be ConvertBitmapToVector
Done


http://gerrit.cloudera.org:8080/#/c/13721/2/src/kudu/common/wire_protocol.cc@923
PS2, Line 923:       if (IS_NULLABLE) {
             :         BitmapChange(dst + offset_to_null_bitmap, dst_col_idx, 
false);
             :       }
             :     }
> seems like these two could be combined into just calling CountSelected once
Done :)


http://gerrit.cloudera.org:8080/#/c/13721/2/src/kudu/common/wire_protocol.cc@935
PS2, Line 935:   bool selected = false;
> I think it would be better to save the selected row idx boundary instead of
The rowblock is used with small size(128) in scan, so maybe it doesn't matter 
to copy full index. But there is no doubt that only save the boundary will 
benefit to the memory cost:)


http://gerrit.cloudera.org:8080/#/c/13721/2/src/kudu/common/wire_protocol.cc@935
PS2, Line 935:   bool selected = false;
> If you call CountSelected above and save the result, you can get a single r
Yea :)



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I19917d1875c46fd4cf98ef8a471b0340a76161e7
Gerrit-Change-Number: 13721
Gerrit-PatchSet: 3
Gerrit-Owner: ZhangYao <[email protected]>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Todd Lipcon <[email protected]>
Gerrit-Reviewer: Yao Xu <[email protected]>
Gerrit-Reviewer: ZhangYao <[email protected]>
Gerrit-Comment-Date: Tue, 16 Jul 2019 16:27:22 +0000
Gerrit-HasComments: Yes

Reply via email to