From a0de05689870ecede28ca91ea0c8474daa7f8d6a Mon Sep 17 00:00:00 2001 From: Finn Williams Date: Wed, 22 Apr 2020 12:27:37 +0100 Subject: IVGCVSW-4719 Remove possible zeros from SendStreamMetaDataPacket offsets Signed-off-by: Finn Williams Change-Id: I8d7ee408c4c73be9b42bb739254b95c476e0e38c --- src/profiling/SendCounterPacket.cpp | 47 ++++++++------------- src/profiling/test/ProfilingTestUtils.cpp | 20 +++++++++ src/profiling/test/ProfilingTestUtils.hpp | 2 + src/profiling/test/ProfilingTests.cpp | 5 +-- src/profiling/test/SendCounterPacketTests.cpp | 59 ++++++++------------------- 5 files changed, 56 insertions(+), 77 deletions(-) diff --git a/src/profiling/SendCounterPacket.cpp b/src/profiling/SendCounterPacket.cpp index 24b86d4427..1ef5440f7b 100644 --- a/src/profiling/SendCounterPacket.cpp +++ b/src/profiling/SendCounterPacket.cpp @@ -10,7 +10,6 @@ #include #include #include -#include #include #include @@ -34,15 +33,12 @@ void SendCounterPacket::SendStreamMetaDataPacket() std::string softwareVersion(GetSoftwareVersion()); std::string processName = GetProcessName().substr(0, 60); - uint32_t infoSize = numeric_cast(info.size()) > 0 ? numeric_cast(info.size()) + 1 : 0; - uint32_t hardwareVersionSize = numeric_cast(hardwareVersion.size()) > 0 ? - numeric_cast(hardwareVersion.size()) + 1 : 0; - uint32_t softwareVersionSize = numeric_cast(softwareVersion.size()) > 0 ? - numeric_cast(softwareVersion.size()) + 1 : 0; - uint32_t processNameSize = numeric_cast(processName.size()) > 0 ? - numeric_cast(processName.size()) + 1 : 0; + uint32_t infoSize = numeric_cast(info.size()) + 1; + uint32_t hardwareVersionSize = numeric_cast(hardwareVersion.size()) + 1; + uint32_t softwareVersionSize = numeric_cast(softwareVersion.size()) + 1; + uint32_t processNameSize = numeric_cast(processName.size()) + 1; - uint32_t sizeUint32 = numeric_cast(sizeof(uint32_t)); + uint32_t sizeUint32 = sizeof(uint32_t); uint32_t headerSize = 2 * sizeUint32; uint32_t bodySize = 10 * sizeUint32; @@ -95,19 +91,19 @@ void SendCounterPacket::SendStreamMetaDataPacket() WriteUint32(writeBuffer, offset, numeric_cast(pid)); // pid offset += sizeUint32; uint32_t poolOffset = bodySize; - WriteUint32(writeBuffer, offset, infoSize ? poolOffset : 0); // offset_info + WriteUint32(writeBuffer, offset, poolOffset); // offset_info offset += sizeUint32; poolOffset += infoSize; - WriteUint32(writeBuffer, offset, hardwareVersionSize ? poolOffset : 0); // offset_hw_version + WriteUint32(writeBuffer, offset, poolOffset); // offset_hw_version offset += sizeUint32; poolOffset += hardwareVersionSize; - WriteUint32(writeBuffer, offset, softwareVersionSize ? poolOffset : 0); // offset_sw_version + WriteUint32(writeBuffer, offset, poolOffset); // offset_sw_version offset += sizeUint32; poolOffset += softwareVersionSize; - WriteUint32(writeBuffer, offset, processNameSize ? poolOffset : 0); // offset_process_name + WriteUint32(writeBuffer, offset, poolOffset); // offset_process_name offset += sizeUint32; poolOffset += processNameSize; - WriteUint32(writeBuffer, offset, packetVersionEntries ? poolOffset : 0); // offset_packet_version_table + WriteUint32(writeBuffer, offset, poolOffset); // offset_packet_version_table offset += sizeUint32; WriteUint32(writeBuffer, offset, 0); // reserved offset += sizeUint32; @@ -120,23 +116,12 @@ void SendCounterPacket::SendStreamMetaDataPacket() offset += infoSize; } - if (hardwareVersionSize) - { - memcpy(&writeBuffer->GetWritableData()[offset], hardwareVersion.c_str(), hardwareVersionSize); - offset += hardwareVersionSize; - } - - if (softwareVersionSize) - { - memcpy(&writeBuffer->GetWritableData()[offset], softwareVersion.c_str(), softwareVersionSize); - offset += softwareVersionSize; - } - - if (processNameSize) - { - memcpy(&writeBuffer->GetWritableData()[offset], processName.c_str(), processNameSize); - offset += processNameSize; - } + memcpy(&writeBuffer->GetWritableData()[offset], hardwareVersion.c_str(), hardwareVersionSize); + offset += hardwareVersionSize; + memcpy(&writeBuffer->GetWritableData()[offset], softwareVersion.c_str(), softwareVersionSize); + offset += softwareVersionSize; + memcpy(&writeBuffer->GetWritableData()[offset], processName.c_str(), processNameSize); + offset += processNameSize; if (packetVersionEntries) { diff --git a/src/profiling/test/ProfilingTestUtils.cpp b/src/profiling/test/ProfilingTestUtils.cpp index 5c63b54b8f..c1386820e8 100644 --- a/src/profiling/test/ProfilingTestUtils.cpp +++ b/src/profiling/test/ProfilingTestUtils.cpp @@ -14,6 +14,26 @@ #include +uint32_t GetStreamMetaDataPacketSize() +{ + uint32_t sizeUint32 = sizeof(uint32_t); + uint32_t payloadSize = 0; + payloadSize += boost::numeric_cast(GetSoftwareInfo().size()) + 1; + payloadSize += boost::numeric_cast(GetHardwareVersion().size()) + 1; + payloadSize += boost::numeric_cast(GetSoftwareVersion().size()) + 1; + payloadSize += boost::numeric_cast(GetProcessName().size()) + 1; + + // Add packetVersionEntries + payloadSize += 6 * 2 * sizeUint32; + // Add packetVersionCountSize + payloadSize += sizeUint32; + + uint32_t headerSize = 2 * sizeUint32; + uint32_t bodySize = 10 * sizeUint32; + + return headerSize + bodySize + payloadSize; +} + inline unsigned int OffsetToNextWord(unsigned int numberOfBytes) { unsigned int uint32_t_size = sizeof(uint32_t); diff --git a/src/profiling/test/ProfilingTestUtils.hpp b/src/profiling/test/ProfilingTestUtils.hpp index 816ffd3dc6..16f1b0c431 100644 --- a/src/profiling/test/ProfilingTestUtils.hpp +++ b/src/profiling/test/ProfilingTestUtils.hpp @@ -17,6 +17,8 @@ using namespace armnn; using namespace armnn::profiling; +uint32_t GetStreamMetaDataPacketSize(); + inline unsigned int OffsetToNextWord(unsigned int numberOfBytes); void VerifyTimelineHeaderBinary(const unsigned char* readableData, diff --git a/src/profiling/test/ProfilingTests.cpp b/src/profiling/test/ProfilingTests.cpp index b3326e0173..64646b461c 100644 --- a/src/profiling/test/ProfilingTests.cpp +++ b/src/profiling/test/ProfilingTests.cpp @@ -2458,10 +2458,7 @@ BOOST_AUTO_TEST_CASE(RequestCounterDirectoryCommandHandlerTest2) BOOST_AUTO_TEST_CASE(CheckProfilingServiceGoodConnectionAcknowledgedPacket) { - // Calculate the size of a Stream Metadata packet - std::string processName = GetProcessName().substr(0, 60); - unsigned int processNameSize = processName.empty() ? 0 : boost::numeric_cast(processName.size()) + 1; - unsigned int streamMetadataPacketsize = 118 + processNameSize; + unsigned int streamMetadataPacketsize = GetStreamMetaDataPacketSize(); // Reset the profiling service to the uninitialized state armnn::Runtime::CreationOptions::ExternalProfilingOptions options; diff --git a/src/profiling/test/SendCounterPacketTests.cpp b/src/profiling/test/SendCounterPacketTests.cpp index a3c237faba..8321e88296 100644 --- a/src/profiling/test/SendCounterPacketTests.cpp +++ b/src/profiling/test/SendCounterPacketTests.cpp @@ -4,6 +4,7 @@ // #include "ProfilingMocks.hpp" +#include "ProfilingTestUtils.hpp" #include "SendCounterPacketTests.hpp" #include @@ -298,14 +299,10 @@ BOOST_AUTO_TEST_CASE(SendStreamMetaDataPacketTest) std::string processName = GetProcessName().substr(0, 60); - uint32_t infoSize = numeric_cast(GetSoftwareInfo().size()) > 0 ? - numeric_cast(GetSoftwareInfo().size()) + 1 : 0; - uint32_t hardwareVersionSize = numeric_cast(GetHardwareVersion().size()) > 0 ? - numeric_cast(GetHardwareVersion().size()) + 1 : 0; - uint32_t softwareVersionSize = numeric_cast(GetSoftwareVersion().size()) > 0 ? - numeric_cast(GetSoftwareVersion().size()) + 1 : 0; - uint32_t processNameSize = numeric_cast(processName.size()) > 0 ? - numeric_cast(processName.size()) + 1 : 0; + uint32_t infoSize = numeric_cast(GetSoftwareInfo().size()) + 1; + uint32_t hardwareVersionSize = numeric_cast(GetHardwareVersion().size()) + 1; + uint32_t softwareVersionSize = numeric_cast(GetSoftwareVersion().size()) + 1; + uint32_t processNameSize = numeric_cast(processName.size()) + 1; uint32_t packetEntries = 6; @@ -337,19 +334,19 @@ BOOST_AUTO_TEST_CASE(SendStreamMetaDataPacketTest) BOOST_TEST(ReadUint32(readBuffer2, offset) == numeric_cast(pid)); offset += sizeUint32; uint32_t poolOffset = 10 * sizeUint32; - BOOST_TEST(ReadUint32(readBuffer2, offset) == (infoSize ? poolOffset : 0)); // offset_info + BOOST_TEST(ReadUint32(readBuffer2, offset) == poolOffset); // offset_info offset += sizeUint32; poolOffset += infoSize; - BOOST_TEST(ReadUint32(readBuffer2, offset) == (hardwareVersionSize ? poolOffset : 0)); // offset_hw_version + BOOST_TEST(ReadUint32(readBuffer2, offset) == poolOffset); // offset_hw_version offset += sizeUint32; poolOffset += hardwareVersionSize; - BOOST_TEST(ReadUint32(readBuffer2, offset) == (softwareVersionSize ? poolOffset : 0)); // offset_sw_version + BOOST_TEST(ReadUint32(readBuffer2, offset) == poolOffset); // offset_sw_version offset += sizeUint32; poolOffset += softwareVersionSize; - BOOST_TEST(ReadUint32(readBuffer2, offset) == (processNameSize ? poolOffset : 0)); // offset_process_name + BOOST_TEST(ReadUint32(readBuffer2, offset) == poolOffset); // offset_process_name offset += sizeUint32; poolOffset += processNameSize; - BOOST_TEST(ReadUint32(readBuffer2, offset) == (packetEntries ? poolOffset : 0)); // offset_packet_version_table + BOOST_TEST(ReadUint32(readBuffer2, offset) == poolOffset); // offset_packet_version_table offset += sizeUint32; BOOST_TEST(ReadUint32(readBuffer2, offset) == 0); // reserved @@ -1785,11 +1782,7 @@ BOOST_AUTO_TEST_CASE(SendThreadTest1) CounterDirectory counterDirectory; sendCounterPacket.SendStreamMetaDataPacket(); - // Get the size of the Stream Metadata Packet - std::string processName = GetProcessName().substr(0, 60); - unsigned int processNameSize = processName.empty() ? 0 : boost::numeric_cast(processName.size()) + 1; - unsigned int streamMetadataPacketsize = 118 + processNameSize; - totalWrittenSize += streamMetadataPacketsize; + totalWrittenSize += GetStreamMetaDataPacketSize(); sendThread.SetReadyToRead(); @@ -1899,11 +1892,7 @@ BOOST_AUTO_TEST_CASE(SendThreadTest2) CounterDirectory counterDirectory; sendCounterPacket.SendStreamMetaDataPacket(); - // Get the size of the Stream Metadata Packet - std::string processName = GetProcessName().substr(0, 60); - unsigned int processNameSize = processName.empty() ? 0 : boost::numeric_cast(processName.size()) + 1; - unsigned int streamMetadataPacketsize = 118 + processNameSize; - totalWrittenSize += streamMetadataPacketsize; + totalWrittenSize += GetStreamMetaDataPacketSize(); sendThread.SetReadyToRead(); @@ -2018,11 +2007,7 @@ BOOST_AUTO_TEST_CASE(SendThreadTest3) CounterDirectory counterDirectory; sendCounterPacket.SendStreamMetaDataPacket(); - // Get the size of the Stream Metadata Packet - std::string processName = GetProcessName().substr(0, 60); - unsigned int processNameSize = processName.empty() ? 0 : boost::numeric_cast(processName.size()) + 1; - unsigned int streamMetadataPacketsize = 118 + processNameSize; - totalWrittenSize += streamMetadataPacketsize; + totalWrittenSize += GetStreamMetaDataPacketSize(); sendThread.SetReadyToRead(); sendCounterPacket.SendCounterDirectoryPacket(counterDirectory); @@ -2116,9 +2101,7 @@ BOOST_AUTO_TEST_CASE(SendCounterPacketTestWithSendThread) SendThread sendThread(profilingStateMachine, bufferManager, sendCounterPacket, -1); sendThread.Start(mockProfilingConnection); - std::string processName = GetProcessName().substr(0, 60); - unsigned int processNameSize = processName.empty() ? 0 : boost::numeric_cast(processName.size()) + 1; - unsigned int streamMetadataPacketsize = 118 + processNameSize; + unsigned int streamMetadataPacketsize = GetStreamMetaDataPacketSize(); sendThread.Stop(); @@ -2173,9 +2156,7 @@ BOOST_AUTO_TEST_CASE(SendThreadBufferTest) auto packetBuffer = bufferManager.GetReadableBuffer(); BOOST_TEST(packetBuffer.get()); - std::string processName = GetProcessName().substr(0, 60); - unsigned int processNameSize = processName.empty() ? 0 : boost::numeric_cast(processName.size()) + 1; - unsigned int streamMetadataPacketsize = 118 + processNameSize; + unsigned int streamMetadataPacketsize = GetStreamMetaDataPacketSize(); BOOST_TEST(packetBuffer->GetSize() == streamMetadataPacketsize); // Recommit to be read by sendCounterPacket @@ -2249,10 +2230,7 @@ BOOST_AUTO_TEST_CASE(SendThreadSendStreamMetadataPacket3) ProfilingStateMachine profilingStateMachine; SetWaitingForAckProfilingState(profilingStateMachine); - // Calculate the size of a Stream Metadata packet - std::string processName = GetProcessName().substr(0, 60); - unsigned int processNameSize = processName.empty() ? 0 : boost::numeric_cast(processName.size()) + 1; - unsigned int streamMetadataPacketsize = 118 + processNameSize; + unsigned int streamMetadataPacketsize = GetStreamMetaDataPacketSize(); MockProfilingConnection mockProfilingConnection; BufferManager bufferManager(3, 1024); @@ -2277,10 +2255,7 @@ BOOST_AUTO_TEST_CASE(SendThreadSendStreamMetadataPacket4) ProfilingStateMachine profilingStateMachine; SetWaitingForAckProfilingState(profilingStateMachine); - // Calculate the size of a Stream Metadata packet - std::string processName = GetProcessName().substr(0, 60); - unsigned int processNameSize = processName.empty() ? 0 : boost::numeric_cast(processName.size()) + 1; - unsigned int streamMetadataPacketsize = 118 + processNameSize; + unsigned int streamMetadataPacketsize = GetStreamMetaDataPacketSize(); MockProfilingConnection mockProfilingConnection; BufferManager bufferManager(3, 1024); -- cgit v1.2.1