HPCC-17253 Optimize resolving expressions and cursors in an BuildCtx

Signed-off-by: Gavin Halliday <gavin.halliday@lexisnexis.com>

ecl/hqlcpp/hqlcpp.cpp

@@ -1588,6 +1588,7 @@ void HqlCppTranslator::cacheOptions()
         DebugOption(options.expirePersists, "expirePersists", true),
         DebugOption(options.defaultPersistExpiry, "defaultPersistExpiry", DEFAULT_PERSIST_EXPIRY_PERIOD),
         DebugOption(options.defaultExpiry, "defaultExpiry", DEFAULT_EXPIRY_PERIOD),
+        DebugOption(options.searchDistanceThreshold, "searchDistanceThreshold", 1000000),
 
         DebugOption(options.checkAsserts,"checkAsserts", true),
         DebugOption(options.assertSortedDistributed,"assertSortedDistributed", false),
@@ -4447,11 +4448,9 @@ void HqlCppTranslator::buildTempExpr(BuildCtx & ctx, BuildCtx & declareCtx, CHql
     case type_dictionary:
         {
             createTempFor(declareCtx, type, tempTarget, modifier, format);
-            IHqlStmt * stmt = subctx.addGroup();
-            stmt->setIncomplete(true);
+            TemporaryGroup group(subctx);
+
             buildDatasetAssign(subctx, tempTarget, expr);
-            stmt->setIncomplete(false);
-            stmt->mergeScopeWithContainer();
             break;
         }
     default:
@@ -4459,11 +4458,9 @@ void HqlCppTranslator::buildTempExpr(BuildCtx & ctx, BuildCtx & declareCtx, CHql
             createTempFor(declareCtx, type, tempTarget, modifier, format);
             if (ignoreSetAll)
                 tempTarget.isAll.clear();
-            IHqlStmt * stmt = subctx.addGroup();
-            stmt->setIncomplete(true);
+
+            TemporaryGroup group(subctx);
             buildExprAssign(subctx, tempTarget, expr);
-            stmt->setIncomplete(false);
-            stmt->mergeScopeWithContainer();
             break;
         }
     }

ecl/hqlcpp/hqlcpp.ipp

@@ -581,6 +581,7 @@ struct HqlCppOptions
     unsigned            subgraphToRegenerate;
     unsigned            defaultPersistExpiry;
     unsigned            defaultExpiry;
+    unsigned            searchDistanceThreshold;
     int                 defaultNumPersistInstances;
     unsigned            reportDFSinfo;
     CompilerType        targetCompiler;

ecl/hqlcpp/hqlcppds.cpp

@@ -4604,16 +4604,12 @@ void HqlCppTranslator::buildRowAssign(BuildCtx & ctx, BoundRow * targetRow, IHql
     }
 
     BuildCtx subctx(ctx);
-    IHqlStmt * stmt = subctx.addGroup();
-    stmt->setIncomplete(true);
+    TemporaryGroup group(subctx);
 
     Owned<BoundRow> rowBuilder = createRowBuilder(subctx, targetRow);
     Owned<IReferenceSelector> createdRef = createReferenceSelector(rowBuilder);
     buildRowAssign(subctx, createdRef, expr);
     finalizeTempRow(subctx, targetRow, rowBuilder);
-
-    stmt->setIncomplete(false);
-    stmt->mergeScopeWithContainer();
 }
 
 

ecl/hqlcpp/hqlhtcpp.cpp

@@ -2180,6 +2180,7 @@ void ActivityInstance::noteChildActivityLocation(IHqlExpression * pass)
 
 void ActivityInstance::buildPrefix()
 {
+    startDistance = querySearchDistance();
     StringBuffer s;
 
     sourceFileSequence.setown(getSizetConstant(translator.beginFunctionGetCppIndex(activityId, isChildActivity())));
@@ -2301,6 +2302,10 @@ void ActivityInstance::buildSuffix()
             addAttributeInt("approxClassSize", approxSize);
     }
 
+    unsigned __int64 searchDistance = querySearchDistance() - startDistance;
+    if (searchDistance > options.searchDistanceThreshold)
+        addAttributeInt("searchDistance", searchDistance);
+
 //  if (!isMember)
 //      classGroupStmt->setIncomplete(false);
 
