Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/15550 )
Change subject: wire_protocol: some simplification and optimization for rowwise encoding ...................................................................... Patch Set 2: (2 comments) http://gerrit.cloudera.org:8080/#/c/15550/2/src/kudu/common/rowblock.h File src/kudu/common/rowblock.h: http://gerrit.cloudera.org:8080/#/c/15550/2/src/kudu/common/rowblock.h@108 PS2, Line 108: // In the case that all rows are selected, sets *selected to 'boost::none' > I felt similarly, but I also don't mind it if it's documented. Alternativel yea, I couldn't find a great way to do this. I can change to a bool return if you guys prefer http://gerrit.cloudera.org:8080/#/c/15550/2/src/kudu/common/rowblock.cc File src/kudu/common/rowblock.cc: http://gerrit.cloudera.org:8080/#/c/15550/2/src/kudu/common/rowblock.cc@91 PS2, Line 91: CHECK_LE(n_rows_, std::numeric_limits<uint16_t>::max()); > What prevent's n_rows_ > 65535? just in practice we don't set rowblock sizes that large since the point of a RowBlock is to fit all the rows into CPU cache. I found some small perf improvement using uint16s here, -- To view, visit http://gerrit.cloudera.org:8080/15550 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I24dfb1bd036fde514ca6494bae0ddc171dd225dd Gerrit-Change-Number: 15550 Gerrit-PatchSet: 2 Gerrit-Owner: Todd Lipcon <t...@apache.org> Gerrit-Reviewer: Andrew Wong <andrew.w...@cloudera.com> Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com> Gerrit-Reviewer: Grant Henke <granthe...@apache.org> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Todd Lipcon <t...@apache.org> Gerrit-Comment-Date: Wed, 25 Mar 2020 05:44:55 +0000 Gerrit-HasComments: Yes
