IVGCVSW-3984 Fix CheckPeriodicCounterCaptureThread

 * PerodicCounterCapture now sleeps instead of terminating
   when reading empty counter data
 * m_IsRunning is now set to false after the thread is joined and
   is no longer atomic
 * removed usages of find() (not thread safe)


Signed-off-by: Finn Williams <Finn.Williams@arm.com>
Change-Id: Ie65d5a9a7e42a31653ec8eed064849355661ef56

4 files changed, 44 insertions, 36 deletions

diff --git a/src/profiling/PeriodicCounterCapture.cpp b/src/profiling/PeriodicCounterCapture.cpp
index 0ccb516ae2..5ba1318a77 100644
--- a/src/profiling/PeriodicCounterCapture.cpp
+++ b/src/profiling/PeriodicCounterCapture.cpp
@@ -6,6 +6,7 @@
 #include "PeriodicCounterCapture.hpp"
 
 #include <boost/log/trivial.hpp>
+#include <iostream>
 
 namespace armnn
 {
@@ -16,14 +17,14 @@ namespace profiling
 void PeriodicCounterCapture::Start()
 {
     // Check if the capture thread is already running
-    if (m_IsRunning.load())
+    if (m_IsRunning)
     {
         // The capture thread is already running
         return;
     }
 
     // Mark the capture thread as running
-    m_IsRunning.store(true);
+    m_IsRunning = true;
 
     // Keep the capture procedure going until the capture thread is signalled to stop
     m_KeepRunning.store(true);
@@ -45,6 +46,8 @@ void PeriodicCounterCapture::Stop()
         // Wait for the capture thread to complete operations
         m_PeriodCaptureThread.join();
     }
+
+    m_IsRunning = false;
 }
 
 CaptureData PeriodicCounterCapture::ReadCaptureData()
@@ -54,16 +57,17 @@ CaptureData PeriodicCounterCapture::ReadCaptureData()
 
 void PeriodicCounterCapture::Capture(const IReadCounterValues& readCounterValues)
 {
-    while (m_KeepRunning.load())
+    do
     {
         // Check if the current capture data indicates that there's data capture
         auto currentCaptureData = ReadCaptureData();
         const std::vector<uint16_t>& counterIds = currentCaptureData.GetCounterIds();
+
         if (currentCaptureData.GetCapturePeriod() == 0 || counterIds.empty())
         {
-            // No data capture, terminate the thread
-            m_KeepRunning.store(false);
-            break;
+            // No data capture, wait the indicated capture period (milliseconds)
+            std::this_thread::sleep_for(std::chrono::milliseconds(5));
+            continue;
         }
 
         std::vector<std::pair<uint16_t, uint32_t>> values;
@@ -107,9 +111,10 @@ void PeriodicCounterCapture::Capture(const IReadCounterValues& readCounterValues
 
         // Wait the indicated capture period (microseconds)
         std::this_thread::sleep_for(std::chrono::microseconds(currentCaptureData.GetCapturePeriod()));
+
     }
+    while (m_KeepRunning.load());
 
-    m_IsRunning.store(false);
 }
 
 } // namespace profiling
diff --git a/src/profiling/PeriodicCounterCapture.hpp b/src/profiling/PeriodicCounterCapture.hpp
index 2e9ac364f1..4a28711d6b 100644
--- a/src/profiling/PeriodicCounterCapture.hpp
+++ b/src/profiling/PeriodicCounterCapture.hpp
@@ -38,14 +38,14 @@ public:
 
     void Start() override;
     void Stop() override;
-    bool IsRunning() const { return m_IsRunning.load(); }
+    bool IsRunning() const { return m_IsRunning; }
 
 private:
     CaptureData ReadCaptureData();
     void Capture(const IReadCounterValues& readCounterValues);
 
     const Holder&             m_CaptureDataHolder;
-    std::atomic<bool>         m_IsRunning;
+    bool                      m_IsRunning;
     std::atomic<bool>         m_KeepRunning;
     std::thread               m_PeriodCaptureThread;
     const IReadCounterValues& m_ReadCounterValues;
diff --git a/src/profiling/PeriodicCounterSelectionCommandHandler.cpp b/src/profiling/PeriodicCounterSelectionCommandHandler.cpp
index db09856dae..3df0f22c1c 100644
--- a/src/profiling/PeriodicCounterSelectionCommandHandler.cpp
+++ b/src/profiling/PeriodicCounterSelectionCommandHandler.cpp
@@ -109,8 +109,16 @@ void PeriodicCounterSelectionCommandHandler::operator()(const Packet& packet)
         // Notify the Send Thread that new data is available in the Counter Stream Buffer
         m_SendCounterPacket.SetReadyToRead();
 
