[SERVER-79095] Reduce calls to VariableEnvironment::rebuild from path lowering Created: 19/Jul/23  Updated: 29/Oct/23  Resolved: 27/Sep/23

Status: Closed
Project: Core Server
Component/s: Query Planning
Affects Version/s: None
Fix Version/s: 7.2.0-rc0

Type: Task Priority: Major - P3
Reporter: Timour Katchaounov Assignee: Henri Nikku
Resolution: Fixed Votes: 0
Labels: M1
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Issue Links:
Duplicate
is duplicated by SERVER-79094 Reduce calls to VariableEnvironment::... Closed
Related
related to SERVER-80730 [CQF] Reduce optimization phases for ... Closed
Assigned Teams:
Query Optimization
Backwards Compatibility: Fully Compatible
Sprint: QO 2023-09-18, QO 2023-10-02
Participants:

 Description   

The path lowering rewrite in path_lower.cpp invokes reference tracking a number of times. This seems to be unnecessary, since path lowering cannot introduce/remove variables during. Since reference tracking is quite expensive, it makes sense to delay reference tracking until the end of path lowering. The following patch illustrates the idea:

diff --git a/src/mongo/db/query/optimizer/opt_phase_manager.cpp b/src/mongo/db/query/optimizer/opt_phase_manager.cpp
index 8b7b218bda0..ce3b380b13f 100644
--- a/src/mongo/db/query/optimizer/opt_phase_manager.cpp
+++ b/src/mongo/db/query/optimizer/opt_phase_manager.cpp
@@ -110,6 +110,22 @@ void OptPhaseManager::runStructuralPhase(C instance, VariableEnvironment& env, A
                 !_debugInfo.exceedsIterationLimit(iterationCount));
     }
 
+    // TODO: Unify the tree comments below, and add them to runStructuralPhases as well.
+
+    // This is needed for cases in which EvalPathLowering is called from a context other than during
+    // PathLowering. If the ABT is modified in a way that adds variable references and definitions
+    // the environment must be updated.
+
+    // This is needed for cases in which EvalFilterLowering is called from a context other than
+    // during PathLowering. If the ABT is modified in a way that adds variable references or
+    // definitions the environment must be updated.
+
+    // During PathLowering we may call EvalPathLowering or EvalFilterLowering. These each may call
+    // rebuild on a subset of the ABT, which will produce invalid references for refs that point to
+    // definitions outside of that subset. Rebuild the tree to avoid leaving those free variables
+    // for the caller.
+
+    env.rebuild(input);
     if (env.hasFreeVariables()) {
         tasserted(6808709, "Plan has free variables: " + generateFreeVarsAssertMsg(env));
     }
@@ -144,7 +160,7 @@ void OptPhaseManager::runStructuralPhases(C1 instance1,
             changed |= instance2.optimize(input);
         }
     }
-
+    env.rebuild(input);
     if (env.hasFreeVariables()) {
         tasserted(6808701, "Plan has free variables: " + generateFreeVarsAssertMsg(env));
     }
diff --git a/src/mongo/db/query/optimizer/rewrites/path_lower.cpp b/src/mongo/db/query/optimizer/rewrites/path_lower.cpp
index cf852899335..d3bd0ebd352 100644
--- a/src/mongo/db/query/optimizer/rewrites/path_lower.cpp
+++ b/src/mongo/db/query/optimizer/rewrites/path_lower.cpp
@@ -41,13 +41,6 @@ bool EvalPathLowering::optimize(ABT& n, bool rebuild) {
 
     algebra::transport<true>(n, *this);
 
-    // This is needed for cases in which EvalPathLowering is called from a context other than during
-    // PathLowering. If the ABT is modified in a way that adds variable references and definitions
-    // the environment must be updated.
-    if (_changed && rebuild) {
-        _env.rebuild(n);
-    }
-
     return _changed;
 }
 
@@ -233,13 +226,6 @@ bool EvalFilterLowering::optimize(ABT& n, bool rebuild) {
 
     algebra::transport<true>(n, *this);
 
-    // This is needed for cases in which EvalFilterLowering is called from a context other than
-    // during PathLowering. If the ABT is modified in a way that adds variable references or
-    // definitions the environment must be updated.
-    if (_changed && rebuild) {
-        _env.rebuild(n);
-    }
-
     return _changed;
 }
 
@@ -430,14 +416,6 @@ bool PathLowering::optimize(ABT& n) {
 
     algebra::transport<true>(n, *this);
 
-    // During PathLowering we may call EvalPathLowering or EvalFilterLowering. These each may call
-    // rebuild on a subset of the ABT, which will produce invalid references for refs that point to
-    // definitions outside of that subset. Rebuild the tree to avoid leaving those free variables
-    // for the caller.
-    if (_changed) {
-        _env.rebuild(n);
-    }
-
     return _changed;
 }
 



 Comments   
Comment by Githook User [ 27/Sep/23 ]

Author:

{'name': 'henrinikku', 'email': 'henri.nikku@mongodb.com', 'username': 'henrinikku'}

Message: SERVER-79095 Reduce calls to VariableEnvironment::rebuild from path lowering
Branch: master
https://github.com/mongodb/mongo/commit/581267c20e13e17d5dd486f7f0cb97503975f14b

Comment by Svilen Mihaylov (Inactive) [ 21/Sep/23 ]

Seems like we have overlap with the linked ticket

Generated at Thu Feb 08 06:40:04 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.