From: Matteo Martincigh Date: Fri, 4 Oct 2019 16:17:42 +0000 (+0100) Subject: IVGCVSW-3937 Refactor and improve the PeriodicCounterCapture class X-Git-Tag: submit/tizen/20200316.035456~186 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=e0e6efc1072358b843f47d2ffffc3d873a4889c6;p=platform%2Fupstream%2Farmnn.git 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 Change-Id: I8c42aa17e17a90cda5cf86eb8ac2d13501ecdadc --- diff --git a/CMakeLists.txt b/CMakeLists.txt index 8ef2949..fc68f3a 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 d8d1f5b..72ca091 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 @@ -17,11 +18,14 @@ class CaptureData { public: CaptureData() - : m_CapturePeriod(0), m_CounterIds() {}; + : m_CapturePeriod(0) + , m_CounterIds() {} CaptureData(uint32_t capturePeriod, std::vector& 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& counterIds); diff --git a/src/profiling/CounterValues.hpp b/src/profiling/ICounterValues.hpp similarity index 99% rename from src/profiling/CounterValues.hpp rename to src/profiling/ICounterValues.hpp index 9c06ff0..5e32ca2 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 diff --git a/src/profiling/IPeriodicCounterCapture.hpp b/src/profiling/IPeriodicCounterCapture.hpp index edc034d..bec41dc 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 3a8236a..0000000 --- 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 5fbaa5d..9002bfc 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 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> 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(timestamp.time_since_epoch().count()), values); - std::this_thread::sleep_for(std::chrono::milliseconds(currentCaptureData.GetCapturePeriod())); - } - } -} -void PeriodicCounterCapture::Start() -{ - bool tstVal = false; + std::vector> 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(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 8a7ff37..2e9ac36 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 #include #include @@ -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 m_IsRunning; - std::thread m_PeriodCaptureThread; - const IReadCounterValue& m_ReadCounterValue; - ISendCounterPacket& m_SendCounterPacket; + void Capture(const IReadCounterValues& readCounterValues); + + const Holder& m_CaptureDataHolder; + std::atomic m_IsRunning; + std::atomic 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 36d95e0..b4cdcac 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 7f3696a..813cccf 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 bc962e3..71d9dcf 100644 --- a/src/profiling/test/ProfilingTests.cpp +++ b/src/profiling/test/ProfilingTests.cpp @@ -13,6 +13,7 @@ #include #include #include +#include #include #include #include @@ -23,7 +24,6 @@ #include #include #include -#include #include @@ -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(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(index, value)); } @@ -2129,7 +2133,7 @@ BOOST_AUTO_TEST_CASE(CheckPeriodicCounterCaptureThread) } private: - std::map m_Data; + std::unordered_map 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();