Merge pull request #14912 from jakesmith/hpcc-25787-slow-appvalues

HPCC-25787 WUQuery very slow if workunit(s) have lots of app values

Reviewed-by: Gavin Halliday <ghalliday@hpccsystems.com>
Merged-by: Gavin Halliday <ghalliday@hpccsystems.com>

common/workunit/workunit.cpp

@@ -3624,7 +3624,7 @@ public:
     virtual IConstWUAppValueIterator & getApplicationValues() const { return *new CArrayIteratorOf<IConstWUAppValue,IConstWUAppValueIterator> (appvalues, 0, (IConstWorkUnitInfo *) this); };
 protected:
     StringAttr wuid, user, jobName, clusterName, timeScheduled, wuscope;
-    mutable CachedTags<CLocalWUAppValue,IConstWUAppValue> appvalues;
+    mutable CachedWUAppValues appvalues;
     unsigned totalThorTime;
     WUState state;
     WUAction action;
@@ -8515,7 +8515,7 @@ void CLocalWorkUnit::setStatistic(StatisticCreatorType creatorType, const char *
             statTree->setProp("@desc", optDescription);
 
         if (statistics.cached)
-            statistics.append(LINK(statTree));
+            statistics.append(statTree); // links statTree
 
         mergeAction = StatsMergeAppend;
     }
@@ -11528,33 +11528,28 @@ void CLocalWUException::setPriority(unsigned _priority)
 
 //==========================================================================================
 
-CLocalWUAppValue::CLocalWUAppValue(IPropertyTree *props, unsigned child) : p(props)
+CLocalWUAppValue::CLocalWUAppValue(const IPropertyTree *_owner, const IPropertyTree *_props) : owner(_owner), props(_props)
 {
-    StringAttrBuilder propPath(prop);
-    propPath.append("*[").append(child).append("]");
 }
 
 const char * CLocalWUAppValue::queryApplication() const
 {
-    return p->queryName();
+    return owner->queryName();
 }
 
 const char * CLocalWUAppValue::queryName() const
 {
-    IPropertyTree* val=p->queryPropTree(prop.str());
-    if(val)
-        return val->queryName();
-    return ""; // Should not happen in normal usage
+    return props->queryName();
 }
 
 const char * CLocalWUAppValue::queryValue() const
 {
-    return p->queryProp(prop.str());
+    return props->queryProp(nullptr);
 }
 
 //==========================================================================================
 
-CLocalWUStatistic::CLocalWUStatistic(IPropertyTree *props) : p(props)
+CLocalWUStatistic::CLocalWUStatistic(const IPropertyTree *props) : p(props)
 {
 }
 

common/workunit/workunit.ipp

@@ -29,11 +29,10 @@
 
 class WORKUNIT_API CLocalWUAppValue : implements IConstWUAppValue, public CInterface
 {
-    Owned<IPropertyTree> p;
-    StringAttr prop;
+    Owned<const IPropertyTree> owner, props;
 public:
     IMPLEMENT_IINTERFACE;
-    CLocalWUAppValue(IPropertyTree *p,unsigned child);
+    CLocalWUAppValue(const IPropertyTree *_owner, const IPropertyTree *_props);
 
     virtual const char *queryApplication() const;
     virtual const char *queryName() const;
@@ -43,11 +42,11 @@ public:
 
 class WORKUNIT_API CLocalWUStatistic : implements IConstWUStatistic, public CInterface
 {
-    Owned<IPropertyTree> p;
+    Owned<const IPropertyTree> p;
 public:
     IMPLEMENT_IINTERFACE;
 
-    CLocalWUStatistic(IPropertyTree *p);
+    CLocalWUStatistic(const IPropertyTree *p);
 
     virtual IStringVal & getCreator(IStringVal & str) const override;
     virtual IStringVal & getDescription(IStringVal & str, bool createDefault) const override;
@@ -66,7 +65,7 @@ public:
 
 //==========================================================================================
 
-template <typename T, typename IT> struct CachedTags
+template <typename T, typename IT, void (* ADD)(const IPropertyTree *p, IArrayOf<IT> &dst)> struct CachedTags
 {
     CachedTags(): cached(false) {}
     void load(IPropertyTree* p,const char* xpath)
@@ -77,9 +76,8 @@ template <typename T, typename IT> struct CachedTags
             Owned<IPropertyTreeIterator> r = p->getElements(xpath);
             for (r->first(); r->isValid(); r->next())
             {
-                IPropertyTree *rp = &r->query();
-                rp->Link();
-                tags.append(*new T(rp));
+                const IPropertyTree &rp = r->query();
+                ADD(&rp, tags); // NB: links items it adds
             }
             cached = true;
         }
@@ -96,23 +94,23 @@ template <typename T, typename IT> struct CachedTags
                 Owned<IPropertyTreeIterator> r = branch->getElements("*");
                 for (r->first(); r->isValid(); r->next())
                 {
-                    IPropertyTree *rp = &r->query();
-                    rp->Link();
-                    tags.append(*new T(rp));
+                    const IPropertyTree &rp = r->query();
+                    ADD(&rp, tags); // NB: links items it adds
                 }
             }
             cached = true;
         }
     }
