Yuqi Du has posted comments on this change. ( http://gerrit.cloudera.org:8080/19794 )
Change subject: [cpp-client] KUDU-3455 Reduce space complexity and speed up hash partition pruning for in-list predicate ...................................................................... Patch Set 8: (11 comments) Thanks very much for your advices, all has fixed. http://gerrit.cloudera.org:8080/#/c/19794/7//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/19794/7//COMMIT_MSG@9 PS7, Line 9: a committed patch, its committed id > It would be better to use the related commit hash i.e. b69dbeb instead, the Done http://gerrit.cloudera.org:8080/#/c/19794/7//COMMIT_MSG@16 PS7, Line 16: > Same. Done http://gerrit.cloudera.org:8080/#/c/19794/7//COMMIT_MSG@21 PS7, Line 21: pe > nit: the Done http://gerrit.cloudera.org:8080/#/c/19794/7//COMMIT_MSG@21 PS7, Line 21: keys, t > nit: keys Done http://gerrit.cloudera.org:8080/#/c/19794/7//COMMIT_MSG@22 PS7, Line 22: longer > nit: longger? longer. DONE http://gerrit.cloudera.org:8080/#/c/19794/7//COMMIT_MSG@24 PS7, Line 24: using > nit: using Done http://gerrit.cloudera.org:8080/#/c/19794/7/src/kudu/common/partition_pruner-test.cc File src/kudu/common/partition_pruner-test.cc: http://gerrit.cloudera.org:8080/#/c/19794/7/src/kudu/common/partition_pruner-test.cc@97 PS7, Line 97: vector<bool > nit: vector Done http://gerrit.cloudera.org:8080/#/c/19794/7/src/kudu/common/partition_pruner.cc File src/kudu/common/partition_pruner.cc: http://gerrit.cloudera.org:8080/#/c/19794/7/src/kudu/common/partition_pruner.cc@186 PS7, Line 186: vector<vector<const void*>> predicate_values_list; > nit: Move this line to L207 where is closer to the place it's used ? Done http://gerrit.cloudera.org:8080/#/c/19794/7/src/kudu/common/partition_pruner.cc@217 PS7, Line 217: DCHECK_NOTNULL(predicate_values_picked); > nit: adds DCHECK to check predicate_values_picked and hash_bucket_bitset wi Done http://gerrit.cloudera.org:8080/#/c/19794/7/src/kudu/common/partition_pruner.cc@219 PS7, Line 219: ol immediately_stop = true; : for (const auto b : *hash_bucket_bitset) { : immediately_stop &= b; : } : i > Why not return immediately once find a true value in the loop? All bitset has set, that means all partitions for this 'hash_dimension' should reserve. Its not necessary to run left. In fact, this is the main optimization for speeding up when in-list predicate values are very long. http://gerrit.cloudera.org:8080/#/c/19794/7/src/kudu/common/partition_pruner.cc@225 PS7, Line 225: } > How about using DCHECK_EQ which is take effect only in DEBUG version? The r Done -- To view, visit http://gerrit.cloudera.org:8080/19794 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie4bea5c10b4ac2c62b85625fe9d2a33ceb4fb2e9 Gerrit-Change-Number: 19794 Gerrit-PatchSet: 8 Gerrit-Owner: Yuqi Du <shenxingwuy...@gmail.com> Gerrit-Reviewer: Abhishek Chennaka <achenn...@cloudera.com> Gerrit-Reviewer: Alexey Serbin <ale...@apache.org> Gerrit-Reviewer: Ashwani Raina <ara...@cloudera.com> Gerrit-Reviewer: KeDeng <kdeng...@gmail.com> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Wang Xixu <1450306...@qq.com> Gerrit-Reviewer: Yifan Zhang <chinazhangyi...@163.com> Gerrit-Reviewer: Yingchun Lai <laiyingc...@apache.org> Gerrit-Reviewer: Yuqi Du <shenxingwuy...@gmail.com> Gerrit-Comment-Date: Fri, 19 May 2023 02:50:36 +0000 Gerrit-HasComments: Yes
