Changes following review

Signed-off-by: Shamser Ahmed <[email protected]>
hpcc-systems · Jan 13, 2025 · d75b6e6 · d75b6e6
1 parent 9462169
commit d75b6e6
Show file tree

Hide file tree

Showing 3 changed files with 17 additions and 8 deletions.
diff --git a/common/thorhelper/thorcommon.cpp b/common/thorhelper/thorcommon.cpp
@@ -1801,8 +1801,6 @@ void ActivityTimeAccumulator::addStatistics(IStatisticGatherer & builder) const
         if (blockedCycles)
             builder.addStatistic(StTimeBlocked, cycle_to_nanosec(blockedCycles));
     }
-    if (lookAheadCycles)
-        builder.addStatistic(StTimeLookAhead, (unsigned __int64)cycle_to_nanosec(lookAheadCycles));
 }
 
 void ActivityTimeAccumulator::addStatistics(CRuntimeStatisticCollection & merged) const
@@ -1816,8 +1814,6 @@ void ActivityTimeAccumulator::addStatistics(CRuntimeStatisticCollection & merged
         if (blockedCycles)
             merged.mergeStatistic(StTimeBlocked, cycle_to_nanosec(blockedCycles));
     }
-    if (lookAheadCycles)
-        merged.mergeStatistic(StTimeLookAhead, (unsigned __int64)cycle_to_nanosec(lookAheadCycles));
 }
 
 void ActivityTimeAccumulator::merge(const ActivityTimeAccumulator & other)

diff --git a/thorlcr/graph/thgraphslave.cpp b/thorlcr/graph/thgraphslave.cpp
@@ -554,7 +554,7 @@ MemoryBuffer &CSlaveActivity::getInitializationData(unsigned slave, MemoryBuffer
     return mb.append(queryInitializationData(slave));
 }
 
-unsigned __int64 CSlaveActivity::queryLocalCycles() const
+unsigned __int64 CSlaveActivity::queryLocalCycles(unsigned __int64 totalCycles, unsigned __int64 blockedCycles, unsigned __int64 lookAheadCycles) const
 {
     unsigned __int64 inputCycles = 0;
     if (1 == inputs.ordinality())
@@ -587,11 +587,10 @@ unsigned __int64 CSlaveActivity::queryLocalCycles() const
                 break;
         }
     }
-    unsigned __int64 processCycles = queryTotalCycles() + queryLookAheadCycles();
+    unsigned __int64 processCycles = totalCycles + lookAheadCycles;
     if (processCycles < inputCycles) // not sure how/if possible, but guard against
         return 0;
     processCycles -= inputCycles;
-    const unsigned __int64 blockedCycles = queryBlockedCycles();
     if (processCycles < blockedCycles)
     {
         ActPrintLog("CSlaveActivity::queryLocalCycles - process %" I64F "uns < blocked %" I64F "uns", cycle_to_nanosec(processCycles), cycle_to_nanosec(blockedCycles));
@@ -600,6 +599,11 @@ unsigned __int64 CSlaveActivity::queryLocalCycles() const
     return processCycles-blockedCycles;
 }
 
+unsigned __int64 CSlaveActivity::queryLocalCycles() const
+{
+    return queryLocalCycles(queryTotalCycles(), queryBlockedCycles(), queryLookAheadCycles());
+}
+
 void CSlaveActivity::serializeStats(MemoryBuffer &mb)
 {
     CriticalBlock b(crit); // JCSMORE not sure what this is protecting..
@@ -619,7 +623,15 @@ void CSlaveActivity::serializeStats(MemoryBuffer &mb)
     queryCodeContext()->gatherStats(serializedStats);
 
     // JCS->GH - should these be serialized as cycles, and a different mapping used on master?
-    serializedStats.setStatistic(StTimeLocalExecute, (unsigned __int64)cycle_to_nanosec(queryLocalCycles()));
+    //
+    // Note: Look ahead cycles are not being kept up to date in slaverStats as multiple objects and threads are updating
+    // look ahead cycles.  At the moment, each thread and objects that generate look ahead cycles, track its own look ahead
+    // cycles and the up to date lookahead cycles is only available with a call to queryLookAheadCycles().  The code would
+    // need to be refactored to change this behaviour.
+    unsigned __int64 lookAheadCycles = queryLookAheadCycles();
+    unsigned __int64 localCycles = queryLocalCycles(queryTotalCycles(), queryBlockedCycles(), lookAheadCycles);
+    serializedStats.setStatistic(StTimeLookAhead, (unsigned __int64)cycle_to_nanosec(lookAheadCycles));
+    serializedStats.setStatistic(StTimeLocalExecute, (unsigned __int64)cycle_to_nanosec(localCycles));
     slaveTimerStats.addStatistics(serializedStats);
     serializedStats.serialize(mb);
     ForEachItemIn(i, outputs)

diff --git a/thorlcr/graph/thgraphslave.hpp b/thorlcr/graph/thgraphslave.hpp
@@ -269,6 +269,7 @@ class graphslave_decl CSlaveActivity : public CActivityBase, public CEdgeProgres
     bool canStall() const;
     bool isFastThrough() const;
     bool suppressLookAhead() const;
+    unsigned __int64 queryLocalCycles(unsigned __int64 totalCycles, unsigned __int64 blockedCycles, unsigned __int64 lookAheadCycles) const;
 
 // IThorDataLink
     virtual CSlaveActivity *queryFromActivity() override { return this; }