IVGCVSW-3440 Fix intermittently failing send thread test
* Simplified the implementation of the MockStreamCounterBuffer
* Minor refactoring
Signed-off-by: Matteo Martincigh <matteo.martincigh@arm.com>
Change-Id: I3aed97daee0ac32d384e1f830e8cc57aae84950b
diff --git a/src/profiling/BufferManager.cpp b/src/profiling/BufferManager.cpp
index 91ad1c5..dbf4466 100644
--- a/src/profiling/BufferManager.cpp
+++ b/src/profiling/BufferManager.cpp
@@ -32,8 +32,8 @@
std::unique_lock<std::mutex> availableListLock(m_AvailableMutex, std::defer_lock);
if (requestedSize > m_MaxBufferSize)
{
- throw armnn::RuntimeException("Maximum buffer size that can be requested is [" +
- std::to_string(m_MaxBufferSize) + "] bytes");
+ throw armnn::InvalidArgumentException("The maximum buffer size that can be requested is [" +
+ std::to_string(m_MaxBufferSize) + "] bytes");
}
availableListLock.lock();
if (m_AvailableList.empty())
diff --git a/src/profiling/IBufferManager.hpp b/src/profiling/IBufferManager.hpp
index 190d9c4..0acdf61 100644
--- a/src/profiling/IBufferManager.hpp
+++ b/src/profiling/IBufferManager.hpp
@@ -18,7 +18,7 @@
class IBufferManager
{
public:
- virtual ~IBufferManager() {};
+ virtual ~IBufferManager() {}
virtual std::unique_ptr<IPacketBuffer> Reserve(unsigned int requestedSize, unsigned int& reservedSize) = 0;
diff --git a/src/profiling/SendCounterPacket.cpp b/src/profiling/SendCounterPacket.cpp
index 33eaeab..0a2f08b 100644
--- a/src/profiling/SendCounterPacket.cpp
+++ b/src/profiling/SendCounterPacket.cpp
@@ -977,17 +977,17 @@
}
// Wait condition lock scope - End
-
-
+ // Get the buffer to read from
std::unique_ptr<IPacketBuffer> packetBuffer = m_BufferManager.GetReadableBuffer();
if (packetBuffer == nullptr)
{
+ // Nothing to read from, ignore and continue
continue;
}
- const unsigned char* readBuffer = packetBuffer->GetReadableData();
- // Get the data to send from the buffer
- unsigned int readBufferSize = packetBuffer->GetSize();
+ // Get the data to send from the buffer
+ const unsigned char* readBuffer = packetBuffer->GetReadableData();
+ unsigned int readBufferSize = packetBuffer->GetSize();
if (readBuffer == nullptr || readBufferSize == 0)
{
// Nothing to send, ignore and continue
@@ -1001,6 +1001,7 @@
m_ProfilingConnection.WritePacket(readBuffer, boost::numeric_cast<uint32_t>(readBufferSize));
}
+ // Mark the packet buffer as read
m_BufferManager.MarkRead(packetBuffer);
}
diff --git a/src/profiling/test/BufferTests.cpp b/src/profiling/test/BufferTests.cpp
index b678350..a2f3c30 100644
--- a/src/profiling/test/BufferTests.cpp
+++ b/src/profiling/test/BufferTests.cpp
@@ -130,7 +130,7 @@
unsigned int reservedSize = 0;
// Cannot reserve buffer bigger than maximum buffer size
- BOOST_CHECK_THROW(bufferManager.Reserve(1024, reservedSize), armnn::RuntimeException);
+ BOOST_CHECK_THROW(bufferManager.Reserve(1024, reservedSize), armnn::InvalidArgumentException);
}
BOOST_AUTO_TEST_CASE(BufferExhaustionTest)
diff --git a/src/profiling/test/SendCounterPacketTests.cpp b/src/profiling/test/SendCounterPacketTests.cpp
index 9659606..ba1d470 100644
--- a/src/profiling/test/SendCounterPacketTests.cpp
+++ b/src/profiling/test/SendCounterPacketTests.cpp
@@ -1694,7 +1694,7 @@
BOOST_AUTO_TEST_CASE(SendThreadTest0)
{
MockProfilingConnection mockProfilingConnection;
- MockStreamCounterBuffer mockStreamCounterBuffer(1, 0);
+ MockStreamCounterBuffer mockStreamCounterBuffer(0);
SendCounterPacket sendCounterPacket(mockProfilingConnection, mockStreamCounterBuffer);
// Try to start the send thread many times, it must only start once
@@ -1718,7 +1718,7 @@
unsigned int totalWrittenSize = 0;
MockProfilingConnection mockProfilingConnection;
- MockStreamCounterBuffer mockStreamCounterBuffer(5,1024);
+ MockStreamCounterBuffer mockStreamCounterBuffer(1024);
SendCounterPacket sendCounterPacket(mockProfilingConnection, mockStreamCounterBuffer);
sendCounterPacket.Start();
@@ -1732,8 +1732,7 @@
// Get the size of the Stream Metadata Packet
std::string processName = GetProcessName().substr(0, 60);
- unsigned int processNameSize = boost::numeric_cast<unsigned int>(processName.size()) > 0 ?
- boost::numeric_cast<unsigned int>(processName.size()) + 1 : 0;
+ unsigned int processNameSize = processName.empty() ? 0 : boost::numeric_cast<unsigned int>(processName.size()) + 1;
unsigned int streamMetadataPacketsize = 118 + processNameSize;
totalWrittenSize += streamMetadataPacketsize;
@@ -1811,15 +1810,15 @@
sendCounterPacket.SetReadyToRead();
- // To test an exact value of the "read size" in the mock buffer, wait a second to allow the send thread to
+ // To test an exact value of the "read size" in the mock buffer, wait two seconds to allow the send thread to
// read all what's remaining in the buffer
std::this_thread::sleep_for(std::chrono::seconds(2));
sendCounterPacket.Stop();
- BOOST_CHECK(mockStreamCounterBuffer.GetReadableBufferSize() == totalWrittenSize);
BOOST_CHECK(mockStreamCounterBuffer.GetCommittedSize() == totalWrittenSize);
- BOOST_CHECK(mockStreamCounterBuffer.GetReadSize() == totalWrittenSize);
+ BOOST_CHECK(mockStreamCounterBuffer.GetReadableSize() == totalWrittenSize);
+ BOOST_CHECK(mockStreamCounterBuffer.GetReadSize() == totalWrittenSize);
}
BOOST_AUTO_TEST_CASE(SendThreadTest2)
@@ -1827,7 +1826,7 @@
unsigned int totalWrittenSize = 0;
MockProfilingConnection mockProfilingConnection;
- MockStreamCounterBuffer mockStreamCounterBuffer(5, 1024);
+ MockStreamCounterBuffer mockStreamCounterBuffer(1024);
SendCounterPacket sendCounterPacket(mockProfilingConnection, mockStreamCounterBuffer);
sendCounterPacket.Start();
@@ -1843,8 +1842,7 @@
// Get the size of the Stream Metadata Packet
std::string processName = GetProcessName().substr(0, 60);
- unsigned int processNameSize = boost::numeric_cast<unsigned int>(processName.size()) > 0 ?
- boost::numeric_cast<unsigned int>(processName.size()) + 1 : 0;
+ unsigned int processNameSize = processName.empty() ? 0 : boost::numeric_cast<unsigned int>(processName.size()) + 1;
unsigned int streamMetadataPacketsize = 118 + processNameSize;
totalWrittenSize += streamMetadataPacketsize;
@@ -1932,15 +1930,15 @@
sendCounterPacket.SetReadyToRead();
- // To test an exact value of the "read size" in the mock buffer, wait a second to allow the send thread to
+ // To test an exact value of the "read size" in the mock buffer, wait two seconds to allow the send thread to
// read all what's remaining in the buffer
std::this_thread::sleep_for(std::chrono::seconds(2));
sendCounterPacket.Stop();
- BOOST_CHECK(mockStreamCounterBuffer.GetReadableBufferSize() == totalWrittenSize);
BOOST_CHECK(mockStreamCounterBuffer.GetCommittedSize() == totalWrittenSize);
- BOOST_CHECK(mockStreamCounterBuffer.GetReadSize() == totalWrittenSize);
+ BOOST_CHECK(mockStreamCounterBuffer.GetReadableSize() == totalWrittenSize);
+ BOOST_CHECK(mockStreamCounterBuffer.GetReadSize() == totalWrittenSize);
}
BOOST_AUTO_TEST_CASE(SendThreadTest3)
@@ -1948,7 +1946,7 @@
unsigned int totalWrittenSize = 0;
MockProfilingConnection mockProfilingConnection;
- MockStreamCounterBuffer mockStreamCounterBuffer(10, 1024);
+ MockStreamCounterBuffer mockStreamCounterBuffer(1024);
SendCounterPacket sendCounterPacket(mockProfilingConnection, mockStreamCounterBuffer);
sendCounterPacket.Start();
@@ -1961,8 +1959,7 @@
// Get the size of the Stream Metadata Packet
std::string processName = GetProcessName().substr(0, 60);
- unsigned int processNameSize = boost::numeric_cast<unsigned int>(processName.size()) > 0 ?
- boost::numeric_cast<unsigned int>(processName.size()) + 1 : 0;
+ unsigned int processNameSize = processName.empty() ? 0 : boost::numeric_cast<unsigned int>(processName.size()) + 1;
unsigned int streamMetadataPacketsize = 118 + processNameSize;
totalWrittenSize += streamMetadataPacketsize;
@@ -2040,10 +2037,11 @@
// thread is not guaranteed to flush the buffer)
sendCounterPacket.Stop();
- BOOST_CHECK(mockStreamCounterBuffer.GetReadableBufferSize() <= totalWrittenSize);
- BOOST_CHECK(mockStreamCounterBuffer.GetCommittedSize() <= totalWrittenSize);
- BOOST_CHECK(mockStreamCounterBuffer.GetReadSize() <= totalWrittenSize);
- BOOST_CHECK(mockStreamCounterBuffer.GetReadSize() <= mockStreamCounterBuffer.GetCommittedSize());
+ BOOST_CHECK(mockStreamCounterBuffer.GetCommittedSize() == totalWrittenSize);
+ BOOST_CHECK(mockStreamCounterBuffer.GetReadableSize() <= totalWrittenSize);
+ BOOST_CHECK(mockStreamCounterBuffer.GetReadSize() <= totalWrittenSize);
+ BOOST_CHECK(mockStreamCounterBuffer.GetReadSize() <= mockStreamCounterBuffer.GetReadableSize());
+ BOOST_CHECK(mockStreamCounterBuffer.GetReadSize() <= mockStreamCounterBuffer.GetCommittedSize());
}
BOOST_AUTO_TEST_SUITE_END()
diff --git a/src/profiling/test/SendCounterPacketTests.hpp b/src/profiling/test/SendCounterPacketTests.hpp
index 243731c..3c3427c 100644
--- a/src/profiling/test/SendCounterPacketTests.hpp
+++ b/src/profiling/test/SendCounterPacketTests.hpp
@@ -46,11 +46,10 @@
{
public:
MockPacketBuffer(unsigned int maxSize)
- : m_MaxSize(maxSize),
- m_Size(0)
- {
- m_Data = std::make_unique<unsigned char[]>(m_MaxSize);
- }
+ : m_MaxSize(maxSize)
+ , m_Size(0)
+ , m_Data(std::make_unique<unsigned char[]>(m_MaxSize))
+ {}
~MockPacketBuffer() {}
@@ -58,7 +57,7 @@
unsigned int GetSize() const override { return m_Size; }
- void MarkRead() override { m_Size = 0;}
+ void MarkRead() override { m_Size = 0; }
void Commit(unsigned int size) override { m_Size = size; }
@@ -126,113 +125,90 @@
class MockStreamCounterBuffer : public IBufferManager
{
public:
- MockStreamCounterBuffer(unsigned int numberOfBuffers = 5, unsigned int maxPacketSize = 4096)
- : m_MaxBufferSize(maxPacketSize)
- , m_ReadableSize(0)
- , m_CommittedSize(0)
- , m_ReadSize(0)
- {
- m_AvailableList.reserve(numberOfBuffers);
- for (unsigned int i = 0; i < numberOfBuffers; ++i)
- {
- std::unique_ptr<IPacketBuffer> buffer = std::make_unique<MockPacketBuffer>(maxPacketSize);
- m_AvailableList.emplace_back(std::move(buffer));
- }
- m_ReadableList.reserve(numberOfBuffers);
- }
+ using IPacketBufferPtr = std::unique_ptr<IPacketBuffer>;
+ MockStreamCounterBuffer(unsigned int maxBufferSize = 4096)
+ : m_MaxBufferSize(maxBufferSize)
+ , m_BufferList()
+ , m_CommittedSize(0)
+ , m_ReadableSize(0)
+ , m_ReadSize(0)
+ {}
~MockStreamCounterBuffer() {}
- std::unique_ptr<IPacketBuffer> Reserve(unsigned int requestedSize, unsigned int& reservedSize) override
+ IPacketBufferPtr Reserve(unsigned int requestedSize, unsigned int& reservedSize) override
{
- std::unique_lock<std::mutex> availableListLock(m_AvailableMutex, std::defer_lock);
+ std::unique_lock<std::mutex> lock(m_Mutex);
+
+ reservedSize = 0;
if (requestedSize > m_MaxBufferSize)
{
- throw armnn::Exception("Maximum buffer size that can be requested is [" +
- std::to_string(m_MaxBufferSize) + "] bytes");
+ throw armnn::InvalidArgumentException("The maximum buffer size that can be requested is [" +
+ std::to_string(m_MaxBufferSize) + "] bytes");
}
- availableListLock.lock();
- if (m_AvailableList.empty())
- {
- throw armnn::profiling::BufferExhaustion("Buffer not available");
- }
- std::unique_ptr<IPacketBuffer> buffer = std::move(m_AvailableList.back());
- m_AvailableList.pop_back();
- availableListLock.unlock();
reservedSize = requestedSize;
- return buffer;
+ return std::make_unique<MockPacketBuffer>(requestedSize);
}
- void Commit(std::unique_ptr<IPacketBuffer>& packetBuffer, unsigned int size) override
+ void Commit(IPacketBufferPtr& packetBuffer, unsigned int size) override
{
- std::unique_lock<std::mutex> readableListLock(m_ReadableMutex, std::defer_lock);
- packetBuffer.get()->Commit(size);
- readableListLock.lock();
- m_ReadableList.push_back(std::move(packetBuffer));
- readableListLock.unlock();
- m_ReadDataAvailable.notify_one();
+ std::unique_lock<std::mutex> lock(m_Mutex);
+
+ packetBuffer->Commit(size);
+ m_BufferList.push_back(std::move(packetBuffer));
m_CommittedSize += size;
}
- void Release(std::unique_ptr<IPacketBuffer>& packetBuffer) override
+ void Release(IPacketBufferPtr& packetBuffer) override
{
- std::unique_lock<std::mutex> availableListLock(m_AvailableMutex, std::defer_lock);
- packetBuffer.get()->Release();
- availableListLock.lock();
- m_AvailableList.push_back(std::move(packetBuffer));
- availableListLock.unlock();
- m_CommittedSize = 0;
- m_ReadSize = 0;
- m_ReadableSize = 0;
+ std::unique_lock<std::mutex> lock(m_Mutex);
+
+ packetBuffer->Release();
}
- std::unique_ptr<IPacketBuffer> GetReadableBuffer() override
+ IPacketBufferPtr GetReadableBuffer() override
{
- std::unique_lock<std::mutex> readableListLock(m_ReadableMutex);
- if (!m_ReadableList.empty())
+ std::unique_lock<std::mutex> lock(m_Mutex);
+
+ if (m_BufferList.empty())
{
- std::unique_ptr<IPacketBuffer> buffer = std::move(m_ReadableList.back());
- m_ReadableSize+=buffer->GetSize();
- m_ReadableList.pop_back();
- readableListLock.unlock();
- return buffer;
+ return nullptr;
}
- return nullptr;
+ IPacketBufferPtr buffer = std::move(m_BufferList.back());
+ m_BufferList.pop_back();
+ m_ReadableSize += buffer->GetSize();
+ return buffer;
}
- void MarkRead(std::unique_ptr<IPacketBuffer>& packetBuffer) override
+ void MarkRead(IPacketBufferPtr& packetBuffer) override
{
- std::unique_lock<std::mutex> availableListLock(m_AvailableMutex, std::defer_lock);
- // increase read size
+ std::unique_lock<std::mutex> lock(m_Mutex);
+
m_ReadSize += packetBuffer->GetSize();
packetBuffer->MarkRead();
- availableListLock.lock();
- m_AvailableList.push_back(std::move(packetBuffer));
- availableListLock.unlock();
}
- unsigned int GetReadableBufferSize() const
- {
- return m_ReadableSize;
- }
- unsigned int GetCommittedSize() const { return m_CommittedSize; }
- unsigned int GetReadSize() const { return m_ReadSize; }
+ unsigned int GetCommittedSize() const { return m_CommittedSize; }
+ unsigned int GetReadableSize() const { return m_ReadableSize; }
+ unsigned int GetReadSize() const { return m_ReadSize; }
private:
+ // The maximum buffer size when creating a new buffer
unsigned int m_MaxBufferSize;
- std::vector<std::unique_ptr<IPacketBuffer>> m_AvailableList;
- std::vector<std::unique_ptr<IPacketBuffer>> m_ReadableList;
- std::mutex m_AvailableMutex;
- std::mutex m_ReadableMutex;
- std::condition_variable m_ReadDataAvailable;
- // The size of the buffer that can be read
- unsigned int m_ReadableSize;
+ // A list of buffers
+ std::vector<IPacketBufferPtr> m_BufferList;
- // The size of the buffer that has been committed for reading
+ // The mutex to synchronize this mock's methods
+ std::mutex m_Mutex;
+
+ // The total size of the buffers that has been committed for reading
unsigned int m_CommittedSize;
- // The size of the buffer that has already been read
+ // The total size of the buffers that can be read
+ unsigned int m_ReadableSize;
+
+ // The total size of the buffers that has already been read
unsigned int m_ReadSize;
};