From 149528e88c081e71fc7ec78e0c301eb2e487adfc Mon Sep 17 00:00:00 2001 From: Matteo Martincigh Date: Thu, 5 Sep 2019 12:02:04 +0100 Subject: [PATCH] IVGCVSW-3691 Basic refactoring in view of upcoming work in the profiler Change-Id: Iea4550b864fc2adb04a3a2411a7ead06b1f60ab9 Signed-off-by: Matteo Martincigh --- src/profiling/CounterDirectory.hpp | 10 ++++- src/profiling/ISendCounterPacket.hpp | 2 +- src/profiling/Packet.hpp | 25 ++++++------ src/profiling/ProfilingUtils.cpp | 12 +++--- src/profiling/SendCounterPacket.cpp | 58 ++++++++++++--------------- src/profiling/SendCounterPacket.hpp | 17 +++++++- src/profiling/test/SendCounterPacketTests.cpp | 11 +++-- 7 files changed, 73 insertions(+), 62 deletions(-) diff --git a/src/profiling/CounterDirectory.hpp b/src/profiling/CounterDirectory.hpp index bbe7bcf..ec1ac27 100644 --- a/src/profiling/CounterDirectory.hpp +++ b/src/profiling/CounterDirectory.hpp @@ -15,6 +15,12 @@ namespace armnn namespace profiling { +class Category +{ +public: + std::string m_Name; +}; + class Device { public: @@ -36,10 +42,12 @@ public: std::string m_Units; }; -class Category +class CounterSet { public: + uint16_t m_Uid; std::string m_Name; + uint16_t m_Count; }; class CounterDirectory final diff --git a/src/profiling/ISendCounterPacket.hpp b/src/profiling/ISendCounterPacket.hpp index 5117c5d..a64e0dd 100644 --- a/src/profiling/ISendCounterPacket.hpp +++ b/src/profiling/ISendCounterPacket.hpp @@ -24,7 +24,7 @@ public: virtual void SendStreamMetaDataPacket() = 0; /// Create and write a CounterDirectoryPacket from the parameters to the buffer. - virtual void SendCounterDirectoryPacket(const Category& category, const std::vector& counters) = 0; + virtual void SendCounterDirectoryPacket(const CounterDirectory& counterDirectory) = 0; /// Create and write a PeriodicCounterCapturePacket from the parameters to the buffer. virtual void SendPeriodicCounterCapturePacket(uint64_t timestamp, const IndexValuePairsVector& values) = 0; diff --git a/src/profiling/Packet.hpp b/src/profiling/Packet.hpp index ce37844..b350f7c 100644 --- a/src/profiling/Packet.hpp +++ b/src/profiling/Packet.hpp @@ -18,19 +18,18 @@ class Packet { public: Packet(uint32_t header, uint32_t length, const char* data) - : m_Header(header), m_Length(length), m_Data(data) - { - m_PacketId = ((header >> 16) & 1023); - m_PacketFamily = (header >> 26); - - if (length == 0) - { - if (m_Data != nullptr) - { - throw armnn::Exception("Data should be null"); - } - } - }; + : m_Header(header), + m_Length(length), + m_Data(data) + { + m_PacketId = ((header >> 16) & 1023); + m_PacketFamily = (header >> 26); + + if (length == 0 && m_Data != nullptr) + { + throw armnn::InvalidArgumentException("Data should be null when length is zero"); + } + } uint32_t GetHeader() const; uint32_t GetPacketFamily() const; diff --git a/src/profiling/ProfilingUtils.cpp b/src/profiling/ProfilingUtils.cpp index 86abef1..015a66e 100644 --- a/src/profiling/ProfilingUtils.cpp +++ b/src/profiling/ProfilingUtils.cpp @@ -35,7 +35,7 @@ void WriteUint32(unsigned char* buffer, unsigned int offset, uint32_t value) { BOOST_ASSERT(buffer); - buffer[offset] = static_cast(value & 0xFF); + buffer[offset] = static_cast(value & 0xFF); buffer[offset + 1] = static_cast((value >> 8) & 0xFF); buffer[offset + 2] = static_cast((value >> 16) & 0xFF); buffer[offset + 3] = static_cast((value >> 24) & 0xFF); @@ -43,9 +43,9 @@ void WriteUint32(unsigned char* buffer, unsigned int offset, uint32_t value) void WriteUint16(unsigned char* buffer, unsigned int offset, uint16_t value) { - BOOST_ASSERT(buffer != nullptr); + BOOST_ASSERT(buffer); - buffer[offset] = static_cast(value & 0xFF); + buffer[offset] = static_cast(value & 0xFF); buffer[offset + 1] = static_cast((value >> 8) & 0xFF); } @@ -71,7 +71,7 @@ uint32_t ReadUint32(const unsigned char* buffer, unsigned int offset) BOOST_ASSERT(buffer); uint32_t value = 0; - value = static_cast(buffer[offset]); + value = static_cast(buffer[offset]); value |= static_cast(buffer[offset + 1]) << 8; value |= static_cast(buffer[offset + 2]) << 16; value |= static_cast(buffer[offset + 3]) << 24; @@ -83,7 +83,7 @@ uint16_t ReadUint16(const unsigned char* buffer, unsigned int offset) BOOST_ASSERT(buffer); uint32_t value = 0; - value = static_cast(buffer[offset]); + value = static_cast(buffer[offset]); value |= static_cast(buffer[offset + 1]) << 8; return static_cast(value); } @@ -115,4 +115,4 @@ std::string GetProcessName() } // namespace profiling -} // namespace armnn \ No newline at end of file +} // namespace armnn diff --git a/src/profiling/SendCounterPacket.cpp b/src/profiling/SendCounterPacket.cpp index 61d32da..94a5db2 100644 --- a/src/profiling/SendCounterPacket.cpp +++ b/src/profiling/SendCounterPacket.cpp @@ -13,9 +13,7 @@ #include #include -#include -#include -#include +#include namespace armnn { @@ -69,17 +67,14 @@ void SendCounterPacket::SendStreamMetaDataPacket() if (reserved < totalSize) { - // Cancel the operation. - m_Buffer.Commit(0); - throw RuntimeException(boost::str(boost::format("No space left in buffer. Unable to reserve (%1%) bytes.") - % totalSize)); + CancelOperationAndThrow( + boost::str(boost::format("No space left in buffer. Unable to reserve (%1%) bytes.") + % totalSize)); } if (writeBuffer == nullptr) { - // Cancel the operation. - m_Buffer.Commit(0); - throw RuntimeException("Error reserving buffer memory."); + CancelOperationAndThrow("Error reserving buffer memory."); } try @@ -121,27 +116,32 @@ void SendCounterPacket::SendStreamMetaDataPacket() // Pool - if (infoSize) { + if (infoSize) + { memcpy(&writeBuffer[offset], info.c_str(), infoSize); offset += infoSize; } - if (hardwareVersionSize) { + if (hardwareVersionSize) + { memcpy(&writeBuffer[offset], hardwareVersion.c_str(), hardwareVersionSize); offset += hardwareVersionSize; } - if (softwareVersionSize) { + if (softwareVersionSize) + { memcpy(&writeBuffer[offset], softwareVersion.c_str(), softwareVersionSize); offset += softwareVersionSize; } - if (processNameSize) { + if (processNameSize) + { memcpy(&writeBuffer[offset], processName.c_str(), processNameSize); offset += processNameSize; } - if (packetVersionEntries) { + if (packetVersionEntries) + { // Packet Version Count WriteUint32(writeBuffer, offset, packetVersionEntries << 16); @@ -160,15 +160,13 @@ void SendCounterPacket::SendStreamMetaDataPacket() } catch(...) { - // Cancel the operation. - m_Buffer.Commit(0); - throw RuntimeException("Error processing packet."); + CancelOperationAndThrow("Error processing packet."); } m_Buffer.Commit(totalSize); } -void SendCounterPacket::SendCounterDirectoryPacket(const Category& category, const std::vector& counters) +void SendCounterPacket::SendCounterDirectoryPacket(const CounterDirectory& counterDirectory) { throw armnn::UnimplementedException(); } @@ -189,17 +187,14 @@ void SendCounterPacket::SendPeriodicCounterCapturePacket(uint64_t timestamp, con if (reserved < totalSize) { - // Cancel the operation. - m_Buffer.Commit(0); - throw profiling::BufferExhaustion( - boost::str(boost::format("No space left in buffer. Unable to reserve (%1%) bytes.") % totalSize)); + CancelOperationAndThrow( + boost::str(boost::format("No space left in buffer. Unable to reserve (%1%) bytes.") + % totalSize)); } if (writeBuffer == nullptr) { - // Cancel the operation. - m_Buffer.Commit(0); - throw RuntimeException("Error reserving buffer memory."); + CancelOperationAndThrow("Error reserving buffer memory."); } // Create header. @@ -242,17 +237,14 @@ void SendCounterPacket::SendPeriodicCounterSelectionPacket(uint32_t capturePerio if (reserved < totalSize) { - // Cancel the operation. - m_Buffer.Commit(0); - throw RuntimeException(boost::str(boost::format("No space left in buffer. Unable to reserve (%1%) bytes.") + CancelOperationAndThrow( + boost::str(boost::format("No space left in buffer. Unable to reserve (%1%) bytes.") % totalSize)); } if (writeBuffer == nullptr) { - // Cancel the operation. - m_Buffer.Commit(0); - throw RuntimeException("Error reserving buffer memory."); + CancelOperationAndThrow("Error reserving buffer memory."); } // Create header. @@ -282,4 +274,4 @@ void SendCounterPacket::SetReadyToRead() } // namespace profiling -} // namespace armnn \ No newline at end of file +} // namespace armnn diff --git a/src/profiling/SendCounterPacket.hpp b/src/profiling/SendCounterPacket.hpp index 0fc3055..3238b35 100644 --- a/src/profiling/SendCounterPacket.hpp +++ b/src/profiling/SendCounterPacket.hpp @@ -20,11 +20,14 @@ class SendCounterPacket : public ISendCounterPacket public: using IndexValuePairsVector = std::vector>; - SendCounterPacket(IBufferWrapper& buffer) : m_Buffer(buffer), m_ReadyToRead(false) {}; + SendCounterPacket(IBufferWrapper& buffer) + : m_Buffer(buffer), + m_ReadyToRead(false) + {} void SendStreamMetaDataPacket() override; - void SendCounterDirectoryPacket(const Category& category, const std::vector& counters) override; + void SendCounterDirectoryPacket(const CounterDirectory& counterDirectory) override; void SendPeriodicCounterCapturePacket(uint64_t timestamp, const IndexValuePairsVector& values) override; @@ -37,6 +40,16 @@ public: static const unsigned int MAX_METADATA_PACKET_LENGTH = 4096; private: + template + void CancelOperationAndThrow(const std::string& errorMessage) + { + // Cancel the operation + m_Buffer.Commit(0); + + // Throw a runtime exception with the given error message + throw ExceptionType(errorMessage); + } + IBufferWrapper& m_Buffer; bool m_ReadyToRead; }; diff --git a/src/profiling/test/SendCounterPacketTests.cpp b/src/profiling/test/SendCounterPacketTests.cpp index 5309560..626a5a6 100644 --- a/src/profiling/test/SendCounterPacketTests.cpp +++ b/src/profiling/test/SendCounterPacketTests.cpp @@ -68,7 +68,7 @@ public: memcpy(buffer, message.c_str(), static_cast(message.size()) + 1); } - void SendCounterDirectoryPacket(const Category& category, const std::vector& counters) override + void SendCounterDirectoryPacket(const CounterDirectory& counterDirectory) override { std::string message("SendCounterDirectoryPacket"); unsigned int reserved = 0; @@ -114,9 +114,8 @@ BOOST_AUTO_TEST_CASE(MockSendCounterPacketTest) BOOST_TEST(strcmp(buffer, "SendStreamMetaDataPacket") == 0); - Category category; - std::vector counters; - sendCounterPacket.SendCounterDirectoryPacket(category, counters); + CounterDirectory counterDirectory(1, "counter_directory", 0, 0, 0); + sendCounterPacket.SendCounterDirectoryPacket(counterDirectory); BOOST_TEST(strcmp(buffer, "SendCounterDirectoryPacket") == 0); @@ -144,7 +143,7 @@ BOOST_AUTO_TEST_CASE(SendPeriodicCounterSelectionPacketTest) uint32_t capturePeriod = 1000; std::vector selectedCounterIds; BOOST_CHECK_THROW(sendPacket1.SendPeriodicCounterSelectionPacket(capturePeriod, selectedCounterIds), - armnn::RuntimeException); + armnn::profiling::BufferExhaustion); // Packet without any counters MockBuffer mockBuffer2(512); @@ -284,7 +283,7 @@ BOOST_AUTO_TEST_CASE(SendStreamMetaDataPacketTest) // Error no space left in buffer MockBuffer mockBuffer1(10); SendCounterPacket sendPacket1(mockBuffer1); - BOOST_CHECK_THROW(sendPacket1.SendStreamMetaDataPacket(), armnn::RuntimeException); + BOOST_CHECK_THROW(sendPacket1.SendStreamMetaDataPacket(), armnn::profiling::BufferExhaustion); // Full metadata packet -- 2.7.4