Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/13749 )
Change subject: Extend benchmark for ColumnarRowBlockToPB. ...................................................................... Patch Set 4: (5 comments) http://gerrit.cloudera.org:8080/#/c/13749/2/src/kudu/common/wire_protocol-test.cc File src/kudu/common/wire_protocol-test.cc: http://gerrit.cloudera.org:8080/#/c/13749/2/src/kudu/common/wire_protocol-test.cc@426 PS2, Line 426: vector<double> select_rates = {1.0}; > It seems that if set column_counts = {3, 30, 300} and select_rates = {1.0, Maybe this will be better if you make 'kNumTrials' not affect the rowblock size itself? You could also change this to a value-parameterized test, so that it turns into 9 separate test cases, and then mark wire_protocol-test as sharded in CMakeLists.txt http://gerrit.cloudera.org:8080/#/c/13749/4/src/kudu/common/wire_protocol-test.cc File src/kudu/common/wire_protocol-test.cc: http://gerrit.cloudera.org:8080/#/c/13749/4/src/kudu/common/wire_protocol-test.cc@93 PS4, Line 93: ResetBenchMarkSchema nit: Benchmark, not BenchMark http://gerrit.cloudera.org:8080/#/c/13749/4/src/kudu/common/wire_protocol-test.cc@151 PS4, Line 151: 10000 * kNumTrials why does the RowBlock size change based on the number of trials? I think we should use a fixed size RowBlock, and probably small (our default value of --scanner_batch_size_rows is 128, which is actually smaller than I thought it would be...) http://gerrit.cloudera.org:8080/#/c/13749/4/src/kudu/common/wire_protocol-test.cc@156 PS4, Line 156: LOG_TIMING(INFO, Substitute("Coverting to PB with column count $0 and row select rate $1 ", typo: Converting http://gerrit.cloudera.org:8080/#/c/13749/4/src/kudu/common/wire_protocol-test.cc@160 PS4, Line 160: faststring direct, indirect; probably would be good to init these outside the loop and just clear them inside the loop, so you don't include allocation in the benchmark -- To view, visit http://gerrit.cloudera.org:8080/13749 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie29937c316be624151e6c51e54545c4f023e603d Gerrit-Change-Number: 13749 Gerrit-PatchSet: 4 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, 09 Jul 2019 03:45:03 +0000 Gerrit-HasComments: Yes
