Wenzhe Zhou has posted comments on this change. ( http://gerrit.cloudera.org:8080/19033 )
Change subject: IMPALA-11604 Planner changes for CPU usage ...................................................................... Patch Set 52: (8 comments) http://gerrit.cloudera.org:8080/#/c/19033/52/be/src/scheduling/scheduler.h File be/src/scheduling/scheduler.h: http://gerrit.cloudera.org:8080/#/c/19033/52/be/src/scheduling/scheduler.h@432 PS52, Line 432: ContainsUnionNode Could we change this new function, ContainsNode and following two new functions as static functions? It seems they don't use any member variables? http://gerrit.cloudera.org:8080/#/c/19033/52/be/src/util/backend-gflag-util.cc File be/src/util/backend-gflag-util.cc: http://gerrit.cloudera.org:8080/#/c/19033/52/be/src/util/backend-gflag-util.cc@209 PS52, Line 209: expression cost from IMPALA-2805 add TODO comments for tune the expression cost http://gerrit.cloudera.org:8080/#/c/19033/52/be/src/util/backend-gflag-util.cc@212 PS52, Line 212: // TODO: Benchmark and tune this config with an optimal value. This flag variable can be set as any positive number. Add what's the effectiveness if the value is too high, or too low. http://gerrit.cloudera.org:8080/#/c/19033/52/common/thrift/ImpalaService.thrift File common/thrift/ImpalaService.thrift: http://gerrit.cloudera.org:8080/#/c/19033/52/common/thrift/ImpalaService.thrift@771 PS52, Line 771: costing nit: processing cost http://gerrit.cloudera.org:8080/#/c/19033/52/common/thrift/Planner.thrift File common/thrift/Planner.thrift: http://gerrit.cloudera.org:8080/#/c/19033/52/common/thrift/Planner.thrift@90 PS52, Line 90: need nit: need to http://gerrit.cloudera.org:8080/#/c/19033/52/fe/src/main/java/org/apache/impala/analysis/AggregateInfo.java File fe/src/main/java/org/apache/impala/analysis/AggregateInfo.java: http://gerrit.cloudera.org:8080/#/c/19033/52/fe/src/main/java/org/apache/impala/analysis/AggregateInfo.java@724 PS52, Line 724: int numInstances Do we need this unused parameter? Don't see @Override annotation for this function http://gerrit.cloudera.org:8080/#/c/19033/52/fe/src/main/java/org/apache/impala/analysis/SortInfo.java File fe/src/main/java/org/apache/impala/analysis/SortInfo.java: http://gerrit.cloudera.org:8080/#/c/19033/52/fe/src/main/java/org/apache/impala/analysis/SortInfo.java@320 PS52, Line 320: int numInstances unused parameter http://gerrit.cloudera.org:8080/#/c/19033/52/fe/src/main/java/org/apache/impala/planner/AggregationNode.java File fe/src/main/java/org/apache/impala/planner/AggregationNode.java: http://gerrit.cloudera.org:8080/#/c/19033/52/fe/src/main/java/org/apache/impala/planner/AggregationNode.java@511 PS52, Line 511: List<ProcessingCost> processingCosts = : Lists.newArrayListWithCapacity(aggInfos_.size()); We can tweak a little to avoid this list and two for loops. -- To view, visit http://gerrit.cloudera.org:8080/19033 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If32dc770dfffcdd0be2b5555a789a7720952c68a Gerrit-Change-Number: 19033 Gerrit-PatchSet: 52 Gerrit-Owner: Qifan Chen <qfc...@hotmail.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Kurt Deschler <kdesc...@cloudera.com> Gerrit-Reviewer: Qifan Chen <qfc...@hotmail.com> Gerrit-Reviewer: Riza Suminto <riza.sumi...@cloudera.com> Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com> Gerrit-Comment-Date: Mon, 27 Feb 2023 20:12:39 +0000 Gerrit-HasComments: Yes