-    void append(IPropertyTree * p)
-    {
-        tags.append(*new T(p));
-    }
 
     operator IArrayOf<IT>&() { return tags; }
     unsigned ordinality() const { return tags.ordinality(); }
     IT & item(unsigned i) const { return tags.item(i); }
 
+    void append(const IPropertyTree * p)
+    {
+        ADD(p, tags); // NB: links p
+    }
+
     void kill()
     {
         cached = false;
@@ -123,66 +121,21 @@ template <typename T, typename IT> struct CachedTags
     IArrayOf<IT> tags;
 };
 
-template <>  struct CachedTags<CLocalWUAppValue, IConstWUAppValue>
+void addStatistic(const IPropertyTree * p, IArrayOf<IConstWUStatistic> &dst)
 {
-    CachedTags(): cached(false) {}
-    void load(IPropertyTree* p,const char* xpath)
-    {
-        if (!cached)
-        {
-            assertex(tags.length() == 0);
-            Owned<IPropertyTreeIterator> r = p->getElements(xpath);
-            for (r->first(); r->isValid(); r->next())
-            {
-                IPropertyTree *rp = &r->query();
-                Owned<IPropertyTreeIterator> v = rp->getElements("*");
-                unsigned pos = 1;
-                for (v->first(); v->isValid(); v->next())
-                {
-                    rp->Link();
-                    tags.append(*new CLocalWUAppValue(rp,pos++));
-                }
-            }
-            cached = true;
-        }
-    }
-    void loadBranch(IPropertyTree* p,const char* xpath)
-    {
-        if (!cached)
-        {
-            assertex(tags.length() == 0);
-            Owned<IPropertyTree> branch = p->getBranch(xpath);
-            if (branch)
-            {
-                Owned<IPropertyTreeIterator> r = branch->getElements("*");
-                for (r->first(); r->isValid(); r->next())
-                {
-                    IPropertyTree *rp = &r->query();
-                    Owned<IPropertyTreeIterator> v = rp->getElements("*");
-                    unsigned pos = 1;
-                    for (v->first(); v->isValid(); v->next())
-                    {
-                        rp->Link();
-                        tags.append(*new CLocalWUAppValue(rp,pos++));
-                    }
-                }
-            }
-            cached = true;
-        }
-    }
+    dst.append(*new CLocalWUStatistic(&OLINK(*p)));
+}
 
+void addWUAppValues(const IPropertyTree *p, IArrayOf<IConstWUAppValue> &dst)
+{
+    Owned<IPropertyTreeIterator> v = p->getElements("*");
+    for (v->first(); v->isValid(); v->next())
+        dst.append(*new CLocalWUAppValue(&OLINK(*p), &v->get()));
+}
 
-    operator IArrayOf<IConstWUAppValue>&() { return tags; }
+typedef CachedTags<CLocalWUStatistic, IConstWUStatistic, addStatistic> CachedStatistics;
+typedef CachedTags<CLocalWUStatistic, IConstWUAppValue, addWUAppValues> CachedWUAppValues;
 
-    void kill()
-    {
-        cached = false;
-        tags.kill();
-    }
-
-    bool cached;
-    IArrayOf<IConstWUAppValue> tags;
-};
 
 //==========================================================================================
 
@@ -215,8 +168,8 @@ protected:
     mutable IArrayOf<IWUResult> results;
     mutable IArrayOf<IWUResult> temporaries;
     mutable IArrayOf<IWUResult> variables;
-    mutable CachedTags<CLocalWUAppValue,IConstWUAppValue> appvalues;
-    mutable CachedTags<CLocalWUStatistic,IConstWUStatistic> statistics;
+    mutable CachedWUAppValues appvalues;
+    mutable CachedStatistics statistics;
     mutable Owned<IUserDescriptor> userDesc;
     Mutex locked;
     Owned<ISecManager> secMgr;