@@ -7108,7 +7113,8 @@ BoundRow * HqlCppTranslator::bindSelectorAsSelf(BuildCtx & ctx, IReferenceSelect
     {
         if (rootRow->querySide() == no_self)
         {
-            ctx.associate(*rootRow);
+            //No need to associate since it is already present in the context
+            //ctx.associate(*rootRow);
             return rootRow;
         }
         return bindSelf(ctx, expr, rootRow->queryBound(), rootRow->queryBuilder());

ecl/hqlcpp/hqlhtcpp.ipp

@@ -229,6 +229,7 @@ public:
     OwnedHqlExpr colocalMember;
     Owned<ParentExtract> nestedExtract;
     SubGraphInfo * subgraph;
+    unsigned __int64 startDistance = 0;
 };
 
 

ecl/hqlcpp/hqlsource.cpp

@@ -1191,6 +1191,7 @@ void SourceBuilder::buildTransformBody(BuildCtx & transformCtx, IHqlExpression *
             OwnedHqlExpr boundSrc = createVariable("left", makeRowReferenceType(physicalRecord));
             transformCtx.associateOwn(*new BoundRow(tableExpr->queryNormalizedSelector(), boundSrc, translator.queryRecordOffsetMap(physicalRecord), no_none, NULL));
         }
+        transformCtx.addGroup();
     }
 
     rootSelfRow = translator.bindSelf(transformCtx, expr, "crSelf");
@@ -1458,6 +1459,7 @@ void SourceBuilder::buildTransformElements(BuildCtx & ctx, IHqlExpression * expr
                 //translator.associateSkipReturnMarker(subctx, failedFilterValue, NULL);
             }
             translator.doTransform(subctx, transform, rowBuilder);
+            ctx.addGroup();
             associateTargetCursor(subctx, ctx, tempRow, rowBuilder, expr);
         }
         break;
@@ -1478,6 +1480,7 @@ void SourceBuilder::buildTransformElements(BuildCtx & ctx, IHqlExpression * expr
                 //translator.associateSkipReturnMarker(subctx, failedFilterValue, NULL);
             }
             translator.doTransform(subctx, transform, rowBuilder);
+            ctx.addGroup();
             associateTargetCursor(subctx, ctx, tempRow, rowBuilder, expr);
         }
         break;

ecl/hqlcpp/hqlstmt.cpp

@@ -34,9 +34,19 @@
 #include "hqlutil.hpp"
 
 #define CLEAR_COPY_THRESHOLD            100
+#ifdef _DEBUG
+//#define TRACK_SEARCH_DISTANCE
+#endif
 
 static unsigned doCalcTotalChildren(const IHqlStmt * stmt);
 
+#ifdef TRACK_SEARCH_DISTANCE
+static unsigned __int64 searchDistance = 0;
+unsigned __int64 querySearchDistance() { return searchDistance; }
+#else
+unsigned __int64 querySearchDistance() { return 0; }
+#endif
+
 //---------------------------------------------------------------------------
 
 struct HQLCPP_API HqlBoundDefinedValue : public HqlDefinedValue
@@ -587,6 +597,10 @@ found:
 
 void BuildCtx::associate(HqlExprAssociation & next)
 {
+#ifdef _DEBUG
+    if (next.represents->getOperator() == no_self)
+        assertex(!queryAssociation(next.represents, next.getKind(), NULL));
+#endif
     assertex(next.represents->queryBody() == next.represents);
     if (!ignoreInput)
     {
@@ -705,72 +719,10 @@ bool BuildCtx::isOuterContext() const
     }
 }
 
