Merge pull request #6520 from ghalliday/issue12187

HPCC-12187 Move classes for gathering selections of statistics

Reviewed-By: Richard Chapman <rchapman@hpccsystems.com>

system/jlib/jstats.cpp

@@ -753,47 +753,37 @@ static int compareUnsigned(unsigned const * left, unsigned const * right)
     return (*left < *right) ? -1 : (*left > *right) ? +1 : 0;
 }
 
-class StatisticsMapping
+StatisticsMapping::StatisticsMapping(StatisticKind kind, ...)
 {
-public:
-    StatisticsMapping(StatisticKind kind, ...)
+    indexToKind.append(kind);
+    va_list args;
+    va_start(args, kind);
+    for (;;)
     {
-        indexToKind.append(kind);
-        va_list args;
-        va_start(args, kind);
-        for (;;)
-        {
-            unsigned next  = va_arg(args, unsigned);
-            if (!next)
-                break;
-            indexToKind.append(next);
-        }
-        va_end(args);
-        process();
+        unsigned next  = va_arg(args, unsigned);
+        if (!next)
+            break;
+        indexToKind.append(next);
     }
+    va_end(args);
+    createMappings();
+}
 
-    unsigned getIndex(StatisticKind kind) const { return kindToIndex.item(kind); }
-    StatisticKind getKind(unsigned index) const { return (StatisticKind)indexToKind.item(index); }
-    unsigned numStatistics() const { return indexToKind.ordinality(); }
+void StatisticsMapping::createMappings()
+{
+    //Possibly not needed, but sort the kinds, so that it is easy to merge/stream the results out in the correct order.
+    indexToKind.sort(compareUnsigned);
 
-protected:
-    void process()
+    //Provide mappings to all statistics to map them to the "unknown" bin by default
+    for (unsigned i=0; i < StMax; i++)
+        kindToIndex.append(numStatistics());
+
+    ForEachItemIn(i, indexToKind)
     {
-        //Possibly not needed, but sort the kinds, so that it is easy to merge/stream the results out in the correct order.
-        indexToKind.sort(compareUnsigned);
-        ForEachItemIn(i, indexToKind)
-        {
-            unsigned kind = indexToKind.item(i);
-            while (kindToIndex.ordinality() < kind)
-                kindToIndex.append(0);
-            kindToIndex.replace(i, kind);
-        }
+        unsigned kind = indexToKind.item(i);
+        kindToIndex.replace(i, kind);
     }
-
-protected:
-    UnsignedArray kindToIndex;
-    UnsignedArray indexToKind;
-};
+}
 
 //--------------------------------------------------------------------------------------------------------------------
 
@@ -1383,61 +1373,33 @@ extern IStatisticGatherer * createStatisticsGatherer(StatisticCreatorType creato
 
 //--------------------------------------------------------------------------------------------------------------------
 
-//This class is used to gather statistics for an activity.
-class CRuntimeStatisticCollection : public IStatisticCollection
+void CRuntimeStatisticCollection::rollupStatistics(unsigned numTargets, IContextLogger * const * targets) const
 {
-public:
-    CRuntimeStatisticCollection(const StatisticsMapping & _mapping) : mapping(_mapping)
-    {
-        unsigned num = mapping.numStatistics();
-        values = new unsigned __int64[num];
-        reset();
-    }
-    ~CRuntimeStatisticCollection()
-    {
-        delete [] values;
-    }
-
-    virtual void addStatistic(StatisticKind kind, unsigned __int64 value)
+    ForEachItem(iStat)
     {
-        unsigned index = queryMapping().getIndex(kind);
-        values[index] += value;
+        StatisticKind kind = getKind(iStat);
+        unsigned __int64 value = values[iStat].getClear();
+        for (unsigned iTarget = 0; iTarget < numTargets; iTarget++)
+            targets[iTarget]->noteStatistic(kind, value, 1);
     }
-    virtual void setStatistic(StatisticKind kind, unsigned __int64 value)
-    {
-        unsigned index = queryMapping().getIndex(kind);
-        values[index] = value;
-    }
-    virtual unsigned __int64 getStatisticValue(StatisticKind kind) const
-    {
-        unsigned index = queryMapping().getIndex(kind);
-        return values[index];
-    }
-    void reset()
-    {
-        unsigned num = mapping.numStatistics();
-        memset(values, 0, sizeof(unsigned __int64) * num);
-    }
-
-    inline const StatisticsMapping & queryMapping() const { return mapping; };
-    inline unsigned ordinality() const { return mapping.numStatistics(); }
-    inline StatisticKind getKind(unsigned i) const { return mapping.getKind(i); }
-
-private:
-    const StatisticsMapping & mapping;
-    unsigned __int64 * values;
-};
-
+    reportIgnoredStats();
+}
 
-void processStatistics(IStatisticGatherer & target, const CRuntimeStatisticCollection & stats)
+void CRuntimeStatisticCollection::recordStatistics(IStatisticGatherer & target, StatsMergeAction mergeAction) const
 {
-    ForEachItemIn(i, stats)
+    ForEachItem(i)
     {
-        StatisticKind kind = stats.getKind(i);
-        target.addStatistic(kind, stats.getStatisticValue(kind));
+        StatisticKind kind = getKind(i);
+        target.updateStatistic(kind, values[i].get(), mergeAction);
     }
+    reportIgnoredStats();
 }
 
+void CRuntimeStatisticCollection::reportIgnoredStats() const
+{
+    if (values[mapping.numStatistics()].getClear())
+        DBGLOG("Some statistics were addded but thrown away");
+}
 
 // ------------------------- old code -------------------------
 

system/jlib/jstats.h

@@ -211,6 +211,8 @@ enum StatisticKind
 
 };
 
