Bankim Bhavsar 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: (7 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 co I basically took one AllNonNull test case with default 3 and 11 as lower and upper range. Switching from a loop that cleared individual bit to clearing bits in a batch by adding a utility method in SelectionVectorView made the difference. Let me run more tests and add details. http://gerrit.cloudera.org:8080/#/c/14380/4/src/kudu/cfile/rle_block.h File src/kudu/cfile/rle_block.h: http://gerrit.cloudera.org:8080/#/c/14380/4/src/kudu/cfile/rle_block.h@426 PS4, Line 426: virtual > nit: drop virtual since it's already an override Done http://gerrit.cloudera.org:8080/#/c/14380/4/src/kudu/cfile/rle_block.h@450 PS4, Line 450: void * > nit: void* Done 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 Good point. Done. 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 Done 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 (b Done 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 Done -- 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: Fri, 11 Oct 2019 00:10:50 +0000 Gerrit-HasComments: Yes
