Ashwani Raina has posted comments on this change. ( http://gerrit.cloudera.org:8080/19568 )
Change subject: [java] KUDU-3455 Reduce space complexity about pruning hash partitions for in-list predicate ...................................................................... Patch Set 8: (8 comments) As of now, I have just taken a cursory look at the main algorithm and test cases. Planning to spend some more time, later, to understand it better in order to provide more valuable feedback. http://gerrit.cloudera.org:8080/#/c/19568/8//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/19568/8//COMMIT_MSG@15 PS8, Line 15: deep nit: Did you mean 'depth' ? http://gerrit.cloudera.org:8080/#/c/19568/8//COMMIT_MSG@15 PS8, Line 15: algorithm nit: approach http://gerrit.cloudera.org:8080/#/c/19568/8//COMMIT_MSG@19 PS8, Line 19: pressure nit: stress http://gerrit.cloudera.org:8080/#/c/19568/8//COMMIT_MSG@20 PS8, Line 20: algorithm's speedup is 100 times or so in the case of older algorithm : not out of memory. nit: algorithm has around 100x gain over older one when latter doesn't cause OOM. http://gerrit.cloudera.org:8080/#/c/19568/8//COMMIT_MSG@23 PS8, Line 23: reminding, the same problems found in cpp-client by reviewing : the codes. nit: reminder, same problem was found to be present in cpp-client per code-review. http://gerrit.cloudera.org:8080/#/c/19568/8/java/kudu-client/src/main/java/org/apache/kudu/client/PartitionPruner.java File java/kudu-client/src/main/java/org/apache/kudu/client/PartitionPruner.java: http://gerrit.cloudera.org:8080/#/c/19568/8/java/kudu-client/src/main/java/org/apache/kudu/client/PartitionPruner.java@657 PS8, Line 657: pruneable nit: prunable http://gerrit.cloudera.org:8080/#/c/19568/8/java/kudu-client/src/main/java/org/apache/kudu/client/PartitionPruner.java@658 PS8, Line 658: The method is a replacement about space complexity optimization of 'pruneHashComponentV1'. nit: The method is an optimized version of 'pruneHashComponent' space-complexity wise. http://gerrit.cloudera.org:8080/#/c/19568/8/java/kudu-client/src/main/java/org/apache/kudu/client/PartitionPruner.java@708 PS8, Line 708: if (level == columnIdxs.size()) { nit: It would really help if you could add a few lines here to summarize the algorithm. For reference, check this out - https://github.com/apache/kudu/blob/2a02969e5c186b22f1bf89555f184680eaf31ca0/src/kudu/client/meta_cache.cc#L441-L460 -- To view, visit http://gerrit.cloudera.org:8080/19568 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Icd6f213cb705e1b2a001562cc7cebe4164281723 Gerrit-Change-Number: 19568 Gerrit-PatchSet: 8 Gerrit-Owner: Yuqi Du <shenxingwuy...@gmail.com> Gerrit-Reviewer: Alexey Serbin <ale...@apache.org> Gerrit-Reviewer: Ashwani Raina <ara...@cloudera.com> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mahesh Reddy <mre...@cloudera.com> 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, 31 Mar 2023 07:39:05 +0000 Gerrit-HasComments: Yes