IVGCVSW-5022 Fix master intermittent failure by providing surge buffer capacity
Change-Id: I028aec48d89d7348836223029aa1e8c315f160fa
Signed-off-by: Jim Flynn <jim.flynn@arm.com>
diff --git a/src/profiling/BufferManager.cpp b/src/profiling/BufferManager.cpp
index a7b71e5..be60f27 100644
--- a/src/profiling/BufferManager.cpp
+++ b/src/profiling/BufferManager.cpp
@@ -14,7 +14,9 @@
BufferManager::BufferManager(unsigned int numberOfBuffers, unsigned int maxPacketSize)
: m_MaxBufferSize(maxPacketSize),
- m_NumberOfBuffers(numberOfBuffers)
+ m_NumberOfBuffers(numberOfBuffers),
+ m_MaxNumberOfBuffers(numberOfBuffers * 3),
+ m_CurrentNumberOfBuffers(numberOfBuffers)
{
Initialize();
}
@@ -30,8 +32,21 @@
availableListLock.lock();
if (m_AvailableList.empty())
{
- availableListLock.unlock();
- return nullptr;
+ if (m_CurrentNumberOfBuffers < m_MaxNumberOfBuffers)
+ {
+ // create a temporary overflow/surge buffer and hand it back
+ m_CurrentNumberOfBuffers++;
+ availableListLock.unlock();
+ IPacketBufferPtr buffer = std::make_unique<PacketBuffer>(m_MaxBufferSize);
+ reservedSize = requestedSize;
+ return buffer;
+ }
+ else
+ {
+ // we have totally busted the limit. call a halt to new memory allocations.
+ availableListLock.unlock();
+ return nullptr;
+ }
}
IPacketBufferPtr buffer = std::move(m_AvailableList.back());
m_AvailableList.pop_back();
@@ -57,6 +72,7 @@
void BufferManager::Initialize()
{
m_AvailableList.reserve(m_NumberOfBuffers);
+ m_CurrentNumberOfBuffers = m_NumberOfBuffers;
for (unsigned int i = 0; i < m_NumberOfBuffers; ++i)
{
IPacketBufferPtr buffer = std::make_unique<PacketBuffer>(m_MaxBufferSize);
@@ -69,7 +85,19 @@
std::unique_lock<std::mutex> availableListLock(m_AvailableMutex, std::defer_lock);
packetBuffer->Release();
availableListLock.lock();
- m_AvailableList.push_back(std::move(packetBuffer));
+ if (m_AvailableList.size() <= m_NumberOfBuffers)
+ {
+ m_AvailableList.push_back(std::move(packetBuffer));
+ }
+ else
+ {
+ // we have been handed a temporary overflow/surge buffer get rid of it
+ packetBuffer->Destroy();
+ if (m_CurrentNumberOfBuffers > m_NumberOfBuffers)
+ {
+ --m_CurrentNumberOfBuffers;
+ }
+ }
availableListLock.unlock();
}
@@ -103,7 +131,19 @@
std::unique_lock<std::mutex> availableListLock(m_AvailableMutex, std::defer_lock);
packetBuffer->MarkRead();
availableListLock.lock();
- m_AvailableList.push_back(std::move(packetBuffer));
+ if (m_AvailableList.size() <= m_NumberOfBuffers)
+ {
+ m_AvailableList.push_back(std::move(packetBuffer));
+ }
+ else
+ {
+ // we have been handed a temporary overflow/surge buffer get rid of it
+ packetBuffer->Destroy();
+ if (m_CurrentNumberOfBuffers > m_NumberOfBuffers)
+ {
+ --m_CurrentNumberOfBuffers;
+ }
+ }
availableListLock.unlock();
}
diff --git a/src/profiling/BufferManager.hpp b/src/profiling/BufferManager.hpp
index f4e63da..4bc7725 100644
--- a/src/profiling/BufferManager.hpp
+++ b/src/profiling/BufferManager.hpp
@@ -51,7 +51,9 @@
// Maximum buffer size
unsigned int m_MaxBufferSize;
// Number of buffers
- unsigned int m_NumberOfBuffers;
+ const unsigned int m_NumberOfBuffers;
+ const unsigned int m_MaxNumberOfBuffers;
+ unsigned int m_CurrentNumberOfBuffers;
// List of available packet buffers
std::vector<IPacketBufferPtr> m_AvailableList;
diff --git a/src/profiling/IPacketBuffer.hpp b/src/profiling/IPacketBuffer.hpp
index 1a97ca7..15687a8 100644
--- a/src/profiling/IPacketBuffer.hpp
+++ b/src/profiling/IPacketBuffer.hpp
@@ -35,6 +35,10 @@
virtual void Release() = 0;
virtual unsigned char* GetWritableData() = 0;
+
+ /// release the memory held and reset internal point to null.
+ /// After this function is invoked the PacketBuffer is unusable.
+ virtual void Destroy() = 0;
};
using IPacketBufferPtr = std::unique_ptr<IPacketBuffer>;
diff --git a/src/profiling/PacketBuffer.cpp b/src/profiling/PacketBuffer.cpp
index 886561a..6010493 100644
--- a/src/profiling/PacketBuffer.cpp
+++ b/src/profiling/PacketBuffer.cpp
@@ -55,6 +55,13 @@
return m_Data.get();
}
+void PacketBuffer::Destroy()
+{
+ m_Data.reset(nullptr);
+ m_Size = 0;
+ m_MaxSize = 0;
+}
+
} // namespace profiling
} // namespace armnn
diff --git a/src/profiling/PacketBuffer.hpp b/src/profiling/PacketBuffer.hpp
index f03b301..02fe3b9 100644
--- a/src/profiling/PacketBuffer.hpp
+++ b/src/profiling/PacketBuffer.hpp
@@ -34,6 +34,8 @@
unsigned char* GetWritableData() override;
+ void Destroy() override;
+
private:
unsigned int m_MaxSize;
unsigned int m_Size;
diff --git a/src/profiling/SendTimelinePacket.cpp b/src/profiling/SendTimelinePacket.cpp
index b749b5d..2ca5f54 100644
--- a/src/profiling/SendTimelinePacket.cpp
+++ b/src/profiling/SendTimelinePacket.cpp
@@ -64,9 +64,13 @@
m_WriteBuffer = m_BufferManager.Reserve(MAX_METADATA_PACKET_LENGTH, reserved);
// Check if there is enough space in the buffer
- if (m_WriteBuffer == nullptr || reserved < m_Offset)
+ if (m_WriteBuffer == nullptr)
{
- throw BufferExhaustion("No space left on buffer", CHECK_LOCATION());
+ throw BufferExhaustion("No free buffers left", CHECK_LOCATION());
+ }
+ if (reserved < m_Offset)
+ {
+ throw BufferExhaustion("Reserved space too small for use", CHECK_LOCATION());
}
if (m_DirectoryPackage)
diff --git a/src/profiling/test/BufferTests.cpp b/src/profiling/test/BufferTests.cpp
index cba209c..0225d75 100644
--- a/src/profiling/test/BufferTests.cpp
+++ b/src/profiling/test/BufferTests.cpp
@@ -146,6 +146,21 @@
BOOST_TEST(packetBuffer.get());
// Cannot reserve buffer when buffer is not available
+ // NOTE: because the buffer manager now has surge capacity of
+ // initial size * 3 we should be able to reserve three
+ // buffers before exhaustion
+ packetBuffer = bufferManager.Reserve(512, reservedSize);
+
+ // Successfully reserved the second buffer with requested size
+ BOOST_TEST(reservedSize == 512);
+ BOOST_TEST(packetBuffer.get());
+
+ packetBuffer = bufferManager.Reserve(512, reservedSize);
+
+ // Successfully reserved the third buffer with requested size
+ BOOST_TEST(reservedSize == 512);
+ BOOST_TEST(packetBuffer.get());
+
auto reservedBuffer = bufferManager.Reserve(512, reservedSize);
BOOST_TEST(reservedSize == 0);
BOOST_TEST(!reservedBuffer.get());
@@ -175,6 +190,19 @@
BOOST_TEST(reservedSize2 == 512);
BOOST_TEST(packetBuffer2.get());
+ // NOTE: the buffer now has a surge capacity of initial size * 3
+ // so we can grab 9 of them prior to exhaustion now
+ for (unsigned int i = 0; i < 6 ; ++i)
+ {
+ // grab another six buffers to exhaust the surge capacity
+ unsigned int reservedSize = 0;
+ auto packetBuffer = bufferManager.Reserve(512, reservedSize);
+
+ // Successfully reserved the third buffer with requested size
+ BOOST_TEST(reservedSize == 512);
+ BOOST_TEST(packetBuffer.get());
+ }
+
// Cannot reserve when buffer is not available
unsigned int reservedSize3 = 0;
auto reservedBuffer = bufferManager.Reserve(512, reservedSize3);
@@ -199,6 +227,20 @@
BOOST_TEST(reservedSize1 == 128);
BOOST_TEST(packetBuffer1.get());
+ // NOTE: now that we have a surge capacity of up to
+ // initial size * 3 we need to allocate four more
+ // buffers to exhaust the manager
+ for (unsigned int i = 0; i < 4 ; ++i)
+ {
+ // grab another six buffers to exhaust the surge capacity
+ unsigned int reservedSize = 0;
+ auto packetBuffer = bufferManager.Reserve(512, reservedSize);
+
+ // Successfully reserved the third buffer with requested size
+ BOOST_TEST(reservedSize == 512);
+ BOOST_TEST(packetBuffer.get());
+ }
+
// Cannot reserve when buffer is not available
unsigned int reservedSize2 = 0;
auto reservedBuffer = bufferManager.Reserve(512, reservedSize2);
@@ -229,6 +271,20 @@
BOOST_TEST(reservedSize1 == 128);
BOOST_TEST(packetBuffer1.get());
+ // NOTE: now that we have a surge capacity of up to
+ // initial size * 3 we need to allocate four more
+ // buffers to exhaust the manager
+ for (unsigned int i = 0; i < 4 ; ++i)
+ {
+ // grab another six buffers to exhaust the surge capacity
+ unsigned int reservedSize = 0;
+ auto packetBuffer = bufferManager.Reserve(512, reservedSize);
+
+ // Successfully reserved the third buffer with requested size
+ BOOST_TEST(reservedSize == 512);
+ BOOST_TEST(packetBuffer.get());
+ }
+
unsigned int reservedSize2 = 0;
auto reservedBuffer = bufferManager.Reserve(512, reservedSize2);
BOOST_TEST(reservedSize2 == 0);
@@ -263,6 +319,20 @@
BOOST_TEST(reservedSize1 == 128);
BOOST_TEST(packetBuffer1.get());
+ // NOTE: now that we have a surge capacity of up to
+ // initial size * 3 we need to allocate four more
+ // buffers to exhaust the manager
+ for (unsigned int i = 0; i < 4 ; ++i)
+ {
+ // grab another six buffers to exhaust the surge capacity
+ unsigned int reservedSize = 0;
+ auto packetBuffer = bufferManager.Reserve(512, reservedSize);
+
+ // Successfully reserved the third buffer with requested size
+ BOOST_TEST(reservedSize == 512);
+ BOOST_TEST(packetBuffer.get());
+ }
+
// Cannot reserve when buffer is not available
unsigned int reservedSize2 = 0;
auto reservedBuffer = bufferManager.Reserve(512, reservedSize2);
diff --git a/src/profiling/test/ProfilingMocks.hpp b/src/profiling/test/ProfilingMocks.hpp
index fe8fbd7..e4f71f9 100644
--- a/src/profiling/test/ProfilingMocks.hpp
+++ b/src/profiling/test/ProfilingMocks.hpp
@@ -193,6 +193,8 @@
unsigned char* GetWritableData() override { return m_Data.get(); }
+ void Destroy() override {m_Data.reset(nullptr); m_Size = 0; m_MaxSize =0;}
+
private:
unsigned int m_MaxSize;
unsigned int m_Size;