From 0270524f96c4e21a755d1c71e46c4e8665918237 Mon Sep 17 00:00:00 2001 From: Colm Donelan Date: Thu, 14 Nov 2019 14:19:07 +0000 Subject: IVGCVSW-4129 Fix thread starvation due to low capture periods * Set default capture period to 10mSec. * Validate capture period in PeriodicCounterSelectionCommandHandler pull it up to 10mSec if it is lower. * Fix segmentation fault in GatordMock when receive thread closes. Signed-off-by: Colm Donelan Change-Id: I9f7ddc70bd99c102c5baef872d28329976a4dc07 --- include/armnn/IRuntime.hpp | 2 +- include/armnn/Types.hpp | 3 +++ .../PeriodicCounterSelectionCommandHandler.cpp | 8 +++++++- src/profiling/test/ProfilingTests.cpp | 24 ++++++++++++---------- tests/profiling/gatordmock/CommandFileParser.cpp | 2 +- tests/profiling/gatordmock/GatordMockService.cpp | 18 +++++++++++++--- tests/profiling/gatordmock/GatordMockService.hpp | 7 +++++++ 7 files changed, 47 insertions(+), 17 deletions(-) diff --git a/include/armnn/IRuntime.hpp b/include/armnn/IRuntime.hpp index 8003fd9514..08db22e4bb 100644 --- a/include/armnn/IRuntime.hpp +++ b/include/armnn/IRuntime.hpp @@ -64,7 +64,7 @@ public: , m_OutgoingCaptureFile("") , m_IncomingCaptureFile("") , m_FileOnly(false) - , m_CapturePeriod(150u) + , m_CapturePeriod(LOWEST_CAPTURE_PERIOD) {} bool m_EnableProfiling; diff --git a/include/armnn/Types.hpp b/include/armnn/Types.hpp index 3d3ab65c4a..94f45302c7 100644 --- a/include/armnn/Types.hpp +++ b/include/armnn/Types.hpp @@ -16,6 +16,9 @@ namespace armnn constexpr unsigned int MaxNumOfTensorDimensions = 5U; +// The lowest performance data capture interval we support is 10 miliseconds. +constexpr unsigned int LOWEST_CAPTURE_PERIOD = 10000u; + /// @enum Status enumeration /// @var Status::Successful /// @var Status::Failure diff --git a/src/profiling/PeriodicCounterSelectionCommandHandler.cpp b/src/profiling/PeriodicCounterSelectionCommandHandler.cpp index 3df0f22c1c..a6b6a050ad 100644 --- a/src/profiling/PeriodicCounterSelectionCommandHandler.cpp +++ b/src/profiling/PeriodicCounterSelectionCommandHandler.cpp @@ -6,6 +6,7 @@ #include "PeriodicCounterSelectionCommandHandler.hpp" #include "ProfilingUtils.hpp" +#include #include #include @@ -82,7 +83,12 @@ void PeriodicCounterSelectionCommandHandler::operator()(const Packet& packet) ParseData(packet, captureData); // Get the capture data - const uint32_t capturePeriod = captureData.GetCapturePeriod(); + uint32_t capturePeriod = captureData.GetCapturePeriod(); + // Validate that the capture period is within the acceptable range. + if (capturePeriod > 0 && capturePeriod < LOWEST_CAPTURE_PERIOD) + { + capturePeriod = LOWEST_CAPTURE_PERIOD; + } const std::vector& counterIds = captureData.GetCounterIds(); // Check whether the selected counter UIDs are valid diff --git a/src/profiling/test/ProfilingTests.cpp b/src/profiling/test/ProfilingTests.cpp index 9c8c6cdec7..4c4ec0ad42 100644 --- a/src/profiling/test/ProfilingTests.cpp +++ b/src/profiling/test/ProfilingTests.cpp @@ -26,6 +26,7 @@ #include #include +#include #include @@ -1612,7 +1613,7 @@ BOOST_AUTO_TEST_CASE(CounterSelectionCommandHandlerParseData) uint32_t sizeOfUint16 = numeric_cast(sizeof(uint16_t)); // Data with period and counters - uint32_t period1 = 10; + uint32_t period1 = armnn::LOWEST_CAPTURE_PERIOD; uint32_t dataLength1 = 8; uint32_t offset = 0; @@ -1656,10 +1657,10 @@ BOOST_AUTO_TEST_CASE(CounterSelectionCommandHandlerParseData) offset += sizeOfUint32; uint32_t period = ReadUint32(readBuffer, offset); - BOOST_TEST(((headerWord0 >> 26) & 0x3F) == 0); // packet family - BOOST_TEST(((headerWord0 >> 16) & 0x3FF) == 4); // packet id - BOOST_TEST(headerWord1 == 8); // data lenght - BOOST_TEST(period == 10); // capture period + BOOST_TEST(((headerWord0 >> 26) & 0x3F) == 0); // packet family + BOOST_TEST(((headerWord0 >> 16) & 0x3FF) == 4); // packet id + BOOST_TEST(headerWord1 == 8); // data length + BOOST_TEST(period == armnn::LOWEST_CAPTURE_PERIOD); // capture period uint16_t counterId = 0; offset += sizeOfUint32; @@ -1672,7 +1673,7 @@ BOOST_AUTO_TEST_CASE(CounterSelectionCommandHandlerParseData) mockBuffer.MarkRead(readBuffer); // Data with period only - uint32_t period2 = 11; + uint32_t period2 = 9000; // We'll specify a value below LOWEST_CAPTURE_PERIOD. It should be pulled upwards. uint32_t dataLength2 = 4; std::unique_ptr uniqueData2 = std::make_unique(dataLength2); @@ -1685,7 +1686,8 @@ BOOST_AUTO_TEST_CASE(CounterSelectionCommandHandlerParseData) const std::vector counterIdsB = holder.GetCaptureData().GetCounterIds(); - BOOST_TEST(holder.GetCaptureData().GetCapturePeriod() == period2); + // Value should have been pulled up from 9000 to LOWEST_CAPTURE_PERIOD. + BOOST_TEST(holder.GetCaptureData().GetCapturePeriod() == armnn::LOWEST_CAPTURE_PERIOD); BOOST_TEST(counterIdsB.size() == 0); readBuffer = mockBuffer.GetReadableBuffer(); @@ -1698,10 +1700,10 @@ BOOST_AUTO_TEST_CASE(CounterSelectionCommandHandlerParseData) offset += sizeOfUint32; period = ReadUint32(readBuffer, offset); - BOOST_TEST(((headerWord0 >> 26) & 0x3F) == 0); // packet family - BOOST_TEST(((headerWord0 >> 16) & 0x3FF) == 4); // packet id - BOOST_TEST(headerWord1 == 4); // data length - BOOST_TEST(period == 11); // capture period + BOOST_TEST(((headerWord0 >> 26) & 0x3F) == 0); // packet family + BOOST_TEST(((headerWord0 >> 16) & 0x3FF) == 4); // packet id + BOOST_TEST(headerWord1 == 4); // data length + BOOST_TEST(period == armnn::LOWEST_CAPTURE_PERIOD); // capture period } BOOST_AUTO_TEST_CASE(CheckConnectionAcknowledged) diff --git a/tests/profiling/gatordmock/CommandFileParser.cpp b/tests/profiling/gatordmock/CommandFileParser.cpp index d08e72cadd..4a8a19b5d2 100644 --- a/tests/profiling/gatordmock/CommandFileParser.cpp +++ b/tests/profiling/gatordmock/CommandFileParser.cpp @@ -23,7 +23,7 @@ void CommandFileParser::ParseFile(std::string CommandFile, GatordMockService& mo std::cout << "Parsing command file: " << CommandFile << std::endl; - while (std::getline(infile, line)) + while (mockService.ReceiveThreadRunning() && std::getline(infile, line)) { std::istringstream iss(line); diff --git a/tests/profiling/gatordmock/GatordMockService.cpp b/tests/profiling/gatordmock/GatordMockService.cpp index 1cdb024273..529ef063dd 100644 --- a/tests/profiling/gatordmock/GatordMockService.cpp +++ b/tests/profiling/gatordmock/GatordMockService.cpp @@ -166,7 +166,11 @@ bool GatordMockService::LaunchReceivingThread() void GatordMockService::WaitForReceivingThread() { - m_CloseReceivingThread.store(true); + // The receiving thread may already have died. + if (m_CloseReceivingThread != true) + { + m_CloseReceivingThread.store(true); + } // Check that the receiving thread is running if (m_ListeningThread.joinable()) { @@ -210,8 +214,16 @@ void GatordMockService::SendPeriodicCounterSelectionList(uint32_t period, std::v void GatordMockService::WaitCommand(uint timeout) { - std::this_thread::sleep_for(std::chrono::microseconds(timeout)); - + // Wait for a maximum of timeout microseconds or if the receive thread has closed. + // There is a certain level of rounding involved in this timing. + uint iterations = timeout / 1000; + std::cout << std::dec << "Wait command with timeout of " << timeout << " iterations = " << iterations << std::endl; + uint count = 0; + while ((this->ReceiveThreadRunning() && (count < iterations))) + { + std::this_thread::sleep_for(std::chrono::microseconds(1000)); + ++count; + } if (m_EchoPackets) { std::cout << std::dec << "Wait command with timeout of " << timeout << " microseconds completed. " << std::endl; diff --git a/tests/profiling/gatordmock/GatordMockService.hpp b/tests/profiling/gatordmock/GatordMockService.hpp index a77d7ff480..c3afc333ca 100644 --- a/tests/profiling/gatordmock/GatordMockService.hpp +++ b/tests/profiling/gatordmock/GatordMockService.hpp @@ -41,6 +41,7 @@ public: GatordMockService(armnn::profiling::CommandHandlerRegistry& registry, bool echoPackets) : m_HandlerRegistry(registry) , m_EchoPackets(echoPackets) + , m_CloseReceivingThread(false) { m_PacketsReceivedCount.store(0, std::memory_order_relaxed); } @@ -86,6 +87,12 @@ public: /// command handling code is added. void WaitForReceivingThread(); + // @return true only if the receive thread is closed or closing. + bool ReceiveThreadRunning() + { + return !m_CloseReceivingThread.load(); + } + /// Send the counter list to ArmNN. void SendPeriodicCounterSelectionList(uint32_t period, std::vector counters); -- cgit v1.2.1