From 2ce8dba8193222f6a48176e7623a5d0133c886a6 Mon Sep 17 00:00:00 2001 From: Brad Davis Date: Fri, 10 Jul 2015 15:06:38 -0700 Subject: [PATCH] Removing heap allocation from property flag parsing, adding some manual tests --- libraries/shared/src/BufferParser.h | 5 +- libraries/shared/src/PropertyFlags.h | 92 ++++++++------- tests/entities/CMakeLists.txt | 12 ++ tests/entities/packet.bin | Bin 0 -> 1387 bytes tests/entities/src/main.cpp | 165 +++++++++++++++++++++++++++ 5 files changed, 231 insertions(+), 43 deletions(-) create mode 100644 tests/entities/CMakeLists.txt create mode 100644 tests/entities/packet.bin create mode 100644 tests/entities/src/main.cpp diff --git a/libraries/shared/src/BufferParser.h b/libraries/shared/src/BufferParser.h index 84bde2be31..d60e7127cd 100644 --- a/libraries/shared/src/BufferParser.h +++ b/libraries/shared/src/BufferParser.h @@ -44,10 +44,7 @@ public: template inline void readFlags(PropertyFlags& result) { - // FIXME doing heap allocation - QByteArray encoded((const char*)(_data + _offset), remaining()); - result.decode(encoded); - _offset += result.getEncodedLength(); + _offset += result.decode(_data + _offset, remaining()); } template diff --git a/libraries/shared/src/PropertyFlags.h b/libraries/shared/src/PropertyFlags.h index de05edc076..0202784c77 100644 --- a/libraries/shared/src/PropertyFlags.h +++ b/libraries/shared/src/PropertyFlags.h @@ -25,6 +25,7 @@ #include #include +#include "ByteCountCoding.h" #include templateclass PropertyFlags { @@ -51,7 +52,8 @@ public: void setHasProperty(Enum flag, bool value = true); bool getHasProperty(Enum flag) const; QByteArray encode(); - void decode(const QByteArray& fromEncoded); + size_t decode(const uint8_t* data, size_t length); + size_t decode(const QByteArray& fromEncoded); operator QByteArray() { return encode(); }; @@ -193,51 +195,63 @@ template inline QByteArray PropertyFlags::encode() { return output; } -template inline void PropertyFlags::decode(const QByteArray& fromEncodedBytes) { +template +inline size_t PropertyFlags::decode(const uint8_t* data, size_t size) { + clear(); + //clear(); // we are cleared out! - clear(); // we are cleared out! + size_t bytesConsumed = 0; + int bitCount = BITS_IN_BYTE * size; - // first convert the ByteArray into a BitArray... - QBitArray encodedBits; - int bitCount = BITS_PER_BYTE * fromEncodedBytes.count(); - encodedBits.resize(bitCount); - - for(int byte = 0; byte < fromEncodedBytes.count(); byte++) { - char originalByte = fromEncodedBytes.at(byte); - for(int bit = 0; bit < BITS_PER_BYTE; bit++) { - int shiftBy = BITS_PER_BYTE - (bit + 1); - char maskBit = ( 1 << shiftBy); - bool bitValue = originalByte & maskBit; - encodedBits.setBit(byte * BITS_PER_BYTE + bit, bitValue); + int encodedByteCount = 1; // there is at least 1 byte (after the leadBits) + int leadBits = 1; // there is always at least 1 lead bit + bool inLeadBits = true; + int bitAt = 0; + int expectedBitCount; // unknown at this point + int lastValueBit; + for (int byte = 0; byte < size; byte++) { + char originalByte = data[byte]; + bytesConsumed++; + unsigned char maskBit = 0x80; // LEFT MOST BIT set + for (int bit = 0; bit < BITS_IN_BYTE; bit++) { + bool bitIsSet = originalByte & maskBit; + // Processing of the lead bits + if (inLeadBits) { + if (bitIsSet) { + encodedByteCount++; + leadBits++; + } else { + inLeadBits = false; // once we hit our first 0, we know we're out of the lead bits + expectedBitCount = (encodedByteCount * BITS_IN_BYTE) - leadBits; + lastValueBit = expectedBitCount + bitAt; + + // check to see if the remainder of our buffer is sufficient + if (expectedBitCount > (bitCount - leadBits)) { + break; + } + } + } else { + if (bitAt > lastValueBit) { + break; + } + + if (bitIsSet) { + setHasProperty(static_cast(bitAt - leadBits), true); + } + } + bitAt++; + maskBit >>= 1; } - } - - // next, read the leading bits to determine the correct number of bytes to decode (may not match the QByteArray) - int encodedByteCount = 0; - int leadBits = 1; - int bitAt; - for (bitAt = 0; bitAt < bitCount; bitAt++) { - if (encodedBits.at(bitAt)) { - encodedByteCount++; - leadBits++; - } else { + if (!inLeadBits && bitAt > lastValueBit) { break; } } - encodedByteCount++; // always at least one byte - _encodedLength = encodedByteCount; + _encodedLength = bytesConsumed; + return bytesConsumed; +} - int expectedBitCount = encodedByteCount * BITS_PER_BYTE; - - // Now, keep reading... - if (expectedBitCount <= (encodedBits.size() - leadBits)) { - int flagsStartAt = bitAt + 1; - for (bitAt = flagsStartAt; bitAt < expectedBitCount; bitAt++) { - if (encodedBits.at(bitAt)) { - setHasProperty((Enum)(bitAt - flagsStartAt)); - } - } - } +template inline size_t PropertyFlags::decode(const QByteArray& fromEncodedBytes) { + return decode(reinterpret_cast(fromEncodedBytes.data()), fromEncodedBytes.size()); } template inline void PropertyFlags::debugDumpBits() { diff --git a/tests/entities/CMakeLists.txt b/tests/entities/CMakeLists.txt new file mode 100644 index 0000000000..44b84dea43 --- /dev/null +++ b/tests/entities/CMakeLists.txt @@ -0,0 +1,12 @@ + +set(TARGET_NAME "entities-test") + +# This is not a testcase -- just set it up as a regular hifi project +setup_hifi_project() + +set_target_properties(${TARGET_NAME} PROPERTIES FOLDER "Tests/manual-tests/") + +# link in the shared libraries +link_hifi_libraries(entities avatars shared octree gpu model fbx networking animation environment) + +copy_dlls_beside_windows_executable() \ No newline at end of file diff --git a/tests/entities/packet.bin b/tests/entities/packet.bin new file mode 100644 index 0000000000000000000000000000000000000000..295117172ded11f54ff48a869fbb449ee8672154 GIT binary patch literal 1387 zcmcJPPiWIn9LIlgRCiF(IjM@c65L_1G;PyvX+g6!VK`@DZU=W6E=!XwlIC?us#|AW zhl*b2pD8jucu-Nni<^Rf4h}(Z!`A89iiqe$chG|}a2iRs)(tylh~MPF%kTFB@B90G z-s>O#(wr%3NT1o=b>}%byv~t)wW)Z>ZiCG5z5#Qi4WP{|n<;}GMblMlp^!L8nk)ct zW?%L(3oZIxW5IMPwGCh_2oUGy=ieFQ+W~-?9m!E_eliyZ##)8}VAcRw4s{Abnl+t$ z0USwlBslez8?Mh>c=Gx1&Sx`iHOY@-Et7T|Y#Etz*J74^GvVNhg6<|OFfP6oICRrw zm*TF8!kF(}40HDAA?DxQS!T*1T+LIoH@r<{C*k0X^opgRQ=AtNX(Dmod}P*f*Fa+noWBc}C) zP*~K|UGjce?Px+MLe{H>sL4npSrm!J3{i7R?cMoGf2z&42m|PcUR;tj-Kf*}h=>PV zmADku@c?B_tWdog7Q3_YPNanKfW#}hScxlq{uj5)QYEe+6wT+gn5xQZNB&zkc+IGd z87RUVvcM}!&s-{z-N_({mrgwhRS%qROk~*8ZyYuEZ@{SC21B{NH~xw0BNZ5@Oqguz zwWaml^*F@f7YCWEt=IE?Pp39NwDvShdk){fQN5s>IiR8R*mUB>$^M4mi7R_cV)V!( zJ`eP&TXR6K$pWnu271qX;JPMEwj^a4drkl7Q$8#>U5hCFwupHeJH=coU7a_U)6$Ns zWu_deyk5y8y4?irrYOQIcm#r$B#{t!pV!L^g5aaw3rYb04d@`RH5&*iivVqD!^-1{ qdoS74x!Y7n^Tf@JBe_0!_p;pvy}6>d&2Uf*u&m$>8Rsxa0>DqVPf;uY literal 0 HcmV?d00001 diff --git a/tests/entities/src/main.cpp b/tests/entities/src/main.cpp new file mode 100644 index 0000000000..8d16d98103 --- /dev/null +++ b/tests/entities/src/main.cpp @@ -0,0 +1,165 @@ +// +// main.cpp +// tests/render-utils/src +// +// Copyright 2014 High Fidelity, Inc. +// +// Distributed under the Apache License, Version 2.0. +// See the accompanying file LICENSE or http://www.apache.org/licenses/LICENSE-2.0.html +// + +#include +#include +#include +#include +#include +#include +#include + +#include +#include +#include + +const QString& getTestResourceDir() { + static QString dir; + if (dir.isEmpty()) { + QDir path(__FILE__); + path.cdUp(); + dir = path.cleanPath(path.absoluteFilePath("../")) + "/"; + qDebug() << "Qml Test Path: " << dir; + } + return dir; +} + +class StopWatch { +public: + void start() { + Q_ASSERT(_start == 0); + _start = usecTimestampNow(); + } + + void stop() { + Q_ASSERT(_start != 0); + _last = usecTimestampNow() - _start; + _start = 0; + _total += _last; + _count++; + } + + quint64 getLast() { + return _last; + } + + quint64 getTotal() { + return _total; + } + + float getAverage() { + return (float)_total / (float)_count; + } + + void reset() { + _last = _start = _total = _count = 0; + } + +private: + size_t _count{ 0 }; + quint64 _total{ 0 }; + quint64 _start{ 0 }; + quint64 _last{ 0 }; +}; + +template +void testByteCountCodedStable(const T& value) { + ByteCountCoded coder((T)value); + auto encoded = coder.encode(); + auto originalEncodedSize = encoded.size(); + for (int i = 0; i < 10; ++i) { + encoded.append(qrand()); + } + ByteCountCoded decoder; + decoder.decode(encoded); + Q_ASSERT(decoder.data == coder.data); + auto consumed = decoder.decode(encoded.data(), encoded.size()); + Q_ASSERT(consumed == originalEncodedSize); + +} + +template +void testByteCountCoded() { + testByteCountCodedStable(0); + testByteCountCodedStable(1); + testByteCountCodedStable(1 << 16); + testByteCountCodedStable(std::numeric_limits::max() >> 16); + testByteCountCodedStable(std::numeric_limits::max() >> 8); + testByteCountCodedStable(std::numeric_limits::max() >> 1); + testByteCountCodedStable(std::numeric_limits::max()); +} + +void testPropertyFlags(uint32_t value) { + EntityPropertyFlags original; + original.clear(); + auto enumSize = sizeof(EntityPropertyList); + for (size_t i = 0; i < sizeof(EntityPropertyList) * 8; ++i) { + original.setHasProperty((EntityPropertyList)i); + } + QByteArray encoded = original.encode(); + auto originalSize = encoded.size(); + for (size_t i = 0; i < sizeof(EntityPropertyList); ++i) { + encoded.append(qrand()); + } + + EntityPropertyFlags decodeOld, decodeNew; + { + decodeOld.decode(encoded); + Q_ASSERT(decodeOld == original); + } + + { + auto decodeSize = decodeNew.decode((const uint8_t*)encoded.data(), encoded.size()); + Q_ASSERT(originalSize == decodeSize); + Q_ASSERT(decodeNew == original); + } +} + +void testPropertyFlags() { + testPropertyFlags(0); + testPropertyFlags(1); + testPropertyFlags(1 << 16); + testPropertyFlags(0xFFFF); +} + +int main(int argc, char** argv) { + QCoreApplication app(argc, argv); + { + auto start = usecTimestampNow(); + for (int i = 0; i < 1000; ++i) { + testPropertyFlags(); + testByteCountCoded(); + testByteCountCoded(); + testByteCountCoded(); + testByteCountCoded(); + } + auto duration = usecTimestampNow() - start; + qDebug() << duration; + + } + DependencyManager::set(NodeType::Unassigned); + + QFile file(getTestResourceDir() + "packet.bin"); + if (!file.open(QIODevice::ReadOnly)) return -1; + QByteArray packet = file.readAll(); + EntityItemPointer item = BoxEntityItem::factory(EntityItemID(), EntityItemProperties()); + ReadBitstreamToTreeParams params; + params.bitstreamVersion = 33; + + auto start = usecTimestampNow(); + for (int i = 0; i < 1000; ++i) { + item->readEntityDataFromBuffer(reinterpret_cast(packet.constData()), packet.size(), params); + } + float duration = (usecTimestampNow() - start); + qDebug() << (duration / 1000.0f); + return 0; +} + +#include "main.moc"