Browse Source

HPCC-10983 Don't constant fold to avoid expression duplication in optimizer

Signed-off-by: Gavin Halliday <gavin.halliday@lexisnexis.com>
Gavin Halliday 11 years ago
parent
commit
07645465e7
1 changed files with 39 additions and 20 deletions
  1. 39 20
      ecl/hql/hqlopt.cpp

+ 39 - 20
ecl/hql/hqlopt.cpp

@@ -321,26 +321,40 @@ IHqlExpression * CTreeOptimizer::swapIntoIf(IHqlExpression * expr, bool force)
     OwnedHqlExpr newLeft = replaceChildDataset(body, left, 0);
     OwnedHqlExpr newRight = replaceChildDataset(body, right, 0);
 
-    OwnedHqlExpr transformedLeft = transform(newLeft);
-    OwnedHqlExpr transformedRight = transform(newRight);
+    HqlExprArray args;
+    args.append(*LINK(cond));
+    args.append(*LINK(newLeft));
+    args.append(*LINK(newRight));
+    OwnedHqlExpr newIf = child->clone(args);
 
-    //Don't bother moving the condition over the if if it doesn't improve the code elsewhere
-    if (force || (newLeft != transformedLeft) || (newRight != transformedRight))
+    if (!alreadyHasUsage(newIf))
+    {
+        incUsage(newLeft);
+        incUsage(newRight);
+    }
+
+    OwnedHqlExpr transformedIf = transform(newIf);
+    if (force || (newIf != transformedIf))
     {
         //Need to call dec on all expressions that are no longer used... left and right still used by newLeft/newRight
-        noteUnused(child);
-        DBGLOG("Optimizer: Swap %s and %s", queryNode0Text(expr), queryNode1Text(child));
-        HqlExprArray args;
-        args.append(*LINK(cond));
-        args.append(*LINK(transformedLeft));
-        args.append(*LINK(transformedRight));
-        OwnedHqlExpr ret = child->clone(args);
-        if (!alreadyHasUsage(ret))
+        if (!alreadyHasUsage(newIf))
         {
-            incUsage(transformedLeft);
-            incUsage(transformedRight);
+            //This may possibly leave left/right linked once if transformed(newLeft) doesn't use left any more.
+            //But a recursiveDecUsage could cause too much to be decremented.
+            if (newLeft != transformedIf->queryChild(1))
+                decUsage(newLeft);
+            if (newRight != transformedIf->queryChild(2))
+                decUsage(newRight);
         }
-        return ret.getClear();
+        noteUnused(child);
+        DBGLOG("Optimizer: Swap %s and %s", queryNode0Text(expr), queryNode1Text(child));
+        return transformedIf.getClear();
+    }
+
+    if (!alreadyHasUsage(newIf))
+    {
+        decUsage(newLeft);
+        decUsage(newRight);
     }
     return LINK(expr);
 }
@@ -1513,6 +1527,9 @@ IHqlExpression * CTreeOptimizer::optimizeProjectInlineTable(IHqlExpression * tra
         if (!next || monitor.isComplex())
             return NULL;
 
+        if (onlyFoldConstant)
+            next.setown(foldScopedHqlExpression(errorProcessor, NULL, next));
+
         if (onlyFoldConstant && !isConstantTransform(next))
             return NULL;
         newValues.append(*ensureTransformType(next, no_transform));
@@ -1954,8 +1971,8 @@ ANewTransformInfo * CTreeOptimizer::createTransformInfo(IHqlExpression * expr)
 IHqlExpression * CTreeOptimizer::expandFields(TableProjectMapper * mapper, IHqlExpression * expr, IHqlExpression * oldDataset, IHqlExpression * newDataset, IExpandCallback * _expandCallback)
 {
     OwnedHqlExpr expandedFilter = mapper->expandFields(expr, oldDataset, newDataset, _expandCallback);
-    if (options & HOOfold)
-        expandedFilter.setown(foldScopedHqlExpression(errorProcessor, newDataset, expandedFilter));
+    //There used to be code to constant fold filters here - but it can cause dataset expressions to become duplicated
+    //causing code to be duplicated.  Only fold expressions that are reduced to constants.
     return expandedFilter.getClear();
 }
 
@@ -3908,10 +3925,12 @@ IHqlExpression * optimizeHqlExpression(IErrorReceiver & errorProcessor, IHqlExpr
     unwindCommaCompound(args, expr);
     optimizeHqlExpression(errorProcessor, newArgs, args, options);
     OwnedHqlExpr optimized = createActionList(newArgs);
+
+    if (expr == optimized)
+        return optimized.getClear();
+
     //If the graph was optimized then it is highly likely there are now constant folding opportunities
-    if (expr != optimized)
-        return foldHqlExpression(errorProcessor, optimized);
-    return optimized.getClear();
+    return foldHqlExpression(errorProcessor, optimized);
 }