From 359ee03dd8b1f16a4f079b30f85d6ce74cf80f2e Mon Sep 17 00:00:00 2001 From: Jan Date: Fri, 25 Oct 2019 11:04:16 +0200 Subject: [PATCH] upgrade vpack library (#10314) --- .../velocypack/include/velocypack/Buffer.h | 32 ++- .../velocypack/include/velocypack/Builder.h | 75 ++++-- .../velocypack/include/velocypack/HexDump.h | 12 +- .../velocypack/include/velocypack/Options.h | 26 ++- 3rdParty/velocypack/include/velocypack/Sink.h | 2 +- .../velocypack/include/velocypack/Slice.h | 14 +- .../include/velocypack/velocypack-aliases.h | 1 + 3rdParty/velocypack/src/Builder.cpp | 217 +++++++++++------- 3rdParty/velocypack/src/HexDump.cpp | 13 +- 3rdParty/velocypack/src/SliceStaticData.cpp | 3 +- arangod/Cluster/ClusterMethods.cpp | 6 +- arangod/Graph/GraphOperations.cpp | 10 +- arangod/MMFiles/MMFilesCollectionKeys.cpp | 4 +- arangod/RestHandler/RestGraphHandler.cpp | 2 +- .../RocksDBReplicationContext.cpp | 4 +- .../RocksDBReplicationTailing.cpp | 2 +- arangod/Transaction/Helpers.cpp | 2 +- arangod/Transaction/Methods.cpp | 2 +- 18 files changed, 269 insertions(+), 158 deletions(-) diff --git a/3rdParty/velocypack/include/velocypack/Buffer.h b/3rdParty/velocypack/include/velocypack/Buffer.h index 8dac5b8dc8..3782792888 100644 --- a/3rdParty/velocypack/include/velocypack/Buffer.h +++ b/3rdParty/velocypack/include/velocypack/Buffer.h @@ -55,11 +55,12 @@ class Buffer { Buffer(Buffer const& that) : Buffer() { if (that._size > 0) { - if (that._size > sizeof(_local)) { - _buffer = static_cast(malloc(checkOverflow(sizeof(T) * that._size))); + if (that._size > sizeof(that._local)) { + _buffer = static_cast(malloc(checkOverflow(that._size))); ensureValidPointer(_buffer); _capacity = that._size; } else { + VELOCYPACK_ASSERT(_buffer == &_local[0]); _capacity = sizeof(_local); } memcpy(_buffer, that._buffer, checkOverflow(that._size)); @@ -73,10 +74,9 @@ class Buffer { // our own buffer is big enough to hold the data initWithNone(); memcpy(_buffer, that._buffer, checkOverflow(that._size)); - } - else { + } else { // our own buffer is not big enough to hold the data - T* buffer = static_cast(malloc(checkOverflow(sizeof(T) * that._size))); + T* buffer = static_cast(malloc(checkOverflow(that._size))); ensureValidPointer(buffer); buffer[0] = '\x00'; memcpy(buffer, that._buffer, checkOverflow(that._size)); @@ -94,8 +94,11 @@ class Buffer { } Buffer(Buffer&& that) noexcept : _buffer(_local), _capacity(sizeof(_local)) { + poison(_buffer, _capacity); + initWithNone(); if (that._buffer == that._local) { - memcpy(_buffer, that._buffer, static_cast(that._size)); + VELOCYPACK_ASSERT(that._capacity == sizeof(that._local)); + memcpy(_buffer, that._buffer, checkOverflow(that._size)); } else { _buffer = that._buffer; _capacity = that._capacity; @@ -104,16 +107,20 @@ class Buffer { } _size = that._size; that._size = 0; + that.initWithNone(); } Buffer& operator=(Buffer&& that) noexcept { if (this != &that) { + if (_buffer != _local) { + free(_buffer); + } if (that._buffer == that._local) { - memcpy(_buffer, that._buffer, static_cast(that._size)); + _buffer = _local; + _capacity = sizeof(_local); + initWithNone(); + memcpy(_buffer, that._buffer, checkOverflow(that._size)); } else { - if (_buffer != _local) { - free(_buffer); - } _buffer = that._buffer; _capacity = that._capacity; that._buffer = that._local; @@ -121,6 +128,7 @@ class Buffer { } _size = that._size; that._size = 0; + that.initWithNone(); } return *this; } @@ -281,11 +289,11 @@ class Buffer { VELOCYPACK_ASSERT(newLen > 0); T* p; if (_buffer != _local) { - p = static_cast(realloc(_buffer, checkOverflow(sizeof(T) * newLen))); + p = static_cast(realloc(_buffer, checkOverflow(newLen))); ensureValidPointer(p); // realloc will have copied the old data } else { - p = static_cast(malloc(checkOverflow(sizeof(T) * newLen))); + p = static_cast(malloc(checkOverflow(newLen))); ensureValidPointer(p); // copy existing data into buffer memcpy(p, _buffer, checkOverflow(_size)); diff --git a/3rdParty/velocypack/include/velocypack/Builder.h b/3rdParty/velocypack/include/velocypack/Builder.h index a6fd2edbaa..db26ec5576 100644 --- a/3rdParty/velocypack/include/velocypack/Builder.h +++ b/3rdParty/velocypack/include/velocypack/Builder.h @@ -91,37 +91,58 @@ class Builder { public: Options const* options; - // create an empty Builder, using default Options - Builder(); - - explicit Builder(Options const* options); + // create an empty Builder, using Options + explicit Builder(Options const* options = &Options::Defaults); + + // create an empty Builder, using an existing buffer explicit Builder(std::shared_ptr> const& buffer, Options const* options = &Options::Defaults); + + // create a Builder that uses an existing Buffer. the Builder will not + // claim ownership for this Buffer explicit Builder(Buffer& buffer, Options const* options = &Options::Defaults); + + // populate a Builder from a Slice explicit Builder(Slice slice, Options const* options = &Options::Defaults); + ~Builder() = default; Builder(Builder const& that); Builder& operator=(Builder const& that); - Builder(Builder&& that); - Builder& operator=(Builder&& that); + Builder(Builder&& that) noexcept; + Builder& operator=(Builder&& that) noexcept; - // get a const reference to the Builder's Buffer object - std::shared_ptr> const& buffer() const { return _buffer; } + // get a reference to the Builder's Buffer object + // note: this object may be a nullptr if the buffer was already stolen + // from the Builder, or if the Builder has no ownership for the Buffer + std::shared_ptr> const& buffer() const { + return _buffer; + } + + Buffer& bufferRef() const { + if (_bufferPtr == nullptr) { + throw Exception(Exception::InternalError, "Builder has no Buffer"); + } + return *_bufferPtr; + } // steal the Builder's Buffer object. afterwards the Builder - // is unusable + // is unusable - note: this may return a nullptr if the Builder does not + // own the Buffer! std::shared_ptr> steal() { // After a steal the Builder is broken! - std::shared_ptr> res = _buffer; - _buffer.reset(); + std::shared_ptr> res(std::move(_buffer)); _bufferPtr = nullptr; - _pos = 0; + _start = nullptr; + clear(); return res; } - uint8_t const* data() const noexcept { return _bufferPtr->data(); } + uint8_t const* data() const noexcept { + VELOCYPACK_ASSERT(_bufferPtr != nullptr); + return _bufferPtr->data(); + } std::string toString() const; @@ -153,6 +174,7 @@ class Builder { (void) checkOverflow(_pos + len); #endif + VELOCYPACK_ASSERT(_bufferPtr != nullptr); _bufferPtr->reserve(len); _start = _bufferPtr->data(); } @@ -161,8 +183,11 @@ class Builder { void clear() noexcept { _pos = 0; _stack.clear(); - VELOCYPACK_ASSERT(_bufferPtr != nullptr); - _bufferPtr->reset(); + _index.clear(); + if (_bufferPtr != nullptr) { + _bufferPtr->reset(); + _start = _bufferPtr->data(); + } _keyWritten = false; } @@ -564,8 +589,8 @@ class Builder { uint8_t* addInternal(char const* attrName, std::size_t attrLength, T const& sub) { bool haveReported = false; if (!_stack.empty()) { - ValueLength& tos = _stack.back(); - if (VELOCYPACK_UNLIKELY(_start[tos] != 0x0b && _start[tos] != 0x14)) { + ValueLength const to = _stack.back(); + if (VELOCYPACK_UNLIKELY(_start[to] != 0x0b && _start[to] != 0x14)) { throw Exception(Exception::BuilderNeedOpenObject); } if (VELOCYPACK_UNLIKELY(_keyWritten)) { @@ -621,9 +646,9 @@ class Builder { void openCompoundValue(uint8_t type) { bool haveReported = false; if (!_stack.empty()) { - ValueLength& tos = _stack.back(); + ValueLength const to = _stack.back(); if (!_keyWritten) { - if (VELOCYPACK_UNLIKELY(_start[tos] != 0x06 && _start[tos] != 0x13)) { + if (VELOCYPACK_UNLIKELY(_start[to] != 0x06 && _start[to] != 0x13)) { throw Exception(Exception::BuilderNeedOpenArray); } reportAdd(); @@ -797,11 +822,13 @@ struct ObjectBuilder final : public BuilderContainer, } ~ObjectBuilder() { try { - builder->close(); + if (!builder->isClosed()) { + builder->close(); + } } catch (...) { // destructors must not throw. however, we can at least // signal something is very wrong in debug mode - VELOCYPACK_ASSERT(false); + VELOCYPACK_ASSERT(builder->isClosed()); } } }; @@ -826,11 +853,13 @@ struct ArrayBuilder final : public BuilderContainer, } ~ArrayBuilder() { try { - builder->close(); + if (!builder->isClosed()) { + builder->close(); + } } catch (...) { // destructors must not throw. however, we can at least // signal something is very wrong in debug mode - VELOCYPACK_ASSERT(false); + VELOCYPACK_ASSERT(builder->isClosed()); } } }; diff --git a/3rdParty/velocypack/include/velocypack/HexDump.h b/3rdParty/velocypack/include/velocypack/HexDump.h index e49cc21782..a6900c58e4 100644 --- a/3rdParty/velocypack/include/velocypack/HexDump.h +++ b/3rdParty/velocypack/include/velocypack/HexDump.h @@ -41,14 +41,15 @@ struct HexDump { HexDump() = delete; HexDump(Slice const& slice, int valuesPerLine = 16, - std::string const& separator = " ") - : slice(slice), valuesPerLine(valuesPerLine), separator(separator) {} + std::string const& separator = " ", std::string const& header = "0x") + : slice(slice), valuesPerLine(valuesPerLine), separator(separator), header(header) {} HexDump(Slice const* slice, int valuesPerLine = 16, - std::string const& separator = " ") - : HexDump(*slice, valuesPerLine, separator) {} + std::string const& separator = " ", std::string const& header = "0x") + : HexDump(*slice, valuesPerLine, separator, header) {} - static std::string toHex(uint8_t value); + static std::string toHex(uint8_t value, std::string const& header = "0x"); + static void appendHex(std::string& result, uint8_t value); std::string toString() const; friend std::ostream& operator<<(std::ostream&, HexDump const&); @@ -56,6 +57,7 @@ struct HexDump { Slice const slice; int valuesPerLine; std::string separator; + std::string header; }; } // namespace arangodb::velocypack diff --git a/3rdParty/velocypack/include/velocypack/Options.h b/3rdParty/velocypack/include/velocypack/Options.h index 00042fd723..8d3644bf0d 100644 --- a/3rdParty/velocypack/include/velocypack/Options.h +++ b/3rdParty/velocypack/include/velocypack/Options.h @@ -39,24 +39,48 @@ struct Options; class Slice; struct CustomTypeHandler { - virtual ~CustomTypeHandler() {} + virtual ~CustomTypeHandler() = default; virtual void dump(Slice const&, Dumper*, Slice const&); virtual std::string toString(Slice const&, Options const*, Slice const&); }; struct Options { + // Behavior to be applied when dumping VelocyPack values that cannot be + // expressed in JSON without data loss enum UnsupportedTypeBehavior { + // convert any non-JSON-representable value to null NullifyUnsupportedType, + // emit a JSON string "(non-representable type ...)" ConvertUnsupportedType, + // throw an exception for any non-JSON-representable value FailOnUnsupportedType }; + // Behavior to be applied when building VelocyPack Array/Object values + // with a Builder + enum PaddingBehavior { + // use padding - fill unused head bytes with zero-bytes (ASCII NUL) in + // order to avoid a later memmove + UsePadding, + // don't pad and do not fill any gaps with zero-bytes (ASCII NUL). + // instead, memmove data down so there is no gap between the head bytes + // and the payload + NoPadding, + // pad in cases the Builder considers it useful, and don't pad in other + // cases when the Builder doesn't consider it useful + Flexible + }; + Options() {} // Dumper behavior when a VPack value is serialized to JSON that // has no JSON equivalent UnsupportedTypeBehavior unsupportedTypeBehavior = FailOnUnsupportedType; + + // Builder behavior w.r.t. padding or memmoving data + PaddingBehavior paddingBehavior = PaddingBehavior::Flexible; + // custom attribute translator for integer keys AttributeTranslator* attributeTranslator = nullptr; // custom type handler used for processing custom types by Dumper and Slicer diff --git a/3rdParty/velocypack/include/velocypack/Sink.h b/3rdParty/velocypack/include/velocypack/Sink.h index c15fcaf226..f203ac7332 100644 --- a/3rdParty/velocypack/include/velocypack/Sink.h +++ b/3rdParty/velocypack/include/velocypack/Sink.h @@ -42,7 +42,7 @@ struct Sink { Sink(Sink const&) = delete; Sink& operator=(Sink const&) = delete; - virtual ~Sink() {} + virtual ~Sink() = default; virtual void push_back(char c) = 0; virtual void append(std::string const& p) = 0; virtual void append(char const* p) = 0; diff --git a/3rdParty/velocypack/include/velocypack/Slice.h b/3rdParty/velocypack/include/velocypack/Slice.h index 43fb16a0b0..890a89835e 100644 --- a/3rdParty/velocypack/include/velocypack/Slice.h +++ b/3rdParty/velocypack/include/velocypack/Slice.h @@ -47,13 +47,12 @@ namespace arangodb { namespace velocypack { +// This class provides read only access to a VPack value, it is +// intentionally light-weight (only one pointer value), such that +// it can easily be used to traverse larger VPack values. + +// A Slice does not own the VPack data it points to! class Slice { - // This class provides read only access to a VPack value, it is - // intentionally light-weight (only one pointer value), such that - // it can easily be used to traverse larger VPack values. - - // A Slice does not own the VPack data it points to! - friend class Builder; friend class ArrayIterator; friend class ObjectIterator; @@ -1105,6 +1104,9 @@ class Slice { } }; +static_assert(!std::is_polymorphic::value, "Slice must not be polymorphic"); +static_assert(!std::has_virtual_destructor::value, "Slice must not have virtual dtor"); + } // namespace arangodb::velocypack } // namespace arangodb diff --git a/3rdParty/velocypack/include/velocypack/velocypack-aliases.h b/3rdParty/velocypack/include/velocypack/velocypack-aliases.h index f1a50be90e..828666517c 100644 --- a/3rdParty/velocypack/include/velocypack/velocypack-aliases.h +++ b/3rdParty/velocypack/include/velocypack/velocypack-aliases.h @@ -70,6 +70,7 @@ using VPackNormalizedCompare = arangodb::velocypack::NormalizedCompare; #ifdef VELOCYPACK_BUFFER_H #ifndef VELOCYPACK_ALIAS_BUFFER #define VELOCYPACK_ALIAS_BUFFER +using VPackCharBuffer = arangodb::velocypack::CharBuffer; using VPackBufferUInt8 = arangodb::velocypack::UInt8Buffer; template using VPackBuffer = arangodb::velocypack::Buffer; #endif diff --git a/3rdParty/velocypack/src/Builder.cpp b/3rdParty/velocypack/src/Builder.cpp index bc5b0c397c..30c3c7b638 100644 --- a/3rdParty/velocypack/src/Builder.cpp +++ b/3rdParty/velocypack/src/Builder.cpp @@ -89,6 +89,7 @@ bool checkAttributeUniquenessUnsortedBrute(ObjectIterator& it) { do { // key(true) guarantees a String as returned type StringRef key = it.key(true).stringRef(); + ValueLength index = it.index(); // compare with all other already looked-at keys for (ValueLength i = 0; i < index; ++i) { @@ -124,19 +125,9 @@ bool checkAttributeUniquenessUnsortedSet(ObjectIterator& it) { return true; } - } // namespace -// create an empty Builder, using default Options -Builder::Builder() - : _buffer(std::make_shared>()), - _bufferPtr(_buffer.get()), - _start(_bufferPtr->data()), - _pos(0), - _keyWritten(false), - options(&Options::Defaults) {} - -// create an empty Builder, with custom Options +// create an empty Builder, using Options Builder::Builder(Options const* options) : _buffer(std::make_shared>()), _bufferPtr(_buffer.get()), @@ -149,9 +140,11 @@ Builder::Builder(Options const* options) } } +// create an empty Builder, using an existing buffer Builder::Builder(std::shared_ptr> const& buffer, Options const* options) : _buffer(buffer), _bufferPtr(_buffer.get()), + _start(nullptr), _pos(0), _keyWritten(false), options(options) { @@ -165,42 +158,62 @@ Builder::Builder(std::shared_ptr> const& buffer, Options const* } } +// create a Builder that uses an existing Buffer. the Builder will not +// claim ownership for this Buffer Builder::Builder(Buffer& buffer, Options const* options) - : _bufferPtr(nullptr), + : _bufferPtr(&buffer), + _start(_bufferPtr->data()), _pos(buffer.size()), _keyWritten(false), options(options) { - _buffer.reset(&buffer, BufferNonDeleter()); - _bufferPtr = _buffer.get(); - _start = _bufferPtr->data(); if (VELOCYPACK_UNLIKELY(options == nullptr)) { throw Exception(Exception::InternalError, "Options cannot be a nullptr"); } } +// populate a Builder from a Slice Builder::Builder(Slice slice, Options const* options) : Builder(options) { add(slice); } Builder::Builder(Builder const& that) - : _buffer(std::make_shared>(*that._buffer)), - _bufferPtr(_buffer.get()), - _start(_bufferPtr->data()), + : _bufferPtr(nullptr), + _start(nullptr), _pos(that._pos), _stack(that._stack), _index(that._index), _keyWritten(that._keyWritten), options(that.options) { VELOCYPACK_ASSERT(options != nullptr); + + if (that._buffer == nullptr) { + _bufferPtr = that._bufferPtr; + } else { + _buffer = std::make_shared>(*that._buffer); + _bufferPtr = _buffer.get(); + } + + if (_bufferPtr != nullptr) { + _start = _bufferPtr->data(); + } } Builder& Builder::operator=(Builder const& that) { if (this != &that) { - _buffer = std::make_shared>(*that._buffer); - _bufferPtr = _buffer.get(); - _start = _bufferPtr->data(); + if (that._buffer == nullptr) { + _buffer.reset(); + _bufferPtr = that._bufferPtr; + } else { + _buffer = std::make_shared>(*that._buffer); + _bufferPtr = _buffer.get(); + } + if (_bufferPtr == nullptr) { + _start = nullptr; + } else { + _start = _bufferPtr->data(); + } _pos = that._pos; _stack = that._stack; _index = that._index; @@ -211,41 +224,50 @@ Builder& Builder::operator=(Builder const& that) { return *this; } -Builder::Builder(Builder&& that) { - if (VELOCYPACK_UNLIKELY(!that.isClosed())) { - throw Exception(Exception::InternalError, "Cannot move an open Builder"); +Builder::Builder(Builder&& that) noexcept + : _buffer(std::move(that._buffer)), + _bufferPtr(nullptr), + _start(nullptr), + _pos(that._pos), + _stack(std::move(that._stack)), + _index(std::move(that._index)), + _keyWritten(that._keyWritten), + options(that.options) { + + if (_buffer != nullptr) { + _bufferPtr = _buffer.get(); + } else { + _bufferPtr = that._bufferPtr; } - _buffer = that._buffer; - _bufferPtr = _buffer.get(); - _start = _bufferPtr->data(); - _pos = that._pos; - _stack.clear(); - _stack.swap(that._stack); - _index.clear(); - _index.swap(that._index); - _keyWritten = that._keyWritten; - options = that.options; - that._pos = 0; - that._keyWritten = false; + if (_bufferPtr != nullptr) { + _start = _bufferPtr->data(); + } + VELOCYPACK_ASSERT(that._buffer == nullptr); + that._bufferPtr = nullptr; + that.clear(); } -Builder& Builder::operator=(Builder&& that) { - if (VELOCYPACK_UNLIKELY(!that.isClosed())) { - throw Exception(Exception::InternalError, "Cannot move an open Builder"); - } +Builder& Builder::operator=(Builder&& that) noexcept { if (this != &that) { - _buffer = that._buffer; - _bufferPtr = _buffer.get(); - _start = _bufferPtr->data(); + _buffer = std::move(that._buffer); + if (_buffer != nullptr) { + _bufferPtr = _buffer.get(); + } else { + _bufferPtr = that._bufferPtr; + } + if (_bufferPtr != nullptr) { + _start = _bufferPtr->data(); + } else { + _start = nullptr; + } _pos = that._pos; - _stack.clear(); - _stack.swap(that._stack); - _index.clear(); - _index.swap(that._index); + _stack = std::move(that._stack); + _index = std::move(that._index); _keyWritten = that._keyWritten; options = that.options; - that._pos = 0; - that._keyWritten = false; + VELOCYPACK_ASSERT(that._buffer == nullptr); + that._bufferPtr = nullptr; + that.clear(); } return *this; } @@ -444,41 +466,50 @@ Builder& Builder::closeArray(ValueLength tos, std::vector& index) { offsetSize = 8; } + if (offsetSize < 8 && + !needIndexTable && + options->paddingBehavior == Options::PaddingBehavior::UsePadding) { + // if we are allowed to use padding, we will pad to 8 bytes anyway. as we are not + // using an index table, we can also use type 0x05 for all Arrays without making + // things worse space-wise + offsetSize = 8; + } + // Maybe we need to move down data: - if (offsetSize == 1) { + if (offsetSize == 1 || offsetSize == 2) { // check if one of the first entries in the array is ValueType::None // (0x00). in this case, we could not distinguish between a None (0x00) // and the optional padding. so we must prevent the memmove here - bool allowMemMove = true; - std::size_t const n = (std::min)(std::size_t(6), index.size()); - for (std::size_t i = 0; i < n; i++) { - if (_start[tos + index[i]] == 0x00) { - allowMemMove = false; - break; - } - } + bool allowMemMove = options->paddingBehavior == Options::PaddingBehavior::NoPadding || + (offsetSize == 1 && options->paddingBehavior == Options::PaddingBehavior::Flexible); if (allowMemMove) { - ValueLength targetPos = 3; - if (!needIndexTable) { - targetPos = 2; - } - if (_pos > (tos + 9)) { - ValueLength len = _pos - (tos + 9); - memmove(_start + tos + targetPos, _start + tos + 9, checkOverflow(len)); - } - ValueLength const diff = 9 - targetPos; - rollback(diff); - if (needIndexTable) { - std::size_t const n = index.size(); - for (std::size_t i = 0; i < n; i++) { - index[i] -= diff; + std::size_t const n = (std::min)(std::size_t(8 - 2 * offsetSize), index.size()); + for (std::size_t i = 0; i < n; i++) { + if (_start[tos + index[i]] == 0x00) { + allowMemMove = false; + break; } - } // Note: if !needIndexTable the index array is now wrong! + } + if (allowMemMove) { + ValueLength targetPos = 1 + 2 * offsetSize; + if (!needIndexTable) { + targetPos -= offsetSize; + } + if (_pos > (tos + 9)) { + ValueLength len = _pos - (tos + 9); + memmove(_start + tos + targetPos, _start + tos + 9, checkOverflow(len)); + } + ValueLength const diff = 9 - targetPos; + rollback(diff); + if (needIndexTable) { + std::size_t const n = index.size(); + for (std::size_t i = 0; i < n; i++) { + index[i] -= diff; + } + } // Note: if !needIndexTable the index array is now wrong! + } } } - // One could move down things in the offsetSize == 2 case as well, - // since we only need 4 bytes in the beginning. However, saving these - // 4 bytes has been sacrificed on the Altar of Performance. // Now build the table: if (needIndexTable) { @@ -558,6 +589,13 @@ Builder& Builder::close() { (head == 0x06 && options->buildUnindexedArrays) || (head == 0x0b && (options->buildUnindexedObjects || index.size() == 1))) { if (closeCompactArrayOrObject(tos, isArray, index)) { + // And, if desired, check attribute uniqueness: + if (options->checkAttributeUniqueness && + index.size() > 1 && + !checkAttributeUniqueness(Slice(_start + tos))) { + // duplicate attribute name! + throw Exception(Exception::DuplicateAttributeName); + } return *this; } // This might fall through, if closeCompactArrayOrObject gave up! @@ -583,9 +621,20 @@ Builder& Builder::close() { // case we would win back 6 bytes but would need one byte per subvalue // for the index table offsetSize = 1; - + // One could move down things in the offsetSize == 2 case as well, + // since we only need 4 bytes in the beginning. However, saving these + // 4 bytes has been sacrificed on the Altar of Performance. + } else if (_pos - tos + 2 * index.size() <= 0xffff) { + offsetSize = 2; + } else if (_pos - tos + 4 * index.size() <= 0xffffffffu) { + offsetSize = 4; + } + + if (offsetSize < 4 && + (options->paddingBehavior == Options::PaddingBehavior::NoPadding || + (offsetSize == 1 && options->paddingBehavior == Options::PaddingBehavior::Flexible))) { // Maybe we need to move down data: - ValueLength targetPos = 3; + ValueLength targetPos = 1 + 2 * offsetSize; if (_pos > (tos + 9)) { ValueLength len = _pos - (tos + 9); memmove(_start + tos + targetPos, _start + tos + 9, checkOverflow(len)); @@ -596,14 +645,6 @@ Builder& Builder::close() { for (std::size_t i = 0; i < n; i++) { index[i] -= diff; } - - // One could move down things in the offsetSize == 2 case as well, - // since we only need 4 bytes in the beginning. However, saving these - // 4 bytes has been sacrificed on the Altar of Performance. - } else if (_pos - tos + 2 * index.size() <= 0xffff) { - offsetSize = 2; - } else if (_pos - tos + 4 * index.size() <= 0xffffffffu) { - offsetSize = 4; } // Now build the table: @@ -1042,7 +1083,7 @@ bool Builder::checkAttributeUniqueness(Slice obj) const { VELOCYPACK_ASSERT(options->checkAttributeUniqueness == true); VELOCYPACK_ASSERT(obj.isObject()); VELOCYPACK_ASSERT(obj.length() >= 2); - + if (obj.isSorted()) { // object attributes are sorted return checkAttributeUniquenessSorted(obj); @@ -1091,7 +1132,7 @@ bool Builder::checkAttributeUniquenessUnsorted(Slice obj) const { // will use an std::unordered_set for O(1) lookups but with heap // allocations ObjectIterator it(obj, true); - + if (it.size() <= ::LinearAttributeUniquenessCutoff) { return ::checkAttributeUniquenessUnsortedBrute(it); } diff --git a/3rdParty/velocypack/src/HexDump.cpp b/3rdParty/velocypack/src/HexDump.cpp index 5647056ef7..cc91c648cb 100644 --- a/3rdParty/velocypack/src/HexDump.cpp +++ b/3rdParty/velocypack/src/HexDump.cpp @@ -31,15 +31,17 @@ using namespace arangodb::velocypack; -std::string HexDump::toHex(uint8_t value) { - std::string result("0x"); +std::string HexDump::toHex(uint8_t value, std::string const& header) { + std::string result(header); + appendHex(result, value); + return result; +} +void HexDump::appendHex(std::string& result, uint8_t value) { uint8_t x = value / 16; result.push_back((x < 10 ? ('0' + x) : ('a' + x - 10))); x = value % 16; result.push_back((x < 10 ? ('0' + x) : ('a' + x - 10))); - - return result; } std::string HexDump::toString() const { @@ -59,7 +61,8 @@ std::string HexDump::toString() const { } } - result.append(HexDump::toHex(it)); + result.append(header); + HexDump::appendHex(result, it); ++current; } diff --git a/3rdParty/velocypack/src/SliceStaticData.cpp b/3rdParty/velocypack/src/SliceStaticData.cpp index 65bd5b7e1c..f2fc60d2a1 100644 --- a/3rdParty/velocypack/src/SliceStaticData.cpp +++ b/3rdParty/velocypack/src/SliceStaticData.cpp @@ -28,9 +28,10 @@ using namespace arangodb::velocypack; +#if __cplusplus < 201703L constexpr uint8_t SliceStaticData::FixedTypeLengths[256]; constexpr ValueType SliceStaticData::TypeMap[256]; constexpr unsigned int SliceStaticData::WidthMap[32]; constexpr unsigned int SliceStaticData::FirstSubMap[32]; constexpr uint64_t SliceStaticData::PrecalculatedHashesForDefaultSeed[256]; - +#endif diff --git a/arangod/Cluster/ClusterMethods.cpp b/arangod/Cluster/ClusterMethods.cpp index 6e4318d114..b394f1be50 100644 --- a/arangod/Cluster/ClusterMethods.cpp +++ b/arangod/Cluster/ClusterMethods.cpp @@ -1824,7 +1824,7 @@ int fetchEdgesFromEngines(transaction::Methods& trx, futures.emplace_back( network::sendRequestRetry(pool, "server:" + engine.first, fuerte::RestVerb::Put, url + StringUtils::itoa(engine.second), - *leased->buffer(), network::Timeout(CL_DEFAULT_TIMEOUT))); + leased->bufferRef(), network::Timeout(CL_DEFAULT_TIMEOUT))); } for (Future& f : futures) { @@ -1912,7 +1912,7 @@ int fetchEdgesFromEngines( futures.emplace_back( network::sendRequestRetry(pool, "server:" + engine.first, fuerte::RestVerb::Put, url + StringUtils::itoa(engine.second), - *leased->buffer(), network::Timeout(CL_DEFAULT_TIMEOUT))); + leased->bufferRef(), network::Timeout(CL_DEFAULT_TIMEOUT))); } for (Future& f : futures) { @@ -1999,7 +1999,7 @@ void fetchVerticesFromEngines( futures.emplace_back( network::sendRequestRetry(pool, "server:" + engine.first, fuerte::RestVerb::Put, url + StringUtils::itoa(engine.second), - *(leased->buffer()), network::Timeout(CL_DEFAULT_TIMEOUT))); + leased->bufferRef(), network::Timeout(CL_DEFAULT_TIMEOUT))); } for (Future& f : futures) { diff --git a/arangod/Graph/GraphOperations.cpp b/arangod/Graph/GraphOperations.cpp index 200a5fbaa1..43879bd38e 100644 --- a/arangod/Graph/GraphOperations.cpp +++ b/arangod/Graph/GraphOperations.cpp @@ -510,8 +510,8 @@ OperationResult GraphOperations::getDocument(std::string const& collectionName, return result; } -GraphOperations::VPackBufferPtr GraphOperations::_getSearchSlice( - const std::string& key, boost::optional& rev) const { +GraphOperations::VPackBufferPtr GraphOperations::_getSearchSlice(std::string const& key, + boost::optional& rev) const { VPackBuilder builder; { VPackObjectBuilder guard(&builder); @@ -521,11 +521,11 @@ GraphOperations::VPackBufferPtr GraphOperations::_getSearchSlice( } } - return builder.buffer(); + return builder.steal(); } -OperationResult GraphOperations::removeEdge(const std::string& definitionName, - const std::string& key, +OperationResult GraphOperations::removeEdge(std::string const& definitionName, + std::string const& key, boost::optional rev, bool waitForSync, bool returnOld) { return removeEdgeOrVertex(definitionName, key, rev, waitForSync, returnOld); diff --git a/arangod/MMFiles/MMFilesCollectionKeys.cpp b/arangod/MMFiles/MMFilesCollectionKeys.cpp index dcef033592..29bb67f554 100644 --- a/arangod/MMFiles/MMFilesCollectionKeys.cpp +++ b/arangod/MMFiles/MMFilesCollectionKeys.cpp @@ -201,7 +201,7 @@ void MMFilesCollectionKeys::dumpDocs(arangodb::velocypack::Builder& result, THROW_ARANGO_EXCEPTION(TRI_ERROR_BAD_PARAMETER); } - auto buffer = result.buffer(); + auto& buffer = result.bufferRef(); size_t offset = 0; for (VPackSlice it : VPackArrayIterator(ids)) { @@ -223,7 +223,7 @@ void MMFilesCollectionKeys::dumpDocs(arangodb::velocypack::Builder& result, TRI_ASSERT(current.isObject()); result.add(current); - if (buffer->byteSize() > maxChunkSize) { + if (buffer.byteSize() > maxChunkSize) { // buffer is full break; } diff --git a/arangod/RestHandler/RestGraphHandler.cpp b/arangod/RestHandler/RestGraphHandler.cpp index 5a549de281..08c751e34d 100644 --- a/arangod/RestHandler/RestGraphHandler.cpp +++ b/arangod/RestHandler/RestGraphHandler.cpp @@ -525,7 +525,7 @@ void RestGraphHandler::generateResultMergedWithObject(VPackSlice obj, result.close(); VPackBuilder merged = VelocyPackHelper::merge(result.slice(), obj, false, false); - writeResult(std::move(*merged.buffer().get()), options); + writeResult(merged.slice(), options); } catch (...) { // Building the error response failed generateError(rest::ResponseCode::SERVER_ERROR, TRI_ERROR_INTERNAL, diff --git a/arangod/RocksDBEngine/RocksDBReplicationContext.cpp b/arangod/RocksDBEngine/RocksDBReplicationContext.cpp index 435cfe2dc1..109b288e43 100644 --- a/arangod/RocksDBEngine/RocksDBReplicationContext.cpp +++ b/arangod/RocksDBEngine/RocksDBReplicationContext.cpp @@ -636,7 +636,7 @@ arangodb::Result RocksDBReplicationContext::dumpDocuments( auto* rcoll = static_cast(cIter->logical->getPhysical()); const uint64_t cObjectId = rcoll->objectId(); - auto buffer = b.buffer(); + auto& buffer = b.bufferRef(); bool hasMore = true; b.openArray(true); size_t oldPos = from; @@ -693,7 +693,7 @@ arangodb::Result RocksDBReplicationContext::dumpDocuments( hasMore = cIter->hasMore(); } - if (buffer->byteSize() > maxChunkSize) { + if (buffer.byteSize() > maxChunkSize) { // result is big enough so that we abort prematurely full = true; } diff --git a/arangod/RocksDBEngine/RocksDBReplicationTailing.cpp b/arangod/RocksDBEngine/RocksDBReplicationTailing.cpp index f4df939002..f20f135bdb 100644 --- a/arangod/RocksDBEngine/RocksDBReplicationTailing.cpp +++ b/arangod/RocksDBEngine/RocksDBReplicationTailing.cpp @@ -673,7 +673,7 @@ RocksDBReplicationResult rocksutils::tailWal(TRI_vocbase_t* vocbase, uint64_t ti // we need to check if the builder is bigger than the chunksize, // only after we printed a full WriteBatch. Otherwise a client might // never read the full writebatch - while (iterator->Valid() && lastTick <= tickEnd && builder.buffer()->size() < chunkSize) { + while (iterator->Valid() && lastTick <= tickEnd && builder.bufferRef().size() < chunkSize) { s = iterator->status(); if (!s.ok()) { LOG_TOPIC("ed096", ERR, Logger::REPLICATION) << "error during WAL scan: " << s.ToString(); diff --git a/arangod/Transaction/Helpers.cpp b/arangod/Transaction/Helpers.cpp index af57db373d..c3c1c090e9 100644 --- a/arangod/Transaction/Helpers.cpp +++ b/arangod/Transaction/Helpers.cpp @@ -392,7 +392,7 @@ OperationResult transaction::helpers::buildCountResult( } resultBuilder.add(VPackValue(result)); } - return OperationResult(Result(), resultBuilder.buffer()); + return OperationResult(Result(), resultBuilder.steal()); } /// @brief creates an id string from a custom _id value and the _key string diff --git a/arangod/Transaction/Methods.cpp b/arangod/Transaction/Methods.cpp index 6e9defa8ad..ffc1499068 100644 --- a/arangod/Transaction/Methods.cpp +++ b/arangod/Transaction/Methods.cpp @@ -2609,7 +2609,7 @@ futures::Future transaction::Methods::countCoordinatorHelper( // return number from cache VPackBuilder resultBuilder; resultBuilder.add(VPackValue(documents)); - return OperationResult(Result(), resultBuilder.buffer()); + return OperationResult(Result(), resultBuilder.steal()); } /// @brief count the number of documents in a collection