+
 HqlExprAssociation * BuildCtx::queryAssociation(IHqlExpression * search, AssocKind kind, HqlExprCopyArray * selectors)
 {
-    HqlStmts * searchStmts = curStmts;
-
-    if (!search)
-        return NULL;
-    search = search->queryBody();
-    unsigned searchMask = kind;
-    if (selectors)
-        searchMask |= AssocCursor;
-
-    // search all statements in the tree before this one, to see
-    // if an expression already exists...  If so return the target
-    // of the assignment.
-    for (;;)
-    {
-        unsigned stmtMask = searchStmts->associationMask;
-        if (stmtMask & searchMask)
-        {
-            //Safe to use the hash iterator if no selectors, or this definition list contains no cursors
-            if ((kind == AssocExpr) && (!selectors || !(stmtMask & AssocCursor)))
-            {
-                HqlExprAssociation * match = searchStmts->exprAssociationCache.find(*search);
-                if (match)
-                    return match;
-            }
-            else
-            {
-                const CIArrayOf<HqlExprAssociation> & defs = searchStmts->defs;
-                if (!selectors)
-                {
-                    ForEachItemInRev(idx, defs)
-                    {
-                        HqlExprAssociation & cur = defs.item(idx);
-                        IHqlExpression * represents = cur.represents.get();
-                        if (represents == search)
-                            if (cur.getKind() == kind)
-                                return &cur;
-                    }
-                }
-                else
-                {
-                    ForEachItemInRev(idx, defs)
-                    {
-                        HqlExprAssociation & cur = defs.item(idx);
-                        IHqlExpression * represents = cur.represents.get();
-                        AssocKind curKind = cur.getKind();
-                        if (curKind == AssocCursor)
-                        {
-                            if (selectors->contains(*represents))
-                                return NULL;
-                        }
-                        if (represents == search)
-                            if (curKind == kind)
-                                return &cur;
-                    }
-                }
-            }
-        }
-
-        HqlStmt * limitStmt = searchStmts->queryStmt();
-        if (!limitStmt)
-            break;
-        searchStmts = limitStmt->queryContainer();
-    }
-    return NULL;
+    return curStmts->queryAssociation(search, kind, selectors);
 }
 
 
@@ -1067,10 +1019,30 @@ HqlStmts::HqlStmts(HqlStmt * _owner) : owner(_owner)
 
 void HqlStmts::appendOwn(HqlExprAssociation & next)
 {
+    AssocKind kind = next.getKind();
+    if (kind == AssocCursor)
+    {
+        //Self selectors are unique, so no need to check if they already exists in the context
+        if (next.represents->getOperator() != no_self)
+        {
+            //If a cursor is hiding another cursor then indicate the hash table can not be used
+            if (queryAssociation(next.represents, kind, nullptr))
+                associationMask |= AssocSequentialSearch;
+        }
+#ifdef _DEBUG
+        else
+        {
+            assertex(!queryAssociation(next.represents, next.getKind(), NULL));
+        }
+#endif
+    }
+
     defs.append(next);
-    associationMask |= next.getKind();
-    if (next.getKind() == AssocExpr)
+    associationMask |= kind;
+    if (kind == AssocExpr)
         exprAssociationCache.replace(next);
+    if (kind == AssocCursor)
+        maxCursor = defs.ordinality();
 }
 
 void HqlStmts::inheritDefinitions(HqlStmts & other)
@@ -1080,10 +1052,12 @@ void HqlStmts::inheritDefinitions(HqlStmts & other)
     {
         HqlExprAssociation & cur = other.defs.item(i);
         defs.append(OLINK(cur));
-        if (cur.getKind() == AssocExpr)
+        AssocKind kind = cur.getKind();
+        if (kind == AssocExpr)
             exprAssociationCache.replace(cur);
+        if (kind == AssocCursor)
+            maxCursor = defs.ordinality();
     }
-
 }
 
 void HqlStmts::appendStmt(HqlStmt & stmt)
@@ -1151,10 +1125,95 @@ bool HqlStmts::zap(HqlExprAssociation & next)
     }
 
     defs.remove(match);
+    if (defs.ordinality() < maxCursor)
+        maxCursor = defs.ordinality();
     return true;
 }
 
 
