IVGCVSW-3937 Refactor and improve the PeriodicCounterCapture class

 * Conformed the PeriodicCounterCapture class to the other thread-based
   classes
 * Code refactoring
 * Renamed CounterValues file to ICounterValues
 * Removed no longer used file
 * Updated unit tests accordingly

Signed-off-by: Matteo Martincigh <matteo.martincigh@arm.com>
Change-Id: I8c42aa17e17a90cda5cf86eb8ac2d13501ecdadc

10 files changed, 111 insertions, 115 deletions

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 8ef2949ed6..fc68f3afb3 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -440,17 +440,16 @@ list(APPEND armnn_sources
     src/profiling/ConnectionAcknowledgedCommandHandler.hpp
     src/profiling/CounterDirectory.cpp
     src/profiling/CounterDirectory.hpp
-    src/profiling/CounterValues.hpp
     src/profiling/EncodeVersion.hpp
     src/profiling/Holder.cpp
     src/profiling/Holder.hpp
     src/profiling/IBufferManager.hpp
     src/profiling/ICounterDirectory.hpp
+    src/profiling/ICounterValues.hpp
     src/profiling/ISendCounterPacket.hpp
     src/profiling/IPacketBuffer.hpp
     src/profiling/IPeriodicCounterCapture.hpp
     src/profiling/IProfilingConnection.hpp
-    src/profiling/IReadCounterValue.hpp
     src/profiling/Packet.cpp
     src/profiling/Packet.hpp
     src/profiling/PacketBuffer.cpp
diff --git a/src/profiling/Holder.hpp b/src/profiling/Holder.hpp
index d8d1f5bfb4..72ca0914a9 100644
--- a/src/profiling/Holder.hpp
+++ b/src/profiling/Holder.hpp
@@ -2,6 +2,7 @@
 // Copyright © 2017 Arm Ltd. All rights reserved.
 // SPDX-License-Identifier: MIT
 //
+
 #pragma once
 
 #include <mutex>
@@ -17,11 +18,14 @@ class CaptureData
 {
 public:
     CaptureData()
-        : m_CapturePeriod(0), m_CounterIds() {};
+        : m_CapturePeriod(0)
+        , m_CounterIds() {}
     CaptureData(uint32_t capturePeriod, std::vector<uint16_t>& counterIds)
-        : m_CapturePeriod(capturePeriod), m_CounterIds(counterIds) {};
+        : m_CapturePeriod(capturePeriod)
+        , m_CounterIds(counterIds) {}
     CaptureData(const CaptureData& captureData)
-        : m_CapturePeriod(captureData.m_CapturePeriod), m_CounterIds(captureData.m_CounterIds) {};
+        : m_CapturePeriod(captureData.m_CapturePeriod)
+        , m_CounterIds(captureData.m_CounterIds) {}
 
     CaptureData& operator= (const CaptureData& captureData);
 
@@ -39,7 +43,7 @@ class Holder
 {
 public:
     Holder()
-        : m_CaptureData() {};
+        : m_CaptureData() {}
     CaptureData GetCaptureData() const;
     void SetCaptureData(uint32_t capturePeriod, const std::vector<uint16_t>& counterIds);
 
diff --git a/src/profiling/CounterValues.hpp b/src/profiling/ICounterValues.hpp
index 9c06ff0a7d..5e32ca2b37 100644
--- a/src/profiling/CounterValues.hpp
+++ b/src/profiling/ICounterValues.hpp
@@ -2,6 +2,7 @@
 // Copyright © 2019 Arm Ltd. All rights reserved.
 // SPDX-License-Identifier: MIT
 //
+
 #pragma once
 
 #include <cstdint>
diff --git a/src/profiling/IPeriodicCounterCapture.hpp b/src/profiling/IPeriodicCounterCapture.hpp
index edc034d20e..bec41dce11 100644
--- a/src/profiling/IPeriodicCounterCapture.hpp
+++ b/src/profiling/IPeriodicCounterCapture.hpp
@@ -13,8 +13,10 @@ namespace profiling
 class IPeriodicCounterCapture
 {
 public:
-    virtual void Start() = 0;
     virtual ~IPeriodicCounterCapture() {}
+
+    virtual void Start() = 0;
+    virtual void Stop() = 0;
 };
 
 } // namespace profiling
diff --git a/src/profiling/IReadCounterValue.hpp b/src/profiling/IReadCounterValue.hpp
deleted file mode 100644
index 3a8236a0f4..0000000000
--- a/src/profiling/IReadCounterValue.hpp
+++ /dev/null
@@ -1,23 +0,0 @@
-//
-// Copyright © 2019 Arm Ltd. All rights reserved.
-// SPDX-License-Identifier: MIT
-//
-
-#pragma once
-
-namespace armnn
-{
-
-namespace profiling
-{
-
-class IReadCounterValue
-{
-public:
-    virtual void GetCounterValue(uint16_t index, uint32_t &value) const = 0;
-    virtual ~IReadCounterValue() {}
-};
-
-} // namespace profiling
-
-} // namespace armnn
\ No newline at end of file
diff --git a/src/profiling/PeriodicCounterCapture.cpp b/src/profiling/PeriodicCounterCapture.cpp
index 5fbaa5d035..9002bfc065 100644
--- a/src/profiling/PeriodicCounterCapture.cpp
+++ b/src/profiling/PeriodicCounterCapture.cpp
@@ -11,82 +11,82 @@ namespace armnn
 namespace profiling
 {
 
-PeriodicCounterCapture::PeriodicCounterCapture(const Holder& data, ISendCounterPacket& packet,
-                                               const IReadCounterValue& readCounterValue)
-    : m_CaptureDataHolder(data)
-    , m_IsRunning(false)
-    , m_ReadCounterValue(readCounterValue)
-    , m_SendCounterPacket(packet)
-{}
+void PeriodicCounterCapture::Start()
+{
+    // Check if the capture thread is already running
+    if (m_IsRunning.load())
+    {
+        // The capture thread is already running
+        return;
+    }
+
+    // Mark the capture thread as running
+    m_IsRunning.store(true);
+
+    // Keep the capture procedure going until the capture thread is signalled to stop
+    m_KeepRunning.store(true);
+
+    // Start the new capture thread.
+    m_PeriodCaptureThread = std::thread(&PeriodicCounterCapture::Capture,
+                                        this,
+                                        std::ref(m_ReadCounterValues));
+}
+
+void PeriodicCounterCapture::Stop()
+{
+    m_KeepRunning.store(false);
+
+    if (m_PeriodCaptureThread.joinable())
+    {
+        m_PeriodCaptureThread.join();
+    }
+}
 
 CaptureData PeriodicCounterCapture::ReadCaptureData()
 {
     return m_CaptureDataHolder.GetCaptureData();
 }
 
-void PeriodicCounterCapture::Functionality(const IReadCounterValue& readCounterValue)
+void PeriodicCounterCapture::Capture(const IReadCounterValues& readCounterValues)
 {
-    bool threadRunning = true;
-
-    while(threadRunning)
+    while (m_KeepRunning.load())
     {
         auto currentCaptureData = ReadCaptureData();
         std::vector<uint16_t> counterIds = currentCaptureData.GetCounterIds();
         if (currentCaptureData.GetCapturePeriod() == 0 || counterIds.empty())
         {
-            threadRunning = false;
-            m_IsRunning.store(false, std::memory_order_relaxed);
+            m_KeepRunning.store(false);
+            break;
         }
-        else
-        {
-            std::vector<std::pair<uint16_t, uint32_t>> values;
-            auto numCounters = counterIds.size();
-            values.reserve(numCounters);
-
-            // Create vector of pairs of CounterIndexes and Values
-            uint32_t counterValue;
-            for (uint16_t index = 0; index < numCounters; ++index)
-            {
-                auto requestedId = counterIds[index];
-                readCounterValue.GetCounterValue(requestedId, counterValue);
-                values.emplace_back(std::make_pair(requestedId, counterValue));
-            }
-
-            #if USE_CLOCK_MONOTONIC_RAW
-                using clock = MonotonicClockRaw;
-            #else
-                using clock = std::chrono::steady_clock;
-            #endif
-            // Take a timestamp
-            auto timestamp = clock::now();
-
-            m_SendCounterPacket.SendPeriodicCounterCapturePacket(
-                    static_cast<uint64_t>(timestamp.time_since_epoch().count()), values);
-            std::this_thread::sleep_for(std::chrono::milliseconds(currentCaptureData.GetCapturePeriod()));
-        }
-    }
-}
 
-void PeriodicCounterCapture::Start()
-{
-    bool tstVal = false;
+        std::vector<std::pair<uint16_t, uint32_t>> values;
+        auto numCounters = counterIds.size();
+        values.reserve(numCounters);
 
-    if (m_IsRunning.compare_exchange_strong(tstVal, true, std::memory_order_relaxed))
-    {
-        // Check that the thread execution is finished.
-        if (m_PeriodCaptureThread.joinable())
+        // Create vector of pairs of CounterIndexes and Values
+        uint32_t counterValue = 0;
+        for (uint16_t index = 0; index < numCounters; ++index)
         {
-            m_PeriodCaptureThread.join();
+            auto requestedId = counterIds[index];
+            counterValue = readCounterValues.GetCounterValue(requestedId);
+            values.emplace_back(std::make_pair(requestedId, counterValue));
         }
-        // Starts the new thread.
-        m_PeriodCaptureThread = std::thread(&PeriodicCounterCapture::Functionality, this,
-                                            std::ref(m_ReadCounterValue));
+
+        #if USE_CLOCK_MONOTONIC_RAW
+            using clock = MonotonicClockRaw;
+        #else
+            using clock = std::chrono::steady_clock;
+        #endif
+
+        // Take a timestamp
+        auto timestamp = clock::now();
+
+        m_SendCounterPacket.SendPeriodicCounterCapturePacket(
+                    static_cast<uint64_t>(timestamp.time_since_epoch().count()), values);
+        std::this_thread::sleep_for(std::chrono::milliseconds(currentCaptureData.GetCapturePeriod()));
     }
-}
 
-void PeriodicCounterCapture::Join()
-{
-    m_PeriodCaptureThread.join();
+    m_IsRunning.store(false);
 }
 
 } // namespace profiling
diff --git a/src/profiling/PeriodicCounterCapture.hpp b/src/profiling/PeriodicCounterCapture.hpp
index 8a7ff37bf9..2e9ac364f1 100644
--- a/src/profiling/PeriodicCounterCapture.hpp
+++ b/src/profiling/PeriodicCounterCapture.hpp
@@ -5,13 +5,13 @@
 
 #pragma once
 
-#include "Holder.hpp"
 #include "IPeriodicCounterCapture.hpp"
+#include "Holder.hpp"
 #include "Packet.hpp"
-#include "IReadCounterValue.hpp"
 #include "SendCounterPacket.hpp"
+#include "ICounterValues.hpp"
 
-#include "WallClockTimer.hpp"
+#include <WallClockTimer.hpp>
 
 #include <atomic>
 #include <chrono>
@@ -27,20 +27,29 @@ namespace profiling
 class PeriodicCounterCapture final : public IPeriodicCounterCapture
 {
 public:
-    PeriodicCounterCapture(const Holder& data, ISendCounterPacket& packet, const IReadCounterValue& readCounterValue);
+    PeriodicCounterCapture(const Holder& data, ISendCounterPacket& packet, const IReadCounterValues& readCounterValue)
+        : m_CaptureDataHolder(data)
+        , m_IsRunning(false)
+        , m_KeepRunning(false)
+        , m_ReadCounterValues(readCounterValue)
+        , m_SendCounterPacket(packet)
+    {}
+    ~PeriodicCounterCapture() { Stop(); }
 
     void Start() override;
-    void Join();
+    void Stop() override;
+    bool IsRunning() const { return m_IsRunning.load(); }
 
 private:
     CaptureData ReadCaptureData();
-    void Functionality(const IReadCounterValue& readCounterValue);
-
-    const Holder&            m_CaptureDataHolder;
-    std::atomic<bool>        m_IsRunning;
-    std::thread              m_PeriodCaptureThread;
-    const IReadCounterValue& m_ReadCounterValue;
-    ISendCounterPacket&      m_SendCounterPacket;
+    void Capture(const IReadCounterValues& readCounterValues);
+
+    const Holder&             m_CaptureDataHolder;
+    std::atomic<bool>         m_IsRunning;
+    std::atomic<bool>         m_KeepRunning;
+    std::thread               m_PeriodCaptureThread;
+    const IReadCounterValues& m_ReadCounterValues;
+    ISendCounterPacket&       m_SendCounterPacket;
 };
 
 } // namespace profiling
diff --git a/src/profiling/ProfilingService.hpp b/src/profiling/ProfilingService.hpp
index 36d95e0b5e..b4cdcac76e 100644
--- a/src/profiling/ProfilingService.hpp
+++ b/src/profiling/ProfilingService.hpp
@@ -8,7 +8,7 @@
 #include "ProfilingStateMachine.hpp"
 #include "ProfilingConnectionFactory.hpp"
 #include "CounterDirectory.hpp"
-#include "CounterValues.hpp"
+#include "ICounterValues.hpp"
 
 namespace armnn
 {
diff --git a/src/profiling/SendCounterPacket.cpp b/src/profiling/SendCounterPacket.cpp
index 7f3696a940..813cccfe6a 100644
--- a/src/profiling/SendCounterPacket.cpp
+++ b/src/profiling/SendCounterPacket.cpp
@@ -913,7 +913,7 @@ void SendCounterPacket::Start()
     // Mark the send thread as running
     m_IsRunning.store(true);
 
-    // Keep the send procedure going until the the send thread is signalled to stop
+    // Keep the send procedure going until the send thread is signalled to stop
     m_KeepRunning.store(true);
 
     // Start the send thread
diff --git a/src/profiling/test/ProfilingTests.cpp b/src/profiling/test/ProfilingTests.cpp
index bc962e3b17..71d9dcf381 100644
--- a/src/profiling/test/ProfilingTests.cpp
+++ b/src/profiling/test/ProfilingTests.cpp
@@ -13,6 +13,7 @@
 #include <CounterDirectory.hpp>
 #include <EncodeVersion.hpp>
 #include <Holder.hpp>
+#include <ICounterValues.hpp>
 #include <Packet.hpp>
 #include <PacketVersionResolver.hpp>
 #include <PeriodicCounterCapture.hpp>
@@ -23,7 +24,6 @@
 #include <RequestCounterDirectoryCommandHandler.hpp>
 #include <Runtime.hpp>
 #include <SocketProfilingConnection.hpp>
-#include <IReadCounterValue.hpp>
 
 #include <armnn/Conversion.hpp>
 
@@ -1769,7 +1769,8 @@ BOOST_AUTO_TEST_CASE(CounterSelectionCommandHandlerParseData)
 
     class TestCaptureThread : public IPeriodicCounterCapture
     {
-        void Start() override {};
+        void Start() override {}
+        void Stop() override {}
     };
 
     const uint32_t packetId = 0x40000;
@@ -2099,26 +2100,29 @@ BOOST_AUTO_TEST_CASE(StringToSwTraceNameStringTest)
 
 BOOST_AUTO_TEST_CASE(CheckPeriodicCounterCaptureThread)
 {
-    class CaptureReader : public IReadCounterValue
+    class CaptureReader : public IReadCounterValues
     {
     public:
         CaptureReader() {}
 
-        void GetCounterValue(uint16_t index, uint32_t &value) const override
+        uint16_t GetCounterCount() const override
         {
-            if (m_Data.count(index))
-            {
-                value = m_Data.at(index);
-            }
-            else
+            return boost::numeric_cast<uint16_t>(m_Data.size());
+        }
+
+        uint32_t GetCounterValue(uint16_t index) const override
+        {
+            if (m_Data.find(index) == m_Data.end())
             {
-                value = 0;
+                return 0;
             }
+
+            return m_Data.at(index);
         }
 
         void SetCounterValue(uint16_t index, uint32_t value)
         {
-            if (!m_Data.count(index))
+            if (m_Data.find(index) == m_Data.end())
             {
                 m_Data.insert(std::pair<uint16_t, uint32_t>(index, value));
             }
@@ -2129,7 +2133,7 @@ BOOST_AUTO_TEST_CASE(CheckPeriodicCounterCaptureThread)
         }
 
     private:
-        std::map<uint16_t, uint32_t> m_Data;
+        std::unordered_map<uint16_t, uint32_t> m_Data;
     };
 
     Holder data;
@@ -2149,7 +2153,7 @@ BOOST_AUTO_TEST_CASE(CheckPeriodicCounterCaptureThread)
 
     PeriodicCounterCapture periodicCounterCapture(std::ref(data), std::ref(sendCounterPacket), captureReader);
 
-    for(unsigned int i = 0; i < numSteps; ++i)
+    for (unsigned int i = 0; i < numSteps; ++i)
     {
         data.SetCaptureData(1, captureIds1);
         captureReader.SetCounterValue(0, valueA * (i + 1));
@@ -2166,7 +2170,7 @@ BOOST_AUTO_TEST_CASE(CheckPeriodicCounterCaptureThread)
         periodicCounterCapture.Start();
     }
 
-    periodicCounterCapture.Join();
+    periodicCounterCapture.Stop();
 
     auto buffer = mockBuffer.GetReadableBuffer();