Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/14380 )
Change subject: [cfile] KUDU-2852 Push predicate evaluation for int type RLE decoder ...................................................................... Patch Set 4: (5 comments) http://gerrit.cloudera.org:8080/#/c/14380/4//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/14380/4//COMMIT_MSG@23 PS4, Line 23: - To benchmark, adjusted all_types-scan-correctness-test with : - large number of rows 100k to 1M with run lengths of 1k to 10k : - predicate that selects subset of rows : - only RLE encoded integer datatypes : Observed 40% to 60% perf improvement in scan compared to not doing decoder : level evaluation (i.e. materializing_iterator_decoder_eval = false) Nice! Would be great if you could post the numbers describing what cases correspond to what times, if you have them available. That'd give some idea for how the improvement varies per selectivity, cardinality, etc. http://gerrit.cloudera.org:8080/#/c/14380/4/src/kudu/common/rowblock.h File src/kudu/common/rowblock.h: http://gerrit.cloudera.org:8080/#/c/14380/4/src/kudu/common/rowblock.h@217 PS4, Line 217: // Clear "nrows" bits from the supplied "offset" in the current view. : void ClearBits(size_t offset, size_t nrows) { : DCHECK_LE(offset + nrows, sel_vec_->nrows() - row_offset_); : BitmapChangeBits(sel_vec_->mutable_bitmap(), row_offset_ + offset, nrows, : false); : } nit: if you reorder the args, you could set the default offset to 0 and get rid of the below method, right? http://gerrit.cloudera.org:8080/#/c/14380/4/src/kudu/tablet/all_types-scan-correctness-test.cc File src/kudu/tablet/all_types-scan-correctness-test.cc: http://gerrit.cloudera.org:8080/#/c/14380/4/src/kudu/tablet/all_types-scan-correctness-test.cc@237 PS4, Line 237: nit: extra space http://gerrit.cloudera.org:8080/#/c/14380/4/src/kudu/tablet/all_types-scan-correctness-test.cc@239 PS4, Line 239: full nit: I found this confusing because I thought there'd be 5 "full" chunks (by definition of a "chunk"). Maybe reword this as 3 "matching" chunks? http://gerrit.cloudera.org:8080/#/c/14380/4/src/kudu/tablet/all_types-scan-correctness-test.cc@262 PS4, Line 262: matching_chunks_in_last_stride * matching_rows_per_chunk + : remainder_matching_rows; nit: seems to be a couple too many spaces -- To view, visit http://gerrit.cloudera.org:8080/14380 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6e05775ec1301d3d0b0365a7704b8e962a20455e Gerrit-Change-Number: 14380 Gerrit-PatchSet: 4 Gerrit-Owner: Bankim Bhavsar <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Andrew Wong <[email protected]> Gerrit-Reviewer: Attila Bukor <[email protected]> Gerrit-Reviewer: Bankim Bhavsar <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Thu, 10 Oct 2019 21:32:22 +0000 Gerrit-HasComments: Yes