+HqlExprAssociation * HqlStmts::queryAssociation(IHqlExpression * search, AssocKind kind, HqlExprCopyArray * selectors)
+{
+    HqlStmts * searchStmts = this;
+    if (!search)
+        return NULL;
+    search = search->queryBody();
+    unsigned searchMask = kind;
+    if (selectors)
+        searchMask |= AssocSequentialSearch;
+
+    // search all statements in the tree before this one, to see
+    // if an expression already exists...  If so return the target
+    // of the assignment.
+    for (;;)
+    {
+        unsigned stmtMask = searchStmts->associationMask;
+        if (stmtMask & searchMask)
+        {
+            //Safe to use the hash iterator if no selectors, or this definition list contains no cursors
+            if (((kind == AssocExpr)) && (!selectors || !(stmtMask & AssocSequentialSearch)))
+            {
+                HqlExprAssociation * match = searchStmts->exprAssociationCache.find(*search);
+                if (match)
+                    return match;
+            }
+            else
+            {
+                const CIArrayOf<HqlExprAssociation> & defs = searchStmts->defs;
+                unsigned max = defs.ordinality();
+                //If searching for a cursor, then restrict the search to the known range.
+                //This is also valid even if selectors != null - since selectors also match AssocCursor.
+                if (kind == AssocCursor)
+                    max = searchStmts->maxCursor;
+                if (!selectors)
+                {
+                    for (unsigned idx=max; idx--; )
+                    {
+                        HqlExprAssociation & cur = defs.item(idx);
+                        IHqlExpression * represents = cur.represents.get();
+#ifdef TRACK_SEARCH_DISTANCE
+                        searchDistance++;
+#endif
+                        if (represents == search)
+                        {
+                            if (cur.getKind() == kind)
+                                return &cur;
+                        }
+                    }
+                }
+                else
+                {
+                    for (unsigned idx=max; idx--; )
+                    {
+                        HqlExprAssociation & cur = defs.item(idx);
+                        IHqlExpression * represents = cur.represents.get();
+                        AssocKind curKind = cur.getKind();
+                        if (curKind == AssocCursor)
+                        {
+                            if (selectors->contains(*represents))
+                                return NULL;
+                        }
+#ifdef TRACK_SEARCH_DISTANCE
+                        searchDistance++;
+#endif
+                        if (represents == search)
+                        {
+                            if (curKind == kind)
+                                return &cur;
+                        }
+                    }
+                }
+            }
+        }
+
+        HqlStmt * limitStmt = searchStmts->queryStmt();
+        if (!limitStmt)
+            break;
+        searchStmts = limitStmt->queryContainer();
+    }
+    return NULL;
+}
+
+
 //---------------------------------------------------------------------------
 
 HqlStmt::HqlStmt(StmtKind _kind, HqlStmts * _container)

ecl/hqlcpp/hqlstmt.hpp

@@ -54,6 +54,7 @@ enum AssocKind
     AssocGraphNode         = 0x0000200,
     AssocSubGraph          = 0x0000400,
     AssocStmt              = 0x0000800,
+    AssocSequentialSearch  = 0x0001000,
  };
 
 class CHqlBoundExpr;
@@ -283,6 +284,29 @@ public:
 };
 
 
+/*
+ * This class is used to temporarily add a group into the BuildCtx to avoid problems with declaring variables too
+ * early, and then merges any associations into the parent statement once creating the result is complete.
+ */
+class TemporaryGroup
+{
+public:
+    TemporaryGroup(BuildCtx & ctx)
+    {
+        group = ctx.addGroup();
+        group->setIncomplete(true);
+    }
+    ~TemporaryGroup()
+    {
+        group->setIncomplete(false);
+        group->mergeScopeWithContainer();
+    }
+private:
+    IHqlStmt * group;
+};
+
+
+
 unsigned calcTotalChildren(const IHqlStmt * stmt);
 
 IHqlExpression * stripTranslatedCasts(IHqlExpression * e);
@@ -290,4 +314,7 @@ IHqlExpression * peepholeAddExpr(IHqlExpression * left, IHqlExpression * right);
 bool rightFollowsLeft(IHqlExpression * left, IHqlExpression * leftLen, IHqlExpression * right);
 extern HQLCPP_API void outputSizeStmts();
 
+/* If TRACK_SEARCH_DISTANCE is defined it returns the distance spent searching for an matching association - otherwise 0 */
+extern HQLCPP_API unsigned __int64 querySearchDistance();
+
 #endif

ecl/hqlcpp/hqlstmt.ipp

@@ -120,12 +120,14 @@ public:
 
     void appendOwn(HqlExprAssociation & next);
     bool zap(HqlExprAssociation & next);
+    HqlExprAssociation * queryAssociation(IHqlExpression * search, AssocKind kind, HqlExprCopyArray * selectors);
 
 protected:
     HqlStmt *                       owner;
     CIArrayOf<HqlExprAssociation>   defs;
     // A bit mask of which types of associations this contains.  Don't worry about false positives.
     unsigned                        associationMask;
+    unsigned                        maxCursor = 0;
     AssociationCache                exprAssociationCache;
 };