-        // Start the Period Counter Capture thread (if not running already)
-        m_PeriodicCounterCapture.Start();
+        if (capturePeriod == 0 || validCounterIds.empty())
+        {
+            // No data capture stop the thread
+            m_PeriodicCounterCapture.Stop();
+        }
+        else
+        {
+            // Start the Period Counter Capture thread (if not running already)
+            m_PeriodicCounterCapture.Start();
+        }
 
         break;
     }
diff --git a/src/profiling/test/ProfilingTests.cpp b/src/profiling/test/ProfilingTests.cpp
index 6437521635..033f64ae24 100644
--- a/src/profiling/test/ProfilingTests.cpp
+++ b/src/profiling/test/ProfilingTests.cpp
@@ -1805,7 +1805,7 @@ BOOST_AUTO_TEST_CASE(CounterSelectionCommandHandlerParseData)
 
     BOOST_TEST(((headerWord0 >> 26) & 0x3F) == 0);  // packet family
     BOOST_TEST(((headerWord0 >> 16) & 0x3FF) == 4); // packet id
-    BOOST_TEST(headerWord1 == 4);                   // data lenght
+    BOOST_TEST(headerWord1 == 4);                   // data length
     BOOST_TEST(period == 11);                       // capture period
 }
 
@@ -2044,42 +2044,46 @@ BOOST_AUTO_TEST_CASE(CheckPeriodicCounterCaptureThread)
     class CaptureReader : public IReadCounterValues
     {
     public:
-        CaptureReader() {}
-
+        CaptureReader(uint16_t counterSize)
+        {
+            for(uint16_t i = 0; i < counterSize; ++i)
+            {
+                m_Data[i] = 0;
+            }
+            m_CounterSize = counterSize;
+        }
+        //not used
         bool IsCounterRegistered(uint16_t counterUid) const override
         {
-            return m_Data.find(counterUid) != m_Data.end();
+            return false;
         }
 
         uint16_t GetCounterCount() const override
         {
-            return boost::numeric_cast<uint16_t>(m_Data.size());
+            return m_CounterSize;
         }
 
         uint32_t GetCounterValue(uint16_t counterUid) const override
         {
-            if (m_Data.find(counterUid) == m_Data.end())
+            if(counterUid > m_CounterSize)
             {
-                return 0;
+                BOOST_FAIL("Invalid counter Uid");
             }
-
             return m_Data.at(counterUid).load();
         }
 
         void SetCounterValue(uint16_t counterUid, uint32_t value)
         {
-            if (m_Data.find(counterUid) == m_Data.end())
-            {
-                m_Data.insert(std::make_pair(counterUid, value));
-            }
-            else
+            if(counterUid > m_CounterSize)
             {
-                m_Data.at(counterUid).store(value);
+                BOOST_FAIL("Invalid counter Uid");
             }
+            m_Data.at(counterUid).store(value);
         }
 
     private:
         std::unordered_map<uint16_t, std::atomic<uint32_t>> m_Data;
+        uint16_t m_CounterSize;
     };
 
     ProfilingStateMachine profilingStateMachine;
@@ -2092,7 +2096,7 @@ BOOST_AUTO_TEST_CASE(CheckPeriodicCounterCaptureThread)
     SendCounterPacket sendCounterPacket(profilingStateMachine, mockBuffer);
 
     std::vector<uint16_t> counterIds;
-    CaptureReader captureReader;
+    CaptureReader captureReader(2);
 
     unsigned int valueA = 10;
     unsigned int valueB = 15;
@@ -2107,18 +2111,9 @@ BOOST_AUTO_TEST_CASE(CheckPeriodicCounterCaptureThread)
         captureReader.SetCounterValue(1, valueB * (i + 1));
 
         periodicCounterCapture.Start();
-
-        std::this_thread::sleep_for(std::chrono::milliseconds(200));
-
-        periodicCounterCapture.Start();
-
-        data.SetCaptureData(0, captureIds2);
-
-        periodicCounterCapture.Start();
+        periodicCounterCapture.Stop();
     }
 
-    periodicCounterCapture.Stop();
-
     auto buffer = mockBuffer.GetReadableBuffer();
 
     uint32_t headerWord0 = ReadUint32(buffer, 0);