From 17b47a1636c5cc352c0c6ebc73cb38ce3ea66f41 Mon Sep 17 00:00:00 2001 From: jackwener Date: Sun, 29 Oct 2023 23:23:58 +0800 Subject: [PATCH] [Performance](Nereids): optimize GroupExpressionMatching --- .../nereids/jobs/cascades/ApplyRuleJob.java | 2 +- .../pattern/GroupExpressionMatching.java | 41 +++++++++---------- .../doris/nereids/pattern/GroupMatching.java | 11 ++--- .../EnforceMissingPropertiesHelper.java | 2 - .../doris/nereids/trees/AbstractTreeNode.java | 13 ------ .../nereids/trees/plans/AbstractPlan.java | 11 +++++ .../nereids_p0/expression/topn_to_max.groovy | 4 +- 7 files changed, 40 insertions(+), 44 deletions(-) diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/jobs/cascades/ApplyRuleJob.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/jobs/cascades/ApplyRuleJob.java index 3e73850b0158d7..5560c369dd6f74 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/jobs/cascades/ApplyRuleJob.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/jobs/cascades/ApplyRuleJob.java @@ -61,7 +61,7 @@ public ApplyRuleJob(GroupExpression groupExpression, Rule rule, JobContext conte } @Override - public void execute() throws AnalysisException { + public final void execute() throws AnalysisException { if (groupExpression.hasApplied(rule) || groupExpression.isUnused()) { return; diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/pattern/GroupExpressionMatching.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/pattern/GroupExpressionMatching.java index f73ddcc88688b2..02421e4041c69c 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/pattern/GroupExpressionMatching.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/pattern/GroupExpressionMatching.java @@ -55,6 +55,7 @@ public GroupExpressionIterator iterator() { public static class GroupExpressionIterator implements Iterator { private final List results = Lists.newArrayList(); private int resultIndex = 0; + private int resultsSize; /** * Constructor. @@ -103,7 +104,7 @@ public GroupExpressionIterator(Pattern pattern, GroupExpression groupExpre // matching children group, one List per child // first dimension is every child group's plan // second dimension is all matched plan in one group - List> childrenPlans = Lists.newArrayListWithCapacity(childrenGroupArity); + List[] childrenPlans = new List[childrenGroupArity]; for (int i = 0; i < childrenGroupArity; ++i) { Group childGroup = groupExpression.child(i); List childrenPlan = matchingChildGroup(pattern, childGroup, i); @@ -116,7 +117,7 @@ public GroupExpressionIterator(Pattern pattern, GroupExpression groupExpre return; } } - childrenPlans.add(childrenPlan); + childrenPlans[i] = childrenPlan; } assembleAllCombinationPlanTree(root, pattern, groupExpression, childrenPlans); } else if (patternArity == 1 && (pattern.hasMultiChild() || pattern.hasMultiGroupChild())) { @@ -127,6 +128,7 @@ public GroupExpressionIterator(Pattern pattern, GroupExpression groupExpre results.add(root); } } + this.resultsSize = results.size(); } private List matchingChildGroup(Pattern parentPattern, @@ -154,40 +156,37 @@ private List matchingChildGroup(Pattern parentPattern, } private void assembleAllCombinationPlanTree(Plan root, Pattern rootPattern, - GroupExpression groupExpression, - List> childrenPlans) { - int[] childrenPlanIndex = new int[childrenPlans.size()]; + GroupExpression groupExpression, List[] childrenPlans) { + int childrenPlansSize = childrenPlans.length; + int[] childrenPlanIndex = new int[childrenPlansSize]; int offset = 0; LogicalProperties logicalProperties = groupExpression.getOwnerGroup().getLogicalProperties(); // assemble all combination of plan tree by current root plan and children plan - while (offset < childrenPlans.size()) { - ImmutableList.Builder childrenBuilder = - ImmutableList.builderWithExpectedSize(childrenPlans.size()); - for (int i = 0; i < childrenPlans.size(); i++) { - childrenBuilder.add(childrenPlans.get(i).get(childrenPlanIndex[i])); + Optional groupExprOption = Optional.of(groupExpression); + Optional logicalPropOption = Optional.of(logicalProperties); + while (offset < childrenPlansSize) { + ImmutableList.Builder childrenBuilder = ImmutableList.builderWithExpectedSize(childrenPlansSize); + for (int i = 0; i < childrenPlansSize; i++) { + childrenBuilder.add(childrenPlans[i].get(childrenPlanIndex[i])); } List children = childrenBuilder.build(); // assemble children: replace GroupPlan to real plan, // withChildren will erase groupExpression, so we must // withGroupExpression too. - Plan rootWithChildren = root.withGroupExprLogicalPropChildren(Optional.of(groupExpression), - Optional.of(logicalProperties), children); + Plan rootWithChildren = root.withGroupExprLogicalPropChildren(groupExprOption, + logicalPropOption, children); if (rootPattern.matchPredicates(rootWithChildren)) { results.add(rootWithChildren); } - offset = 0; - while (true) { + for (offset = 0; offset < childrenPlansSize; offset++) { childrenPlanIndex[offset]++; - if (childrenPlanIndex[offset] == childrenPlans.get(offset).size()) { + if (childrenPlanIndex[offset] == childrenPlans[offset].size()) { + // Reset the index when it reaches the size of the current child plan list childrenPlanIndex[offset] = 0; - offset++; - if (offset == childrenPlans.size()) { - break; - } } else { - break; + break; // Break the loop when the index is within the size of the current child plan list } } } @@ -195,7 +194,7 @@ private void assembleAllCombinationPlanTree(Plan root, Pattern rootPattern @Override public boolean hasNext() { - return resultIndex < results.size(); + return resultIndex < resultsSize; } @Override diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/pattern/GroupMatching.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/pattern/GroupMatching.java index b3e59590d37a96..d80fb40b3a5158 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/pattern/GroupMatching.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/pattern/GroupMatching.java @@ -45,11 +45,12 @@ public static List getAllMatchingPlans(Pattern pattern, Group group) { matchingPlans.add(plan); } } - for (GroupExpression groupExpression : group.getPhysicalExpressions()) { - for (Plan plan : new GroupExpressionMatching(pattern, groupExpression)) { - matchingPlans.add(plan); - } - } + // Jackwener: We don't need to match physical expressions. + // for (GroupExpression groupExpression : group.getPhysicalExpressions()) { + // for (Plan plan : new GroupExpressionMatching(pattern, groupExpression)) { + // matchingPlans.add(plan); + // } + // } } return matchingPlans; } diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/properties/EnforceMissingPropertiesHelper.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/properties/EnforceMissingPropertiesHelper.java index d548e3254c47d8..bd9f103c7c704c 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/properties/EnforceMissingPropertiesHelper.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/properties/EnforceMissingPropertiesHelper.java @@ -38,13 +38,11 @@ public class EnforceMissingPropertiesHelper { private static final EventProducer ENFORCER_TRACER = new EventProducer(EnforcerEvent.class, EventChannel.getDefaultChannel().addConsumers(new LogConsumer(EnforcerEvent.class, EventChannel.LOG))); - private final JobContext context; private final GroupExpression groupExpression; private Cost curTotalCost; public EnforceMissingPropertiesHelper(JobContext context, GroupExpression groupExpression, Cost curTotalCost) { - this.context = context; this.groupExpression = groupExpression; this.curTotalCost = curTotalCost; } diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/AbstractTreeNode.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/AbstractTreeNode.java index 0305ae2afad699..7a545ec17be8a2 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/AbstractTreeNode.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/AbstractTreeNode.java @@ -17,10 +17,6 @@ package org.apache.doris.nereids.trees; -import org.apache.doris.nereids.trees.expressions.StatementScopeIdGenerator; -import org.apache.doris.nereids.trees.plans.ObjectId; -import org.apache.doris.planner.PlanNodeId; - import com.google.common.collect.ImmutableList; import java.util.List; @@ -33,7 +29,6 @@ */ public abstract class AbstractTreeNode> implements TreeNode { - protected final ObjectId id = StatementScopeIdGenerator.newObjectId(); protected final List children; // TODO: Maybe we should use a GroupPlan to avoid TreeNode hold the GroupExpression. // https://github.com/apache/doris/pull/9807#discussion_r884829067 @@ -59,12 +54,4 @@ public List children() { public int arity() { return children.size(); } - - /** - * used for PhysicalPlanTranslator only - * @return PlanNodeId - */ - public PlanNodeId translatePlanNodeId() { - return id.toPlanNodeId(); - } } diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/AbstractPlan.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/AbstractPlan.java index 38a209ff55f49b..c223dd43b6ebac 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/AbstractPlan.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/AbstractPlan.java @@ -24,9 +24,11 @@ import org.apache.doris.nereids.trees.AbstractTreeNode; import org.apache.doris.nereids.trees.expressions.ExprId; import org.apache.doris.nereids.trees.expressions.Slot; +import org.apache.doris.nereids.trees.expressions.StatementScopeIdGenerator; import org.apache.doris.nereids.util.MutableState; import org.apache.doris.nereids.util.MutableState.EmptyMutableState; import org.apache.doris.nereids.util.TreeStringUtils; +import org.apache.doris.planner.PlanNodeId; import org.apache.doris.statistics.Statistics; import com.google.common.base.Supplier; @@ -45,6 +47,7 @@ */ public abstract class AbstractPlan extends AbstractTreeNode implements Plan { public static final String FRAGMENT_ID = "fragment"; + protected final ObjectId id = StatementScopeIdGenerator.newObjectId(); protected final Statistics statistics; protected final PlanType type; @@ -168,4 +171,12 @@ public Optional getMutableState(String key) { public void setMutableState(String key, Object state) { this.mutableState = this.mutableState.set(key, state); } + + /** + * used for PhysicalPlanTranslator only + * @return PlanNodeId + */ + public PlanNodeId translatePlanNodeId() { + return id.toPlanNodeId(); + } } diff --git a/regression-test/suites/nereids_p0/expression/topn_to_max.groovy b/regression-test/suites/nereids_p0/expression/topn_to_max.groovy index ae848b5a244f3a..4c05b42ccc3d06 100644 --- a/regression-test/suites/nereids_p0/expression/topn_to_max.groovy +++ b/regression-test/suites/nereids_p0/expression/topn_to_max.groovy @@ -31,7 +31,7 @@ suite("test_topn_to_max") { group by k1; ''' res = sql ''' - explain rewritten plan select k1, max(k2) + explain rewritten plan select k1, topn(k2, 1) from test_topn_to_max group by k1; ''' @@ -42,7 +42,7 @@ suite("test_topn_to_max") { from test_topn_to_max; ''' res = sql ''' - explain rewritten plan select max(k2) + explain rewritten plan select topn(k2, 1) from test_topn_to_max; ''' assertTrue(res.toString().contains("max"))