Skip to content

Commit f6efad8

Browse files
def-claude
andcommitted
transform: algorithmic improvements to optimizer CPU hotspots
Profiling shows EquivalencePropagation alone consuming ~24% of total optimizer CPU. This commit makes several algorithmic improvements to reduce cloning, redundant work, and fixpoint iterations: **EquivalencePropagation (equivalence_propagation.rs):** - Precompute `base = join_equivs + outer_equivs` once per Join, then clone per child, instead of cloning both separately N times - Add `could_apply()` guard to skip clone+reduce_expr+reduce+rollback for expressions the reducer's remap cannot affect - Remove redundant `minimize()` after `permute()` in Join input setup, since `permute()` already calls `minimize(None)` internally - Replace full `relation.clone()` for change detection with a `Cell<bool>` flag set conservatively at mutation sites **EquivalenceClasses (analysis/equivalences.rs):** - Add `could_apply()` method to `ExpressionReducer` trait that walks the expression tree checking if any subexpression matches a remap key - Skip clone+reduce in `minimize_once()` for literals and column refs **RedundantJoin (redundant_join.rs):** - Remove all redundant join inputs in one pass instead of one per fixpoint iteration, avoiding costly re-runs of all transforms for each removal Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent ee7b07a commit f6efad8

File tree

3 files changed

+228
-100
lines changed

3 files changed

+228
-100
lines changed

src/transform/src/analysis/equivalences.rs

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -845,6 +845,10 @@ impl EquivalenceClasses {
845845
for expr in class.iter_mut() {
846846
self.remap.reduce_child(expr);
847847
if let Some(columns) = columns {
848+
// Skip clone + reduce for expressions already in reduced form.
849+
if expr.is_literal() || matches!(expr, MirScalarExpr::Column(..)) {
850+
continue;
851+
}
848852
let orig_expr = expr.clone();
849853
expr.reduce(columns);
850854
if expr.contains_err() {
@@ -1136,6 +1140,12 @@ pub trait ExpressionReducer {
11361140
/// Attempt to replace `expr` itself with another expression.
11371141
/// Returns true if it does so.
11381142
fn replace(&self, expr: &mut MirScalarExpr) -> bool;
1143+
/// Check if `reduce_expr` could potentially modify `expr`.
1144+
/// Returns true conservatively if uncertain. A false return guarantees
1145+
/// that `reduce_expr` would not change `expr`.
1146+
fn could_apply(&self, _expr: &MirScalarExpr) -> bool {
1147+
true
1148+
}
11391149
/// Attempt to replace any subexpressions of `expr` with other expressions.
11401150
/// Returns true if it does so.
11411151
fn reduce_expr(&self, expr: &mut MirScalarExpr) -> bool {
@@ -1197,6 +1207,30 @@ impl ExpressionReducer for BTreeMap<MirScalarExpr, MirScalarExpr> {
11971207
}
11981208
false
11991209
}
1210+
/// Check if `expr` or any of its subexpressions (matching `reduce_expr` traversal)
1211+
/// is a key in the BTreeMap. If not, `reduce_expr` would be a no-op.
1212+
fn could_apply(&self, expr: &MirScalarExpr) -> bool {
1213+
if self.is_empty() {
1214+
return false;
1215+
}
1216+
if self.contains_key(expr) {
1217+
return true;
1218+
}
1219+
match expr {
1220+
MirScalarExpr::CallBinary { expr1, expr2, .. } => {
1221+
self.could_apply(expr1) || self.could_apply(expr2)
1222+
}
1223+
MirScalarExpr::CallUnary { expr, .. } => self.could_apply(expr),
1224+
MirScalarExpr::CallVariadic { exprs, .. } => {
1225+
exprs.iter().any(|e| self.could_apply(e))
1226+
}
1227+
MirScalarExpr::If { then, els, .. } => {
1228+
// cond is not reduced (matching reduce_child behavior)
1229+
self.could_apply(then) || self.could_apply(els)
1230+
}
1231+
_ => false,
1232+
}
1233+
}
12001234
}
12011235

12021236
/// True iff the aggregate function returns an input datum.

0 commit comments

Comments
 (0)