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 <Colm.Donelan@arm.com>
Change-Id: I9f7ddc70bd99c102c5baef872d28329976a4dc07

diff --git a/include/armnn/IRuntime.hpp b/include/armnn/IRuntime.hpp
index 8003fd9..08db22e 100644 (file)
--- 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 3d3ab65..94f4530 100644 (file)
--- 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 3df0f22..a6b6a05 100644 (file)
--- a/src/profiling/PeriodicCounterSelectionCommandHandler.cpp
+++ b/src/profiling/PeriodicCounterSelectionCommandHandler.cpp
@@ -6,6 +6,7 @@
 #include "PeriodicCounterSelectionCommandHandler.hpp"
 #include "ProfilingUtils.hpp"
 
+#include <armnn/Types.hpp>
 #include <boost/numeric/conversion/cast.hpp>
 #include <boost/format.hpp>
 
@@ -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<uint16_t>& 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 9c8c6cd..4c4ec0a 100644 (file)
--- a/src/profiling/test/ProfilingTests.cpp
+++ b/src/profiling/test/ProfilingTests.cpp
@@ -26,6 +26,7 @@
 #include <SendTimelinePacket.hpp>
 
 #include <armnn/Conversion.hpp>
+#include <armnn/Types.hpp>
 
 #include <armnn/Utils.hpp>
 
@@ -1612,7 +1613,7 @@ BOOST_AUTO_TEST_CASE(CounterSelectionCommandHandlerParseData)
     uint32_t sizeOfUint16 = numeric_cast<uint32_t>(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<unsigned char[]> uniqueData2 = std::make_unique<unsigned char[]>(dataLength2);
@@ -1685,7 +1686,8 @@ BOOST_AUTO_TEST_CASE(CounterSelectionCommandHandlerParseData)
 
     const std::vector<uint16_t> 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 d08e72c..4a8a19b 100644 (file)
--- 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 1cdb024..529ef06 100644 (file)
--- 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 a77d7ff..c3afc33 100644 (file)
--- 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<uint16_t> counters);