+//---------------------------------------------------------------------------------------------------------------------
+
 interface IStatistic : extends IInterface
 {
 public:
@@ -307,6 +309,17 @@ public:
     virtual IStatisticCollection * getResult() = 0;
 };
 
+//All filtering should go through this interface - so we can extend and allow AND/OR filters at a later date.
+interface IStatisticsFilter : public IInterface
+{
+public:
+    virtual bool matches(StatisticCreatorType curCreatorType, const char * curCreator, StatisticScopeType curScopeType, const char * curScope, StatisticMeasure curMeasure, StatisticKind curKind) const = 0;
+    //These are a bit arbitrary...
+    virtual bool queryMergeSources() const = 0;
+    virtual const char * queryScope() const = 0;
+
+};
+
 class StatsScopeBlock
 {
 public:
@@ -322,6 +335,8 @@ protected:
     IStatisticGatherer & gatherer;
 };
 
+//---------------------------------------------------------------------------------------------------------------------
+
 class StatsSubgraphScope : public StatsScopeBlock
 {
 public:
@@ -349,16 +364,7 @@ public:
     }
 };
 
-//All filtering should go through this interface - so we can extend and allow AND/OR filters at a later date.
-interface IStatisticsFilter : public IInterface
-{
-public:
-    virtual bool matches(StatisticCreatorType curCreatorType, const char * curCreator, StatisticScopeType curScopeType, const char * curScope, StatisticMeasure curMeasure, StatisticKind curKind) const = 0;
-    //These are a bit arbitrary...
-    virtual bool queryMergeSources() const = 0;
-    virtual const char * queryScope() const = 0;
-
-};
+//---------------------------------------------------------------------------------------------------------------------
 
 class ScopedItemFilter
 {
@@ -424,6 +430,124 @@ protected:
     bool mergeSources;
 };
 
+//---------------------------------------------------------------------------------------------------------------------
+
+class StatisticsMapping
+{
+public:
+    //Takes a list of StatisticKind terminated by StKindNone
+    StatisticsMapping(StatisticKind kind, ...);
+
+    inline unsigned getIndex(StatisticKind kind) const
+    {
+        dbgassertex(kind >= StKindNone && kind < StMax);
+        return kindToIndex.item(kind);
+    }
+    inline StatisticKind getKind(unsigned index) const { return (StatisticKind)indexToKind.item(index); }
+    inline unsigned numStatistics() const { return indexToKind.ordinality(); }
+
+protected:
+    void createMappings();
+
+protected:
+    UnsignedArray kindToIndex;
+    UnsignedArray indexToKind;
+};
+
+//---------------------------------------------------------------------------------------------------------------------
+
+//MORE: We probably want to have functions that peform the atomic equivalents
+class CRuntimeStatistic
+{
+public:
+    inline void add(unsigned __int64 delta) { value += delta; }
+    inline void addAtomic(unsigned __int64 delta) { value += delta; }
+    inline unsigned __int64 get() const { return value; }
+    inline unsigned __int64 getClear()
+    {
+        unsigned __int64 ret = value;
+        value -= ret;
+        return ret;
+    }
+    inline unsigned __int64 getClearAtomic()
+    {
+        unsigned __int64 ret = value;
+        value -= ret; // should be atomic dec...
+        return ret;
+    }
+    inline void clear() { set(0); }
+    inline void set(unsigned __int64 delta) { value = delta; }
+
+protected:
+    unsigned __int64 value;
+};
+
+//This class is used to gather statistics for an activity - it has no notion of scope.
+interface IContextLogger;
+class CRuntimeStatisticCollection
+{
+public:
+    CRuntimeStatisticCollection(const StatisticsMapping & _mapping) : mapping(_mapping)
+    {
+        unsigned num = mapping.numStatistics();
+        values = new CRuntimeStatistic[num+1]; // extra entry is to gather unexpected stats
+    }
+    ~CRuntimeStatisticCollection()
+    {
+        delete [] values;
+    }
+
+    inline CRuntimeStatistic & queryStatistic(StatisticKind kind)
+    {
+        unsigned index = queryMapping().getIndex(kind);
+        return values[index];
+    }
+    inline const CRuntimeStatistic & queryStatistic(StatisticKind kind) const
+    {
+        unsigned index = queryMapping().getIndex(kind);
+        return values[index];
+    }
+
+    void addStatistic(StatisticKind kind, unsigned __int64 value)
+    {
+        queryStatistic(kind).add(value);
+    }
+    void setStatistic(StatisticKind kind, unsigned __int64 value)
+    {
+        queryStatistic(kind).set(value);
+    }
+    unsigned __int64 getStatisticValue(StatisticKind kind) const
+    {
+        return queryStatistic(kind).get();
+    }
+    void reset()
+    {
+        unsigned num = mapping.numStatistics();
+        for (unsigned i = 0; i <= num; i++)
+            values[i].clear();
+        memset(values, 0, sizeof(unsigned __int64) * num);
+    }
+
+    inline const StatisticsMapping & queryMapping() const { return mapping; };
+    inline unsigned ordinality() const { return mapping.numStatistics(); }
+    inline StatisticKind getKind(unsigned i) const { return mapping.getKind(i); }
+
+    void rollupStatistics(IContextLogger * target) { rollupStatistics(1, &target); }
+    void rollupStatistics(unsigned num, IContextLogger * const * targets) const;
+
+    void recordStatistics(IStatisticGatherer & target, StatsMergeAction mergeAction) const;
+
+protected:
+    void reportIgnoredStats() const;
+
+private:
+    const StatisticsMapping & mapping;
+    CRuntimeStatistic * values;
+};
+
+
+//---------------------------------------------------------------------------------------------------------------------
+
 //A class for minimizing the overhead of collecting timestamps.
 class jlib_decl OptimizedTimestamp
 {