From 9b71461d69951737c3213185d8023c2b760ce35b Mon Sep 17 00:00:00 2001 From: jsteemann Date: Fri, 15 Sep 2017 12:10:57 +0200 Subject: [PATCH 1/9] change some of the underlying data structures of hash tables for mmfiles this provides a 20%+ speedup on some lookup operations --- arangod/MMFiles/MMFilesEdgeIndex.cpp | 78 +-------- arangod/MMFiles/MMFilesEdgeIndex.h | 74 +++++++- arangod/MMFiles/MMFilesHashIndex.cpp | 132 ++------------ arangod/MMFiles/MMFilesHashIndex.h | 202 ++++++++++++++-------- arangod/MMFiles/MMFilesPrimaryIndex.cpp | 53 +----- arangod/MMFiles/MMFilesPrimaryIndex.h | 53 +++++- arangod/MMFiles/MMFilesRevisionsCache.cpp | 26 +-- arangod/MMFiles/MMFilesRevisionsCache.h | 34 +++- lib/Basics/AssocMulti.h | 76 +++----- lib/Basics/AssocUnique.h | 77 +++------ lib/Basics/AssocUniqueHelpers.h | 6 +- 11 files changed, 356 insertions(+), 455 deletions(-) diff --git a/arangod/MMFiles/MMFilesEdgeIndex.cpp b/arangod/MMFiles/MMFilesEdgeIndex.cpp index c5528b207a94..f088f07c69db 100644 --- a/arangod/MMFiles/MMFilesEdgeIndex.cpp +++ b/arangod/MMFiles/MMFilesEdgeIndex.cpp @@ -54,65 +54,6 @@ static std::vector> const IndexAttributes{{arangodb::basics::AttributeName("_from", false)}, {arangodb::basics::AttributeName("_to", false)}}; -/// @brief hashes an edge key -static uint64_t HashElementKey(void*, VPackSlice const* key) { - TRI_ASSERT(key != nullptr); - // we can get away with the fast hash function here, as edge - // index values are restricted to strings - return MMFilesSimpleIndexElement::hash(*key); -} - -/// @brief hashes an edge -static uint64_t HashElementEdge(void*, MMFilesSimpleIndexElement const& element, - bool byKey) { - if (byKey) { - return element.hash(); - } - - TRI_voc_rid_t revisionId = element.revisionId(); - return fasthash64_uint64(revisionId, 0x56781234); -} - -/// @brief checks if key and element match -static bool IsEqualKeyEdge(void* userData, VPackSlice const* left, - MMFilesSimpleIndexElement const& right) { - TRI_ASSERT(left != nullptr); - IndexLookupContext* context = static_cast(userData); - TRI_ASSERT(context != nullptr); - - try { - VPackSlice tmp = right.slice(context); - TRI_ASSERT(tmp.isString()); - return left->equals(tmp); - } catch (...) { - return false; - } -} - -/// @brief checks for elements are equal -static bool IsEqualElementEdge(void*, MMFilesSimpleIndexElement const& left, - MMFilesSimpleIndexElement const& right) { - return left.revisionId() == right.revisionId(); -} - -/// @brief checks for elements are equal -static bool IsEqualElementEdgeByKey(void* userData, - MMFilesSimpleIndexElement const& left, - MMFilesSimpleIndexElement const& right) { - IndexLookupContext* context = static_cast(userData); - try { - VPackSlice lSlice = left.slice(context); - VPackSlice rSlice = right.slice(context); - - TRI_ASSERT(lSlice.isString()); - TRI_ASSERT(rSlice.isString()); - - return lSlice.equals(rSlice); - } catch (...) { - return false; - } -} - MMFilesEdgeIndexIterator::MMFilesEdgeIndexIterator( LogicalCollection* collection, transaction::Methods* trx, ManagedDocumentResult* mmdr, arangodb::MMFilesEdgeIndex const* index, @@ -192,8 +133,6 @@ MMFilesEdgeIndex::MMFilesEdgeIndex(TRI_idx_iid_t iid, {arangodb::basics::AttributeName(StaticStrings::ToString, false)}}), false, false), - _edgesFrom(nullptr), - _edgesTo(nullptr), _numBuckets(1) { TRI_ASSERT(iid != 0); @@ -206,18 +145,9 @@ MMFilesEdgeIndex::MMFilesEdgeIndex(TRI_idx_iid_t iid, auto context = [this]() -> std::string { return this->context(); }; - _edgesFrom = new TRI_MMFilesEdgeIndexHash_t( - HashElementKey, HashElementEdge, IsEqualKeyEdge, IsEqualElementEdge, - IsEqualElementEdgeByKey, _numBuckets, 64, context); - - _edgesTo = new TRI_MMFilesEdgeIndexHash_t( - HashElementKey, HashElementEdge, IsEqualKeyEdge, IsEqualElementEdge, - IsEqualElementEdgeByKey, _numBuckets, 64, context); -} + _edgesFrom.reset(new TRI_MMFilesEdgeIndexHash_t(MMFilesEdgeIndexHelper(), _numBuckets, 64, context)); -MMFilesEdgeIndex::~MMFilesEdgeIndex() { - delete _edgesFrom; - delete _edgesTo; + _edgesTo.reset(new TRI_MMFilesEdgeIndexHash_t(MMFilesEdgeIndexHelper(), _numBuckets, 64, context)); } /// @brief return a selectivity estimate for the index @@ -522,7 +452,7 @@ IndexIterator* MMFilesEdgeIndex::createEqIterator( bool const isFrom = (attrNode->stringEquals(StaticStrings::FromString)); return new MMFilesEdgeIndexIterator(_collection, trx, mmdr, this, - isFrom ? _edgesFrom : _edgesTo, keys); + isFrom ? _edgesFrom.get() : _edgesTo.get(), keys); } /// @brief create the iterator @@ -552,7 +482,7 @@ IndexIterator* MMFilesEdgeIndex::createInIterator( bool const isFrom = (attrNode->stringEquals(StaticStrings::FromString)); return new MMFilesEdgeIndexIterator(_collection, trx, mmdr, this, - isFrom ? _edgesFrom : _edgesTo, keys); + isFrom ? _edgesFrom.get() : _edgesTo.get(), keys); } /// @brief add a single value node to the iterator's keys diff --git a/arangod/MMFiles/MMFilesEdgeIndex.h b/arangod/MMFiles/MMFilesEdgeIndex.h index 25ca8f1697fc..7972ebb8f233 100644 --- a/arangod/MMFiles/MMFilesEdgeIndex.h +++ b/arangod/MMFiles/MMFilesEdgeIndex.h @@ -26,6 +26,7 @@ #include "Basics/AssocMulti.h" #include "Basics/Common.h" +#include "Basics/fasthash.h" #include "Indexes/Index.h" #include "Indexes/IndexIterator.h" #include "MMFiles/MMFilesIndexElement.h" @@ -42,8 +43,69 @@ class LocalTaskQueue; class MMFilesEdgeIndex; +struct MMFilesEdgeIndexHelper { + /// @brief hashes an edge key + static inline uint64_t HashKey(void*, VPackSlice const* key) { + TRI_ASSERT(key != nullptr); + // we can get away with the fast hash function here, as edge + // index values are restricted to strings + return MMFilesSimpleIndexElement::hash(*key); + } + + /// @brief hashes an edge + static inline uint64_t HashElement(void*, MMFilesSimpleIndexElement const& element, + bool byKey) { + if (byKey) { + return element.hash(); + } + + TRI_voc_rid_t revisionId = element.revisionId(); + return fasthash64_uint64(revisionId, 0x56781234); + } + + /// @brief checks if key and element match + inline bool IsEqualKeyElement(void* userData, VPackSlice const* left, + MMFilesSimpleIndexElement const& right) const { + TRI_ASSERT(left != nullptr); + IndexLookupContext* context = static_cast(userData); + TRI_ASSERT(context != nullptr); + + try { + VPackSlice tmp = right.slice(context); + TRI_ASSERT(tmp.isString()); + return left->equals(tmp); + } catch (...) { + return false; + } + } + + /// @brief checks for elements are equal + inline bool IsEqualElementElement(void*, MMFilesSimpleIndexElement const& left, + MMFilesSimpleIndexElement const& right) const { + return left.revisionId() == right.revisionId(); + } + + /// @brief checks for elements are equal + inline bool IsEqualElementElementByKey(void* userData, + MMFilesSimpleIndexElement const& left, + MMFilesSimpleIndexElement const& right) const { + IndexLookupContext* context = static_cast(userData); + try { + VPackSlice lSlice = left.slice(context); + VPackSlice rSlice = right.slice(context); + + TRI_ASSERT(lSlice.isString()); + TRI_ASSERT(rSlice.isString()); + + return lSlice.equals(rSlice); + } catch (...) { + return false; + } + } +}; + typedef arangodb::basics::AssocMulti + MMFilesSimpleIndexElement, uint32_t, false, MMFilesEdgeIndexHelper> TRI_MMFilesEdgeIndexHash_t; class MMFilesEdgeIndexIterator final : public IndexIterator { @@ -79,8 +141,6 @@ class MMFilesEdgeIndex final : public Index { MMFilesEdgeIndex(TRI_idx_iid_t, arangodb::LogicalCollection*); - ~MMFilesEdgeIndex(); - public: IndexType type() const override { return Index::TRI_IDX_TYPE_EDGE_INDEX; } @@ -120,9 +180,9 @@ class MMFilesEdgeIndex final : public Index { bool hasBatchInsert() const override { return true; } - TRI_MMFilesEdgeIndexHash_t* from() { return _edgesFrom; } + TRI_MMFilesEdgeIndexHash_t* from() { return _edgesFrom.get(); } - TRI_MMFilesEdgeIndexHash_t* to() { return _edgesTo; } + TRI_MMFilesEdgeIndexHash_t* to() { return _edgesTo.get(); } bool supportsFilterCondition(arangodb::aql::AstNode const*, arangodb::aql::Variable const*, size_t, size_t&, @@ -164,10 +224,10 @@ class MMFilesEdgeIndex final : public Index { private: /// @brief the hash table for _from - TRI_MMFilesEdgeIndexHash_t* _edgesFrom; + std::unique_ptr _edgesFrom; /// @brief the hash table for _to - TRI_MMFilesEdgeIndexHash_t* _edgesTo; + std::unique_ptr _edgesTo; /// @brief number of buckets effectively used by the index size_t _numBuckets; diff --git a/arangod/MMFiles/MMFilesHashIndex.cpp b/arangod/MMFiles/MMFilesHashIndex.cpp index 3891cfc452cc..5fe776ff00ca 100644 --- a/arangod/MMFiles/MMFilesHashIndex.cpp +++ b/arangod/MMFiles/MMFilesHashIndex.cpp @@ -211,82 +211,6 @@ void MMFilesHashIndexLookupBuilder::buildNextSearchValue() { _builder->close(); // End of search Array } -/// @brief determines if two elements are equal -static bool IsEqualElementElementUnique(void*, - MMFilesHashIndexElement const* left, - MMFilesHashIndexElement const* right) { - // this is quite simple - return left->revisionId() == right->revisionId(); -} - -/// @brief determines if two elements are equal -static bool IsEqualElementElementMulti(void* userData, - MMFilesHashIndexElement const* left, - MMFilesHashIndexElement const* right) { - TRI_ASSERT(left != nullptr); - TRI_ASSERT(right != nullptr); - - if (left->revisionId() != right->revisionId()) { - return false; - } - if (left->hash() != right->hash()) { - return false; - } - - IndexLookupContext* context = static_cast(userData); - TRI_ASSERT(context != nullptr); - - for (size_t i = 0; i < context->numFields(); ++i) { - VPackSlice leftData = left->slice(context, i); - VPackSlice rightData = right->slice(context, i); - - int res = - arangodb::basics::VelocyPackHelper::compare(leftData, rightData, false); - - if (res != 0) { - return false; - } - } - - return true; -} - -/// @brief given a key generates a hash integer -static uint64_t HashKey(void*, VPackSlice const* key) { - return MMFilesHashIndexElement::hash(*key); -} - -/// @brief determines if a key corresponds to an element -static bool IsEqualKeyElementMulti(void* userData, VPackSlice const* left, - MMFilesHashIndexElement const* right) { - TRI_ASSERT(left->isArray()); - TRI_ASSERT(right->revisionId() != 0); - IndexLookupContext* context = static_cast(userData); - TRI_ASSERT(context != nullptr); - - // TODO: is it a performance improvement to compare the hash values first? - VPackArrayIterator it(*left); - - while (it.valid()) { - int res = arangodb::basics::VelocyPackHelper::compare(it.value(), right->slice(context, it.index()), false); - - if (res != 0) { - return false; - } - - it.next(); - } - - return true; -} - -/// @brief determines if a key corresponds to an element -static bool IsEqualKeyElementUnique(void* userData, VPackSlice const* left, - uint64_t, - MMFilesHashIndexElement const* right) { - return IsEqualKeyElementMulti(userData, left, right); -} - MMFilesHashIndexIterator::MMFilesHashIndexIterator( LogicalCollection* collection, transaction::Methods* trx, ManagedDocumentResult* mmdr, MMFilesHashIndex const* index, @@ -390,42 +314,18 @@ void MMFilesHashIndexIteratorVPack::reset() { /// @brief create the unique array MMFilesHashIndex::UniqueArray::UniqueArray( - size_t numPaths, TRI_HashArray_t* hashArray, HashElementFunc* hashElement, - IsEqualElementElementByKey* isEqualElElByKey) - : _hashArray(hashArray), - _hashElement(hashElement), - _isEqualElElByKey(isEqualElElByKey), + size_t numPaths, std::unique_ptr hashArray) + : _hashArray(std::move(hashArray)), _numPaths(numPaths) { TRI_ASSERT(_hashArray != nullptr); - TRI_ASSERT(_hashElement != nullptr); - TRI_ASSERT(_isEqualElElByKey != nullptr); -} - -/// @brief destroy the unique array -MMFilesHashIndex::UniqueArray::~UniqueArray() { - delete _hashArray; - delete _hashElement; - delete _isEqualElElByKey; } /// @brief create the multi array MMFilesHashIndex::MultiArray::MultiArray( - size_t numPaths, TRI_HashArrayMulti_t* hashArray, - HashElementFunc* hashElement, IsEqualElementElementByKey* isEqualElElByKey) - : _hashArray(hashArray), - _hashElement(hashElement), - _isEqualElElByKey(isEqualElElByKey), + size_t numPaths, std::unique_ptr hashArray) + : _hashArray(std::move(hashArray)), _numPaths(numPaths) { TRI_ASSERT(_hashArray != nullptr); - TRI_ASSERT(_hashElement != nullptr); - TRI_ASSERT(_isEqualElElByKey != nullptr); -} - -/// @brief destroy the multi array -MMFilesHashIndex::MultiArray::~MultiArray() { - delete _hashArray; - delete _hashElement; - delete _isEqualElElByKey; } MMFilesHashIndex::MMFilesHashIndex(TRI_idx_iid_t iid, @@ -442,35 +342,23 @@ MMFilesHashIndex::MMFilesHashIndex(TRI_idx_iid_t iid, indexBuckets = static_cast(physical->indexBuckets()); } - auto func = std::make_unique(); - auto compare = std::make_unique(_paths.size(), - _useExpansion); - if (_unique) { auto array = std::make_unique( - HashKey, *(func.get()), IsEqualKeyElementUnique, - IsEqualElementElementUnique, *(compare.get()), indexBuckets, + MMFilesUniqueHashIndexHelper(_paths.size(), _useExpansion), + indexBuckets, [this]() -> std::string { return this->context(); }); - _uniqueArray = new MMFilesHashIndex::UniqueArray(numPaths(), array.get(), - func.get(), compare.get()); - array.release(); + _uniqueArray = new MMFilesHashIndex::UniqueArray(numPaths(), std::move(array)); } else { _multiArray = nullptr; auto array = std::make_unique( - HashKey, *(func.get()), IsEqualKeyElementMulti, - IsEqualElementElementMulti, *(compare.get()), indexBuckets, 64, + MMFilesMultiHashIndexHelper(_paths.size(), _useExpansion), + indexBuckets, 64, [this]() -> std::string { return this->context(); }); - _multiArray = new MMFilesHashIndex::MultiArray(numPaths(), array.get(), - func.get(), compare.get()); - - array.release(); + _multiArray = new MMFilesHashIndex::MultiArray(numPaths(), std::move(array)); } - compare.release(); - - func.release(); } /// @brief destroys the index diff --git a/arangod/MMFiles/MMFilesHashIndex.h b/arangod/MMFiles/MMFilesHashIndex.h index 44c09a59de3a..1dc706877767 100644 --- a/arangod/MMFiles/MMFilesHashIndex.h +++ b/arangod/MMFiles/MMFilesHashIndex.h @@ -30,6 +30,7 @@ #include "Basics/VelocyPackHelper.h" #include "Basics/fasthash.h" #include "Indexes/IndexIterator.h" +#include "Indexes/IndexLookupContext.h" #include "MMFiles/MMFilesIndexElement.h" #include "MMFiles/MMFilesPathBasedIndex.h" #include "Transaction/Helpers.h" @@ -49,6 +50,128 @@ class LocalTaskQueue; class MMFilesHashIndex; +struct MMFilesHashIndexHelper { + MMFilesHashIndexHelper(size_t n, bool allowExpansion) + : _numFields(n), _allowExpansion(allowExpansion) {} + + static inline uint64_t HashKey(void*, VPackSlice const* key) { + return MMFilesHashIndexElement::hash(*key); + } + + static inline uint64_t HashElement(void*, MMFilesHashIndexElement const* element, bool byKey) { + uint64_t hash = element->hash(); + + if (byKey) { + return hash; + } + + TRI_voc_rid_t revisionId = element->revisionId(); + return fasthash64_uint64(revisionId, hash); + } + + /// @brief determines if a key corresponds to an element + inline bool IsEqualKeyElement(void* userData, VPackSlice const* left, + MMFilesHashIndexElement const* right) const { + TRI_ASSERT(left->isArray()); + TRI_ASSERT(right->revisionId() != 0); + IndexLookupContext* context = static_cast(userData); + TRI_ASSERT(context != nullptr); + + // TODO: is it a performance improvement to compare the hash values first? + VPackArrayIterator it(*left); + + while (it.valid()) { + int res = arangodb::basics::VelocyPackHelper::compare(it.value(), right->slice(context, it.index()), false); + + if (res != 0) { + return false; + } + + it.next(); + } + + return true; + } + + inline bool IsEqualElementElementByKey(void* userData, + MMFilesHashIndexElement const* left, + MMFilesHashIndexElement const* right) const { + TRI_ASSERT(left->revisionId() != 0); + TRI_ASSERT(right->revisionId() != 0); + + if (!_allowExpansion && left->revisionId() == right->revisionId()) { + return true; + } + + IndexLookupContext* context = static_cast(userData); + + for (size_t i = 0; i < _numFields; ++i) { + VPackSlice leftData = left->slice(context, i); + VPackSlice rightData = right->slice(context, i); + + int res = arangodb::basics::VelocyPackHelper::compare(leftData, + rightData, false); + + if (res != 0) { + return false; + } + } + + return true; + } + + size_t const _numFields; + bool const _allowExpansion; +}; + +struct MMFilesUniqueHashIndexHelper : public MMFilesHashIndexHelper { + MMFilesUniqueHashIndexHelper(size_t n, bool allowExpansion) : MMFilesHashIndexHelper(n, allowExpansion) {} + + /// @brief determines if two elements are equal + inline bool IsEqualElementElement(void*, + MMFilesHashIndexElement const* left, + MMFilesHashIndexElement const* right) const { + // this is quite simple + return left->revisionId() == right->revisionId(); + } +}; + +struct MMFilesMultiHashIndexHelper : public MMFilesHashIndexHelper { + MMFilesMultiHashIndexHelper(size_t n, bool allowExpansion) : MMFilesHashIndexHelper(n, allowExpansion) {} + + /// @brief determines if two elements are equal + inline bool IsEqualElementElement(void* userData, + MMFilesHashIndexElement const* left, + MMFilesHashIndexElement const* right) const { + TRI_ASSERT(left != nullptr); + TRI_ASSERT(right != nullptr); + + if (left->revisionId() != right->revisionId()) { + return false; + } + if (left->hash() != right->hash()) { + return false; + } + + IndexLookupContext* context = static_cast(userData); + TRI_ASSERT(context != nullptr); + + for (size_t i = 0; i < context->numFields(); ++i) { + VPackSlice leftData = left->slice(context, i); + VPackSlice rightData = right->slice(context, i); + + int res = + arangodb::basics::VelocyPackHelper::compare(leftData, rightData, false); + + if (res != 0) { + return false; + } + } + + return true; + } +}; + /// @brief Class to build Slice lookups out of AST Conditions class MMFilesHashIndexLookupBuilder { private: @@ -225,94 +348,29 @@ class MMFilesHashIndex final : public MMFilesPathBasedIndex { std::unordered_set& found) const; /// @brief given an element generates a hash integer - private: - class HashElementFunc { - public: - HashElementFunc() {} - - uint64_t operator()(void* userData, MMFilesHashIndexElement const* element, - bool byKey = true) { - uint64_t hash = element->hash(); - - if (byKey) { - return hash; - } - - TRI_voc_rid_t revisionId = element->revisionId(); - return fasthash64_uint64(revisionId, hash); - } - }; - - /// @brief determines if a key corresponds to an element - class IsEqualElementElementByKey { - size_t _numFields; - bool _allowExpansion; - - public: - IsEqualElementElementByKey(size_t n, bool allowExpansion) - : _numFields(n), _allowExpansion(allowExpansion) {} - - bool operator()(void* userData, MMFilesHashIndexElement const* left, - MMFilesHashIndexElement const* right) { - TRI_ASSERT(left->revisionId() != 0); - TRI_ASSERT(right->revisionId() != 0); - - if (!_allowExpansion && left->revisionId() == right->revisionId()) { - return true; - } - - IndexLookupContext* context = static_cast(userData); - - for (size_t i = 0; i < _numFields; ++i) { - VPackSlice leftData = left->slice(context, i); - VPackSlice rightData = right->slice(context, i); - - int res = arangodb::basics::VelocyPackHelper::compare(leftData, - rightData, false); - - if (res != 0) { - return false; - } - } - - return true; - } - }; - private: /// @brief the actual hash index (unique type) typedef arangodb::basics::AssocUnique + MMFilesHashIndexElement*, MMFilesUniqueHashIndexHelper> TRI_HashArray_t; struct UniqueArray { UniqueArray() = delete; - UniqueArray(size_t numPaths, TRI_HashArray_t*, HashElementFunc*, - IsEqualElementElementByKey*); - - ~UniqueArray(); - - TRI_HashArray_t* _hashArray; // the hash array itself, unique values - HashElementFunc* _hashElement; // hash function for elements - IsEqualElementElementByKey* _isEqualElElByKey; // comparison func + UniqueArray(size_t numPaths, std::unique_ptr); + std::unique_ptr _hashArray; // the hash array itself, unique values size_t _numPaths; }; /// @brief the actual hash index (multi type) typedef arangodb::basics::AssocMulti< - arangodb::velocypack::Slice, MMFilesHashIndexElement*, uint32_t, false> + arangodb::velocypack::Slice, MMFilesHashIndexElement*, uint32_t, false, MMFilesMultiHashIndexHelper> TRI_HashArrayMulti_t; struct MultiArray { MultiArray() = delete; - MultiArray(size_t numPaths, TRI_HashArrayMulti_t*, HashElementFunc*, - IsEqualElementElementByKey*); - ~MultiArray(); - - TRI_HashArrayMulti_t* - _hashArray; // the hash array itself, non-unique values - HashElementFunc* _hashElement; // hash function for elements - IsEqualElementElementByKey* _isEqualElElByKey; // comparison func + MultiArray(size_t numPaths, std::unique_ptr); + + std::unique_ptr _hashArray; // the hash array itself, non-unique values size_t _numPaths; }; diff --git a/arangod/MMFiles/MMFilesPrimaryIndex.cpp b/arangod/MMFiles/MMFilesPrimaryIndex.cpp index 4453d0a37d2a..94adb16e7790 100644 --- a/arangod/MMFiles/MMFilesPrimaryIndex.cpp +++ b/arangod/MMFiles/MMFilesPrimaryIndex.cpp @@ -53,44 +53,6 @@ static std::vector> const IndexAttributes{{arangodb::basics::AttributeName("_id", false)}, {arangodb::basics::AttributeName("_key", false)}}; -static inline uint64_t HashKey(void*, uint8_t const* key) { - return MMFilesSimpleIndexElement::hash(VPackSlice(key)); -} - -static inline uint64_t HashElement(void*, - MMFilesSimpleIndexElement const& element) { - return element.hash(); -} - -/// @brief determines if a key corresponds to an element -static bool IsEqualKeyElement(void* userData, uint8_t const* key, uint64_t hash, - MMFilesSimpleIndexElement const& right) { - IndexLookupContext* context = static_cast(userData); - TRI_ASSERT(context != nullptr); - - try { - VPackSlice tmp = right.slice(context); - TRI_ASSERT(tmp.isString()); - return VPackSlice(key).equals(tmp); - } catch (...) { - return false; - } -} - -/// @brief determines if two elements are equal -static bool IsEqualElementElement(void* userData, - MMFilesSimpleIndexElement const& left, - MMFilesSimpleIndexElement const& right) { - IndexLookupContext* context = static_cast(userData); - TRI_ASSERT(context != nullptr); - - VPackSlice l = left.slice(context); - VPackSlice r = right.slice(context); - TRI_ASSERT(l.isString()); - TRI_ASSERT(r.isString()); - return l.equals(r); -} - MMFilesPrimaryIndexIterator::MMFilesPrimaryIndexIterator( LogicalCollection* collection, transaction::Methods* trx, ManagedDocumentResult* mmdr, MMFilesPrimaryIndex const* index, @@ -211,8 +173,7 @@ MMFilesPrimaryIndex::MMFilesPrimaryIndex( std::vector>( {{arangodb::basics::AttributeName(StaticStrings::KeyString, false)}}), - /*unique*/ true , /*sparse*/ false), - _primaryIndex(nullptr) { + /*unique*/ true , /*sparse*/ false) { size_t indexBuckets = 1; if (collection != nullptr) { @@ -223,14 +184,10 @@ MMFilesPrimaryIndex::MMFilesPrimaryIndex( indexBuckets = static_cast(physical->indexBuckets()); } - _primaryIndex = new MMFilesPrimaryIndexImpl( - HashKey, HashElement, IsEqualKeyElement, IsEqualElementElement, - IsEqualElementElement, indexBuckets, - [this]() -> std::string { return this->context(); }); + _primaryIndex.reset(new MMFilesPrimaryIndexImpl(MMFilesPrimaryIndexHelper(), indexBuckets, + [this]() -> std::string { return this->context(); })); } -MMFilesPrimaryIndex::~MMFilesPrimaryIndex() { delete _primaryIndex; } - /// @brief return the number of documents from the index size_t MMFilesPrimaryIndex::size() const { return _primaryIndex->size(); } @@ -349,7 +306,7 @@ IndexIterator* MMFilesPrimaryIndex::allIterator(transaction::Methods* trx, ManagedDocumentResult* mmdr, bool reverse) const { return new MMFilesAllIndexIterator(_collection, trx, mmdr, this, - _primaryIndex, reverse); + _primaryIndex.get(), reverse); } /// @brief request an iterator over all elements in the index in @@ -358,7 +315,7 @@ IndexIterator* MMFilesPrimaryIndex::allIterator(transaction::Methods* trx, IndexIterator* MMFilesPrimaryIndex::anyIterator( transaction::Methods* trx, ManagedDocumentResult* mmdr) const { return new MMFilesAnyIndexIterator(_collection, trx, mmdr, this, - _primaryIndex); + _primaryIndex.get()); } /// @brief a method to iterate over all elements in the index in diff --git a/arangod/MMFiles/MMFilesPrimaryIndex.h b/arangod/MMFiles/MMFilesPrimaryIndex.h index 2f316ef563ad..b6d3e7d98bbb 100644 --- a/arangod/MMFiles/MMFilesPrimaryIndex.h +++ b/arangod/MMFiles/MMFilesPrimaryIndex.h @@ -28,6 +28,8 @@ #include "Basics/Common.h" #include "Indexes/Index.h" #include "Indexes/IndexIterator.h" +#include "Indexes/IndexLookupContext.h" +#include "MMFiles/MMFilesIndexElement.h" #include "VocBase/voc-types.h" #include "VocBase/vocbase.h" @@ -43,7 +45,52 @@ namespace transaction { class Methods; } -typedef arangodb::basics::AssocUnique +struct MMFilesPrimaryIndexHelper { + static inline uint64_t HashKey(void*, uint8_t const* key) { + return MMFilesSimpleIndexElement::hash(VPackSlice(key)); + } + + static inline uint64_t HashElement(void*, MMFilesSimpleIndexElement const& element, bool) { + return element.hash(); + } + + /// @brief determines if a key corresponds to an element + inline bool IsEqualKeyElement(void* userData, uint8_t const* key, + MMFilesSimpleIndexElement const& right) const { + IndexLookupContext* context = static_cast(userData); + TRI_ASSERT(context != nullptr); + + try { + VPackSlice tmp = right.slice(context); + TRI_ASSERT(tmp.isString()); + return VPackSlice(key).equals(tmp); + } catch (...) { + return false; + } + } + + /// @brief determines if two elements are equal + inline bool IsEqualElementElement(void* userData, + MMFilesSimpleIndexElement const& left, + MMFilesSimpleIndexElement const& right) const { + IndexLookupContext* context = static_cast(userData); + TRI_ASSERT(context != nullptr); + + VPackSlice l = left.slice(context); + VPackSlice r = right.slice(context); + TRI_ASSERT(l.isString()); + TRI_ASSERT(r.isString()); + return l.equals(r); + } + + inline bool IsEqualElementElementByKey(void* userData, + MMFilesSimpleIndexElement const& left, + MMFilesSimpleIndexElement const& right) const { + return IsEqualElementElement(userData, left, right); + } +}; + +typedef arangodb::basics::AssocUnique MMFilesPrimaryIndexImpl; class MMFilesPrimaryIndexIterator final : public IndexIterator { @@ -126,8 +173,6 @@ class MMFilesPrimaryIndex final : public Index { explicit MMFilesPrimaryIndex(arangodb::LogicalCollection*); - ~MMFilesPrimaryIndex(); - public: IndexType type() const override { return Index::TRI_IDX_TYPE_PRIMARY_INDEX; } @@ -249,7 +294,7 @@ class MMFilesPrimaryIndex final : public Index { private: /// @brief the actual index - MMFilesPrimaryIndexImpl* _primaryIndex; + std::unique_ptr _primaryIndex; }; } diff --git a/arangod/MMFiles/MMFilesRevisionsCache.cpp b/arangod/MMFiles/MMFilesRevisionsCache.cpp index bd54f58f03bd..cb27f343053c 100644 --- a/arangod/MMFiles/MMFilesRevisionsCache.cpp +++ b/arangod/MMFiles/MMFilesRevisionsCache.cpp @@ -30,32 +30,8 @@ using namespace arangodb; -namespace { -static inline uint64_t HashKey(void*, TRI_voc_rid_t const* key) { - return std::hash()(*key); -// return XXH64(key, sizeof(TRI_voc_rid_t), 0x12345678); -} - -static inline uint64_t HashElement(void*, MMFilesDocumentPosition const& element) { - return std::hash()(element.revisionId()); -// TRI_voc_rid_t revisionId = element.revisionId(); -// return HashKey(nullptr, &revisionId); -} - -static bool IsEqualKeyElement(void*, TRI_voc_rid_t const* key, - uint64_t hash, MMFilesDocumentPosition const& element) { - return *key == element.revisionId(); -} - -static bool IsEqualElementElement(void*, MMFilesDocumentPosition const& left, - MMFilesDocumentPosition const& right) { - return left.revisionId() == right.revisionId(); -} - -} // namespace - MMFilesRevisionsCache::MMFilesRevisionsCache() - : _positions(HashKey, HashElement, IsEqualKeyElement, IsEqualElementElement, IsEqualElementElement, 8, []() -> std::string { return "mmfiles revisions"; }) {} + : _positions(MMFilesRevisionsCacheHelper(), 8, []() -> std::string { return "mmfiles revisions"; }) {} MMFilesRevisionsCache::~MMFilesRevisionsCache() {} diff --git a/arangod/MMFiles/MMFilesRevisionsCache.h b/arangod/MMFiles/MMFilesRevisionsCache.h index b714d85f03c2..33411a57c8b6 100644 --- a/arangod/MMFiles/MMFilesRevisionsCache.h +++ b/arangod/MMFiles/MMFilesRevisionsCache.h @@ -27,6 +27,8 @@ #include "Basics/Common.h" #include "Basics/AssocUnique.h" #include "Basics/ReadWriteLock.h" +#include "Basics/fasthash.h" +#include "Logger/Logger.h" #include "MMFiles/MMFilesDocumentPosition.h" #include "VocBase/voc-types.h" @@ -34,6 +36,32 @@ struct MMFilesMarker; namespace arangodb { +struct MMFilesRevisionsCacheHelper { + static inline uint64_t HashKey(void*, TRI_voc_rid_t const* key) { + return fasthash64_uint64(*key, 0xdeadbeef); + return std::hash()(*key); + } + + static inline uint64_t HashElement(void*, MMFilesDocumentPosition const& element, bool) { + return fasthash64_uint64(element.revisionId(), 0xdeadbeef); + } + + inline bool IsEqualKeyElement(void*, TRI_voc_rid_t const* key, + MMFilesDocumentPosition const& element) const { + return *key == element.revisionId(); + } + + inline bool IsEqualElementElement(void*, MMFilesDocumentPosition const& left, + MMFilesDocumentPosition const& right) const { + return left.revisionId() == right.revisionId(); + } + + inline bool IsEqualElementElementByKey(void* userData, MMFilesDocumentPosition const& left, + MMFilesDocumentPosition const& right) const { + return IsEqualElementElement(userData, left, right); + } +}; + class MMFilesRevisionsCache { public: MMFilesRevisionsCache(); @@ -54,9 +82,9 @@ class MMFilesRevisionsCache { MMFilesDocumentPosition fetchAndRemove(TRI_voc_rid_t revisionId); private: - mutable arangodb::basics::ReadWriteLock _lock; - - arangodb::basics::AssocUnique _positions; + mutable basics::ReadWriteLock _lock; + + basics::AssocUnique _positions; }; } // namespace arangodb diff --git a/lib/Basics/AssocMulti.h b/lib/Basics/AssocMulti.h index 900fb24889c1..1aacb469dc0a 100644 --- a/lib/Basics/AssocMulti.h +++ b/lib/Basics/AssocMulti.h @@ -94,8 +94,8 @@ namespace basics { /// //////////////////////////////////////////////////////////////////////////////// -template +template class AssocMulti { private: typedef void UserData; @@ -103,13 +103,6 @@ class AssocMulti { public: static IndexType const INVALID_INDEX = ((IndexType)0) - 1; - typedef std::function HashKeyFuncType; - typedef std::function - HashElementFuncType; - typedef std::function - IsEqualKeyElementFuncType; - typedef std::function - IsEqualElementElementFuncType; typedef std::function CallbackElementFuncType; private: @@ -117,6 +110,7 @@ class AssocMulti { typedef arangodb::basics::IndexBucket Bucket; + AssocMultiHelper _helper; std::vector _buckets; size_t _bucketsMask; @@ -133,24 +127,15 @@ class AssocMulti { uint64_t _nrProbesD; // statistics: number of misses while removing #endif - HashKeyFuncType const _hashKey; - HashElementFuncType const _hashElement; - IsEqualKeyElementFuncType const _isEqualKeyElement; - IsEqualElementElementFuncType const _isEqualElementElement; - IsEqualElementElementFuncType const _isEqualElementElementByKey; - std::function _contextCallback; IndexType _initialSize; public: - AssocMulti(HashKeyFuncType hashKey, HashElementFuncType hashElement, - IsEqualKeyElementFuncType isEqualKeyElement, - IsEqualElementElementFuncType isEqualElementElement, - IsEqualElementElementFuncType isEqualElementElementByKey, + AssocMulti(AssocMultiHelper&& helper, size_t numberBuckets = 1, IndexType initialSize = 64, std::function contextCallback = []() -> std::string { return ""; }) - : + : _helper(std::move(helper)), #ifdef TRI_INTERNAL_STATS _nrFinds(0), _nrAdds(0), @@ -160,11 +145,6 @@ class AssocMulti { _nrProbesF(0), _nrProbesD(0), #endif - _hashKey(hashKey), - _hashElement(hashElement), - _isEqualKeyElement(isEqualKeyElement), - _isEqualElementElement(isEqualElementElement), - _isEqualElementElementByKey(isEqualElementElementByKey), _contextCallback(contextCallback), _initialSize(initialSize) { @@ -272,7 +252,7 @@ class AssocMulti { #endif // compute the hash by the key only first - uint64_t hashByKey = _hashElement(userData, element, true); + uint64_t hashByKey = _helper.HashElement(userData, element, true); Bucket& b = _buckets[hashByKey & _bucketsMask]; auto result = @@ -360,7 +340,7 @@ class AssocMulti { } std::shared_ptr worker; - worker.reset(new Partitioner(queue, _hashElement, contextDestroyer, + worker.reset(new Partitioner(queue, AssocMultiHelper::HashElement, contextDestroyer, data, lower, upper, contextCreator(), bucketFlags, bucketMapLocker, allBuckets, inserters)); @@ -466,7 +446,7 @@ class AssocMulti { b._table[i].value && (b._table[i].prev != INVALID_INDEX || (useHashCache && b._table[i].readHashCache() != hashByKey) || - !_isEqualElementElementByKey(userData, element, b._table[i].value))) { + !_helper.IsEqualElementElementByKey(userData, element, b._table[i].value))) { i = incr(b, i); #ifdef TRI_INTERNAL_STATS // update statistics @@ -491,7 +471,7 @@ class AssocMulti { // list of which we want to make element a member. Perhaps an // equal element is right here: if (checkEquality && - _isEqualElementElement(userData, element, b._table[i].value)) { + _helper.IsEqualElementElement(userData, element, b._table[i].value)) { Element old = b._table[i].value; if (overwrite) { TRI_ASSERT(!useHashCache || b._table[i].readHashCache() == hashByKey); @@ -694,7 +674,7 @@ class AssocMulti { } // compute the hash - uint64_t hashByKey = _hashKey(userData, key); + uint64_t hashByKey = _helper.HashKey(userData, key); Bucket const& b = _buckets[hashByKey & _bucketsMask]; IndexType hashIndex = hashToIndex(hashByKey); IndexType i = hashIndex % b._nrAlloc; @@ -708,7 +688,7 @@ class AssocMulti { while (b._table[i].value && (b._table[i].prev != INVALID_INDEX || (useHashCache && b._table[i].readHashCache() != hashByKey) || - !_isEqualKeyElement(userData, key, b._table[i].value))) { + !_helper.IsEqualKeyElement(userData, key, b._table[i].value))) { i = incr(b, i); #ifdef TRI_INTERNAL_STATS _nrProbesF++; @@ -757,7 +737,7 @@ class AssocMulti { } // compute the hash - uint64_t hashByKey = _hashElement(userData, element, true); + uint64_t hashByKey = _helper.HashElement(userData, element, true); Bucket const& b = _buckets[hashByKey & _bucketsMask]; IndexType hashIndex = hashToIndex(hashByKey); IndexType i = hashIndex % b._nrAlloc; @@ -772,7 +752,7 @@ class AssocMulti { b._table[i].value && (b._table[i].prev != INVALID_INDEX || (useHashCache && b._table[i].readHashCache() != hashByKey) || - !_isEqualElementElementByKey(userData, element, b._table[i].value))) { + !_helper.IsEqualElementElementByKey(userData, element, b._table[i].value))) { i = incr(b, i); #ifdef TRI_INTERNAL_STATS _nrProbesF++; @@ -822,7 +802,7 @@ class AssocMulti { return; } - uint64_t hashByKey = _hashElement(userData, element, true); + uint64_t hashByKey = _helper.HashElement(userData, element, true); Bucket const& b = _buckets[hashByKey & _bucketsMask]; uint64_t hashByElm; IndexType i = findElementPlace(userData, b, element, true, hashByElm); @@ -838,7 +818,7 @@ class AssocMulti { while (b._table[i].value && (b._table[i].prev != INVALID_INDEX || (useHashCache && b._table[i].readHashCache() != hashByKey) || - !_isEqualElementElementByKey(userData, element, + !_helper.IsEqualElementElementByKey(userData, element, b._table[i].value))) { i = incr(b, i); #ifdef TRI_INTERNAL_STATS @@ -935,7 +915,7 @@ class AssocMulti { if (useHashCache) { // We need to exchange the hashCache value by that of the key: b->_table[i].writeHashCache( - _hashElement(userData, b->_table[i].value, true)); + _helper.HashElement(userData, b->_table[i].value, true)); } #ifdef TRI_CHECK_MULTI_POINTER_HASH check(userData, false, false); @@ -1063,7 +1043,7 @@ class AssocMulti { if (useHashCache) { hashByKey = oldTable[j].readHashCache(); } else { - hashByKey = _hashElement(userData, oldTable[j].value, true); + hashByKey = _helper.HashElement(userData, oldTable[j].value, true); } IndexType insertPosition = insertFirst(userData, copy, oldTable[j].value, hashByKey); @@ -1078,7 +1058,7 @@ class AssocMulti { if (useHashCache) { hashByElm = oldTable[k].readHashCache(); } else { - hashByElm = _hashElement(userData, oldTable[k].value, false); + hashByElm = _helper.HashElement(userData, oldTable[k].value, false); } insertFurther(userData, copy, oldTable[k].value, hashByKey, hashByElm, insertPosition); @@ -1147,7 +1127,7 @@ class AssocMulti { if (b._table[i].prev == INVALID_INDEX) { // We are the first in a linked list. uint64_t hashByKey = - _hashElement(userData, b._table[i].value, true); + _helper.HashElement(userData, b._table[i].value, true); hashIndex = hashToIndex(hashByKey); j = hashIndex % b._nrAlloc; if (useHashCache && b._table[i].readHashCache() != hashByKey) { @@ -1156,7 +1136,7 @@ class AssocMulti { for (k = j; k != i;) { if (!b._table[k].value || (b._table[k].prev == INVALID_INDEX && - _isEqualElementElementByKey(userData, b._table[i].value, + _helper.IsEqualElementElementByKey(userData, b._table[i].value, b._table[k].value))) { ok = false; std::cout << "Alarm pos bykey: " << i << std::endl; @@ -1166,7 +1146,7 @@ class AssocMulti { } else { // We are not the first in a linked list. uint64_t hashByElm = - _hashElement(userData, b._table[i].value, false); + _helper.HashElement(userData, b._table[i].value, false); hashIndex = hashToIndex(hashByElm); j = hashIndex % b._nrAlloc; if (useHashCache && b._table[i].readHashCache() != hashByElm) { @@ -1174,7 +1154,7 @@ class AssocMulti { } for (k = j; k != i;) { if (!b._table[k].value || - _isEqualElementElement(userData, b._table[i].value, + _helper.IsEqualElementElement(userData, b._table[i].value, b._table[k].value)) { ok = false; std::cout << "Alarm unique: " << k << ", " << i << std::endl; @@ -1209,14 +1189,14 @@ class AssocMulti { // pointer into the table, which is either empty or points to // an entry that compares equal to element. - hashByElm = _hashElement(userData, element, false); + hashByElm = _helper.HashElement(userData, element, false); IndexType hashindex = hashToIndex(hashByElm); IndexType i = hashindex % b._nrAlloc; while (b._table[i].value && (!checkEquality || (useHashCache && b._table[i].readHashCache() != hashByElm) || - !_isEqualElementElement(userData, element, b._table[i].value))) { + !_helper.IsEqualElementElement(userData, element, b._table[i].value))) { i = incr(b, i); #ifdef TRI_INTERNAL_STATS _nrProbes++; @@ -1234,7 +1214,7 @@ class AssocMulti { // This performs a complete lookup for an element. It returns a slot // number. This slot is either empty or contains an element that // compares equal to element. - uint64_t hashByKey = _hashElement(userData, element, true); + uint64_t hashByKey = _helper.HashElement(userData, element, true); Bucket const& b = _buckets[hashByKey & _bucketsMask]; buck = const_cast(&b); IndexType hashIndex = hashToIndex(hashByKey); @@ -1246,7 +1226,7 @@ class AssocMulti { b._table[i].value && (b._table[i].prev != INVALID_INDEX || (useHashCache && b._table[i].readHashCache() != hashByKey) || - !_isEqualElementElementByKey(userData, element, b._table[i].value))) { + !_helper.IsEqualElementElementByKey(userData, element, b._table[i].value))) { i = incr(b, i); #ifdef TRI_INTERNAL_STATS _nrProbes++; @@ -1255,7 +1235,7 @@ class AssocMulti { if (b._table[i].value) { // It might be right here! - if (_isEqualElementElement(userData, element, b._table[i].value)) { + if (_helper.IsEqualElementElement(userData, element, b._table[i].value)) { return i; } @@ -1326,7 +1306,7 @@ class AssocMulti { // Find out where this element ought to be: // If it is the start of one of the linked lists, we need to hash // by key, otherwise, we hash by the full identity of the element: - uint64_t hash = _hashElement(userData, b._table[j].value, + uint64_t hash = _helper.HashElement(userData, b._table[j].value, b._table[j].prev == INVALID_INDEX); IndexType hashIndex = hashToIndex(hash); IndexType k = hashIndex % b._nrAlloc; diff --git a/lib/Basics/AssocUnique.h b/lib/Basics/AssocUnique.h index c4d6111d9949..3fdd7f1ae950 100644 --- a/lib/Basics/AssocUnique.h +++ b/lib/Basics/AssocUnique.h @@ -51,51 +51,30 @@ namespace basics { /// @brief associative array //////////////////////////////////////////////////////////////////////////////// -template +template class AssocUnique { private: typedef void UserData; typedef arangodb::basics::BucketPosition BucketPosition; public: - typedef std::function HashKeyFuncType; - typedef std::function - HashElementFuncType; - typedef std::function - IsEqualKeyElementFuncType; - typedef std::function - IsEqualElementElementFuncType; - typedef std::function CallbackElementFuncType; typedef arangodb::basics::IndexBucket Bucket; private: + AssocUniqueHelper _helper; std::vector _buckets; size_t _bucketsMask; - HashKeyFuncType const _hashKey; - HashElementFuncType const _hashElement; - IsEqualKeyElementFuncType const _isEqualKeyElement; - IsEqualElementElementFuncType const _isEqualElementElement; - IsEqualElementElementFuncType const _isEqualElementElementByKey; - std::function _contextCallback; public: - AssocUnique(HashKeyFuncType hashKey, HashElementFuncType hashElement, - IsEqualKeyElementFuncType isEqualKeyElement, - IsEqualElementElementFuncType isEqualElementElement, - IsEqualElementElementFuncType isEqualElementElementByKey, + AssocUnique(AssocUniqueHelper&& helper, size_t numberBuckets = 1, std::function contextCallback = []() -> std::string { return ""; }) - : _hashKey(hashKey), - _hashElement(hashElement), - _isEqualKeyElement(isEqualKeyElement), - _isEqualElementElement(isEqualElementElement), - _isEqualElementElementByKey(isEqualElementElementByKey), + : _helper(std::move(helper)), _contextCallback(contextCallback) { // Make the number of buckets a power of two: size_t ex = 0; @@ -180,7 +159,7 @@ class AssocUnique { if (element) { uint64_t i, k; - i = k = _hashElement(userData, element) % n; + i = k = _helper.HashElement(userData, element, true) % n; for (; i < n && copy._table[i]; ++i) ; @@ -251,12 +230,12 @@ class AssocUnique { uint64_t k = i; for (; i < n && b._table[i] && - !_isEqualElementElementByKey(userData, element, b._table[i]); + !_helper.IsEqualElementElementByKey(userData, element, b._table[i]); ++i) ; if (i == n) { for (i = 0; i < k && b._table[i] && - !_isEqualElementElementByKey(userData, element, b._table[i]); + !_helper.IsEqualElementElementByKey(userData, element, b._table[i]); ++i) ; } @@ -371,7 +350,7 @@ class AssocUnique { ////////////////////////////////////////////////////////////////////////////// Element find(UserData* userData, Element const& element) const { - uint64_t i = _hashElement(userData, element); + uint64_t i = _helper.HashElement(userData, element, true); Bucket const& b = _buckets[i & _bucketsMask]; uint64_t const n = b._nrAlloc; @@ -379,12 +358,12 @@ class AssocUnique { uint64_t k = i; for (; i < n && b._table[i] && - !_isEqualElementElementByKey(userData, element, b._table[i]); + !_helper.IsEqualElementElementByKey(userData, element, b._table[i]); ++i) ; if (i == n) { for (i = 0; i < k && b._table[i] && - !_isEqualElementElementByKey(userData, element, b._table[i]); + !_helper.IsEqualElementElementByKey(userData, element, b._table[i]); ++i) ; } @@ -403,7 +382,7 @@ class AssocUnique { ////////////////////////////////////////////////////////////////////////////// Element findByKey(UserData* userData, Key const* key) const { - uint64_t hash = _hashKey(userData, key); + uint64_t hash = _helper.HashKey(userData, key); uint64_t i = hash; uint64_t bucketId = i & _bucketsMask; Bucket const& b = _buckets[static_cast(bucketId)]; @@ -413,12 +392,12 @@ class AssocUnique { uint64_t k = i; for (; i < n && b._table[i] && - !_isEqualKeyElement(userData, key, hash, b._table[i]); + !_helper.IsEqualKeyElement(userData, key, b._table[i]); ++i) ; if (i == n) { for (i = 0; i < k && b._table[i] && - !_isEqualKeyElement(userData, key, hash, b._table[i]); + !_helper.IsEqualKeyElement(userData, key, b._table[i]); ++i) ; } @@ -432,7 +411,7 @@ class AssocUnique { } Element* findByKeyRef(UserData* userData, Key const* key) const { - uint64_t hash = _hashKey(userData, key); + uint64_t hash = _helper.HashKey(userData, key); uint64_t i = hash; uint64_t bucketId = i & _bucketsMask; Bucket const& b = _buckets[static_cast(bucketId)]; @@ -442,12 +421,12 @@ class AssocUnique { uint64_t k = i; for (; i < n && b._table[i] && - !_isEqualKeyElement(userData, key, hash, b._table[i]); + !_helper.IsEqualKeyElement(userData, key, b._table[i]); ++i) ; if (i == n) { for (i = 0; i < k && b._table[i] && - !_isEqualKeyElement(userData, key, hash, b._table[i]); + !_helper.IsEqualKeyElement(userData, key, b._table[i]); ++i) ; } @@ -469,7 +448,7 @@ class AssocUnique { Element findByKey(UserData* userData, Key const* key, BucketPosition& position, uint64_t& hash) const { - hash = _hashKey(userData, key); + hash = _helper.HashKey(userData, key); uint64_t i = hash; uint64_t bucketId = i & _bucketsMask; Bucket const& b = _buckets[static_cast(bucketId)]; @@ -479,12 +458,12 @@ class AssocUnique { uint64_t k = i; for (; i < n && b._table[i] && - !_isEqualKeyElement(userData, key, hash, b._table[i]); + !_helper.IsEqualKeyElement(userData, key, b._table[i]); ++i) ; if (i == n) { for (i = 0; i < k && b._table[i] && - !_isEqualKeyElement(userData, key, hash, b._table[i]); + !_helper.IsEqualKeyElement(userData, key, b._table[i]); ++i) ; } @@ -507,7 +486,7 @@ class AssocUnique { ////////////////////////////////////////////////////////////////////////////// int insert(UserData* userData, Element const& element) { - uint64_t hash = _hashElement(userData, element); + uint64_t hash = _helper.HashElement(userData, element, true); Bucket& b = _buckets[hash & _bucketsMask]; if (!checkResize(userData, b, 0)) { @@ -624,7 +603,7 @@ class AssocUnique { } std::shared_ptr worker; - worker.reset(new Partitioner(queue, _hashElement, contextDestroyer, + worker.reset(new Partitioner(queue, AssocUniqueHelper::HashElement, contextDestroyer, data, lower, upper, contextCreator(), bucketFlags, bucketMapLocker, allBuckets, inserters)); @@ -657,7 +636,7 @@ class AssocUnique { uint64_t k = TRI_IncModU64(i, n); while (b._table[k]) { - uint64_t j = _hashElement(userData, b._table[k]) % n; + uint64_t j = _helper.HashElement(userData, b._table[k], true) % n; if ((i < k && !(i < j && j <= k)) || (k < i && !(i < j || j <= k))) { b._table[i] = b._table[k]; @@ -680,7 +659,7 @@ class AssocUnique { ////////////////////////////////////////////////////////////////////////////// Element removeByKey(UserData* userData, Key const* key) { - uint64_t hash = _hashKey(userData, key); + uint64_t hash = _helper.HashKey(userData, key); uint64_t i = hash; Bucket& b = _buckets[i & _bucketsMask]; @@ -689,12 +668,12 @@ class AssocUnique { uint64_t k = i; for (; i < n && b._table[i] && - !_isEqualKeyElement(userData, key, hash, b._table[i]); + !_helper.IsEqualKeyElement(userData, key, b._table[i]); ++i) ; if (i == n) { for (i = 0; i < k && b._table[i] && - !_isEqualKeyElement(userData, key, hash, b._table[i]); + !_helper.IsEqualKeyElement(userData, key, b._table[i]); ++i) ; } @@ -713,7 +692,7 @@ class AssocUnique { ////////////////////////////////////////////////////////////////////////////// Element remove(UserData* userData, Element const& element) { - uint64_t i = _hashElement(userData, element); + uint64_t i = _helper.HashElement(userData, element, true); Bucket& b = _buckets[i & _bucketsMask]; uint64_t const n = b._nrAlloc; @@ -721,12 +700,12 @@ class AssocUnique { uint64_t k = i; for (; i < n && b._table[i] && - !_isEqualElementElement(userData, element, b._table[i]); + !_helper.IsEqualElementElement(userData, element, b._table[i]); ++i) ; if (i == n) { for (i = 0; i < k && b._table[i] && - !_isEqualElementElement(userData, element, b._table[i]); + !_helper.IsEqualElementElement(userData, element, b._table[i]); ++i) ; } diff --git a/lib/Basics/AssocUniqueHelpers.h b/lib/Basics/AssocUniqueHelpers.h index 25148f1cafa6..781846c4f3c6 100644 --- a/lib/Basics/AssocUniqueHelpers.h +++ b/lib/Basics/AssocUniqueHelpers.h @@ -122,7 +122,7 @@ class UniquePartitionerTask : public LocalTask { typedef UniqueInserterTask Inserter; typedef std::vector> DocumentsPerBucket; - std::function _hashElement; + std::function _hashElement; std::function _contextDestroyer; std::shared_ptr const> _data; std::vector const* _elements; @@ -141,7 +141,7 @@ class UniquePartitionerTask : public LocalTask { public: UniquePartitionerTask( std::shared_ptr queue, - std::function hashElement, + std::function hashElement, std::function const& contextDestroyer, std::shared_ptr const> data, size_t lower, size_t upper, void* userData, @@ -170,7 +170,7 @@ class UniquePartitionerTask : public LocalTask { _allBuckets->size()); // initialize to number of buckets for (size_t i = _lower; i < _upper; ++i) { - uint64_t hashByKey = _hashElement(_userData, (*_elements)[i]); + uint64_t hashByKey = _hashElement(_userData, (*_elements)[i], true); auto bucketId = hashByKey & _bucketsMask; partitions[bucketId].emplace_back((*_elements)[i], hashByKey); From 804c3065a8137895438196231b646e0b5ce135e0 Mon Sep 17 00:00:00 2001 From: jsteemann Date: Fri, 15 Sep 2017 13:26:09 +0200 Subject: [PATCH 2/9] speed up full collection scans in mmfiles engine by using a batch lookup method for the revisions cache --- arangod/MMFiles/MMFilesCollection.cpp | 18 ++++++++++++++++ arangod/MMFiles/MMFilesCollection.h | 5 +++++ arangod/MMFiles/MMFilesPrimaryIndex.cpp | 26 +++++++++++++++++++++++ arangod/MMFiles/MMFilesPrimaryIndex.h | 2 ++ arangod/MMFiles/MMFilesRevisionsCache.cpp | 13 ++++++++++++ arangod/MMFiles/MMFilesRevisionsCache.h | 1 + 6 files changed, 65 insertions(+) diff --git a/arangod/MMFiles/MMFilesCollection.cpp b/arangod/MMFiles/MMFilesCollection.cpp index 6a23e3aa9c72..19bc455e4dcb 100644 --- a/arangod/MMFiles/MMFilesCollection.cpp +++ b/arangod/MMFiles/MMFilesCollection.cpp @@ -1985,6 +1985,20 @@ bool MMFilesCollection::readDocumentWithCallback(transaction::Methods* trx, return false; } +size_t MMFilesCollection::readDocumentWithCallback(transaction::Methods* trx, + std::vector>& tokens, + IndexIterator::DocumentCallback const& cb) { + size_t count = 0; + batchLookupRevisionVPack(tokens); + for (auto const& it : tokens) { + if (it.second) { + cb(MMFilesToken{it.first}, VPackSlice(it.second)); + ++count; + } + } + return count; +} + bool MMFilesCollection::readDocumentConditional( transaction::Methods* trx, DocumentIdentifierToken const& token, TRI_voc_tick_t maxTick, ManagedDocumentResult& result) { @@ -2978,6 +2992,10 @@ uint8_t const* MMFilesCollection::lookupRevisionVPackConditional( return vpack; } +void MMFilesCollection::batchLookupRevisionVPack(std::vector>& revisions) const { + _revisionsCache.batchLookup(revisions); +} + MMFilesDocumentPosition MMFilesCollection::insertRevision( TRI_voc_rid_t revisionId, uint8_t const* dataptr, TRI_voc_fid_t fid, bool isInWal, bool shouldLock) { diff --git a/arangod/MMFiles/MMFilesCollection.h b/arangod/MMFiles/MMFilesCollection.h index 33db08af1475..3c5bc34959e3 100644 --- a/arangod/MMFiles/MMFilesCollection.h +++ b/arangod/MMFiles/MMFilesCollection.h @@ -342,6 +342,10 @@ class MMFilesCollection final : public PhysicalCollection { bool readDocumentWithCallback(transaction::Methods* trx, DocumentIdentifierToken const& token, IndexIterator::DocumentCallback const& cb) override; + + size_t readDocumentWithCallback(transaction::Methods* trx, + std::vector>& tokens, + IndexIterator::DocumentCallback const& cb); bool readDocumentConditional(transaction::Methods* trx, DocumentIdentifierToken const& token, @@ -482,6 +486,7 @@ class MMFilesCollection final : public PhysicalCollection { uint8_t const* lookupRevisionVPackConditional(TRI_voc_rid_t revisionId, TRI_voc_tick_t maxTick, bool excludeWal) const; + void batchLookupRevisionVPack(std::vector>& revisions) const; bool addIndex(std::shared_ptr idx); void addIndexLocal(std::shared_ptr idx); diff --git a/arangod/MMFiles/MMFilesPrimaryIndex.cpp b/arangod/MMFiles/MMFilesPrimaryIndex.cpp index 94adb16e7790..18cf79048201 100644 --- a/arangod/MMFiles/MMFilesPrimaryIndex.cpp +++ b/arangod/MMFiles/MMFilesPrimaryIndex.cpp @@ -118,6 +118,32 @@ bool MMFilesAllIndexIterator::next(TokenCallback const& cb, size_t limit) { return true; } +bool MMFilesAllIndexIterator::nextDocument(DocumentCallback const& cb, size_t limit) { + _revisions.clear(); + _revisions.reserve(limit); + + bool done = false; + while (limit > 0) { + MMFilesSimpleIndexElement element; + if (_reverse) { + element = _index->findSequentialReverse(&_context, _position); + } else { + element = _index->findSequential(&_context, _position, _total); + } + if (element) { + _revisions.emplace_back(std::make_pair(element.revisionId(), nullptr)); + --limit; + } else { + done = true; + break; + } + } + + auto physical = static_cast(_collection->getPhysical()); + physical->readDocumentWithCallback(_trx, _revisions, cb); + return !done; +} + // Skip the first count-many entries void MMFilesAllIndexIterator::skip(uint64_t count, uint64_t& skipped) { while (count > 0) { diff --git a/arangod/MMFiles/MMFilesPrimaryIndex.h b/arangod/MMFiles/MMFilesPrimaryIndex.h index b6d3e7d98bbb..2259e500f9cf 100644 --- a/arangod/MMFiles/MMFilesPrimaryIndex.h +++ b/arangod/MMFiles/MMFilesPrimaryIndex.h @@ -129,6 +129,7 @@ class MMFilesAllIndexIterator final : public IndexIterator { char const* typeName() const override { return "all-index-iterator"; } bool next(TokenCallback const& cb, size_t limit) override; + bool nextDocument(DocumentCallback const& cb, size_t limit) override; void skip(uint64_t count, uint64_t& skipped) override; @@ -137,6 +138,7 @@ class MMFilesAllIndexIterator final : public IndexIterator { private: MMFilesPrimaryIndexImpl const* _index; arangodb::basics::BucketPosition _position; + std::vector> _revisions; bool const _reverse; uint64_t _total; }; diff --git a/arangod/MMFiles/MMFilesRevisionsCache.cpp b/arangod/MMFiles/MMFilesRevisionsCache.cpp index cb27f343053c..4282514b2165 100644 --- a/arangod/MMFiles/MMFilesRevisionsCache.cpp +++ b/arangod/MMFiles/MMFilesRevisionsCache.cpp @@ -42,6 +42,19 @@ MMFilesDocumentPosition MMFilesRevisionsCache::lookup(TRI_voc_rid_t revisionId) return _positions.findByKey(nullptr, &revisionId); } +void MMFilesRevisionsCache::batchLookup(std::vector>& revisions) const { + READ_LOCKER(locker, _lock); + + for (auto& it : revisions) { + MMFilesDocumentPosition const old = _positions.findByKey(nullptr, &it.first); + if (old) { + uint8_t const* vpack = static_cast(old.dataptr()); + TRI_ASSERT(VPackSlice(vpack).isObject()); + it.second = vpack; + } + } +} + void MMFilesRevisionsCache::sizeHint(int64_t hint) { WRITE_LOCKER(locker, _lock); if (hint > 256) { diff --git a/arangod/MMFiles/MMFilesRevisionsCache.h b/arangod/MMFiles/MMFilesRevisionsCache.h index 33411a57c8b6..4fecc17e1f9f 100644 --- a/arangod/MMFiles/MMFilesRevisionsCache.h +++ b/arangod/MMFiles/MMFilesRevisionsCache.h @@ -74,6 +74,7 @@ class MMFilesRevisionsCache { size_t memoryUsage(); void clear(); MMFilesDocumentPosition lookup(TRI_voc_rid_t revisionId) const; + void batchLookup(std::vector>& revisions) const; MMFilesDocumentPosition insert(TRI_voc_rid_t revisionId, uint8_t const* dataptr, TRI_voc_fid_t fid, bool isInWal, bool shouldLock); void insert(MMFilesDocumentPosition const& position, bool shouldLock); void update(TRI_voc_rid_t revisionId, uint8_t const* dataptr, TRI_voc_fid_t fid, bool isInWal); From 2dbe857d83518b159f7d6a5323d9deaab5c5240b Mon Sep 17 00:00:00 2001 From: jsteemann Date: Fri, 15 Sep 2017 15:25:25 +0200 Subject: [PATCH 3/9] fix compile errors --- ...ciative-multi-pointer-nohashcache-test.cpp | 80 ++++++++--------- .../Basics/associative-multi-pointer-test.cpp | 89 +++++++++---------- 2 files changed, 80 insertions(+), 89 deletions(-) diff --git a/tests/Basics/associative-multi-pointer-nohashcache-test.cpp b/tests/Basics/associative-multi-pointer-nohashcache-test.cpp index aed3b95ab001..0c84d3e53cf4 100644 --- a/tests/Basics/associative-multi-pointer-nohashcache-test.cpp +++ b/tests/Basics/associative-multi-pointer-nohashcache-test.cpp @@ -41,12 +41,6 @@ using namespace std; // --SECTION-- private macros // ----------------------------------------------------------------------------- -#define INIT_MULTI \ - arangodb::basics::AssocMulti a1( \ - HashKey, HashElement, IsEqualKeyElement, IsEqualElementElement, IsEqualElementElementByKey); - -#define DESTROY_MULTI ; - #define ELEMENT(name, v, k) \ data_container_t name; \ name.key = k; \ @@ -59,43 +53,55 @@ struct data_container_t { data_container_t (int key, int value) : value(value), key(key) {}; }; -static uint64_t HashKey (void* userData, void const* e) { - int const* key = (int const*) e; - - return fasthash64(key, sizeof(int), 0x12345678); -} +struct AssocMultiTestHelper { + static inline uint64_t HashKey(void* userData, void const* e) { + int const* key = (int const*) e; + return fasthash64(key, sizeof(int), 0x12345678); + } -static uint64_t HashElement (void* userData, void const* e, bool byKey) { - data_container_t const* element = (data_container_t const*) e; + static inline uint64_t HashElement(void* userData, + void const* e, + bool byKey) { + data_container_t const* element = (data_container_t const*) e; - if (byKey) { - return fasthash64(&element->key, sizeof(element->key), 0x12345678); - } - else { - return fasthash64(&element->value, sizeof(element->value), 0x12345678); + if (byKey) { + return fasthash64(&element->key, sizeof(element->key), 0x12345678); + } + else { + return fasthash64(&element->value, sizeof(element->value), 0x12345678); + } } -} -static bool IsEqualKeyElement (void* userData, void const* k, void const* r) { - int const* key = (int const*) k; - data_container_t const* element = (data_container_t const*) r; + bool IsEqualKeyElement(void* userData, + void const* k, + void const* r) const { + int const* key = (int const*) k; + data_container_t const* element = (data_container_t const*) r; - return *key == element->key; -} + return *key == element->key; + } -static bool IsEqualElementElement (void* userData, void const* l, void const* r) { - data_container_t const* left = (data_container_t const*) l; - data_container_t const* right = (data_container_t const*) r; + bool IsEqualElementElement(void* userData, + void const* l, + void const* r) const { + data_container_t const* left = (data_container_t const*) l; + data_container_t const* right = (data_container_t const*) r; - return left->value == right->value; -} + return left->value == right->value; + } -static bool IsEqualElementElementByKey (void* userData, void const* l, void const* r) { - data_container_t const* left = (data_container_t const*) l; - data_container_t const* right = (data_container_t const*) r; + bool IsEqualElementElementByKey(void* userData, + void const* l, + void const* r) const { + data_container_t const* left = (data_container_t const*) l; + data_container_t const* right = (data_container_t const*) r; - return left->key == right->key; -} + return left->key == right->key; + } +}; + +#define INIT_MULTI \ + arangodb::basics::AssocMulti a1((AssocMultiTestHelper())); // ----------------------------------------------------------------------------- // --SECTION-- private constants @@ -118,8 +124,6 @@ SECTION("tst_init") { INIT_MULTI CHECK((uint32_t) 0 == a1.size()); - - DESTROY_MULTI } //////////////////////////////////////////////////////////////////////////////// @@ -139,8 +143,6 @@ SECTION("tst_insert_few") { CHECK(&e1 == a1.remove(nullptr, &e1)); CHECK((uint32_t) 0 == a1.size()); CHECK(r == a1.lookup(nullptr, &e1)); - - DESTROY_MULTI } // Note MODULUS must be a divisor of NUMBER_OF_ELEMENTS @@ -262,8 +264,6 @@ SECTION("tst_insert_delete_many") { } v.clear(); delete one_more; - - DESTROY_MULTI } } diff --git a/tests/Basics/associative-multi-pointer-test.cpp b/tests/Basics/associative-multi-pointer-test.cpp index d3f768f758ca..7026ab1b9fd4 100644 --- a/tests/Basics/associative-multi-pointer-test.cpp +++ b/tests/Basics/associative-multi-pointer-test.cpp @@ -41,12 +41,6 @@ using namespace std; // --SECTION-- private macros // ----------------------------------------------------------------------------- -#define INIT_MULTI \ - arangodb::basics::AssocMulti a1( \ - HashKey, HashElement, IsEqualKeyElement, IsEqualElementElement, IsEqualElementElementByKey); - -#define DESTROY_MULTI ; - #define ELEMENT(name, v, k) \ data_container_t name; \ name.key = k; \ @@ -59,52 +53,55 @@ struct data_container_t { data_container_t (int key, int value) : value(value), key(key) {}; }; -static uint64_t HashKey (void* userData, - void const* e) { - int const* key = (int const*) e; - - return fasthash64(key, sizeof(int), 0x12345678); -} +struct AssocMultiTestHelper { + static inline uint64_t HashKey(void* userData, void const* e) { + int const* key = (int const*) e; + return fasthash64(key, sizeof(int), 0x12345678); + } -static uint64_t HashElement (void* userData, - void const* e, - bool byKey) { - data_container_t const* element = (data_container_t const*) e; + static inline uint64_t HashElement(void* userData, + void const* e, + bool byKey) { + data_container_t const* element = (data_container_t const*) e; - if (byKey) { - return fasthash64(&element->key, sizeof(element->key), 0x12345678); - } - else { - return fasthash64(&element->value, sizeof(element->value), 0x12345678); + if (byKey) { + return fasthash64(&element->key, sizeof(element->key), 0x12345678); + } + else { + return fasthash64(&element->value, sizeof(element->value), 0x12345678); + } } -} -static bool IsEqualKeyElement (void* userData, - void const* k, - void const* r) { - int const* key = (int const*) k; - data_container_t const* element = (data_container_t const*) r; + bool IsEqualKeyElement(void* userData, + void const* k, + void const* r) const { + int const* key = (int const*) k; + data_container_t const* element = (data_container_t const*) r; - return *key == element->key; -} + return *key == element->key; + } -static bool IsEqualElementElement (void* userData, - void const* l, - void const* r) { - data_container_t const* left = (data_container_t const*) l; - data_container_t const* right = (data_container_t const*) r; + bool IsEqualElementElement(void* userData, + void const* l, + void const* r) const { + data_container_t const* left = (data_container_t const*) l; + data_container_t const* right = (data_container_t const*) r; - return left->value == right->value; -} + return left->value == right->value; + } -static bool IsEqualElementElementByKey (void* userData, - void const* l, - void const* r) { - data_container_t const* left = (data_container_t const*) l; - data_container_t const* right = (data_container_t const*) r; + bool IsEqualElementElementByKey(void* userData, + void const* l, + void const* r) const { + data_container_t const* left = (data_container_t const*) l; + data_container_t const* right = (data_container_t const*) r; - return left->key == right->key; -} + return left->key == right->key; + } +}; + +#define INIT_MULTI \ + arangodb::basics::AssocMulti a1((AssocMultiTestHelper())); // ----------------------------------------------------------------------------- // --SECTION-- private constants @@ -127,8 +124,6 @@ SECTION("tst_init") { INIT_MULTI CHECK((uint32_t) 0 == a1.size()); - - DESTROY_MULTI } //////////////////////////////////////////////////////////////////////////////// @@ -148,8 +143,6 @@ SECTION("tst_insert_few") { CHECK(&e1 == a1.remove(nullptr, &e1)); CHECK((uint32_t) 0 == a1.size()); CHECK(r == a1.lookup(nullptr, &e1)); - - DESTROY_MULTI } // Note MODULUS must be a divisor of NUMBER_OF_ELEMENTS @@ -272,8 +265,6 @@ SECTION("tst_insert_delete_many") { } v.clear(); delete one_more; - - DESTROY_MULTI } } From 388ea1a7de8d977a4e6ca7ccf15b3822cc5fbc4c Mon Sep 17 00:00:00 2001 From: jsteemann Date: Mon, 18 Sep 2017 09:15:28 +0200 Subject: [PATCH 4/9] some cleanup --- arangod/MMFiles/MMFilesRevisionsCache.cpp | 17 ++++------ arangod/MMFiles/MMFilesRevisionsCache.h | 2 -- lib/Basics/fasthash.cpp | 41 ++++++----------------- lib/Basics/fasthash.h | 21 +++++++++++- 4 files changed, 36 insertions(+), 45 deletions(-) diff --git a/arangod/MMFiles/MMFilesRevisionsCache.cpp b/arangod/MMFiles/MMFilesRevisionsCache.cpp index 4282514b2165..ebe332d262e2 100644 --- a/arangod/MMFiles/MMFilesRevisionsCache.cpp +++ b/arangod/MMFiles/MMFilesRevisionsCache.cpp @@ -24,7 +24,6 @@ #include "MMFilesRevisionsCache.h" #include "Basics/ReadLocker.h" #include "Basics/WriteLocker.h" -#include "Basics/xxhash.h" #include "Logger/Logger.h" #include "MMFiles/MMFilesDatafileHelper.h" @@ -110,7 +109,7 @@ void MMFilesRevisionsCache::update(TRI_voc_rid_t revisionId, uint8_t const* data WRITE_LOCKER(locker, _lock); MMFilesDocumentPosition* old = _positions.findByKeyRef(nullptr, &revisionId); - if (old == nullptr) { + if (old == nullptr || !(*old)) { return; } @@ -122,12 +121,12 @@ void MMFilesRevisionsCache::update(TRI_voc_rid_t revisionId, uint8_t const* data bool MMFilesRevisionsCache::updateConditional(TRI_voc_rid_t revisionId, MMFilesMarker const* oldPosition, MMFilesMarker const* newPosition, TRI_voc_fid_t newFid, bool isInWal) { WRITE_LOCKER(locker, _lock); - MMFilesDocumentPosition old = _positions.findByKey(nullptr, &revisionId); - if (!old) { + MMFilesDocumentPosition* old = _positions.findByKeyRef(nullptr, &revisionId); + if (old == nullptr || !(*old)) { return false; } - uint8_t const* vpack = static_cast(old.dataptr()); + uint8_t const* vpack = static_cast(old->dataptr()); TRI_ASSERT(vpack != nullptr); MMFilesMarker const* markerPtr = reinterpret_cast(vpack - MMFilesDatafileHelper::VPackOffset(TRI_DF_MARKER_VPACK_DOCUMENT)); @@ -137,12 +136,8 @@ bool MMFilesRevisionsCache::updateConditional(TRI_voc_rid_t revisionId, MMFilesM return false; } - _positions.removeByKey(nullptr, &revisionId); - - old.dataptr(reinterpret_cast(newPosition) + MMFilesDatafileHelper::VPackOffset(TRI_DF_MARKER_VPACK_DOCUMENT)); - old.fid(newFid, isInWal); - - _positions.insert(nullptr, old); + old->dataptr(reinterpret_cast(newPosition) + MMFilesDatafileHelper::VPackOffset(TRI_DF_MARKER_VPACK_DOCUMENT)); + old->fid(newFid, isInWal); return true; } diff --git a/arangod/MMFiles/MMFilesRevisionsCache.h b/arangod/MMFiles/MMFilesRevisionsCache.h index 4fecc17e1f9f..d028faa8d29b 100644 --- a/arangod/MMFiles/MMFilesRevisionsCache.h +++ b/arangod/MMFiles/MMFilesRevisionsCache.h @@ -28,7 +28,6 @@ #include "Basics/AssocUnique.h" #include "Basics/ReadWriteLock.h" #include "Basics/fasthash.h" -#include "Logger/Logger.h" #include "MMFiles/MMFilesDocumentPosition.h" #include "VocBase/voc-types.h" @@ -39,7 +38,6 @@ namespace arangodb { struct MMFilesRevisionsCacheHelper { static inline uint64_t HashKey(void*, TRI_voc_rid_t const* key) { return fasthash64_uint64(*key, 0xdeadbeef); - return std::hash()(*key); } static inline uint64_t HashElement(void*, MMFilesDocumentPosition const& element, bool) { diff --git a/lib/Basics/fasthash.cpp b/lib/Basics/fasthash.cpp index af7a3945c495..8915f49a9a02 100644 --- a/lib/Basics/fasthash.cpp +++ b/lib/Basics/fasthash.cpp @@ -25,27 +25,6 @@ #include "fasthash.h" -static constexpr uint64_t m = 0x880355f21e6d1965ULL; - -// Compression function for Merkle-Damgard construction. -// This function is generated using the framework provided. - -static inline uint64_t mix(uint64_t h) { - h ^= h >> 23; - h *= 0x2127599bf4325c37ULL; - h ^= h >> 47; - - return h; -} - -uint64_t fasthash64_uint64(uint64_t value, uint64_t seed) { - uint64_t h = seed ^ 4619197404915747624ULL; // this is h = seed ^ (sizeof(uint64_t) * m), but prevents VS warning C4307: integral constant overflow - h ^= mix(value); - h *= m; - - return mix(h); -} - uint64_t fasthash64(const void* buf, size_t len, uint64_t seed) { #ifndef TRI_UNALIGNED_ACCESS // byte-wise hashing to support platforms that don't permit @@ -53,7 +32,7 @@ uint64_t fasthash64(const void* buf, size_t len, uint64_t seed) { // memory access strategy of fasthash64) uint8_t const* pos = (uint8_t const*)buf; uint8_t const* end = pos + len; - uint64_t h = seed ^ (len * m); + uint64_t h = seed ^ (len * fasthash_m); while (pos != end) { len = end - pos; @@ -79,14 +58,14 @@ uint64_t fasthash64(const void* buf, size_t len, uint64_t seed) { v ^= (uint64_t)pos[1] << 8; case 1: v ^= (uint64_t)pos[0]; - h ^= mix(v); - h *= m; + h ^= fasthash_mix(v); + h *= fasthash_m; } pos += len; } - return mix(h); + return fasthash_mix(h); #else // uint64_t-wise hashing for platforms that allow dereferencing // unaligned pointers to uint64_t memory @@ -94,13 +73,13 @@ uint64_t fasthash64(const void* buf, size_t len, uint64_t seed) { uint64_t const* pos = (uint64_t const*)buf; uint64_t const* end = pos + (len / 8); const unsigned char* pos2; - uint64_t h = seed ^ (len * m); + uint64_t h = seed ^ (len * fasthash_m); uint64_t v; while (pos != end) { v = *pos++; - h ^= mix(v); - h *= m; + h ^= fasthash_mix(v); + h *= fasthash_m; } pos2 = (const unsigned char*)pos; @@ -121,11 +100,11 @@ uint64_t fasthash64(const void* buf, size_t len, uint64_t seed) { v ^= (uint64_t)pos2[1] << 8; case 1: v ^= (uint64_t)pos2[0]; - h ^= mix(v); - h *= m; + h ^= fasthash_mix(v); + h *= fasthash_m; } - return mix(h); + return fasthash_mix(h); #endif } diff --git a/lib/Basics/fasthash.h b/lib/Basics/fasthash.h index 7ca29252f319..8691d3f5cefd 100644 --- a/lib/Basics/fasthash.h +++ b/lib/Basics/fasthash.h @@ -25,9 +25,28 @@ #ifndef LIB_BASICS_FASTHASH_H #define LIB_BASICS_FASTHASH_H 1 + #include "Basics/Common.h" -uint64_t fasthash64_uint64(uint64_t value, uint64_t seed); +static constexpr uint64_t fasthash_m = 0x880355f21e6d1965ULL; + +// Compression function for Merkle-Damgard construction. +// This function is generated using the framework provided. +static inline uint64_t fasthash_mix(uint64_t h) { + h ^= h >> 23; + h *= 0x2127599bf4325c37ULL; + h ^= h >> 47; + + return h; +} + +static inline uint64_t fasthash64_uint64(uint64_t value, uint64_t seed) { + uint64_t h = seed ^ 4619197404915747624ULL; // this is h = seed ^ (sizeof(uint64_t) * m), but prevents VS warning C4307: integral constant overflow + h ^= fasthash_mix(value); + h *= fasthash_m; + + return fasthash_mix(h); +} /** * fasthash32 - 32-bit implementation of fasthash From 1c74c8c0c2c37f2f4d0065cba5dcc1e74a4bcac3 Mon Sep 17 00:00:00 2001 From: jsteemann Date: Tue, 19 Sep 2017 08:44:54 +0200 Subject: [PATCH 5/9] some more micro optimizations --- arangod/Aql/AqlItemBlock.cpp | 37 ++------------ arangod/Aql/AqlItemBlock.h | 6 +-- arangod/Aql/AqlValue.cpp | 36 +++++++------- arangod/Aql/AqlValue.h | 61 +++++------------------- arangod/Aql/CollectBlock.cpp | 12 ++--- arangod/Aql/DocumentProducingBlock.h | 2 +- arangod/Aql/EnumerateCollectionBlock.cpp | 2 +- arangod/Aql/EnumerateListBlock.cpp | 2 +- arangod/Aql/IndexBlock.cpp | 15 ++++-- arangod/Aql/ModificationBlocks.cpp | 4 +- arangod/MMFiles/MMFilesIndexElement.h | 14 +++--- lib/Basics/VelocyPackHelper.cpp | 27 +++++------ 12 files changed, 76 insertions(+), 142 deletions(-) diff --git a/arangod/Aql/AqlItemBlock.cpp b/arangod/Aql/AqlItemBlock.cpp index 36df0933d36b..1d95af7c7dcb 100644 --- a/arangod/Aql/AqlItemBlock.cpp +++ b/arangod/Aql/AqlItemBlock.cpp @@ -207,10 +207,8 @@ void AqlItemBlock::destroy() { } /// @brief shrink the block to the specified number of rows -/// if sweep is set, then the superfluous rows are cleaned -/// if sweep is not set, the caller has to ensure that the -/// superfluous rows are empty -void AqlItemBlock::shrink(size_t nrItems, bool sweep) { +/// the superfluous rows are cleaned +void AqlItemBlock::shrink(size_t nrItems) { TRI_ASSERT(nrItems > 0); if (nrItems == _nrItems) { @@ -224,33 +222,6 @@ void AqlItemBlock::shrink(size_t nrItems, bool sweep) { "cannot use shrink() to increase block"); } - if (sweep) { - // erase all stored values in the region that we freed - for (size_t i = nrItems; i < _nrItems; ++i) { - for (RegisterId j = 0; j < _nrRegs; ++j) { - AqlValue& a(_data[_nrRegs * i + j]); - - if (a.requiresDestruction()) { - auto it = _valueCount.find(a); - - if (it != _valueCount.end()) { - TRI_ASSERT((*it).second > 0); - - if (--((*it).second) == 0) { - decreaseMemoryUsage(a.memoryUsage()); - a.destroy(); - try { - _valueCount.erase(it); - } catch (...) { - } - } - } - } - a.erase(); - } - } - } - decreaseMemoryUsage(sizeof(AqlValue) * (_nrItems - nrItems) * _nrRegs); // adjust the size of the block @@ -295,8 +266,8 @@ void AqlItemBlock::rescale(size_t nrItems, RegisterId nrRegs) { void AqlItemBlock::clearRegisters( std::unordered_set const& toClear) { - for (auto const& reg : toClear) { - for (size_t i = 0; i < _nrItems; i++) { + for (size_t i = 0; i < _nrItems; i++) { + for (auto const& reg : toClear) { AqlValue& a(_data[_nrRegs * i + reg]); if (a.requiresDestruction()) { diff --git a/arangod/Aql/AqlItemBlock.h b/arangod/Aql/AqlItemBlock.h index 5da593c46e25..d6504b687d66 100644 --- a/arangod/Aql/AqlItemBlock.h +++ b/arangod/Aql/AqlItemBlock.h @@ -296,10 +296,8 @@ class AqlItemBlock { inline size_t capacity() const { return _data.size(); } /// @brief shrink the block to the specified number of rows - /// if sweep is set, then the superfluous rows are cleaned - /// if sweep is not set, the caller has to ensure that the - /// superfluous rows are empty - void shrink(size_t nrItems, bool sweep); + /// the superfluous rows are cleaned + void shrink(size_t nrItems); /// @brief rescales the block to the specified dimensions /// note that the block should be empty before rescaling to prevent diff --git a/arangod/Aql/AqlValue.cpp b/arangod/Aql/AqlValue.cpp index 7ec16ae34a48..defe9f0fdafe 100644 --- a/arangod/Aql/AqlValue.cpp +++ b/arangod/Aql/AqlValue.cpp @@ -43,8 +43,8 @@ using namespace arangodb::aql; /// @brief hashes the value uint64_t AqlValue::hash(transaction::Methods* trx, uint64_t seed) const { switch (type()) { - case VPACK_SLICE_POINTER: case VPACK_INLINE: + case VPACK_SLICE_POINTER: case VPACK_MANAGED_SLICE: case VPACK_MANAGED_BUFFER: { // we must use the slow hash function here, because a value may have @@ -191,8 +191,8 @@ std::string const & AqlValue::getTypeString() const noexcept { /// @brief get the (array) length (note: this treats ranges as arrays, too!) size_t AqlValue::length() const { switch (type()) { - case VPACK_SLICE_POINTER: case VPACK_INLINE: + case VPACK_SLICE_POINTER: case VPACK_MANAGED_SLICE: case VPACK_MANAGED_BUFFER: { return static_cast(slice().length()); @@ -293,8 +293,8 @@ AqlValue AqlValue::getKeyAttribute(transaction::Methods* trx, mustDestroy = false; switch (type()) { case VPACK_SLICE_POINTER: - // fall-through intentional doCopy = false; + // fall-through intentional case VPACK_INLINE: // fall-through intentional case VPACK_MANAGED_SLICE: @@ -570,8 +570,8 @@ AqlValue AqlValue::get(transaction::Methods* trx, bool AqlValue::hasKey(transaction::Methods* trx, std::string const& name) const { switch (type()) { - case VPACK_SLICE_POINTER: case VPACK_INLINE: + case VPACK_SLICE_POINTER: case VPACK_MANAGED_SLICE: case VPACK_MANAGED_BUFFER: { VPackSlice s(slice()); @@ -596,8 +596,8 @@ double AqlValue::toDouble(transaction::Methods* trx) const { double AqlValue::toDouble(transaction::Methods* trx, bool& failed) const { failed = false; switch (type()) { - case VPACK_SLICE_POINTER: case VPACK_INLINE: + case VPACK_SLICE_POINTER: case VPACK_MANAGED_SLICE: case VPACK_MANAGED_BUFFER: { VPackSlice s(slice()); @@ -663,8 +663,8 @@ double AqlValue::toDouble(transaction::Methods* trx, bool& failed) const { /// @brief get the numeric value of an AqlValue int64_t AqlValue::toInt64(transaction::Methods* trx) const { switch (type()) { - case VPACK_SLICE_POINTER: case VPACK_INLINE: + case VPACK_SLICE_POINTER: case VPACK_MANAGED_SLICE: case VPACK_MANAGED_BUFFER: { VPackSlice s(slice()); @@ -715,8 +715,8 @@ int64_t AqlValue::toInt64(transaction::Methods* trx) const { /// @brief whether or not the contained value evaluates to true bool AqlValue::toBoolean() const { switch (type()) { - case VPACK_SLICE_POINTER: case VPACK_INLINE: + case VPACK_SLICE_POINTER: case VPACK_MANAGED_SLICE: case VPACK_MANAGED_BUFFER: { VPackSlice s(slice()); @@ -810,8 +810,8 @@ v8::Handle AqlValue::toV8( v8::Isolate* isolate, transaction::Methods* trx) const { switch (type()) { - case VPACK_SLICE_POINTER: case VPACK_INLINE: + case VPACK_SLICE_POINTER: case VPACK_MANAGED_SLICE: case VPACK_MANAGED_BUFFER: { VPackOptions* options = trx->transactionContext()->getVPackOptions(); @@ -911,8 +911,8 @@ void AqlValue::toVelocyPack(transaction::Methods* trx, AqlValue AqlValue::materialize(transaction::Methods* trx, bool& hasCopied, bool resolveExternals) const { switch (type()) { - case VPACK_SLICE_POINTER: case VPACK_INLINE: + case VPACK_SLICE_POINTER: case VPACK_MANAGED_SLICE: case VPACK_MANAGED_BUFFER: { hasCopied = false; @@ -939,6 +939,10 @@ AqlValue AqlValue::materialize(transaction::Methods* trx, bool& hasCopied, /// @brief clone a value AqlValue AqlValue::clone() const { switch (type()) { + case VPACK_INLINE: { + // copy internal data + return AqlValue(slice()); + } case VPACK_SLICE_POINTER: { if (isManagedDocument()) { // copy from externally managed document. this will not copy the data @@ -947,10 +951,6 @@ AqlValue AqlValue::clone() const { // copy from regular pointer. this may copy the data return AqlValue(_data.pointer); } - case VPACK_INLINE: { - // copy internal data - return AqlValue(slice()); - } case VPACK_MANAGED_SLICE: { return AqlValue(AqlValueHintCopy(_data.slice)); } @@ -986,8 +986,8 @@ AqlValue AqlValue::clone() const { /// @brief destroy the value's internals void AqlValue::destroy() noexcept { switch (type()) { - case VPACK_SLICE_POINTER: case VPACK_INLINE: { + case VPACK_SLICE_POINTER: // nothing to do return; } @@ -1018,9 +1018,6 @@ void AqlValue::destroy() noexcept { /// @brief return the slice from the value VPackSlice AqlValue::slice() const { switch (type()) { - case VPACK_SLICE_POINTER: { - return VPackSlice(_data.pointer); - } case VPACK_INLINE: { VPackSlice s(&_data.internal[0]); if (s.isExternal()) { @@ -1028,6 +1025,9 @@ VPackSlice AqlValue::slice() const { } return s; } + case VPACK_SLICE_POINTER: { + return VPackSlice(_data.pointer); + } case VPACK_MANAGED_SLICE: { VPackSlice s(_data.slice); if (s.isExternal()) { @@ -1137,8 +1137,8 @@ int AqlValue::Compare(transaction::Methods* trx, AqlValue const& left, // if we get here, types are equal or can be treated as being equal switch (leftType) { - case VPACK_SLICE_POINTER: case VPACK_INLINE: + case VPACK_SLICE_POINTER: case VPACK_MANAGED_SLICE: case VPACK_MANAGED_BUFFER: { return arangodb::basics::VelocyPackHelper::compare( diff --git a/arangod/Aql/AqlValue.h b/arangod/Aql/AqlValue.h index 11ccbab8236e..86ff8b18523a 100644 --- a/arangod/Aql/AqlValue.h +++ b/arangod/Aql/AqlValue.h @@ -109,8 +109,8 @@ struct AqlValue final { /// @brief AqlValueType, indicates what sort of value we have enum AqlValueType : uint8_t { + VPACK_INLINE = 0, // contains vpack data, inline VPACK_SLICE_POINTER, // contains a pointer to a vpack document, memory is not managed! - VPACK_INLINE, // contains vpack data, inline VPACK_MANAGED_SLICE, // contains vpack, via pointer to a managed uint8_t slice VPACK_MANAGED_BUFFER, // contains vpack, via pointer to a managed buffer DOCVEC, // a vector of blocks of results coming from a subquery, managed @@ -424,9 +424,8 @@ struct AqlValue final { bool isArray() const noexcept; // @brief return a string describing the content of this aqlvalue - std::string const & getTypeString() const noexcept; + std::string const& getTypeString() const noexcept; - /// @brief get the (array) length (note: this treats ranges as arrays, too!) size_t length() const; @@ -517,8 +516,8 @@ struct AqlValue final { size_t memoryUsage() const noexcept { auto const t = type(); switch (t) { - case VPACK_SLICE_POINTER: case VPACK_INLINE: + case VPACK_SLICE_POINTER: return 0; case VPACK_MANAGED_SLICE: try { @@ -696,29 +695,12 @@ struct hash { std::hash ptrHash; arangodb::aql::AqlValue::AqlValueType type = x.type(); size_t res = intHash(type); - switch (type) { - case arangodb::aql::AqlValue::VPACK_SLICE_POINTER: { - return res ^ ptrHash(x._data.pointer); - } - case arangodb::aql::AqlValue::VPACK_INLINE: { - return res ^ static_cast(arangodb::velocypack::Slice(&x._data.internal[0]).hash()); - } - case arangodb::aql::AqlValue::VPACK_MANAGED_SLICE: { - return res ^ ptrHash(x._data.slice); - } - case arangodb::aql::AqlValue::VPACK_MANAGED_BUFFER: { - return res ^ ptrHash(x._data.buffer); - } - case arangodb::aql::AqlValue::DOCVEC: { - return res ^ ptrHash(x._data.docvec); - } - case arangodb::aql::AqlValue::RANGE: { - return res ^ ptrHash(x._data.range); - } + if (type == arangodb::aql::AqlValue::VPACK_INLINE) { + return res ^ static_cast(arangodb::velocypack::Slice(&x._data.internal[0]).hash()); } - - TRI_ASSERT(false); - return 0; + // treat all other pointer types the same, because they will + // have the same bit representations + return res ^ ptrHash(x._data.pointer); } }; @@ -730,29 +712,12 @@ struct equal_to { if (type != b.type()) { return false; } - switch (type) { - case arangodb::aql::AqlValue::VPACK_SLICE_POINTER: { - return a._data.pointer == b._data.pointer; - } - case arangodb::aql::AqlValue::VPACK_INLINE: { - return arangodb::velocypack::Slice(&a._data.internal[0]).equals(arangodb::velocypack::Slice(&b._data.internal[0])); - } - case arangodb::aql::AqlValue::VPACK_MANAGED_SLICE: { - return a._data.slice == b._data.slice; - } - case arangodb::aql::AqlValue::VPACK_MANAGED_BUFFER: { - return a._data.buffer == b._data.buffer; - } - case arangodb::aql::AqlValue::DOCVEC: { - return a._data.docvec == b._data.docvec; - } - case arangodb::aql::AqlValue::RANGE: { - return a._data.range == b._data.range; - } + if (type == arangodb::aql::AqlValue::VPACK_INLINE) { + return arangodb::velocypack::Slice(&a._data.internal[0]).equals(arangodb::velocypack::Slice(&b._data.internal[0])); } - - TRI_ASSERT(false); - return false; + // treat all other pointer types the same, because they will + // have the same bit representations + return a._data.pointer == b._data.pointer; } }; diff --git a/arangod/Aql/CollectBlock.cpp b/arangod/Aql/CollectBlock.cpp index a62f3119b3da..9cd1fefec9a5 100644 --- a/arangod/Aql/CollectBlock.cpp +++ b/arangod/Aql/CollectBlock.cpp @@ -335,7 +335,7 @@ int SortedCollectBlock::getOrSkipSome(size_t atLeast, size_t atMost, for (auto& it : _groupRegisters) { int cmp = AqlValue::Compare( _trx, _currentGroup.groupValues[i], - cur->getValue(_pos, it.second), false); + cur->getValueReference(_pos, it.second), false); if (cmp != 0) { // group change @@ -415,7 +415,7 @@ int SortedCollectBlock::getOrSkipSome(size_t atLeast, size_t atMost, TRI_ASSERT(cur != nullptr); emitGroup(cur, res.get(), skipped, skipping); ++skipped; - res->shrink(skipped, false); + res->shrink(skipped); } else { ++skipped; } @@ -454,7 +454,7 @@ int SortedCollectBlock::getOrSkipSome(size_t atLeast, size_t atMost, if (!skipping) { TRI_ASSERT(skipped > 0); - res->shrink(skipped, false); + res->shrink(skipped); } result = res.release(); @@ -671,7 +671,7 @@ int HashedCollectBlock::getOrSkipSome(size_t atLeast, size_t atMost, result->setValue(row, _aggregateRegisters[j++].first, r->stealValue()); } - } else if (en->_count) { + } else { // set group count in result register TRI_ASSERT(!it.second->empty()); result->setValue(row, _collectRegister, @@ -703,8 +703,6 @@ int HashedCollectBlock::getOrSkipSome(size_t atLeast, size_t atMost, THROW_ARANGO_EXCEPTION(TRI_ERROR_DEBUG); } - throwIfKilled(); // check if we were aborted - groupValues.clear(); // for hashing simply re-use the aggregate registers, without cloning @@ -777,6 +775,8 @@ int HashedCollectBlock::getOrSkipSome(size_t atLeast, size_t atMost, if (++_pos >= cur->size()) { _buffer.pop_front(); _pos = 0; + + throwIfKilled(); // check if we were aborted bool hasMore = !_buffer.empty(); diff --git a/arangod/Aql/DocumentProducingBlock.h b/arangod/Aql/DocumentProducingBlock.h index 44080ccd2ee2..45045d0370f9 100644 --- a/arangod/Aql/DocumentProducingBlock.h +++ b/arangod/Aql/DocumentProducingBlock.h @@ -48,7 +48,7 @@ class DocumentProducingBlock { virtual ~DocumentProducingBlock() = default; public: - bool produceResult() const { return _produceResult; } + inline bool produceResult() const { return _produceResult; } private: DocumentProducingFunction buildCallback() const; diff --git a/arangod/Aql/EnumerateCollectionBlock.cpp b/arangod/Aql/EnumerateCollectionBlock.cpp index 3f7172d9ab96..23ad19c6e2ed 100644 --- a/arangod/Aql/EnumerateCollectionBlock.cpp +++ b/arangod/Aql/EnumerateCollectionBlock.cpp @@ -216,7 +216,7 @@ AqlItemBlock* EnumerateCollectionBlock::getSome(size_t, // atLeast, if (send < atMost) { // The collection did not have enough results - res->shrink(send, false); + res->shrink(send); } // Clear out registers no longer needed later: diff --git a/arangod/Aql/EnumerateListBlock.cpp b/arangod/Aql/EnumerateListBlock.cpp index fc67b0738f3b..8ef42dfa436b 100644 --- a/arangod/Aql/EnumerateListBlock.cpp +++ b/arangod/Aql/EnumerateListBlock.cpp @@ -75,7 +75,7 @@ AqlItemBlock* EnumerateListBlock::getSome(size_t, size_t atMost) { return nullptr; } - std::unique_ptr res(nullptr); + std::unique_ptr res; do { // repeatedly try to get more stuff from upstream diff --git a/arangod/Aql/IndexBlock.cpp b/arangod/Aql/IndexBlock.cpp index db2ffc8bc71d..3fe23862ccdd 100644 --- a/arangod/Aql/IndexBlock.cpp +++ b/arangod/Aql/IndexBlock.cpp @@ -447,8 +447,13 @@ bool IndexBlock::readIndex( } TRI_ASSERT(atMost >= _returned); - - if (_cursor->nextDocument(callback, atMost - _returned)) { + + + // TODO: optimize for the case when produceResult() is false + // in this case we do not need to fetch the documents at all + bool res = _cursor->nextDocument(callback, atMost - _returned); + + if (res) { // We have returned enough. // And this index could return more. // We are good. @@ -507,11 +512,11 @@ AqlItemBlock* IndexBlock::getSome(size_t atLeast, size_t atMost) { if (_indexes.size() > 1) { // Activate uniqueness checks callback = [&](DocumentIdentifierToken const& token, VPackSlice slice) { - TRI_ASSERT(res.get() != nullptr); + TRI_ASSERT(res != nullptr); if (!_isLastIndex) { // insert & check for duplicates in one go if (!_alreadyReturned.emplace(token._data).second) { - // Document already in list. Skip this + // Document already in list. Skip it return; } } else { @@ -599,7 +604,7 @@ AqlItemBlock* IndexBlock::getSome(size_t atLeast, size_t atMost) { return nullptr; } if (_returned < atMost) { - res->shrink(_returned, false); + res->shrink(_returned); } // Clear out registers no longer needed later: diff --git a/arangod/Aql/ModificationBlocks.cpp b/arangod/Aql/ModificationBlocks.cpp index 448eb2b6f599..db8a42e98ca6 100644 --- a/arangod/Aql/ModificationBlocks.cpp +++ b/arangod/Aql/ModificationBlocks.cpp @@ -129,7 +129,7 @@ AqlItemBlock* ModificationBlock::getSome(size_t atLeast, size_t atMost) { std::unique_ptr res( ExecutionBlock::getSomeWithoutRegisterClearout(atLeast, atMost)); - if (res.get() == nullptr) { + if (res == nullptr) { break; } @@ -142,7 +142,7 @@ AqlItemBlock* ModificationBlock::getSome(size_t atLeast, size_t atMost) { replyBlocks.reset(work(blocks)); - if (replyBlocks.get() != nullptr) { + if (replyBlocks != nullptr) { break; } } diff --git a/arangod/MMFiles/MMFilesIndexElement.h b/arangod/MMFiles/MMFilesIndexElement.h index 76cb4f58579c..4b7bfa95ce46 100644 --- a/arangod/MMFiles/MMFilesIndexElement.h +++ b/arangod/MMFiles/MMFilesIndexElement.h @@ -206,7 +206,7 @@ struct MMFilesSimpleIndexElement { public: constexpr MMFilesSimpleIndexElement() : _revisionId(0), _hashAndOffset(0) {} MMFilesSimpleIndexElement(TRI_voc_rid_t revisionId, arangodb::velocypack::Slice const& value, uint32_t offset); - MMFilesSimpleIndexElement(MMFilesSimpleIndexElement const& other) : _revisionId(other._revisionId), _hashAndOffset(other._hashAndOffset) {} + MMFilesSimpleIndexElement(MMFilesSimpleIndexElement const& other) noexcept : _revisionId(other._revisionId), _hashAndOffset(other._hashAndOffset) {} MMFilesSimpleIndexElement& operator=(MMFilesSimpleIndexElement const& other) noexcept { _revisionId = other._revisionId; _hashAndOffset = other._hashAndOffset; @@ -214,16 +214,16 @@ struct MMFilesSimpleIndexElement { } /// @brief get the revision id of the document - inline TRI_voc_rid_t revisionId() const { return _revisionId; } - inline uint64_t hash() const { return _hashAndOffset & 0xFFFFFFFFULL; } - inline uint32_t offset() const { return static_cast((_hashAndOffset & 0xFFFFFFFF00000000ULL) >> 32); } + inline TRI_voc_rid_t revisionId() const noexcept { return _revisionId; } + inline uint64_t hash() const noexcept { return _hashAndOffset & 0xFFFFFFFFULL; } + inline uint32_t offset() const noexcept { return static_cast((_hashAndOffset & 0xFFFFFFFF00000000ULL) >> 32); } arangodb::velocypack::Slice slice(IndexLookupContext*) const; - inline operator bool() const { return _revisionId != 0; } - inline bool operator==(MMFilesSimpleIndexElement const& other) const { + inline operator bool() const noexcept { return _revisionId != 0; } + inline bool operator==(MMFilesSimpleIndexElement const& other) const noexcept { return _revisionId == other._revisionId && _hashAndOffset == other._hashAndOffset; } - inline bool operator<(MMFilesSimpleIndexElement const& other) const { + inline bool operator<(MMFilesSimpleIndexElement const& other) const noexcept { return _revisionId < other._revisionId; } diff --git a/lib/Basics/VelocyPackHelper.cpp b/lib/Basics/VelocyPackHelper.cpp index 7c9b38a4f21a..f1c587269d21 100644 --- a/lib/Basics/VelocyPackHelper.cpp +++ b/lib/Basics/VelocyPackHelper.cpp @@ -392,12 +392,12 @@ int VelocyPackHelper::compareNumberValues(VPackValueType lhsType, // fallthrough to double comparison } - double left = lhs.getNumericValue(); - double right = rhs.getNumericValue(); - if (left == right) { + double l = lhs.getNumericValue(); + double r = rhs.getNumericValue(); + if (l == r) { return 0; } - return (left < right ? -1 : 1); + return (l < r ? -1 : 1); } ////////////////////////////////////////////////////////////////////////////// @@ -413,18 +413,17 @@ int VelocyPackHelper::compareStringValues(char const* left, VPackValueLength nl, size_t len = static_cast(nl < nr ? nl : nr); res = memcmp(left, right, len); } - if (res < 0) { - return -1; - } - if (res > 0) { - return 1; + + if (res != 0) { + return (res < 0 ? -1 : 1); } + // res == 0 if (nl == nr) { return 0; } // res == 0, but different string lengths - return nl < nr ? -1 : 1; + return (nl < nr ? -1 : 1); } //////////////////////////////////////////////////////////////////////////////// @@ -719,12 +718,8 @@ int VelocyPackHelper::compare(VPackSlice lhs, VPackSlice rhs, bool useUTF8, int8_t lWeight = TypeWeight(lhs); int8_t rWeight = TypeWeight(rhs); - if (lWeight < rWeight) { - return -1; - } - - if (lWeight > rWeight) { - return 1; + if (lWeight != rWeight) { + return (lWeight < rWeight ? -1 : 1); } TRI_ASSERT(lWeight == rWeight); From 1e293e7cee66e099e078ed19e71a355ad141ec6d Mon Sep 17 00:00:00 2001 From: jsteemann Date: Tue, 19 Sep 2017 09:04:18 +0200 Subject: [PATCH 6/9] simplify method --- arangod/Aql/AqlValue.cpp | 28 +++++++++------------------- arangod/Aql/AqlValue.h | 3 +-- 2 files changed, 10 insertions(+), 21 deletions(-) diff --git a/arangod/Aql/AqlValue.cpp b/arangod/Aql/AqlValue.cpp index defe9f0fdafe..2a2e42292f2d 100644 --- a/arangod/Aql/AqlValue.cpp +++ b/arangod/Aql/AqlValue.cpp @@ -159,33 +159,23 @@ bool AqlValue::isArray() const noexcept { } } -std::array AqlValue::typeStrings = { { - "none", - "null", - "bool", - "number", - "string", - "object", - "array", - "unknown"} }; - -std::string const & AqlValue::getTypeString() const noexcept { +char const* AqlValue::getTypeString() const noexcept { if(isNone()) { - return typeStrings[0]; + return "none"; } else if(isNull(true)) { - return typeStrings[1]; + return "null"; } else if(isBoolean()) { - return typeStrings[2]; + return "bool"; } else if(isNumber()) { - return typeStrings[3]; + return "number"; } else if(isString()) { - return typeStrings[4]; + return "string"; } else if(isObject()) { - return typeStrings[5]; + return "object"; } else if(isArray()){ - return typeStrings[6]; + return "array"; } - return typeStrings[7]; + return "unknown"; } /// @brief get the (array) length (note: this treats ranges as arrays, too!) diff --git a/arangod/Aql/AqlValue.h b/arangod/Aql/AqlValue.h index 86ff8b18523a..6ae390e09f87 100644 --- a/arangod/Aql/AqlValue.h +++ b/arangod/Aql/AqlValue.h @@ -103,7 +103,6 @@ struct AqlValueHintTransferOwnership { struct AqlValue final { friend struct std::hash; friend struct std::equal_to; - static std::array typeStrings; public: @@ -424,7 +423,7 @@ struct AqlValue final { bool isArray() const noexcept; // @brief return a string describing the content of this aqlvalue - std::string const& getTypeString() const noexcept; + char const* getTypeString() const noexcept; /// @brief get the (array) length (note: this treats ranges as arrays, too!) size_t length() const; From 2c8d70efcd3e890452e1426a28999866ce544b0b Mon Sep 17 00:00:00 2001 From: jsteemann Date: Wed, 20 Sep 2017 12:00:16 +0200 Subject: [PATCH 7/9] simplify some AqlValue construction APIs --- arangod/Aql/Aggregator.cpp | 12 +- arangod/Aql/AqlValue.cpp | 40 +-- arangod/Aql/AqlValue.h | 87 ++++-- arangod/Aql/AttributeAccessor.cpp | 4 +- arangod/Aql/DocumentProducingBlock.cpp | 23 +- arangod/Aql/Expression.cpp | 311 +++++++++++----------- arangod/Aql/Expression.h | 28 +- arangod/Aql/FixedVarExpressionContext.cpp | 2 +- arangod/Aql/Functions.cpp | 300 ++++++++++----------- arangod/Aql/V8Expression.cpp | 2 +- arangod/Graph/ClusterTraverserCache.cpp | 6 +- arangod/Graph/ShortestPathResult.cpp | 2 +- 12 files changed, 410 insertions(+), 407 deletions(-) diff --git a/arangod/Aql/Aggregator.cpp b/arangod/Aql/Aggregator.cpp index 89a47247ec30..fdf8471263f4 100644 --- a/arangod/Aql/Aggregator.cpp +++ b/arangod/Aql/Aggregator.cpp @@ -130,7 +130,7 @@ void AggregatorMin::reduce(AqlValue const& cmpValue) { AqlValue AggregatorMin::stealValue() { if (value.isEmpty()) { - return AqlValue(arangodb::basics::VelocyPackHelper::NullValue()); + return AqlValue(AqlValueHintNull()); } AqlValue copy = value; value.erase(); @@ -151,7 +151,7 @@ void AggregatorMax::reduce(AqlValue const& cmpValue) { AqlValue AggregatorMax::stealValue() { if (value.isEmpty()) { - return AqlValue(arangodb::basics::VelocyPackHelper::NullValue()); + return AqlValue(AqlValueHintNull()); } AqlValue copy = value; value.erase(); @@ -184,7 +184,7 @@ void AggregatorSum::reduce(AqlValue const& cmpValue) { AqlValue AggregatorSum::stealValue() { if (invalid || std::isnan(sum) || sum == HUGE_VAL || sum == -HUGE_VAL) { - return AqlValue(arangodb::basics::VelocyPackHelper::NullValue()); + return AqlValue(AqlValueHintNull()); } builder.clear(); @@ -223,7 +223,7 @@ void AggregatorAverage::reduce(AqlValue const& cmpValue) { AqlValue AggregatorAverage::stealValue() { if (invalid || count == 0 || std::isnan(sum) || sum == HUGE_VAL || sum == -HUGE_VAL) { - return AqlValue(arangodb::basics::VelocyPackHelper::NullValue()); + return AqlValue(AqlValueHintNull()); } TRI_ASSERT(count > 0); @@ -267,7 +267,7 @@ void AggregatorVarianceBase::reduce(AqlValue const& cmpValue) { AqlValue AggregatorVariance::stealValue() { if (invalid || count == 0 || (count == 1 && !population) || std::isnan(sum) || sum == HUGE_VAL || sum == -HUGE_VAL) { - return AqlValue(arangodb::basics::VelocyPackHelper::NullValue()); + return AqlValue(AqlValueHintNull()); } TRI_ASSERT(count > 0); @@ -289,7 +289,7 @@ AqlValue AggregatorVariance::stealValue() { AqlValue AggregatorStddev::stealValue() { if (invalid || count == 0 || (count == 1 && !population) || std::isnan(sum) || sum == HUGE_VAL || sum == -HUGE_VAL) { - return AqlValue(arangodb::basics::VelocyPackHelper::NullValue()); + return AqlValue(AqlValueHintNull()); } TRI_ASSERT(count > 0); diff --git a/arangod/Aql/AqlValue.cpp b/arangod/Aql/AqlValue.cpp index 2a2e42292f2d..a9eea884504f 100644 --- a/arangod/Aql/AqlValue.cpp +++ b/arangod/Aql/AqlValue.cpp @@ -160,22 +160,22 @@ bool AqlValue::isArray() const noexcept { } char const* AqlValue::getTypeString() const noexcept { - if(isNone()) { + if (isNone()) { return "none"; - } else if(isNull(true)) { + } else if (isNull(true)) { return "null"; - } else if(isBoolean()) { + } else if (isBoolean()) { return "bool"; - } else if(isNumber()) { + } else if (isNumber()) { return "number"; - } else if(isString()) { + } else if (isString()) { return "string"; - } else if(isObject()) { + } else if (isObject()) { return "object"; - } else if(isArray()){ + } else if (isArray()){ return "array"; } - return "unknown"; + return "none"; } /// @brief get the (array) length (note: this treats ranges as arrays, too!) @@ -274,7 +274,7 @@ AqlValue AqlValue::at(transaction::Methods* trx, } // default is to return null - return AqlValue(arangodb::basics::VelocyPackHelper::NullValue()); + return AqlValue(AqlValueHintNull()); } /// @brief get the _key attribute from an object/document @@ -313,7 +313,7 @@ AqlValue AqlValue::getKeyAttribute(transaction::Methods* trx, } // default is to return null - return AqlValue(arangodb::basics::VelocyPackHelper::NullValue()); + return AqlValue(AqlValueHintNull()); } /// @brief get the _id attribute from an object/document @@ -357,7 +357,7 @@ AqlValue AqlValue::getIdAttribute(transaction::Methods* trx, } // default is to return null - return AqlValue(arangodb::basics::VelocyPackHelper::NullValue()); + return AqlValue(AqlValueHintNull()); } /// @brief get the _from attribute from an object/document @@ -396,7 +396,7 @@ AqlValue AqlValue::getFromAttribute(transaction::Methods* trx, } // default is to return null - return AqlValue(arangodb::basics::VelocyPackHelper::NullValue()); + return AqlValue(AqlValueHintNull()); } /// @brief get the _to attribute from an object/document @@ -435,7 +435,7 @@ AqlValue AqlValue::getToAttribute(transaction::Methods* trx, } // default is to return null - return AqlValue(arangodb::basics::VelocyPackHelper::NullValue()); + return AqlValue(AqlValueHintNull()); } /// @brief get the (object) element by name @@ -480,7 +480,7 @@ AqlValue AqlValue::get(transaction::Methods* trx, } // default is to return null - return AqlValue(arangodb::basics::VelocyPackHelper::NullValue()); + return AqlValue(AqlValueHintNull()); } /// @brief get the (object) element(s) by name @@ -489,7 +489,7 @@ AqlValue AqlValue::get(transaction::Methods* trx, bool& mustDestroy, bool doCopy) const { mustDestroy = false; if (names.empty()) { - return AqlValue(arangodb::basics::VelocyPackHelper::NullValue()); + return AqlValue(AqlValueHintNull()); } switch (type()) { @@ -518,7 +518,7 @@ AqlValue AqlValue::get(transaction::Methods* trx, if (s.isNone()) { // not found - return AqlValue(arangodb::basics::VelocyPackHelper::NullValue()); + return AqlValue(AqlValueHintNull()); } else if (s.isCustom()) { // _id needs special treatment if (i + 1 == n) { @@ -527,9 +527,9 @@ AqlValue AqlValue::get(transaction::Methods* trx, return AqlValue(transaction::helpers::extractIdString(trx->resolver(), s, prev)); } // x._id.y - return AqlValue(arangodb::basics::VelocyPackHelper::NullValue()); + return AqlValue(AqlValueHintNull()); } else if (i + 1 < n && !s.isObject()) { - return AqlValue(arangodb::basics::VelocyPackHelper::NullValue()); + return AqlValue(AqlValueHintNull()); } } @@ -553,7 +553,7 @@ AqlValue AqlValue::get(transaction::Methods* trx, } // default is to return null - return AqlValue(arangodb::basics::VelocyPackHelper::NullValue()); + return AqlValue(AqlValueHintNull()); } /// @brief check whether an object has a specific key @@ -936,7 +936,7 @@ AqlValue AqlValue::clone() const { case VPACK_SLICE_POINTER: { if (isManagedDocument()) { // copy from externally managed document. this will not copy the data - return AqlValue(AqlValueHintNoCopy(_data.pointer)); + return AqlValue(AqlValueHintDocumentNoCopy(_data.pointer)); } // copy from regular pointer. this may copy the data return AqlValue(_data.pointer); diff --git a/arangod/Aql/AqlValue.h b/arangod/Aql/AqlValue.h index 6ae390e09f87..278213174bd1 100644 --- a/arangod/Aql/AqlValue.h +++ b/arangod/Aql/AqlValue.h @@ -86,18 +86,42 @@ struct AqlValueHintCopy { }; // no-op struct used only internally to indicate that we want -// to NOT copy the data behind the passed pointer -struct AqlValueHintNoCopy { - explicit AqlValueHintNoCopy(uint8_t const* ptr) : ptr(ptr) {} +// to NOT copy the database document data behind the passed pointer +struct AqlValueHintDocumentNoCopy { + explicit AqlValueHintDocumentNoCopy(uint8_t const* v) : ptr(v) {} uint8_t const* ptr; }; -// no-op struct used only internally to indicate that we want -// to pass the ownership of the data behind the passed pointer -// to the callee -struct AqlValueHintTransferOwnership { - explicit AqlValueHintTransferOwnership(uint8_t* ptr) : ptr(ptr) {} - uint8_t* ptr; +struct AqlValueHintNull { + constexpr AqlValueHintNull() noexcept {} +}; + +struct AqlValueHintBool { + explicit AqlValueHintBool(bool v) noexcept : value(v) {} + bool const value; +}; + +struct AqlValueHintZero { + constexpr AqlValueHintZero() noexcept {} +}; + +struct AqlValueHintDouble { + explicit AqlValueHintDouble(double v) noexcept : value(v) {} + double const value; +}; + +struct AqlValueHintInt { + explicit AqlValueHintInt(int64_t v) noexcept : value(v) {} + explicit AqlValueHintInt(int v) noexcept : value(int64_t(v)) {} + int64_t const value; +}; + +struct AqlValueHintUInt { + explicit AqlValueHintUInt(uint64_t v) noexcept : value(v) {} +#ifdef TRI_OVERLOAD_FUNCS_SIZE_T + explicit AqlValueHintUInt(size_t v) noexcept : value(uint64_t(v)) {} +#endif + uint64_t const value; }; struct AqlValue final { @@ -176,14 +200,30 @@ struct AqlValue final { } // construct boolean value type + // DEPRECATED explicit AqlValue(bool value) { VPackSlice slice(value ? arangodb::basics::VelocyPackHelper::TrueValue() : arangodb::basics::VelocyPackHelper::FalseValue()); memcpy(_data.internal, slice.begin(), static_cast(slice.byteSize())); setType(AqlValueType::VPACK_INLINE); } + + explicit AqlValue(AqlValueHintNull const&) noexcept { + _data.internal[0] = 0x18; // null in VPack + setType(AqlValueType::VPACK_INLINE); + } + + explicit AqlValue(AqlValueHintBool const& v) noexcept { + _data.internal[0] = v.value ? 0x1a : 0x19; // true/false in VPack + setType(AqlValueType::VPACK_INLINE); + } + + explicit AqlValue(AqlValueHintZero const&) noexcept { + _data.internal[0] = 0x30; // 0 in VPack + setType(AqlValueType::VPACK_INLINE); + } // construct from a double value - explicit AqlValue(double value) { + explicit AqlValue(double value) noexcept { if (std::isnan(value) || !std::isfinite(value) || value == HUGE_VAL || value == -HUGE_VAL) { // null _data.internal[0] = 0x18; @@ -203,6 +243,9 @@ struct AqlValue final { setType(AqlValueType::VPACK_INLINE); } + // construct from a double value + explicit AqlValue(AqlValueHintDouble const&v) noexcept : AqlValue(v.value) {} + // construct from an int64 value explicit AqlValue(int64_t value) noexcept { if (value >= 0 && value <= 9) { @@ -251,6 +294,11 @@ struct AqlValue final { setType(AqlValueType::VPACK_INLINE); } + // construct from a uint64 value + explicit AqlValue(AqlValueHintUInt const& v) noexcept : AqlValue(v.value) {} + + explicit AqlValue(AqlValueHintInt const& v) noexcept : AqlValue(v.value) {} + // construct from char* and length, copying the string AqlValue(char const* value, size_t length) { TRI_ASSERT(value != nullptr); @@ -315,22 +363,15 @@ struct AqlValue final { } // construct from pointer, not copying! - explicit AqlValue(AqlValueHintNoCopy const& ptr) noexcept { - setPointer(ptr.ptr); + explicit AqlValue(AqlValueHintDocumentNoCopy const& v) noexcept { + setPointer(v.ptr); TRI_ASSERT(!VPackSlice(_data.pointer).isExternal()); } // construct from pointer, copying the data behind the pointer - explicit AqlValue(AqlValueHintCopy const& ptr) { - TRI_ASSERT(ptr.ptr != nullptr); - initFromSlice(VPackSlice(ptr.ptr)); - } - - // construct from pointer, taking over the ownership - explicit AqlValue(AqlValueHintTransferOwnership const& ptr) { - TRI_ASSERT(ptr.ptr != nullptr); - _data.slice = ptr.ptr; - setType(AqlValueType::VPACK_MANAGED_SLICE); + explicit AqlValue(AqlValueHintCopy const& v) { + TRI_ASSERT(v.ptr != nullptr); + initFromSlice(VPackSlice(v.ptr)); } // construct from Builder, copying contents @@ -422,7 +463,7 @@ struct AqlValue final { /// as arrays, too!) bool isArray() const noexcept; - // @brief return a string describing the content of this aqlvalue + // @brief return a string describing the content of this AqlValue char const* getTypeString() const noexcept; /// @brief get the (array) length (note: this treats ranges as arrays, too!) diff --git a/arangod/Aql/AttributeAccessor.cpp b/arangod/Aql/AttributeAccessor.cpp index af7ba1a7e626..67f5740f039b 100644 --- a/arangod/Aql/AttributeAccessor.cpp +++ b/arangod/Aql/AttributeAccessor.cpp @@ -89,7 +89,7 @@ AqlValue AttributeAccessor::getSystem(transaction::Methods* trx, return value.getToAttribute(trx, mustDestroy, true); default: { mustDestroy = false; - return AqlValue(arangodb::basics::VelocyPackHelper::NullValue()); + return AqlValue(AqlValueHintNull()); } } } @@ -108,7 +108,7 @@ AqlValue AttributeAccessor::getDynamic(transaction::Methods* trx, return value.get(trx, _attributeParts, mustDestroy, true); default: { mustDestroy = false; - return AqlValue(arangodb::basics::VelocyPackHelper::NullValue()); + return AqlValue(AqlValueHintNull()); } } } diff --git a/arangod/Aql/DocumentProducingBlock.cpp b/arangod/Aql/DocumentProducingBlock.cpp index 90d1cf380195..6db5d9fc3ce0 100644 --- a/arangod/Aql/DocumentProducingBlock.cpp +++ b/arangod/Aql/DocumentProducingBlock.cpp @@ -28,6 +28,7 @@ #include "Aql/IndexNode.h" #include "Aql/Variable.h" #include "Basics/Exceptions.h" +#include "Basics/StaticStrings.h" #include "StorageEngine/EngineSelectorFeature.h" #include "StorageEngine/StorageEngine.h" #include "Transaction/Helpers.h" @@ -59,17 +60,17 @@ DocumentProducingBlock::DocumentProducingFunction DocumentProducingBlock::buildC auto const& projection = _node->projection(); if (projection.size() == 1) { - if (projection[0] == "_id") { + if (projection[0] == StaticStrings::IdString) { // return _id attribute return [this](AqlItemBlock* res, VPackSlice slice, size_t registerId, size_t& row, size_t fromRow) { VPackSlice found = transaction::helpers::extractIdFromDocument(slice); if (found.isCustom()) { // _id as a custom type needs special treatment res->emplaceValue(row, static_cast(registerId), - transaction::helpers::extractIdString(_trxPtr->resolver(), found, slice)); + transaction::helpers::extractIdString(_trxPtr->resolver(), found, slice)); } else { res->emplaceValue(row, static_cast(registerId), - AqlValueHintCopy(found.begin())); + AqlValueHintCopy(found.begin())); } if (row != fromRow) { // re-use already copied AQLValues @@ -77,16 +78,16 @@ DocumentProducingBlock::DocumentProducingFunction DocumentProducingBlock::buildC } ++row; }; - } else if (projection[0] == "_key") { + } else if (projection[0] == StaticStrings::KeyString) { // return _key attribute return [this](AqlItemBlock* res, VPackSlice slice, size_t registerId, size_t& row, size_t fromRow) { VPackSlice found = transaction::helpers::extractKeyFromDocument(slice); if (_useRawDocumentPointers) { res->emplaceValue(row, static_cast(registerId), - AqlValueHintNoCopy(found.begin())); + AqlValueHintDocumentNoCopy(found.begin())); } else { res->emplaceValue(row, static_cast(registerId), - AqlValueHintCopy(found.begin())); + AqlValueHintCopy(found.begin())); } if (row != fromRow) { // re-use already copied AQLValues @@ -104,15 +105,15 @@ DocumentProducingBlock::DocumentProducingFunction DocumentProducingBlock::buildC if (slice.isNone()) { // attribute not found res->emplaceValue(row, static_cast(registerId), - VPackSlice::nullSlice()); + VPackSlice::nullSlice()); } else { uint8_t const* vpack = slice.begin(); if (_useRawDocumentPointers) { res->emplaceValue(row, static_cast(registerId), - AqlValueHintNoCopy(vpack)); + AqlValueHintDocumentNoCopy(vpack)); } else { res->emplaceValue(row, static_cast(registerId), - AqlValueHintCopy(vpack)); + AqlValueHintCopy(vpack)); } } if (row != fromRow) { @@ -128,10 +129,10 @@ DocumentProducingBlock::DocumentProducingFunction DocumentProducingBlock::buildC uint8_t const* vpack = slice.begin(); if (_useRawDocumentPointers) { res->emplaceValue(row, static_cast(registerId), - AqlValueHintNoCopy(vpack)); + AqlValueHintDocumentNoCopy(vpack)); } else { res->emplaceValue(row, static_cast(registerId), - AqlValueHintCopy(vpack)); + AqlValueHintCopy(vpack)); } if (row != fromRow) { // re-use already copied AQLValues diff --git a/arangod/Aql/Expression.cpp b/arangod/Aql/Expression.cpp index 437d3d3b685b..dfe5124a6ff3 100644 --- a/arangod/Aql/Expression.cpp +++ b/arangod/Aql/Expression.cpp @@ -56,7 +56,7 @@ using VelocyPackHelper = arangodb::basics::VelocyPackHelper; namespace { /// @brief register warning -static void RegisterWarning(arangodb::aql::Ast const* ast, +static void registerWarning(arangodb::aql::Ast const* ast, char const* functionName, int code) { std::string msg; @@ -86,12 +86,12 @@ Expression::Expression(ExecutionPlan* plan, Ast* ast, AstNode* node) : _plan(plan), _ast(ast), _node(node), + _func(nullptr), // this will reset all pointers in the union _type(UNPROCESSED), _canThrow(true), _canRunOnDBServer(false), _isDeterministic(false), _hasDeterminedAttributes(false), - _built(false), _attributes(), _expressionContext(nullptr) { TRI_ASSERT(_ast != nullptr); @@ -104,31 +104,7 @@ Expression::Expression(ExecutionPlan* plan, Ast* ast, arangodb::velocypack::Slic /// @brief destroy the expression Expression::~Expression() { - if (_built) { - switch (_type) { - case JSON: - TRI_ASSERT(_data != nullptr); - delete[] _data; - break; - - case ATTRIBUTE_SYSTEM: - case ATTRIBUTE_DYNAMIC: { - TRI_ASSERT(_accessor != nullptr); - delete _accessor; - break; - } - - case V8: - delete _func; - break; - - case SIMPLE: - case UNPROCESSED: { - // nothing to do - break; - } - } - } + freeInternals(); } /// @brief return all variables used in the expression @@ -139,12 +115,9 @@ void Expression::variables(std::unordered_set& result) const { /// @brief execute the expression AqlValue Expression::execute(transaction::Methods* trx, ExpressionContext* ctx, bool& mustDestroy) { - if (!_built) { - buildExpression(trx); - } + buildExpression(trx); TRI_ASSERT(_type != UNPROCESSED); - TRI_ASSERT(_built); _expressionContext = ctx; // and execute @@ -182,7 +155,6 @@ AqlValue Expression::execute(transaction::Methods* trx, ExpressionContext* ctx, THROW_ARANGO_EXCEPTION_MESSAGE(TRI_ERROR_INTERNAL, "invalid expression type"); - } /// @brief execute the expression @@ -206,9 +178,9 @@ void Expression::replaceVariables( if ((_type == ATTRIBUTE_SYSTEM || _type == ATTRIBUTE_DYNAMIC) && _accessor != nullptr) { _accessor->replaceVariable(replacements); + } else if (_type == V8) { + freeInternals(); } - - invalidate(); } /// @brief replace a variable reference in the expression with another @@ -221,27 +193,7 @@ void Expression::replaceVariableReference(Variable const* variable, _node = _ast->replaceVariableReference(const_cast(_node), variable, node); - invalidate(); - - if (_type == ATTRIBUTE_SYSTEM || _type == ATTRIBUTE_DYNAMIC) { - if (_built) { - delete _accessor; - _accessor = nullptr; - _built = false; - } - // must even set back the expression type so the expression will be analyzed - // again - _type = UNPROCESSED; - } else if (_type == SIMPLE) { - // must rebuild the expression completely, as it may have changed drastically - _built = false; - _type = UNPROCESSED; - _node->clearFlagsRecursive(); // recursively delete the node's flags - } - - const_cast(_node)->clearFlags(); - _attributes.clear(); - _hasDeterminedAttributes = false; + invalidateAfterReplacements(); } void Expression::replaceAttributeAccess(Variable const* variable, @@ -250,21 +202,44 @@ void Expression::replaceAttributeAccess(Variable const* variable, TRI_ASSERT(_node != nullptr); _node = _ast->replaceAttributeAccess(const_cast(_node), variable, attribute); - invalidate(); + invalidateAfterReplacements(); +} - if (_type == ATTRIBUTE_SYSTEM || _type == ATTRIBUTE_DYNAMIC) { - if (_built) { +/// @brief free the internal data structures +void Expression::freeInternals() noexcept { + switch (_type) { + case JSON: + delete[] _data; + _data = nullptr; + break; + + case ATTRIBUTE_SYSTEM: + case ATTRIBUTE_DYNAMIC: { delete _accessor; _accessor = nullptr; - _built = false; + break; } + + case V8: + delete _func; + _func = nullptr; + break; + + case SIMPLE: + case UNPROCESSED: { + // nothing to do + break; + } + } +} + +/// @brief reset internal attributes after variables in the expression were changed +void Expression::invalidateAfterReplacements() { + if (_type == ATTRIBUTE_SYSTEM || _type == ATTRIBUTE_DYNAMIC || _type == SIMPLE || _type == V8) { + freeInternals(); // must even set back the expression type so the expression will be analyzed // again _type = UNPROCESSED; - } else if (_type == SIMPLE) { - // must rebuild the expression completely, as it may have changed drastically - _built = false; - _type = UNPROCESSED; _node->clearFlagsRecursive(); // recursively delete the node's flags } @@ -280,11 +255,7 @@ void Expression::replaceAttributeAccess(Variable const* variable, void Expression::invalidate() { if (_type == V8) { // V8 expressions need a special handling - if (_built) { - delete _func; - _func = nullptr; - _built = false; - } + freeInternals(); } // we do not need to invalidate the other expression type // expression data will be freed in the destructor @@ -375,106 +346,126 @@ bool Expression::findInArray(AqlValue const& left, AqlValue const& right, return false; } +void Expression::initConstantExpression() { + _canThrow = false; + _canRunOnDBServer = true; + _isDeterministic = true; + _data = nullptr; + + _type = JSON; +} + +void Expression::initSimpleExpression() { + _canThrow = _node->canThrow(); + _canRunOnDBServer = _node->canRunOnDBServer(); + _isDeterministic = _node->isDeterministic(); + + _type = SIMPLE; + + if (_node->type != NODE_TYPE_ATTRIBUTE_ACCESS) { + return; + } + + // optimization for attribute accesses + TRI_ASSERT(_node->numMembers() == 1); + auto member = _node->getMemberUnchecked(0); + std::vector parts{_node->getString()}; + + while (member->type == NODE_TYPE_ATTRIBUTE_ACCESS) { + parts.insert(parts.begin(), member->getString()); + member = member->getMemberUnchecked(0); + } + + if (member->type != NODE_TYPE_REFERENCE) { + return; + } + auto v = static_cast(member->getData()); + + bool dataIsFromCollection = false; + if (_plan != nullptr) { + // check if the variable we are referring to is set by + // a collection enumeration/index enumeration + auto setter = _plan->getVarSetBy(v->id); + if (setter != nullptr && + (setter->getType() == ExecutionNode::INDEX || setter->getType() == ExecutionNode::ENUMERATE_COLLECTION)) { + // it is + dataIsFromCollection = true; + } + } + + // specialize the simple expression into an attribute accessor + _accessor = new AttributeAccessor(std::move(parts), v, dataIsFromCollection); + if (_accessor->isDynamic()) { + _type = ATTRIBUTE_DYNAMIC; + } else { + _type = ATTRIBUTE_SYSTEM; + } +} + +void Expression::initV8Expression() { + _canThrow = _node->canThrow(); + _canRunOnDBServer = _node->canRunOnDBServer(); + _isDeterministic = _node->isDeterministic(); + _func = nullptr; + + _type = V8; + + if (_hasDeterminedAttributes) { + return; + } + + // determine all top-level attributes used in expression only once + // as this might be expensive + bool isSafeForOptimization; + _attributes = + Ast::getReferencedAttributes(_node, isSafeForOptimization); + + if (!isSafeForOptimization) { + _attributes.clear(); + // unfortunately there are not only top-level attribute accesses but + // also other accesses, e.g. the index values or accesses of the whole + // value. + // for example, we cannot optimize LET x = a +1 or LET x = a[0], but LET + // x = a._key + } + + _hasDeterminedAttributes = true; +} + /// @brief analyze the expression (determine its type etc.) -void Expression::analyzeExpression() { +void Expression::initExpression() { TRI_ASSERT(_type == UNPROCESSED); - TRI_ASSERT(_built == false); if (_node->isConstant()) { // expression is a constant value - _type = JSON; - _canThrow = false; - _canRunOnDBServer = true; - _isDeterministic = true; - _data = nullptr; + initConstantExpression(); } else if (_node->isSimple()) { // expression is a simple expression - _type = SIMPLE; - _canThrow = _node->canThrow(); - _canRunOnDBServer = _node->canRunOnDBServer(); - _isDeterministic = _node->isDeterministic(); - - if (_node->type == NODE_TYPE_ATTRIBUTE_ACCESS) { - TRI_ASSERT(_node->numMembers() == 1); - auto member = _node->getMemberUnchecked(0); - std::vector parts{_node->getString()}; - - while (member->type == NODE_TYPE_ATTRIBUTE_ACCESS) { - parts.insert(parts.begin(), member->getString()); - member = member->getMemberUnchecked(0); - } - - if (member->type == NODE_TYPE_REFERENCE) { - auto v = static_cast(member->getData()); - - bool dataIsFromCollection = false; - if (_plan != nullptr) { - // check if the variable we are referring to is set by - // a collection enumeration/index enumeration - auto setter = _plan->getVarSetBy(v->id); - if (setter != nullptr && - (setter->getType() == ExecutionNode::INDEX || setter->getType() == ExecutionNode::ENUMERATE_COLLECTION)) { - // it is - dataIsFromCollection = true; - } - } - - // specialize the simple expression into an attribute accessor - _accessor = new AttributeAccessor(std::move(parts), v, dataIsFromCollection); - if (_accessor->isDynamic()) { - _type = ATTRIBUTE_DYNAMIC; - } else { - _type = ATTRIBUTE_SYSTEM; - } - _built = true; - } - } + initSimpleExpression(); } else { // expression is a V8 expression - _type = V8; - _canThrow = _node->canThrow(); - _canRunOnDBServer = _node->canRunOnDBServer(); - _isDeterministic = _node->isDeterministic(); - _func = nullptr; - - if (!_hasDeterminedAttributes) { - // determine all top-level attributes used in expression only once - // as this might be expensive - _hasDeterminedAttributes = true; - - bool isSafeForOptimization; - _attributes = - Ast::getReferencedAttributes(_node, isSafeForOptimization); - - if (!isSafeForOptimization) { - _attributes.clear(); - // unfortunately there are not only top-level attribute accesses but - // also other accesses, e.g. the index values or accesses of the whole - // value. - // for example, we cannot optimize LET x = a +1 or LET x = a[0], but LET - // x = a._key - } - } + initV8Expression(); } + + TRI_ASSERT(_type != UNPROCESSED); } /// @brief build the expression void Expression::buildExpression(transaction::Methods* trx) { - TRI_ASSERT(!_built); - if (_type == UNPROCESSED) { - analyzeExpression(); + initExpression(); } - if (_type == JSON) { - TRI_ASSERT(_data == nullptr); + TRI_ASSERT(_type != UNPROCESSED); + + if (_type == JSON && _data == nullptr) { // generate a constant value transaction::BuilderLeaser builder(trx); _node->toVelocyPackValue(*builder.get()); _data = new uint8_t[static_cast(builder->size())]; memcpy(_data, builder->data(), static_cast(builder->size())); - } else if (_type == V8) { + } else if (_type == V8 && _func == nullptr) { // generate a V8 expression _func = _ast->query()->v8Executor()->generateExpression(_node); @@ -484,8 +475,6 @@ void Expression::buildExpression(transaction::Methods* trx) { _func->setAttributeRestrictions(_attributes); } } - - _built = true; } /// @brief execute an expression of type SIMPLE, the convention is that @@ -673,7 +662,7 @@ AqlValue Expression::executeSimpleExpressionIndexedAccess( // fall-through to returning null } - return AqlValue(VelocyPackHelper::NullValue()); + return AqlValue(AqlValueHintNull()); } /// @brief execute an expression of type SIMPLE with ARRAY @@ -1090,7 +1079,7 @@ AqlValue Expression::executeSimpleExpressionNaryAndOr( if (count == 0) { // There is nothing to evaluate. So this is always true mustDestroy = true; - return AqlValue(true); + return AqlValue(AqlValueHintBool(true)); } // AND @@ -1108,7 +1097,7 @@ AqlValue Expression::executeSimpleExpressionNaryAndOr( } } mustDestroy = true; - return AqlValue(true); + return AqlValue(AqlValueHintBool(true)); } // OR @@ -1125,7 +1114,7 @@ AqlValue Expression::executeSimpleExpressionNaryAndOr( } } mustDestroy = true; - return AqlValue(false); + return AqlValue(AqlValueHintBool(false)); } /// @brief execute an expression of type SIMPLE with COMPARISON @@ -1148,7 +1137,7 @@ AqlValue Expression::executeSimpleExpressionComparison( if (!right.isArray()) { // right operand must be an array, otherwise we return false // do not throw, but return "false" instead - return AqlValue(false); + return AqlValue(AqlValueHintBool(false)); } bool result = findInArray(left, right, trx, node); @@ -1207,7 +1196,7 @@ AqlValue Expression::executeSimpleExpressionArrayComparison( if (!left.isArray()) { // left operand must be an array // do not throw, but return "false" instead - return AqlValue(false); + return AqlValue(AqlValueHintBool(false)); } if (node->type == NODE_TYPE_OPERATOR_BINARY_ARRAY_IN || @@ -1216,7 +1205,7 @@ AqlValue Expression::executeSimpleExpressionArrayComparison( if (!right.isArray()) { // right operand must be an array, otherwise we return false // do not throw, but return "false" instead - return AqlValue(false); + return AqlValue(AqlValueHintBool(false)); } } @@ -1226,10 +1215,10 @@ AqlValue Expression::executeSimpleExpressionArrayComparison( if (Quantifier::IsAllOrNone(node->getMember(2))) { // [] ALL ... // [] NONE ... - return AqlValue(true); + return AqlValue(AqlValueHintBool(true)); } else { // [] ANY ... - return AqlValue(false); + return AqlValue(AqlValueHintBool(false)); } } @@ -1569,14 +1558,14 @@ AqlValue Expression::executeSimpleExpressionArithmetic( if (r == 0.0) { if (node->type == NODE_TYPE_OPERATOR_BINARY_DIV) { // division by zero - RegisterWarning(_ast, "/", TRI_ERROR_QUERY_DIVISION_BY_ZERO); + registerWarning(_ast, "/", TRI_ERROR_QUERY_DIVISION_BY_ZERO); TRI_ASSERT(!mustDestroy); - return AqlValue(VelocyPackHelper::NullValue()); + return AqlValue(AqlValueHintNull()); } else if (node->type == NODE_TYPE_OPERATOR_BINARY_MOD) { // modulo zero - RegisterWarning(_ast, "%", TRI_ERROR_QUERY_DIVISION_BY_ZERO); + registerWarning(_ast, "%", TRI_ERROR_QUERY_DIVISION_BY_ZERO); TRI_ASSERT(!mustDestroy); - return AqlValue(VelocyPackHelper::NullValue()); + return AqlValue(AqlValueHintNull()); } } @@ -1600,7 +1589,7 @@ AqlValue Expression::executeSimpleExpressionArithmetic( result = fmod(l, r); break; default: - return AqlValue(VelocyPackHelper::ZeroValue()); + return AqlValue(AqlValueHintZero()); } // this will convert NaN, +inf & -inf to null diff --git a/arangod/Aql/Expression.h b/arangod/Aql/Expression.h index dc42ee38b5c7..21257a45e7c3 100644 --- a/arangod/Aql/Expression.h +++ b/arangod/Aql/Expression.h @@ -84,7 +84,7 @@ class Expression { /// @brief whether or not the expression can throw an exception inline bool canThrow() { if (_type == UNPROCESSED) { - analyzeExpression(); + initExpression(); } return _canThrow; } @@ -92,7 +92,7 @@ class Expression { /// @brief whether or not the expression can safely run on a DB server inline bool canRunOnDBServer() { if (_type == UNPROCESSED) { - analyzeExpression(); + initExpression(); } return _canRunOnDBServer; } @@ -100,7 +100,7 @@ class Expression { /// @brief whether or not the expression is deterministic inline bool isDeterministic() { if (_type == UNPROCESSED) { - analyzeExpression(); + initExpression(); } return _isDeterministic; } @@ -133,7 +133,7 @@ class Expression { /// @brief check whether this is a JSON expression inline bool isJson() { if (_type == UNPROCESSED) { - analyzeExpression(); + initExpression(); } return _type == JSON; } @@ -141,7 +141,7 @@ class Expression { /// @brief check whether this is a V8 expression inline bool isV8() { if (_type == UNPROCESSED) { - analyzeExpression(); + initExpression(); } return _type == V8; } @@ -149,7 +149,7 @@ class Expression { /// @brief get expression type as string std::string typeString() { if (_type == UNPROCESSED) { - analyzeExpression(); + initExpression(); } switch (_type) { @@ -213,14 +213,23 @@ class Expression { void clearVariable(Variable const* variable) { _variables.erase(variable); } private: + /// @brief free the internal data structures + void freeInternals() noexcept; + + /// @brief reset internal attributes after variables in the expression were changed + void invalidateAfterReplacements(); /// @brief find a value in an array bool findInArray(AqlValue const&, AqlValue const&, transaction::Methods*, AstNode const*) const; + void initConstantExpression(); + void initSimpleExpression(); + void initV8Expression(); + /// @brief analyze the expression (determine its type etc.) - void analyzeExpression(); + void initExpression(); /// @brief build the expression (if appropriate, compile it into /// executable code) @@ -343,9 +352,7 @@ class Expression { /// if the expression is a constant, it will be stored as plain JSON instead union { V8Expression* _func; - uint8_t* _data; - AttributeAccessor* _accessor; }; @@ -365,9 +372,6 @@ class Expression { /// determined bool _hasDeterminedAttributes; - /// @brief whether or not the expression has been built/compiled - bool _built; - /// @brief the top-level attributes used in the expression, grouped /// by variable name std::unordered_map> diff --git a/arangod/Aql/FixedVarExpressionContext.cpp b/arangod/Aql/FixedVarExpressionContext.cpp index 78ab952bd8c9..33a514f0698d 100644 --- a/arangod/Aql/FixedVarExpressionContext.cpp +++ b/arangod/Aql/FixedVarExpressionContext.cpp @@ -45,7 +45,7 @@ AqlValue FixedVarExpressionContext::getVariableValue(Variable const* variable, b auto it = _vars.find(variable); if (it == _vars.end()) { TRI_ASSERT(false); - return AqlValue(arangodb::basics::VelocyPackHelper::NullValue()); + return AqlValue(AqlValueHintNull()); } if (doCopy) { mustDestroy = true; diff --git a/arangod/Aql/Functions.cpp b/arangod/Aql/Functions.cpp index 45ed6ff697a9..dbf268ed9390 100644 --- a/arangod/Aql/Functions.cpp +++ b/arangod/Aql/Functions.cpp @@ -23,11 +23,6 @@ #include "Functions.h" -#include -#include -#include -#include - #include "ApplicationFeatures/ApplicationServer.h" #include "Aql/Function.h" #include "Aql/Query.h" @@ -50,14 +45,19 @@ #include "VocBase/LogicalCollection.h" #include "VocBase/ManagedDocumentResult.h" +#include +#include +#include +#include + +#include "unicode/unistr.h" + using namespace arangodb; using namespace arangodb::aql; /// @brief convert a number value into an AqlValue -static AqlValue NumberValue(transaction::Methods* trx, int value) { - transaction::BuilderLeaser builder(trx); - builder->add(VPackValue(value)); - return AqlValue(builder.get()); +static inline AqlValue NumberValue(transaction::Methods* trx, int value) { + return AqlValue(AqlValueHintInt(value)); } /// @brief convert a number value into an AqlValue @@ -65,15 +65,13 @@ static AqlValue NumberValue(transaction::Methods* trx, double value, bool nullif if (std::isnan(value) || !std::isfinite(value) || value == HUGE_VAL || value == -HUGE_VAL) { if (nullify) { // convert to null - return AqlValue(arangodb::basics::VelocyPackHelper::NullValue()); + return AqlValue(AqlValueHintNull()); } // convert to 0 - return AqlValue(arangodb::basics::VelocyPackHelper::ZeroValue()); + return AqlValue(AqlValueHintZero()); } - transaction::BuilderLeaser builder(trx); - builder->add(VPackValue(value)); - return AqlValue(builder.get()); + return AqlValue(AqlValueHintDouble(value)); } /// @brief validate the number of parameters @@ -531,7 +529,7 @@ AqlValue Functions::MergeParameters(arangodb::aql::Query* query, for (auto const& it : VPackArrayIterator(initialSlice)) { if (!it.isObject()) { RegisterInvalidArgumentWarning(query, funcName); - return AqlValue(arangodb::basics::VelocyPackHelper::NullValue()); + return AqlValue(AqlValueHintNull()); } builder = arangodb::basics::VelocyPackHelper::merge(builder.slice(), it, false, recursive); @@ -541,7 +539,7 @@ AqlValue Functions::MergeParameters(arangodb::aql::Query* query, if (!initial.isObject()) { RegisterInvalidArgumentWarning(query, funcName); - return AqlValue(arangodb::basics::VelocyPackHelper::NullValue()); + return AqlValue(AqlValueHintNull()); } // merge in all other arguments @@ -550,7 +548,7 @@ AqlValue Functions::MergeParameters(arangodb::aql::Query* query, if (!param.isObject()) { RegisterInvalidArgumentWarning(query, funcName); - return AqlValue(arangodb::basics::VelocyPackHelper::NullValue()); + return AqlValue(AqlValueHintNull()); } AqlValueMaterializer materializer(trx); @@ -586,7 +584,7 @@ AqlValue Functions::IsNull(arangodb::aql::Query* query, transaction::Methods* trx, VPackFunctionParameters const& parameters) { AqlValue a = ExtractFunctionParameterValue(trx, parameters, 0); - return AqlValue(a.isNull(true)); + return AqlValue(AqlValueHintBool(a.isNull(true))); } /// @brief function IS_BOOL @@ -594,7 +592,7 @@ AqlValue Functions::IsBool(arangodb::aql::Query* query, transaction::Methods* trx, VPackFunctionParameters const& parameters) { AqlValue a = ExtractFunctionParameterValue(trx, parameters, 0); - return AqlValue(a.isBoolean()); + return AqlValue(AqlValueHintBool(a.isBoolean())); } /// @brief function IS_NUMBER @@ -602,7 +600,7 @@ AqlValue Functions::IsNumber(arangodb::aql::Query* query, transaction::Methods* trx, VPackFunctionParameters const& parameters) { AqlValue a = ExtractFunctionParameterValue(trx, parameters, 0); - return AqlValue(a.isNumber()); + return AqlValue(AqlValueHintBool(a.isNumber())); } /// @brief function IS_STRING @@ -610,7 +608,7 @@ AqlValue Functions::IsString(arangodb::aql::Query* query, transaction::Methods* trx, VPackFunctionParameters const& parameters) { AqlValue a = ExtractFunctionParameterValue(trx, parameters, 0); - return AqlValue(a.isString()); + return AqlValue(AqlValueHintBool(a.isString())); } /// @brief function IS_ARRAY @@ -618,7 +616,7 @@ AqlValue Functions::IsArray(arangodb::aql::Query* query, transaction::Methods* trx, VPackFunctionParameters const& parameters) { AqlValue a = ExtractFunctionParameterValue(trx, parameters, 0); - return AqlValue(a.isArray()); + return AqlValue(AqlValueHintBool(a.isArray())); } /// @brief function IS_OBJECT @@ -626,7 +624,7 @@ AqlValue Functions::IsObject(arangodb::aql::Query* query, transaction::Methods* trx, VPackFunctionParameters const& parameters) { AqlValue a = ExtractFunctionParameterValue(trx, parameters, 0); - return AqlValue(a.isObject()); + return AqlValue(AqlValueHintBool(a.isObject())); } /// @brief function TYPENAME @@ -634,23 +632,9 @@ AqlValue Functions::Typename(arangodb::aql::Query* query, transaction::Methods* trx, VPackFunctionParameters const& parameters) { AqlValue value = ExtractFunctionParameterValue(trx, parameters, 0); + char const* type = value.getTypeString(); - if (value.isObject()) { - return AqlValue(TRI_CHAR_LENGTH_PAIR("object")); - } - if (value.isArray()) { - return AqlValue(TRI_CHAR_LENGTH_PAIR("array")); - } - if (value.isString()) { - return AqlValue(TRI_CHAR_LENGTH_PAIR("string")); - } - if (value.isNumber()) { - return AqlValue(TRI_CHAR_LENGTH_PAIR("number")); - } - if (value.isBoolean()) { - return AqlValue(TRI_CHAR_LENGTH_PAIR("bool")); - } - return AqlValue(TRI_CHAR_LENGTH_PAIR("null")); + return AqlValue(TRI_CHAR_LENGTH_PAIR(type)); } /// @brief function TO_NUMBER @@ -662,12 +646,10 @@ AqlValue Functions::ToNumber(arangodb::aql::Query* query, double value = a.toDouble(trx, failed); if (failed) { - return AqlValue(arangodb::basics::VelocyPackHelper::ZeroValue()); + return AqlValue(AqlValueHintZero()); } - - transaction::BuilderLeaser builder(trx); - builder->add(VPackValue(value)); - return AqlValue(builder.get()); + + return AqlValue(AqlValueHintDouble(value)); } /// @brief function TO_STRING @@ -735,13 +717,10 @@ AqlValue Functions::Length(arangodb::aql::Query* query, ValidateParameters(parameters, "LENGTH", 1, 1); - transaction::BuilderLeaser builder(trx); - AqlValue value = ExtractFunctionParameterValue(trx, parameters, 0); if (value.isArray()) { // shortcut! - builder->add(VPackValue(value.length())); - return AqlValue(builder->slice()); + return AqlValue(AqlValueHintUInt(value.length())); } size_t length = 0; @@ -768,8 +747,8 @@ AqlValue Functions::Length(arangodb::aql::Query* query, } else if (value.isObject()) { length = static_cast(value.length()); } - builder->add(VPackValue(static_cast(length))); - return AqlValue(builder.get()); + + return AqlValue(AqlValueHintUInt(length)); } /// @brief function FIRST @@ -782,11 +761,11 @@ AqlValue Functions::First(arangodb::aql::Query* query, if (!value.isArray()) { // not an array RegisterWarning(query, "FIRST", TRI_ERROR_QUERY_ARRAY_EXPECTED); - return AqlValue(arangodb::basics::VelocyPackHelper::NullValue()); + return AqlValue(AqlValueHintNull()); } if (value.length() == 0) { - return AqlValue(arangodb::basics::VelocyPackHelper::NullValue()); + return AqlValue(AqlValueHintNull()); } bool mustDestroy; @@ -803,13 +782,13 @@ AqlValue Functions::Last(arangodb::aql::Query* query, if (!value.isArray()) { // not an array RegisterWarning(query, "LAST", TRI_ERROR_QUERY_ARRAY_EXPECTED); - return AqlValue(arangodb::basics::VelocyPackHelper::NullValue()); + return AqlValue(AqlValueHintNull()); } VPackValueLength const n = value.length(); if (n == 0) { - return AqlValue(arangodb::basics::VelocyPackHelper::NullValue()); + return AqlValue(AqlValueHintNull()); } bool mustDestroy; @@ -826,20 +805,20 @@ AqlValue Functions::Nth(arangodb::aql::Query* query, if (!value.isArray()) { // not an array RegisterWarning(query, "NTH", TRI_ERROR_QUERY_ARRAY_EXPECTED); - return AqlValue(arangodb::basics::VelocyPackHelper::NullValue()); + return AqlValue(AqlValueHintNull()); } VPackValueLength const n = value.length(); if (n == 0) { - return AqlValue(arangodb::basics::VelocyPackHelper::NullValue()); + return AqlValue(AqlValueHintNull()); } AqlValue position = ExtractFunctionParameterValue(trx, parameters, 1); int64_t index = position.toInt64(trx); if (index < 0 || index >= static_cast(n)) { - return AqlValue(arangodb::basics::VelocyPackHelper::NullValue()); + return AqlValue(AqlValueHintNull()); } bool mustDestroy; @@ -907,7 +886,7 @@ AqlValue Functions::Contains(arangodb::aql::Query* query, } // return boolean - return AqlValue(result != -1); + return AqlValue(AqlValueHintBool(result != -1)); } /// @brief function CONCAT @@ -1017,8 +996,6 @@ AqlValue Functions::CharLength(arangodb::aql::Query* query, VPackFunctionParameters const& parameters) { ValidateParameters(parameters, "CHAR_LENGTH", 1, 1); - transaction::BuilderLeaser builder(trx); - AqlValue value = ExtractFunctionParameterValue(trx, parameters, 0); size_t length = 0; @@ -1059,18 +1036,9 @@ AqlValue Functions::CharLength(arangodb::aql::Query* query, length = TRI_CharLengthUtf8String(p, l); } - builder->add(VPackValue(static_cast(length))); - return AqlValue(builder.get()); + return AqlValue(AqlValueHintUInt(length)); } -// #include "unicode/utypes.h" -// #include "unicode/uchar.h" -// #include "unicode/locid.h" -// #include "unicode/ustring.h" -// #include "unicode/ucnv.h" -#include "unicode/unistr.h" - - /// @brief function LOWER AqlValue Functions::Lower(arangodb::aql::Query* query, transaction::Methods* trx, @@ -1089,7 +1057,7 @@ AqlValue Functions::Lower(arangodb::aql::Query* query, s.toLower(NULL); s.toUTF8String(utf8); - return AqlValue(utf8.c_str(), utf8.length()); + return AqlValue(utf8); } /// @brief function UPPER @@ -1110,7 +1078,7 @@ AqlValue Functions::Upper(arangodb::aql::Query* query, s.toUpper(NULL); s.toUTF8String(utf8); - return AqlValue(utf8.c_str(), utf8.length()); + return AqlValue(utf8); } /// @brief function LIKE @@ -1132,7 +1100,7 @@ AqlValue Functions::Like(arangodb::aql::Query* query, if (matcher == nullptr) { // compiling regular expression failed RegisterWarning(query, "LIKE", TRI_ERROR_QUERY_INVALID_REGEX); - return AqlValue(arangodb::basics::VelocyPackHelper::NullValue()); + return AqlValue(AqlValueHintNull()); } // extract value @@ -1147,10 +1115,10 @@ AqlValue Functions::Like(arangodb::aql::Query* query, if (error) { // compiling regular expression failed RegisterWarning(query, "LIKE", TRI_ERROR_QUERY_INVALID_REGEX); - return AqlValue(arangodb::basics::VelocyPackHelper::NullValue()); + return AqlValue(AqlValueHintNull()); } - return AqlValue(result); + return AqlValue(AqlValueHintBool(result)); } /// @brief function REGEX_TEST @@ -1172,7 +1140,7 @@ AqlValue Functions::RegexTest(arangodb::aql::Query* query, if (matcher == nullptr) { // compiling regular expression failed RegisterWarning(query, "REGEX_TEST", TRI_ERROR_QUERY_INVALID_REGEX); - return AqlValue(arangodb::basics::VelocyPackHelper::NullValue()); + return AqlValue(AqlValueHintNull()); } // extract value @@ -1187,10 +1155,10 @@ AqlValue Functions::RegexTest(arangodb::aql::Query* query, if (error) { // compiling regular expression failed RegisterWarning(query, "REGEX_TEST", TRI_ERROR_QUERY_INVALID_REGEX); - return AqlValue(arangodb::basics::VelocyPackHelper::NullValue()); + return AqlValue(AqlValueHintNull()); } - return AqlValue(result); + return AqlValue(AqlValueHintBool(result)); } /// @brief function REGEX_REPLACE @@ -1212,7 +1180,7 @@ AqlValue Functions::RegexReplace(arangodb::aql::Query* query, if (matcher == nullptr) { // compiling regular expression failed RegisterWarning(query, "REGEX_REPLACE", TRI_ERROR_QUERY_INVALID_REGEX); - return AqlValue(arangodb::basics::VelocyPackHelper::NullValue()); + return AqlValue(AqlValueHintNull()); } // extract value @@ -1231,7 +1199,7 @@ AqlValue Functions::RegexReplace(arangodb::aql::Query* query, if (error) { // compiling regular expression failed RegisterWarning(query, "REGEX_REPLACE", TRI_ERROR_QUERY_INVALID_REGEX); - return AqlValue(arangodb::basics::VelocyPackHelper::NullValue()); + return AqlValue(AqlValueHintNull()); } return AqlValue(result); @@ -1242,7 +1210,7 @@ AqlValue Functions::Passthru(arangodb::aql::Query* query, transaction::Methods* trx, VPackFunctionParameters const& parameters) { if (parameters.empty()) { - return AqlValue(arangodb::basics::VelocyPackHelper::NullValue()); + return AqlValue(AqlValueHintNull()); } return ExtractFunctionParameterValue(trx, parameters, 0).clone(); @@ -1257,7 +1225,7 @@ AqlValue Functions::Unset(arangodb::aql::Query* query, if (!value.isObject()) { RegisterInvalidArgumentWarning(query, "UNSET"); - return AqlValue(arangodb::basics::VelocyPackHelper::NullValue()); + return AqlValue(AqlValueHintNull()); } std::unordered_set names; @@ -1279,7 +1247,7 @@ AqlValue Functions::UnsetRecursive(arangodb::aql::Query* query, if (!value.isObject()) { RegisterInvalidArgumentWarning(query, "UNSET_RECURSIVE"); - return AqlValue(arangodb::basics::VelocyPackHelper::NullValue()); + return AqlValue(AqlValueHintNull()); } std::unordered_set names; @@ -1301,7 +1269,7 @@ AqlValue Functions::Keep(arangodb::aql::Query* query, if (!value.isObject()) { RegisterInvalidArgumentWarning(query, "KEEP"); - return AqlValue(arangodb::basics::VelocyPackHelper::NullValue()); + return AqlValue(AqlValueHintNull()); } std::unordered_set names; @@ -1336,14 +1304,14 @@ AqlValue Functions::Has(arangodb::aql::Query* query, size_t const n = parameters.size(); if (n < 2) { // no parameters - return AqlValue(false); + return AqlValue(AqlValueHintBool(false)); } AqlValue value = ExtractFunctionParameterValue(trx, parameters, 0); if (!value.isObject()) { // not an object - return AqlValue(false); + return AqlValue(AqlValueHintBool(false)); } AqlValue name = ExtractFunctionParameterValue(trx, parameters, 1); @@ -1357,7 +1325,7 @@ AqlValue Functions::Has(arangodb::aql::Query* query, p = name.slice().copyString(); } - return AqlValue(value.hasKey(trx, p)); + return AqlValue(AqlValueHintBool(value.hasKey(trx, p))); } /// @brief function ATTRIBUTES @@ -1368,7 +1336,7 @@ AqlValue Functions::Attributes(arangodb::aql::Query* query, if (n < 1) { // no parameters - return AqlValue(arangodb::basics::VelocyPackHelper::NullValue()); + return AqlValue(AqlValueHintNull()); } AqlValue value = ExtractFunctionParameterValue(trx, parameters, 0); @@ -1376,7 +1344,7 @@ AqlValue Functions::Attributes(arangodb::aql::Query* query, // not an object RegisterWarning(query, "ATTRIBUTES", TRI_ERROR_QUERY_FUNCTION_ARGUMENT_TYPE_MISMATCH); - return AqlValue(arangodb::basics::VelocyPackHelper::NullValue()); + return AqlValue(AqlValueHintNull()); } bool const removeInternal = GetBooleanParameter(trx, parameters, 1, false); @@ -1432,7 +1400,7 @@ AqlValue Functions::Values(arangodb::aql::Query* query, if (n < 1) { // no parameters - return AqlValue(arangodb::basics::VelocyPackHelper::NullValue()); + return AqlValue(AqlValueHintNull()); } AqlValue value = ExtractFunctionParameterValue(trx, parameters, 0); @@ -1440,7 +1408,7 @@ AqlValue Functions::Values(arangodb::aql::Query* query, // not an object RegisterWarning(query, "VALUES", TRI_ERROR_QUERY_FUNCTION_ARGUMENT_TYPE_MISMATCH); - return AqlValue(arangodb::basics::VelocyPackHelper::NullValue()); + return AqlValue(AqlValueHintNull()); } bool const removeInternal = GetBooleanParameter(trx, parameters, 1, false); @@ -1487,7 +1455,7 @@ AqlValue Functions::Min(arangodb::aql::Query* query, if (!value.isArray()) { // not an array RegisterWarning(query, "MIN", TRI_ERROR_QUERY_ARRAY_EXPECTED); - return AqlValue(arangodb::basics::VelocyPackHelper::NullValue()); + return AqlValue(AqlValueHintNull()); } AqlValueMaterializer materializer(trx); @@ -1504,7 +1472,7 @@ AqlValue Functions::Min(arangodb::aql::Query* query, } } if (minValue.isNone()) { - return AqlValue(arangodb::basics::VelocyPackHelper::NullValue()); + return AqlValue(AqlValueHintNull()); } return AqlValue(minValue); } @@ -1518,7 +1486,7 @@ AqlValue Functions::Max(arangodb::aql::Query* query, if (!value.isArray()) { // not an array RegisterWarning(query, "MAX", TRI_ERROR_QUERY_ARRAY_EXPECTED); - return AqlValue(arangodb::basics::VelocyPackHelper::NullValue()); + return AqlValue(AqlValueHintNull()); } AqlValueMaterializer materializer(trx); @@ -1531,7 +1499,7 @@ AqlValue Functions::Max(arangodb::aql::Query* query, } } if (maxValue.isNone()) { - return AqlValue(arangodb::basics::VelocyPackHelper::NullValue()); + return AqlValue(AqlValueHintNull()); } return AqlValue(maxValue); } @@ -1545,7 +1513,7 @@ AqlValue Functions::Sum(arangodb::aql::Query* query, if (!value.isArray()) { // not an array RegisterWarning(query, "SUM", TRI_ERROR_QUERY_ARRAY_EXPECTED); - return AqlValue(arangodb::basics::VelocyPackHelper::NullValue()); + return AqlValue(AqlValueHintNull()); } AqlValueMaterializer materializer(trx); @@ -1556,7 +1524,7 @@ AqlValue Functions::Sum(arangodb::aql::Query* query, continue; } if (!it.isNumber()) { - return AqlValue(arangodb::basics::VelocyPackHelper::NullValue()); + return AqlValue(AqlValueHintNull()); } double const number = it.getNumericValue(); @@ -1577,7 +1545,7 @@ AqlValue Functions::Average(arangodb::aql::Query* query, if (!value.isArray()) { // not an array RegisterWarning(query, "AVERAGE", TRI_ERROR_QUERY_ARRAY_EXPECTED); - return AqlValue(arangodb::basics::VelocyPackHelper::NullValue()); + return AqlValue(AqlValueHintNull()); } AqlValueMaterializer materializer(trx); @@ -1591,7 +1559,7 @@ AqlValue Functions::Average(arangodb::aql::Query* query, } if (!v.isNumber()) { RegisterWarning(query, "AVERAGE", TRI_ERROR_QUERY_ARRAY_EXPECTED); - return AqlValue(arangodb::basics::VelocyPackHelper::NullValue()); + return AqlValue(AqlValueHintNull()); } // got a numeric value @@ -1607,7 +1575,7 @@ AqlValue Functions::Average(arangodb::aql::Query* query, return NumberValue(trx, sum / static_cast(count), false); } - return AqlValue(arangodb::basics::VelocyPackHelper::NullValue()); + return AqlValue(AqlValueHintNull()); } /// @brief function SLEEP @@ -1619,13 +1587,13 @@ AqlValue Functions::Sleep(arangodb::aql::Query* query, if (!value.isNumber() || value.toDouble(trx) < 0) { RegisterWarning(query, "SLEEP", TRI_ERROR_QUERY_FUNCTION_ARGUMENT_TYPE_MISMATCH); - return AqlValue(arangodb::basics::VelocyPackHelper::NullValue()); + return AqlValue(AqlValueHintNull()); } double const until = TRI_microtime() + value.toDouble(trx); while (TRI_microtime() < until) { - usleep(25000); + usleep(30000); if (query->killed()) { THROW_ARANGO_EXCEPTION(TRI_ERROR_QUERY_KILLED); @@ -1633,7 +1601,7 @@ AqlValue Functions::Sleep(arangodb::aql::Query* query, THROW_ARANGO_EXCEPTION(TRI_ERROR_SHUTTING_DOWN); } } - return AqlValue(arangodb::basics::VelocyPackHelper::NullValue()); + return AqlValue(AqlValueHintNull()); } /// @brief function RANDOM_TOKEN @@ -1716,7 +1684,7 @@ AqlValue Functions::Hash(arangodb::aql::Query* query, // without precision loss when storing in JavaScript etc. uint64_t hash = value.hash(trx) & 0x0007ffffffffffffULL; - return AqlValue(hash); + return AqlValue(AqlValueHintUInt(hash)); } /// @brief function UNIQUE @@ -1730,7 +1698,7 @@ AqlValue Functions::Unique(arangodb::aql::Query* query, if (!value.isArray()) { // not an array RegisterWarning(query, "UNIQUE", TRI_ERROR_QUERY_ARRAY_EXPECTED); - return AqlValue(arangodb::basics::VelocyPackHelper::NullValue()); + return AqlValue(AqlValueHintNull()); } AqlValueMaterializer materializer(trx); @@ -1767,7 +1735,7 @@ AqlValue Functions::SortedUnique(arangodb::aql::Query* query, if (!value.isArray()) { // not an array // this is an internal function - do NOT issue a warning here - return AqlValue(arangodb::basics::VelocyPackHelper::NullValue()); + return AqlValue(AqlValueHintNull()); } AqlValueMaterializer materializer(trx); @@ -1805,7 +1773,7 @@ AqlValue Functions::Union(arangodb::aql::Query* query, if (!value.isArray()) { // not an array RegisterInvalidArgumentWarning(query, "UNION"); - return AqlValue(arangodb::basics::VelocyPackHelper::NullValue()); + return AqlValue(AqlValueHintNull()); } TRI_IF_FAILURE("AqlFunctions::OutOfMemory1") { @@ -1852,7 +1820,7 @@ AqlValue Functions::UnionDistinct(arangodb::aql::Query* query, if (!value.isArray()) { // not an array RegisterInvalidArgumentWarning(query, "UNION_DISTINCT"); - return AqlValue(arangodb::basics::VelocyPackHelper::NullValue()); + return AqlValue(AqlValueHintNull()); } materializers.emplace_back(trx); @@ -1909,7 +1877,7 @@ AqlValue Functions::Intersection(arangodb::aql::Query* query, if (!value.isArray()) { // not an array RegisterWarning(query, "INTERSECTION", TRI_ERROR_QUERY_ARRAY_EXPECTED); - return AqlValue(arangodb::basics::VelocyPackHelper::NullValue()); + return AqlValue(AqlValueHintNull()); } materializers.emplace_back(trx); @@ -1980,7 +1948,7 @@ AqlValue Functions::Outersection(arangodb::aql::Query* query, if (!value.isArray()) { // not an array RegisterWarning(query, "OUTERSECTION", TRI_ERROR_QUERY_ARRAY_EXPECTED); - return AqlValue(arangodb::basics::VelocyPackHelper::NullValue()); + return AqlValue(AqlValueHintNull()); } materializers.emplace_back(trx); @@ -2033,7 +2001,7 @@ AqlValue Functions::Distance(arangodb::aql::Query* query, if (!lat1.isNumber() || !lon1.isNumber() || !lat2.isNumber() || !lon2.isNumber()) { RegisterWarning(query, "DISTANCE", TRI_ERROR_QUERY_FUNCTION_ARGUMENT_TYPE_MISMATCH); - return AqlValue(arangodb::basics::VelocyPackHelper::NullValue()); + return AqlValue(AqlValueHintNull()); } bool failed; @@ -2050,7 +2018,7 @@ AqlValue Functions::Distance(arangodb::aql::Query* query, if (error) { RegisterWarning(query, "DISTANCE", TRI_ERROR_QUERY_FUNCTION_ARGUMENT_TYPE_MISMATCH); - return AqlValue(arangodb::basics::VelocyPackHelper::NullValue()); + return AqlValue(AqlValueHintNull()); } auto toRadians = [](double degrees) -> double { @@ -2081,7 +2049,7 @@ AqlValue Functions::Flatten(arangodb::aql::Query* query, AqlValue list = ExtractFunctionParameterValue(trx, parameters, 0); if (!list.isArray()) { RegisterWarning(query, "FLATTEN", TRI_ERROR_QUERY_ARRAY_EXPECTED); - return AqlValue(arangodb::basics::VelocyPackHelper::NullValue()); + return AqlValue(AqlValueHintNull()); } size_t maxDepth = 1; @@ -2119,7 +2087,7 @@ AqlValue Functions::Zip(arangodb::aql::Query* query, keys.length() != values.length()) { RegisterWarning(query, "ZIP", TRI_ERROR_QUERY_FUNCTION_ARGUMENT_TYPE_MISMATCH); - return AqlValue(arangodb::basics::VelocyPackHelper::NullValue()); + return AqlValue(AqlValueHintNull()); } VPackValueLength n = keys.length(); @@ -2177,7 +2145,7 @@ AqlValue Functions::JsonParse(arangodb::aql::Query* query, if (!slice.isString()) { RegisterWarning(query, "JSON_PARSE", TRI_ERROR_QUERY_FUNCTION_ARGUMENT_TYPE_MISMATCH); - return AqlValue(arangodb::basics::VelocyPackHelper::NullValue()); + return AqlValue(AqlValueHintNull()); } VPackValueLength l; @@ -2189,7 +2157,7 @@ AqlValue Functions::JsonParse(arangodb::aql::Query* query, } catch (...) { RegisterWarning(query, "JSON_PARSE", TRI_ERROR_QUERY_FUNCTION_ARGUMENT_TYPE_MISMATCH); - return AqlValue(arangodb::basics::VelocyPackHelper::NullValue()); + return AqlValue(AqlValueHintNull()); } } @@ -2216,7 +2184,7 @@ AqlValue Functions::ParseIdentifier( if (identifier.empty()) { RegisterWarning(query, "PARSE_IDENTIFIER", TRI_ERROR_QUERY_FUNCTION_ARGUMENT_TYPE_MISMATCH); - return AqlValue(arangodb::basics::VelocyPackHelper::NullValue()); + return AqlValue(AqlValueHintNull()); } std::vector parts = @@ -2225,7 +2193,7 @@ AqlValue Functions::ParseIdentifier( if (parts.size() != 2) { RegisterWarning(query, "PARSE_IDENTIFIER", TRI_ERROR_QUERY_FUNCTION_ARGUMENT_TYPE_MISMATCH); - return AqlValue(arangodb::basics::VelocyPackHelper::NullValue()); + return AqlValue(AqlValueHintNull()); } transaction::BuilderLeaser builder(trx); @@ -2247,7 +2215,7 @@ AqlValue Functions::Slice(arangodb::aql::Query* query, if (!baseArray.isArray()) { RegisterWarning(query, "SLICE", TRI_ERROR_QUERY_FUNCTION_ARGUMENT_TYPE_MISMATCH); - return AqlValue(arangodb::basics::VelocyPackHelper::NullValue()); + return AqlValue(AqlValueHintNull()); } // determine lower bound @@ -2313,7 +2281,7 @@ AqlValue Functions::Minus(arangodb::aql::Query* query, if (!baseArray.isArray()) { RegisterWarning(query, "MINUS", TRI_ERROR_QUERY_FUNCTION_ARGUMENT_TYPE_MISMATCH); - return AqlValue(arangodb::basics::VelocyPackHelper::NullValue()); + return AqlValue(AqlValueHintNull()); } auto options = trx->transactionContextPtr()->getVPackOptions(); @@ -2340,7 +2308,7 @@ AqlValue Functions::Minus(arangodb::aql::Query* query, if (!next.isArray()) { RegisterWarning(query, "MINUS", TRI_ERROR_QUERY_FUNCTION_ARGUMENT_TYPE_MISMATCH); - return AqlValue(arangodb::basics::VelocyPackHelper::NullValue()); + return AqlValue(AqlValueHintNull()); } AqlValueMaterializer materializer(trx); @@ -2380,7 +2348,7 @@ AqlValue Functions::Document(arangodb::aql::Query* query, GetDocumentByIdentifier(trx, colName, identifier, true, *builder.get()); if (builder->isEmpty()) { // not found - return AqlValue(arangodb::basics::VelocyPackHelper::NullValue()); + return AqlValue(AqlValueHintNull()); } return AqlValue(builder.get()); } @@ -2398,14 +2366,14 @@ AqlValue Functions::Document(arangodb::aql::Query* query, builder->close(); return AqlValue(builder.get()); } - return AqlValue(arangodb::basics::VelocyPackHelper::NullValue()); + return AqlValue(AqlValueHintNull()); } AqlValue collectionValue = ExtractFunctionParameterValue(trx, parameters, 0); if (!collectionValue.isString()) { RegisterWarning(query, "DOCUMENT", TRI_ERROR_QUERY_FUNCTION_ARGUMENT_TYPE_MISMATCH); - return AqlValue(arangodb::basics::VelocyPackHelper::NullValue()); + return AqlValue(AqlValueHintNull()); } std::string collectionName(collectionValue.slice().copyString()); @@ -2415,7 +2383,7 @@ AqlValue Functions::Document(arangodb::aql::Query* query, std::string identifier(id.slice().copyString()); GetDocumentByIdentifier(trx, collectionName, identifier, true, *builder.get()); if (builder->isEmpty()) { - return AqlValue(arangodb::basics::VelocyPackHelper::NullValue()); + return AqlValue(AqlValueHintNull()); } return AqlValue(builder.get()); } @@ -2438,7 +2406,7 @@ AqlValue Functions::Document(arangodb::aql::Query* query, } // Id has invalid format - return AqlValue(arangodb::basics::VelocyPackHelper::NullValue()); + return AqlValue(AqlValueHintNull()); } /// @brief function ROUND @@ -2722,7 +2690,7 @@ AqlValue Functions::FirstDocument(arangodb::aql::Query* query, } } - return AqlValue(arangodb::basics::VelocyPackHelper::NullValue()); + return AqlValue(AqlValueHintNull()); } /// @brief function FIRST_LIST @@ -2737,7 +2705,7 @@ AqlValue Functions::FirstList(arangodb::aql::Query* query, } } - return AqlValue(arangodb::basics::VelocyPackHelper::NullValue()); + return AqlValue(AqlValueHintNull()); } /// @brief function PUSH @@ -2762,7 +2730,7 @@ AqlValue Functions::Push(arangodb::aql::Query* query, if (!list.isArray()) { RegisterInvalidArgumentWarning(query, "PUSH"); - return AqlValue(arangodb::basics::VelocyPackHelper::NullValue()); + return AqlValue(AqlValueHintNull()); } transaction::BuilderLeaser builder(trx); @@ -2794,13 +2762,13 @@ AqlValue Functions::Pop(arangodb::aql::Query* query, AqlValue list = ExtractFunctionParameterValue(trx, parameters, 0); if (list.isNull(true)) { - return AqlValue(arangodb::basics::VelocyPackHelper::NullValue()); + return AqlValue(AqlValueHintNull()); } if (!list.isArray()) { RegisterWarning(query, "POP", TRI_ERROR_QUERY_FUNCTION_ARGUMENT_TYPE_MISMATCH); - return AqlValue(arangodb::basics::VelocyPackHelper::NullValue()); + return AqlValue(AqlValueHintNull()); } AqlValueMaterializer materializer(trx); @@ -2851,7 +2819,7 @@ AqlValue Functions::Append(arangodb::aql::Query* query, if (!l.isArray()) { RegisterInvalidArgumentWarning(query, "APPEND"); - return AqlValue(arangodb::basics::VelocyPackHelper::NullValue()); + return AqlValue(AqlValueHintNull()); } std::unordered_set added; @@ -2902,7 +2870,7 @@ AqlValue Functions::Unshift(arangodb::aql::Query* query, if (!list.isNull(true) && !list.isArray()) { RegisterInvalidArgumentWarning(query, "UNSHIFT"); - return AqlValue(arangodb::basics::VelocyPackHelper::NullValue()); + return AqlValue(AqlValueHintNull()); } AqlValue toAppend = ExtractFunctionParameterValue(trx, parameters, 1); @@ -2946,12 +2914,12 @@ AqlValue Functions::Shift(arangodb::aql::Query* query, AqlValue list = ExtractFunctionParameterValue(trx, parameters, 0); if (list.isNull(true)) { - return AqlValue(arangodb::basics::VelocyPackHelper::NullValue()); + return AqlValue(AqlValueHintNull()); } if (!list.isArray()) { RegisterInvalidArgumentWarning(query, "SHIFT"); - return AqlValue(arangodb::basics::VelocyPackHelper::NullValue()); + return AqlValue(AqlValueHintNull()); } transaction::BuilderLeaser builder(trx); @@ -2988,7 +2956,7 @@ AqlValue Functions::RemoveValue(arangodb::aql::Query* query, if (!list.isArray()) { RegisterInvalidArgumentWarning(query, "REMOVE_VALUE"); - return AqlValue(arangodb::basics::VelocyPackHelper::NullValue()); + return AqlValue(AqlValueHintNull()); } auto options = trx->transactionContextPtr()->getVPackOptions(); @@ -3048,7 +3016,7 @@ AqlValue Functions::RemoveValues(arangodb::aql::Query* query, if (!list.isArray() || !values.isArray()) { RegisterInvalidArgumentWarning(query, "REMOVE_VALUES"); - return AqlValue(arangodb::basics::VelocyPackHelper::NullValue()); + return AqlValue(AqlValueHintNull()); } auto options = trx->transactionContextPtr()->getVPackOptions(); @@ -3083,7 +3051,7 @@ AqlValue Functions::RemoveNth(arangodb::aql::Query* query, if (!list.isArray()) { RegisterInvalidArgumentWarning(query, "REMOVE_NTH"); - return AqlValue(arangodb::basics::VelocyPackHelper::NullValue()); + return AqlValue(AqlValueHintNull()); } double const count = static_cast(list.length()); @@ -3126,7 +3094,7 @@ AqlValue Functions::NotNull(arangodb::aql::Query* query, return element.clone(); } } - return AqlValue(arangodb::basics::VelocyPackHelper::NullValue()); + return AqlValue(AqlValueHintNull()); } /// @brief function CURRENT_DATABASE @@ -3172,7 +3140,7 @@ AqlValue Functions::VarianceSample( if (!list.isArray()) { RegisterWarning(query, "VARIANCE_SAMPLE", TRI_ERROR_QUERY_ARRAY_EXPECTED); - return AqlValue(arangodb::basics::VelocyPackHelper::NullValue()); + return AqlValue(AqlValueHintNull()); } double value = 0.0; @@ -3181,11 +3149,11 @@ AqlValue Functions::VarianceSample( if (!Variance(trx, list, value, count)) { RegisterWarning(query, "VARIANCE_SAMPLE", TRI_ERROR_QUERY_INVALID_ARITHMETIC_VALUE); - return AqlValue(arangodb::basics::VelocyPackHelper::NullValue()); + return AqlValue(AqlValueHintNull()); } if (count < 2) { - return AqlValue(arangodb::basics::VelocyPackHelper::NullValue()); + return AqlValue(AqlValueHintNull()); } return NumberValue(trx, value / (count - 1), true); @@ -3202,7 +3170,7 @@ AqlValue Functions::VariancePopulation( if (!list.isArray()) { RegisterWarning(query, "VARIANCE_POPULATION", TRI_ERROR_QUERY_ARRAY_EXPECTED); - return AqlValue(arangodb::basics::VelocyPackHelper::NullValue()); + return AqlValue(AqlValueHintNull()); } double value = 0.0; @@ -3211,11 +3179,11 @@ AqlValue Functions::VariancePopulation( if (!Variance(trx, list, value, count)) { RegisterWarning(query, "VARIANCE_POPULATION", TRI_ERROR_QUERY_INVALID_ARITHMETIC_VALUE); - return AqlValue(arangodb::basics::VelocyPackHelper::NullValue()); + return AqlValue(AqlValueHintNull()); } if (count < 1) { - return AqlValue(arangodb::basics::VelocyPackHelper::NullValue()); + return AqlValue(AqlValueHintNull()); } return NumberValue(trx, value / count, true); @@ -3231,7 +3199,7 @@ AqlValue Functions::StdDevSample( if (!list.isArray()) { RegisterWarning(query, "STDDEV_SAMPLE", TRI_ERROR_QUERY_ARRAY_EXPECTED); - return AqlValue(arangodb::basics::VelocyPackHelper::NullValue()); + return AqlValue(AqlValueHintNull()); } double value = 0.0; @@ -3240,11 +3208,11 @@ AqlValue Functions::StdDevSample( if (!Variance(trx, list, value, count)) { RegisterWarning(query, "STDDEV_SAMPLE", TRI_ERROR_QUERY_INVALID_ARITHMETIC_VALUE); - return AqlValue(arangodb::basics::VelocyPackHelper::NullValue()); + return AqlValue(AqlValueHintNull()); } if (count < 2) { - return AqlValue(arangodb::basics::VelocyPackHelper::NullValue()); + return AqlValue(AqlValueHintNull()); } return NumberValue(trx, std::sqrt(value / (count - 1)), true); @@ -3260,7 +3228,7 @@ AqlValue Functions::StdDevPopulation( if (!list.isArray()) { RegisterWarning(query, "STDDEV_POPULATION", TRI_ERROR_QUERY_ARRAY_EXPECTED); - return AqlValue(arangodb::basics::VelocyPackHelper::NullValue()); + return AqlValue(AqlValueHintNull()); } double value = 0.0; @@ -3269,11 +3237,11 @@ AqlValue Functions::StdDevPopulation( if (!Variance(trx, list, value, count)) { RegisterWarning(query, "STDDEV_POPULATION", TRI_ERROR_QUERY_INVALID_ARITHMETIC_VALUE); - return AqlValue(arangodb::basics::VelocyPackHelper::NullValue()); + return AqlValue(AqlValueHintNull()); } if (count < 1) { - return AqlValue(arangodb::basics::VelocyPackHelper::NullValue()); + return AqlValue(AqlValueHintNull()); } return NumberValue(trx, std::sqrt(value / count), true); @@ -3289,17 +3257,17 @@ AqlValue Functions::Median(arangodb::aql::Query* query, if (!list.isArray()) { RegisterWarning(query, "MEDIAN", TRI_ERROR_QUERY_ARRAY_EXPECTED); - return AqlValue(arangodb::basics::VelocyPackHelper::NullValue()); + return AqlValue(AqlValueHintNull()); } std::vector values; if (!SortNumberList(trx, list, values)) { RegisterWarning(query, "MEDIAN", TRI_ERROR_QUERY_INVALID_ARITHMETIC_VALUE); - return AqlValue(arangodb::basics::VelocyPackHelper::NullValue()); + return AqlValue(AqlValueHintNull()); } if (values.empty()) { - return AqlValue(arangodb::basics::VelocyPackHelper::NullValue()); + return AqlValue(AqlValueHintNull()); } size_t const l = values.size(); size_t midpoint = l / 2; @@ -3320,7 +3288,7 @@ AqlValue Functions::Percentile(arangodb::aql::Query* query, if (!list.isArray()) { RegisterWarning(query, "PERCENTILE", TRI_ERROR_QUERY_ARRAY_EXPECTED); - return AqlValue(arangodb::basics::VelocyPackHelper::NullValue()); + return AqlValue(AqlValueHintNull()); } AqlValue border = ExtractFunctionParameterValue(trx, parameters, 1); @@ -3328,7 +3296,7 @@ AqlValue Functions::Percentile(arangodb::aql::Query* query, if (!border.isNumber()) { RegisterWarning(query, "PERCENTILE", TRI_ERROR_QUERY_FUNCTION_ARGUMENT_TYPE_MISMATCH); - return AqlValue(arangodb::basics::VelocyPackHelper::NullValue()); + return AqlValue(AqlValueHintNull()); } bool unused = false; @@ -3336,7 +3304,7 @@ AqlValue Functions::Percentile(arangodb::aql::Query* query, if (p <= 0.0 || p > 100.0) { RegisterWarning(query, "PERCENTILE", TRI_ERROR_QUERY_FUNCTION_ARGUMENT_TYPE_MISMATCH); - return AqlValue(arangodb::basics::VelocyPackHelper::NullValue()); + return AqlValue(AqlValueHintNull()); } bool useInterpolation = false; @@ -3346,7 +3314,7 @@ AqlValue Functions::Percentile(arangodb::aql::Query* query, if (!methodValue.isString()) { RegisterWarning(query, "PERCENTILE", TRI_ERROR_QUERY_FUNCTION_ARGUMENT_TYPE_MISMATCH); - return AqlValue(arangodb::basics::VelocyPackHelper::NullValue()); + return AqlValue(AqlValueHintNull()); } std::string method = methodValue.slice().copyString(); if (method == "interpolation") { @@ -3356,7 +3324,7 @@ AqlValue Functions::Percentile(arangodb::aql::Query* query, } else { RegisterWarning(query, "PERCENTILE", TRI_ERROR_QUERY_FUNCTION_ARGUMENT_TYPE_MISMATCH); - return AqlValue(arangodb::basics::VelocyPackHelper::NullValue()); + return AqlValue(AqlValueHintNull()); } } @@ -3364,11 +3332,11 @@ AqlValue Functions::Percentile(arangodb::aql::Query* query, if (!SortNumberList(trx, list, values)) { RegisterWarning(query, "PERCENTILE", TRI_ERROR_QUERY_INVALID_ARITHMETIC_VALUE); - return AqlValue(arangodb::basics::VelocyPackHelper::NullValue()); + return AqlValue(AqlValueHintNull()); } if (values.empty()) { - return AqlValue(arangodb::basics::VelocyPackHelper::NullValue()); + return AqlValue(AqlValueHintNull()); } size_t l = values.size(); @@ -3386,7 +3354,7 @@ AqlValue Functions::Percentile(arangodb::aql::Query* query, return NumberValue(trx, values[l - 1], true); } if (pos <= 0) { - return AqlValue(arangodb::basics::VelocyPackHelper::NullValue()); + return AqlValue(AqlValueHintNull()); } double const delta = idx - pos; @@ -3401,7 +3369,7 @@ AqlValue Functions::Percentile(arangodb::aql::Query* query, return NumberValue(trx, values[l - 1], true); } if (pos <= 0) { - return AqlValue(arangodb::basics::VelocyPackHelper::NullValue()); + return AqlValue(AqlValueHintNull()); } return NumberValue(trx, values[static_cast(pos) - 1], true); @@ -3434,7 +3402,7 @@ AqlValue Functions::Range(arangodb::aql::Query* query, if (step == 0.0 || (from < to && step < 0.0) || (from > to && step > 0.0)) { RegisterWarning(query, "RANGE", TRI_ERROR_QUERY_FUNCTION_ARGUMENT_TYPE_MISMATCH); - return AqlValue(arangodb::basics::VelocyPackHelper::NullValue()); + return AqlValue(AqlValueHintNull()); } transaction::BuilderLeaser builder(trx); @@ -3462,7 +3430,7 @@ AqlValue Functions::Position(arangodb::aql::Query* query, if (!list.isArray()) { RegisterWarning(query, "POSITION", TRI_ERROR_QUERY_ARRAY_EXPECTED); - return AqlValue(arangodb::basics::VelocyPackHelper::NullValue()); + return AqlValue(AqlValueHintNull()); } bool returnIndex = false; @@ -3515,7 +3483,7 @@ AqlValue Functions::IsSameCollection( RegisterWarning(query, "IS_SAME_COLLECTION", TRI_ERROR_QUERY_FUNCTION_ARGUMENT_TYPE_MISMATCH); - return AqlValue(arangodb::basics::VelocyPackHelper::NullValue()); + return AqlValue(AqlValueHintNull()); } #include "Pregel/PregelFeature.h" diff --git a/arangod/Aql/V8Expression.cpp b/arangod/Aql/V8Expression.cpp index 209897472471..e5680c5d8e7b 100644 --- a/arangod/Aql/V8Expression.cpp +++ b/arangod/Aql/V8Expression.cpp @@ -145,7 +145,7 @@ AqlValue V8Expression::execute(v8::Isolate* isolate, Query* query, if (result->IsUndefined()) { // expression does not have any (defined) value. replace with null mustDestroy = false; - return AqlValue(basics::VelocyPackHelper::NullValue()); + return AqlValue(AqlValueHintNull()); } // expression had a result. convert it to JSON diff --git a/arangod/Graph/ClusterTraverserCache.cpp b/arangod/Graph/ClusterTraverserCache.cpp index 364752c1e70d..476b6fa987e6 100644 --- a/arangod/Graph/ClusterTraverserCache.cpp +++ b/arangod/Graph/ClusterTraverserCache.cpp @@ -50,7 +50,7 @@ aql::AqlValue ClusterTraverserCache::fetchEdgeAqlResult(EdgeDocumentToken const& TRI_ASSERT(ServerState::instance()->isCoordinator()); // FIXME: the ClusterTraverserCache lifetime is shorter then the query lifetime // therefore we cannot get away here without copying the result - //return aql::AqlValue(aql::AqlValueHintNoCopy(token.vpack())); + //return aql::AqlValue(aql::AqlValueHintDocumentNoCopy(token.vpack())); return aql::AqlValue(VPackSlice(token.vpack())); // will copy slice } @@ -63,11 +63,11 @@ aql::AqlValue ClusterTraverserCache::fetchVertexAqlResult(StringRef id) { if (it == _cache.end()) { LOG_TOPIC(ERR, Logger::GRAPHS) << __FUNCTION__ << " vertex not found"; // Document not found return NULL - return aql::AqlValue(VelocyPackHelper::NullValue()); + return aql::AqlValue(aql::AqlValueHintNull()); } // FIXME: the ClusterTraverserCache lifetime is shorter then the query lifetime // therefore we cannot get away here without copying the result - //return aql::AqlValue(aql::AqlValueHintNoCopy(it->second.begin())); + //return aql::AqlValue(aql::AqlValueHintDocumentNoCopy(it->second.begin())); return aql::AqlValue(it->second); // will copy slice } diff --git a/arangod/Graph/ShortestPathResult.cpp b/arangod/Graph/ShortestPathResult.cpp index 22b3179a6fdf..e9cfba7cd18a 100644 --- a/arangod/Graph/ShortestPathResult.cpp +++ b/arangod/Graph/ShortestPathResult.cpp @@ -53,7 +53,7 @@ void ShortestPathResult::clear() { AqlValue ShortestPathResult::edgeToAqlValue(TraverserCache* cache, size_t position) const { if (position == 0) { // First Edge is defined as NULL - return AqlValue(arangodb::basics::VelocyPackHelper::NullValue()); + return AqlValue(AqlValueHintNull()); } TRI_ASSERT(position - 1 < _edges.size()); return cache->fetchEdgeAqlResult(_edges[position - 1]); From 22b915569bebb9377f743437f091eb0e18fd6872 Mon Sep 17 00:00:00 2001 From: jsteemann Date: Fri, 22 Sep 2017 10:50:13 +0200 Subject: [PATCH 8/9] make AqlValue construction more type-safe --- arangod/Aql/AqlValue.cpp | 2 +- arangod/Aql/AqlValue.h | 25 ++++++------------------- arangod/Aql/CollectBlock.cpp | 2 +- arangod/Aql/Expression.cpp | 32 ++++++++++++++++---------------- arangod/Aql/Functions.cpp | 4 ++-- 5 files changed, 26 insertions(+), 39 deletions(-) diff --git a/arangod/Aql/AqlValue.cpp b/arangod/Aql/AqlValue.cpp index a9eea884504f..cee794c7dccf 100644 --- a/arangod/Aql/AqlValue.cpp +++ b/arangod/Aql/AqlValue.cpp @@ -266,7 +266,7 @@ AqlValue AqlValue::at(transaction::Methods* trx, if (position >= 0 && position < static_cast(n)) { // only look up the value if it is within array bounds - return AqlValue(_data.range->at(static_cast(position))); + return AqlValue(AqlValueHintInt(_data.range->at(static_cast(position)))); } // fall-through intentional break; diff --git a/arangod/Aql/AqlValue.h b/arangod/Aql/AqlValue.h index 278213174bd1..0eeec9106148 100644 --- a/arangod/Aql/AqlValue.h +++ b/arangod/Aql/AqlValue.h @@ -199,14 +199,6 @@ struct AqlValue final { setType(AqlValueType::DOCVEC); } - // construct boolean value type - // DEPRECATED - explicit AqlValue(bool value) { - VPackSlice slice(value ? arangodb::basics::VelocyPackHelper::TrueValue() : arangodb::basics::VelocyPackHelper::FalseValue()); - memcpy(_data.internal, slice.begin(), static_cast(slice.byteSize())); - setType(AqlValueType::VPACK_INLINE); - } - explicit AqlValue(AqlValueHintNull const&) noexcept { _data.internal[0] = 0x18; // null in VPack setType(AqlValueType::VPACK_INLINE); @@ -223,7 +215,8 @@ struct AqlValue final { } // construct from a double value - explicit AqlValue(double value) noexcept { + explicit AqlValue(AqlValueHintDouble const&v) noexcept { + double value = v.value; if (std::isnan(value) || !std::isfinite(value) || value == HUGE_VAL || value == -HUGE_VAL) { // null _data.internal[0] = 0x18; @@ -243,11 +236,9 @@ struct AqlValue final { setType(AqlValueType::VPACK_INLINE); } - // construct from a double value - explicit AqlValue(AqlValueHintDouble const&v) noexcept : AqlValue(v.value) {} - // construct from an int64 value - explicit AqlValue(int64_t value) noexcept { + explicit AqlValue(AqlValueHintInt const& v) noexcept { + int64_t value = v.value; if (value >= 0 && value <= 9) { // a smallint _data.internal[0] = static_cast(0x30U + value); @@ -276,7 +267,8 @@ struct AqlValue final { } // construct from a uint64 value - explicit AqlValue(uint64_t value) noexcept { + explicit AqlValue(AqlValueHintUInt const& v) noexcept { + uint64_t value = v.value; if (value <= 9) { // a smallint _data.internal[0] = static_cast(0x30U + value); @@ -294,11 +286,6 @@ struct AqlValue final { setType(AqlValueType::VPACK_INLINE); } - // construct from a uint64 value - explicit AqlValue(AqlValueHintUInt const& v) noexcept : AqlValue(v.value) {} - - explicit AqlValue(AqlValueHintInt const& v) noexcept : AqlValue(v.value) {} - // construct from char* and length, copying the string AqlValue(char const* value, size_t length) { TRI_ASSERT(value != nullptr); diff --git a/arangod/Aql/CollectBlock.cpp b/arangod/Aql/CollectBlock.cpp index 9cd1fefec9a5..d89881b56b1c 100644 --- a/arangod/Aql/CollectBlock.cpp +++ b/arangod/Aql/CollectBlock.cpp @@ -512,7 +512,7 @@ void SortedCollectBlock::emitGroup(AqlItemBlock const* cur, AqlItemBlock* res, if (static_cast(_exeNode)->_count) { // only set group count in result register - res->emplaceValue(row, _collectRegister, static_cast(_currentGroup.groupLength)); + res->emplaceValue(row, _collectRegister, AqlValueHintUInt(static_cast(_currentGroup.groupLength))); } else if (static_cast(_exeNode)->_expressionVariable != nullptr) { // copy expression result into result register diff --git a/arangod/Aql/Expression.cpp b/arangod/Aql/Expression.cpp index dfe5124a6ff3..1570a98b3387 100644 --- a/arangod/Aql/Expression.cpp +++ b/arangod/Aql/Expression.cpp @@ -965,7 +965,7 @@ AqlValue Expression::executeSimpleExpressionNot( bool const operandIsTrue = operand.toBoolean(); mustDestroy = false; // only a boolean - return AqlValue(!operandIsTrue); + return AqlValue(AqlValueHintBool(!operandIsTrue)); } /// @brief execute an expression of type SIMPLE with + @@ -983,10 +983,10 @@ AqlValue Expression::executeSimpleExpressionPlus(AstNode const* node, if (s.isSmallInt() || s.isInt()) { // can use int64 - return AqlValue(s.getNumber()); + return AqlValue(AqlValueHintInt(s.getNumber())); } else if (s.isUInt()) { // can use uint64 - return AqlValue(s.getUInt()); + return AqlValue(AqlValueHintUInt(s.getUInt())); } // fallthrouh intentional } @@ -999,7 +999,7 @@ AqlValue Expression::executeSimpleExpressionPlus(AstNode const* node, value = 0.0; } - return AqlValue(+value); + return AqlValue(AqlValueHintDouble(+value)); } /// @brief execute an expression of type SIMPLE with - @@ -1016,12 +1016,12 @@ AqlValue Expression::executeSimpleExpressionMinus(AstNode const* node, VPackSlice const s = operand.slice(); if (s.isSmallInt()) { // can use int64 - return AqlValue(-s.getNumber()); + return AqlValue(AqlValueHintInt(-s.getNumber())); } else if (s.isInt()) { int64_t v = s.getNumber(); if (v != INT64_MIN) { // can use int64 - return AqlValue(-v); + return AqlValue(AqlValueHintInt(-v)); } } // fallthrouh intentional @@ -1035,7 +1035,7 @@ AqlValue Expression::executeSimpleExpressionMinus(AstNode const* node, value = 0.0; } - return AqlValue(-value); + return AqlValue(AqlValueHintDouble(-value)); } /// @brief execute an expression of type SIMPLE with AND @@ -1147,7 +1147,7 @@ AqlValue Expression::executeSimpleExpressionComparison( result = !result; } - return AqlValue(result); + return AqlValue(AqlValueHintBool(result)); } // all other comparison operators... @@ -1160,17 +1160,17 @@ AqlValue Expression::executeSimpleExpressionComparison( switch (node->type) { case NODE_TYPE_OPERATOR_BINARY_EQ: - return AqlValue(compareResult == 0); + return AqlValue(AqlValueHintBool(compareResult == 0)); case NODE_TYPE_OPERATOR_BINARY_NE: - return AqlValue(compareResult != 0); + return AqlValue(AqlValueHintBool(compareResult != 0)); case NODE_TYPE_OPERATOR_BINARY_LT: - return AqlValue(compareResult < 0); + return AqlValue(AqlValueHintBool(compareResult < 0)); case NODE_TYPE_OPERATOR_BINARY_LE: - return AqlValue(compareResult <= 0); + return AqlValue(AqlValueHintBool(compareResult <= 0)); case NODE_TYPE_OPERATOR_BINARY_GT: - return AqlValue(compareResult > 0); + return AqlValue(AqlValueHintBool(compareResult > 0)); case NODE_TYPE_OPERATOR_BINARY_GE: - return AqlValue(compareResult >= 0); + return AqlValue(AqlValueHintBool(compareResult >= 0)); default: std::string msg("unhandled type '"); msg.append(node->getTypeString()); @@ -1305,7 +1305,7 @@ AqlValue Expression::executeSimpleExpressionArrayComparison( } TRI_ASSERT(!mustDestroy); - return AqlValue(overallResult); + return AqlValue(AqlValueHintBool(overallResult)); } /// @brief execute an expression of type SIMPLE with TERNARY @@ -1593,5 +1593,5 @@ AqlValue Expression::executeSimpleExpressionArithmetic( } // this will convert NaN, +inf & -inf to null - return AqlValue(result); + return AqlValue(AqlValueHintDouble(result)); } diff --git a/arangod/Aql/Functions.cpp b/arangod/Aql/Functions.cpp index dbf268ed9390..94cda471c524 100644 --- a/arangod/Aql/Functions.cpp +++ b/arangod/Aql/Functions.cpp @@ -671,7 +671,7 @@ AqlValue Functions::ToBool(arangodb::aql::Query* query, transaction::Methods* trx, VPackFunctionParameters const& parameters) { AqlValue a = ExtractFunctionParameterValue(trx, parameters, 0); - return AqlValue(a.toBoolean()); + return AqlValue(AqlValueHintBool(a.toBoolean())); } /// @brief function TO_ARRAY @@ -3478,7 +3478,7 @@ AqlValue Functions::IsSameCollection( std::string const second = ExtractCollectionName(trx, parameters, 1); if (!first.empty() && !second.empty()) { - return AqlValue(first == second); + return AqlValue(AqlValueHintBool(first == second)); } RegisterWarning(query, "IS_SAME_COLLECTION", From 7ef74a4f8e02c433645ab05ff00906436507b4ae Mon Sep 17 00:00:00 2001 From: jsteemann Date: Mon, 25 Sep 2017 13:03:25 +0200 Subject: [PATCH 9/9] some API cleanup --- arangod/Aql/BindParameters.cpp | 11 +-- arangod/Aql/BindParameters.h | 4 +- arangod/Aql/EnumerateCollectionBlock.cpp | 2 +- arangod/Aql/IndexBlock.cpp | 2 +- arangod/Graph/BaseOptions.cpp | 3 +- arangod/Graph/EdgeCollectionInfo.cpp | 8 +- arangod/Pregel/GraphStore.cpp | 4 +- arangod/RestHandler/RestEdgesHandler.cpp | 2 +- arangod/RestHandler/RestSimpleHandler.cpp | 32 ++------ arangod/RocksDBEngine/RocksDBEdgeIndex.cpp | 1 + arangod/RocksDBEngine/RocksDBPrimaryIndex.cpp | 7 +- arangod/Transaction/Methods.cpp | 33 ++------ arangod/Transaction/Methods.h | 6 +- arangod/Utils/OperationCursor.cpp | 82 +------------------ arangod/Utils/OperationCursor.h | 15 +--- arangod/V8Server/v8-query.cpp | 26 +----- js/server/modules/@arangodb/aql.js | 2 +- .../modules/@arangodb/arango-collection.js | 2 +- 18 files changed, 54 insertions(+), 188 deletions(-) diff --git a/arangod/Aql/BindParameters.cpp b/arangod/Aql/BindParameters.cpp index dfe481549cfd..8bdf60f2fdf4 100644 --- a/arangod/Aql/BindParameters.cpp +++ b/arangod/Aql/BindParameters.cpp @@ -80,17 +80,19 @@ void BindParameters::process() { /// @brief strip collection name prefixes from the parameters /// the values must be a VelocyPack array -VPackBuilder BindParameters::StripCollectionNames(VPackSlice const& keys, - char const* collectionName) { +void BindParameters::stripCollectionNames(VPackSlice const& keys, + std::string const& collectionName, + VPackBuilder& result) { + char const* c = collectionName.c_str(); + TRI_ASSERT(keys.isArray()); - VPackBuilder result; result.openArray(); for (auto const& element : VPackArrayIterator(keys)) { if (element.isString()) { VPackValueLength l; char const* s = element.getString(l); auto p = static_cast(memchr(s, '/', static_cast(l))); - if (p != nullptr && strncmp(s, collectionName, p - s) == 0) { + if (p != nullptr && strncmp(s, c, p - s) == 0) { // key begins with collection name + '/', now strip it in place for // further comparisons result.add(VPackValue( @@ -101,6 +103,5 @@ VPackBuilder BindParameters::StripCollectionNames(VPackSlice const& keys, result.add(element); } result.close(); - return result; } diff --git a/arangod/Aql/BindParameters.h b/arangod/Aql/BindParameters.h index d69fcf1fe46a..012a6c5f7f45 100644 --- a/arangod/Aql/BindParameters.h +++ b/arangod/Aql/BindParameters.h @@ -65,8 +65,8 @@ class BindParameters { /// @brief strip collection name prefixes from the parameters /// the values must be a VelocyPack array - static arangodb::velocypack::Builder StripCollectionNames( - arangodb::velocypack::Slice const&, char const*); + static void stripCollectionNames(arangodb::velocypack::Slice const&, + std::string const& collectionName, arangodb::velocypack::Builder& result); private: /// @brief process the parameters diff --git a/arangod/Aql/EnumerateCollectionBlock.cpp b/arangod/Aql/EnumerateCollectionBlock.cpp index 23ad19c6e2ed..7898b40ceec5 100644 --- a/arangod/Aql/EnumerateCollectionBlock.cpp +++ b/arangod/Aql/EnumerateCollectionBlock.cpp @@ -50,7 +50,7 @@ EnumerateCollectionBlock::EnumerateCollectionBlock( _trx->indexScan(_collection->getName(), (ep->_random ? transaction::Methods::CursorType::ANY : transaction::Methods::CursorType::ALL), - _mmdr.get(), 0, UINT64_MAX, 1000, false)) { + _mmdr.get(), false)) { TRI_ASSERT(_cursor->successful()); } diff --git a/arangod/Aql/IndexBlock.cpp b/arangod/Aql/IndexBlock.cpp index 3fe23862ccdd..4747405a3b72 100644 --- a/arangod/Aql/IndexBlock.cpp +++ b/arangod/Aql/IndexBlock.cpp @@ -698,7 +698,7 @@ arangodb::OperationCursor* IndexBlock::orderCursor(size_t currentIndex) { IndexNode const* node = static_cast(getPlanNode()); _cursors[currentIndex].reset(_trx->indexScanForCondition( _indexes[currentIndex], conditionNode, node->outVariable(), _mmdr.get(), - UINT64_MAX, transaction::Methods::defaultBatchSize(), node->_reverse)); + node->_reverse)); } else { // cursor for index already exists, reset and reuse it _cursors[currentIndex]->reset(); diff --git a/arangod/Graph/BaseOptions.cpp b/arangod/Graph/BaseOptions.cpp index b18b763ad7b2..3dea0e9ee2ff 100644 --- a/arangod/Graph/BaseOptions.cpp +++ b/arangod/Graph/BaseOptions.cpp @@ -418,8 +418,7 @@ EdgeCursor* BaseOptions::nextCursorLocal(ManagedDocumentResult* mmdr, std::vector csrs; csrs.reserve(info.idxHandles.size()); for (auto const& it : info.idxHandles) { - csrs.emplace_back(_trx->indexScanForCondition(it, node, _tmpVar, mmdr, - UINT64_MAX, 1000, false)); + csrs.emplace_back(_trx->indexScanForCondition(it, node, _tmpVar, mmdr, false)); } opCursors.emplace_back(std::move(csrs)); } diff --git a/arangod/Graph/EdgeCollectionInfo.cpp b/arangod/Graph/EdgeCollectionInfo.cpp index aa00d72fe0ee..4d8fcac2ec5b 100644 --- a/arangod/Graph/EdgeCollectionInfo.cpp +++ b/arangod/Graph/EdgeCollectionInfo.cpp @@ -80,11 +80,11 @@ std::unique_ptr EdgeCollectionInfo::getEdges( if (_dir == TRI_EDGE_OUT) { res.reset(_trx->indexScanForCondition( _forwardIndexId, _searchBuilder.getOutboundCondition(), - _searchBuilder.getVariable(), mmdr, UINT64_MAX, 1000, false)); + _searchBuilder.getVariable(), mmdr, false)); } else { res.reset(_trx->indexScanForCondition( _forwardIndexId, _searchBuilder.getInboundCondition(), - _searchBuilder.getVariable(), mmdr, UINT64_MAX, 1000, false)); + _searchBuilder.getVariable(), mmdr, false)); } return res; } @@ -117,11 +117,11 @@ std::unique_ptr EdgeCollectionInfo::getReverseEdges( if (_dir == TRI_EDGE_OUT) { res.reset(_trx->indexScanForCondition( _backwardIndexId, _searchBuilder.getInboundCondition(), - _searchBuilder.getVariable(), mmdr, UINT64_MAX, 1000, false)); + _searchBuilder.getVariable(), mmdr, false)); } else { res.reset(_trx->indexScanForCondition( _backwardIndexId, _searchBuilder.getOutboundCondition(), - _searchBuilder.getVariable(), mmdr, UINT64_MAX, 1000, false)); + _searchBuilder.getVariable(), mmdr, false)); } return res; } diff --git a/arangod/Pregel/GraphStore.cpp b/arangod/Pregel/GraphStore.cpp index 194bbf20a92c..8d1c11880745 100644 --- a/arangod/Pregel/GraphStore.cpp +++ b/arangod/Pregel/GraphStore.cpp @@ -360,8 +360,8 @@ void GraphStore::_loadVertices(size_t i, ManagedDocumentResult mmdr; std::unique_ptr cursor = - trx->indexScan(vertexShard, transaction::Methods::CursorType::ALL, &mmdr, - 0, UINT64_MAX, 1000, false); + trx->indexScan(vertexShard, transaction::Methods::CursorType::ALL, &mmdr, false); + if (cursor->failed()) { THROW_ARANGO_EXCEPTION_FORMAT(cursor->code, "while looking up shard '%s'", vertexShard.c_str()); diff --git a/arangod/RestHandler/RestEdgesHandler.cpp b/arangod/RestHandler/RestEdgesHandler.cpp index b43397e5cde7..4ae37a018519 100644 --- a/arangod/RestHandler/RestEdgesHandler.cpp +++ b/arangod/RestHandler/RestEdgesHandler.cpp @@ -82,7 +82,7 @@ void RestEdgesHandler::readCursor( ManagedDocumentResult mmdr; std::unique_ptr cursor(trx.indexScanForCondition( - indexId, condition, var, &mmdr, UINT64_MAX, 1000, false)); + indexId, condition, var, &mmdr, false)); if (cursor->failed()) { THROW_ARANGO_EXCEPTION(cursor->code); diff --git a/arangod/RestHandler/RestSimpleHandler.cpp b/arangod/RestHandler/RestSimpleHandler.cpp index bc2ca1d00dc6..cd45f1079c5e 100644 --- a/arangod/RestHandler/RestSimpleHandler.cpp +++ b/arangod/RestHandler/RestSimpleHandler.cpp @@ -323,11 +323,8 @@ void RestSimpleHandler::lookupByKeys(VPackSlice const& slice) { auto bindVars = std::make_shared(); bindVars->openObject(); bindVars->add("@collection", VPackValue(collectionName)); - VPackBuilder strippedBuilder = - arangodb::aql::BindParameters::StripCollectionNames( - keys, collectionName.c_str()); - - bindVars->add("keys", strippedBuilder.slice()); + bindVars->add(VPackValue("keys")); + arangodb::aql::BindParameters::stripCollectionNames(keys, collectionName, *bindVars.get()); bindVars->close(); std::string const aql( @@ -351,12 +348,6 @@ void RestSimpleHandler::lookupByKeys(VPackSlice const& slice) { return; } - size_t resultSize = 10; - VPackSlice qResult = queryResult.result->slice(); - if (qResult.isArray()) { - resultSize = static_cast(qResult.length()); - } - VPackBuffer resultBuffer; VPackBuilder result(resultBuffer); { @@ -364,34 +355,25 @@ void RestSimpleHandler::lookupByKeys(VPackSlice const& slice) { resetResponse(rest::ResponseCode::OK); response->setContentType(rest::ContentType::JSON); + result.add(VPackValue("documents")); + VPackSlice qResult = queryResult.result->slice(); if (qResult.isArray()) { // This is for internal use of AQL Traverser only. // Should not be documented VPackSlice const postFilter = slice.get("filter"); TRI_ASSERT(postFilter.isNone()); - result.add(VPackValue("documents")); - result.add(qResult); - queryResult.result = nullptr; - } else { - result.add(VPackValue("documents")); - result.add(qResult); - queryResult.result = nullptr; } + result.addExternal(queryResult.result->slice().begin()); result.add("error", VPackValue(false)); result.add("code", VPackValue(static_cast(_response->responseCode()))); - - // reserve a few bytes per result document by default - int res = response->reservePayload(32 * resultSize); - - if (res != TRI_ERROR_NO_ERROR) { - THROW_ARANGO_EXCEPTION(res); - } } generateResult(rest::ResponseCode::OK, std::move(resultBuffer), queryResult.context); + + queryResult.result = nullptr; } catch (...) { unregisterQuery(); throw; diff --git a/arangod/RocksDBEngine/RocksDBEdgeIndex.cpp b/arangod/RocksDBEngine/RocksDBEdgeIndex.cpp index f821e0d56c2d..2e56bca70a03 100644 --- a/arangod/RocksDBEngine/RocksDBEdgeIndex.cpp +++ b/arangod/RocksDBEngine/RocksDBEdgeIndex.cpp @@ -328,6 +328,7 @@ bool RocksDBEdgeIndexIterator::nextExtra(ExtraCallback const& cb, if (needRocksLookup) { lookupInRocksDB(fromTo); } + _keysIterator.next(); } TRI_ASSERT(limit == 0); diff --git a/arangod/RocksDBEngine/RocksDBPrimaryIndex.cpp b/arangod/RocksDBEngine/RocksDBPrimaryIndex.cpp index c5a61012c8cb..ceb67268f8e7 100644 --- a/arangod/RocksDBEngine/RocksDBPrimaryIndex.cpp +++ b/arangod/RocksDBEngine/RocksDBPrimaryIndex.cpp @@ -104,9 +104,12 @@ bool RocksDBPrimaryIndexIterator::next(TokenCallback const& cb, size_t limit) { while (limit > 0) { // TODO: prevent copying of the value into result, as we don't need it here! RocksDBToken token = _index->lookupKey(_trx, StringRef(*_iterator)); - cb(token); + if (token.revisionId()) { + cb(token); + + --limit; + } - --limit; _iterator.next(); if (!_iterator.valid()) { return false; diff --git a/arangod/Transaction/Methods.cpp b/arangod/Transaction/Methods.cpp index 426b01ce0ad1..99b778cdd320 100644 --- a/arangod/Transaction/Methods.cpp +++ b/arangod/Transaction/Methods.cpp @@ -832,8 +832,7 @@ OperationResult transaction::Methods::anyLocal( ManagedDocumentResult mmdr; std::unique_ptr cursor = - indexScan(collectionName, transaction::Methods::CursorType::ANY, &mmdr, - skip, limit, 1000, false); + indexScan(collectionName, transaction::Methods::CursorType::ANY, &mmdr, false); cursor->allDocuments([&resultBuilder](DocumentIdentifierToken const& token, VPackSlice slice) { resultBuilder.add(slice); @@ -2232,8 +2231,7 @@ OperationResult transaction::Methods::allLocal( ManagedDocumentResult mmdr; std::unique_ptr cursor = - indexScan(collectionName, transaction::Methods::CursorType::ALL, &mmdr, - skip, limit, 1000, false); + indexScan(collectionName, transaction::Methods::CursorType::ALL, &mmdr, false); if (cursor->failed()) { return OperationResult(cursor->code); @@ -2620,17 +2618,12 @@ std::pair transaction::Methods::getIndexForSortCondition( OperationCursor* transaction::Methods::indexScanForCondition( IndexHandle const& indexId, arangodb::aql::AstNode const* condition, arangodb::aql::Variable const* var, ManagedDocumentResult* mmdr, - uint64_t limit, uint64_t batchSize, bool reverse) { + bool reverse) { if (_state->isCoordinator()) { // The index scan is only available on DBServers and Single Server. THROW_ARANGO_EXCEPTION(TRI_ERROR_CLUSTER_ONLY_ON_DBSERVER); } - if (limit == 0) { - // nothing to do - return new OperationCursor(TRI_ERROR_NO_ERROR); - } - auto idx = indexId.getIndex(); if (nullptr == idx) { THROW_ARANGO_EXCEPTION_MESSAGE(TRI_ERROR_BAD_PARAMETER, @@ -2646,7 +2639,7 @@ OperationCursor* transaction::Methods::indexScanForCondition( return new OperationCursor(TRI_ERROR_OUT_OF_MEMORY); } - return new OperationCursor(iterator.release(), limit, batchSize); + return new OperationCursor(iterator.release(), defaultBatchSize()); } /// @brief factory for OperationCursor objects @@ -2654,8 +2647,8 @@ OperationCursor* transaction::Methods::indexScanForCondition( /// calling this method std::unique_ptr transaction::Methods::indexScan( std::string const& collectionName, CursorType cursorType, - ManagedDocumentResult* mmdr, uint64_t skip, uint64_t limit, - uint64_t batchSize, bool reverse) { + ManagedDocumentResult* mmdr, + bool reverse) { // For now we assume indexId is the iid part of the index. if (_state->isCoordinator()) { @@ -2663,11 +2656,6 @@ std::unique_ptr transaction::Methods::indexScan( THROW_ARANGO_EXCEPTION(TRI_ERROR_CLUSTER_ONLY_ON_DBSERVER); } - if (limit == 0) { - // nothing to do - return std::make_unique(TRI_ERROR_NO_ERROR); - } - TRI_voc_cid_t cid = addCollectionAtRuntime(collectionName); TransactionCollection* trxColl = trxCollection(cid); if (trxColl == nullptr) { @@ -2696,13 +2684,8 @@ std::unique_ptr transaction::Methods::indexScan( return std::make_unique(TRI_ERROR_OUT_OF_MEMORY); } - if (skip > 0) { - uint64_t unused = 0; - iterator->skip(skip, unused); - } - - return std::make_unique(iterator.release(), limit, - batchSize); + return std::make_unique(iterator.release(), + defaultBatchSize()); } /// @brief return the collection diff --git a/arangod/Transaction/Methods.h b/arangod/Transaction/Methods.h index 65c77a4d54c6..ae01be894d13 100644 --- a/arangod/Transaction/Methods.h +++ b/arangod/Transaction/Methods.h @@ -351,8 +351,7 @@ class Methods { OperationCursor* indexScanForCondition(IndexHandle const&, arangodb::aql::AstNode const*, arangodb::aql::Variable const*, - ManagedDocumentResult*, - uint64_t, uint64_t, bool); + ManagedDocumentResult*, bool reverse); /// @brief factory for OperationCursor objects /// note: the caller must have read-locked the underlying collection when @@ -361,8 +360,7 @@ class Methods { std::unique_ptr indexScan(std::string const& collectionName, CursorType cursorType, ManagedDocumentResult*, - uint64_t skip, uint64_t limit, - uint64_t batchSize, bool reverse); + bool reverse); /// @brief test if a collection is already locked ENTERPRISE_VIRT bool isLocked(arangodb::LogicalCollection*, diff --git a/arangod/Utils/OperationCursor.cpp b/arangod/Utils/OperationCursor.cpp index 82d5cf032830..577c1894f952 100644 --- a/arangod/Utils/OperationCursor.cpp +++ b/arangod/Utils/OperationCursor.cpp @@ -50,7 +50,6 @@ void OperationCursor::reset() { if (_indexIterator != nullptr) { _indexIterator->reset(); _hasMore = true; - _limit = _originalLimit; } } @@ -65,32 +64,8 @@ bool OperationCursor::next(IndexIterator::TokenCallback const& callback, uint64_ batchSize = _batchSize; } - size_t atMost = static_cast(batchSize > _limit ? _limit : batchSize); - -#ifdef ARANGODB_ENABLE_MAINTAINER_MODE - // We add wrapper around Callback that validates that - // the callback has been called at least once. - bool called = false; - auto cb = [&](DocumentIdentifierToken const& token) { - called = true; - callback(token); - }; - _hasMore = _indexIterator->next(cb, atMost); - if (_hasMore) { - // If the index says it has more elements than it need - // to call callback at least once. - // Otherweise progress is not guaranteed. - TRI_ASSERT(called); - } -#else + size_t atMost = static_cast(batchSize); _hasMore = _indexIterator->next(callback, atMost); -#endif - - if (_hasMore) { - // We got atMost many callbacks - TRI_ASSERT(_limit >= atMost); - _limit -= atMost; - } return _hasMore; } @@ -104,32 +79,8 @@ bool OperationCursor::nextDocument(IndexIterator::DocumentCallback const& callba batchSize = _batchSize; } - size_t atMost = static_cast(batchSize > _limit ? _limit : batchSize); - -#ifdef ARANGODB_ENABLE_MAINTAINER_MODE - // We add wrapper around Callback that validates that - // the callback has been called at least once. - bool called = false; - auto cb = [&](DocumentIdentifierToken const& token, VPackSlice slice) { - called = true; - callback(token, slice); - }; - _hasMore = _indexIterator->nextDocument(cb, atMost); - if (_hasMore) { - // If the index says it has more elements than it need - // to call callback at least once. - // Otherweise progress is not guaranteed. - TRI_ASSERT(called); - } -#else + size_t atMost = static_cast(batchSize); _hasMore = _indexIterator->nextDocument(callback, atMost); -#endif - - if (_hasMore) { - // We got atMost many callbacks - TRI_ASSERT(_limit >= atMost); - _limit -= atMost; - } return _hasMore; } @@ -150,32 +101,9 @@ bool OperationCursor::nextWithExtra(IndexIterator::ExtraCallback const& callback if (batchSize == UINT64_MAX) { batchSize = _batchSize; } - size_t atMost = static_cast(batchSize > _limit ? _limit : batchSize); - -#ifdef ARANGODB_ENABLE_MAINTAINER_MODE - // We add wrapper around Callback that validates that - // the callback has been called at least once. - bool called = false; - auto cb = [&](DocumentIdentifierToken const& token, VPackSlice extra) { - called = true; - callback(token, extra); - }; - _hasMore = _indexIterator->nextExtra(cb, atMost); - if (_hasMore) { - // If the index says it has more elements than it need - // to call callback at least once. - // Otherweise progress is not guaranteed. - TRI_ASSERT(called); - } -#else + + size_t atMost = static_cast(batchSize); _hasMore = _indexIterator->nextExtra(callback, atMost); -#endif - - if (_hasMore) { - // We got atMost many callbacks - TRI_ASSERT(_limit >= atMost); - _limit -= atMost; - } return _hasMore; } @@ -192,13 +120,11 @@ int OperationCursor::skip(uint64_t toSkip, uint64_t& skipped) { if (toSkip > _limit) { // Short-cut, we jump to the end - _limit = 0; _hasMore = false; return TRI_ERROR_NO_ERROR; } _indexIterator->skip(toSkip, skipped); - _limit -= skipped; if (skipped != toSkip || _limit == 0) { _hasMore = false; } diff --git a/arangod/Utils/OperationCursor.h b/arangod/Utils/OperationCursor.h index ce8e056665af..5cd2cea30e10 100644 --- a/arangod/Utils/OperationCursor.h +++ b/arangod/Utils/OperationCursor.h @@ -48,7 +48,6 @@ struct OperationCursor { std::unique_ptr _indexIterator; bool _hasMore; uint64_t _limit; - uint64_t const _originalLimit; uint64_t const _batchSize; public: @@ -56,20 +55,14 @@ struct OperationCursor { : code(code), _hasMore(false), _limit(0), - _originalLimit(0), _batchSize(1000) {} - OperationCursor(IndexIterator* iterator, uint64_t limit, uint64_t batchSize) + OperationCursor(IndexIterator* iterator, uint64_t batchSize) : code(TRI_ERROR_NO_ERROR), _indexIterator(iterator), - _hasMore(true), - _limit(limit), // _limit is modified later on - _originalLimit(limit), - _batchSize(batchSize) { - if (_indexIterator == nullptr) { - _hasMore = false; - } - } + _hasMore(_indexIterator != nullptr), + _limit(UINT64_MAX), // _limit is modified later on + _batchSize(batchSize) {} ~OperationCursor() {} diff --git a/arangod/V8Server/v8-query.cpp b/arangod/V8Server/v8-query.cpp index 017f02ba961f..a822463a0ca2 100644 --- a/arangod/V8Server/v8-query.cpp +++ b/arangod/V8Server/v8-query.cpp @@ -190,11 +190,6 @@ static void JS_AllQuery(v8::FunctionCallbackInfo const& args) { TRI_V8_TRY_CATCH_BEGIN(isolate); v8::HandleScope scope(isolate); - // expecting two arguments - if (args.Length() != 2) { - TRI_V8_THROW_EXCEPTION_USAGE("ALL(, )"); - } - arangodb::LogicalCollection* collection = TRI_UnwrapClass(args.Holder(), WRP_VOCBASE_COL_TYPE); @@ -203,18 +198,6 @@ static void JS_AllQuery(v8::FunctionCallbackInfo const& args) { TRI_V8_THROW_EXCEPTION_INTERNAL("cannot extract collection"); } - // extract skip and limit - int64_t skip = 0; - uint64_t limit = UINT64_MAX; - - if (!args[0]->IsNull() && !args[0]->IsUndefined()) { - skip = TRI_ObjectToInt64(args[0]); - } - - if (!args[1]->IsNull() && !args[1]->IsUndefined()) { - limit = TRI_ObjectToUInt64(args[1], false); - } - std::string const collectionName(collection->name()); std::shared_ptr transactionContext = @@ -230,8 +213,7 @@ static void JS_AllQuery(v8::FunctionCallbackInfo const& args) { // We directly read the entire cursor. so batchsize == limit std::unique_ptr opCursor = - trx.indexScan(collectionName, transaction::Methods::CursorType::ALL, nullptr, skip, - limit, limit, false); + trx.indexScan(collectionName, transaction::Methods::CursorType::ALL, nullptr, false); if (opCursor->failed()) { TRI_V8_THROW_EXCEPTION(opCursor->code); @@ -447,10 +429,8 @@ static void JS_LookupByKeys(v8::FunctionCallbackInfo const& args) { TRI_V8_THROW_EXCEPTION(res); } - VPackBuilder strippedBuilder = - arangodb::aql::BindParameters::StripCollectionNames(keys.slice(), collection->name().c_str()); - - bindVars->add("keys", strippedBuilder.slice()); + bindVars->add(VPackValue("keys")); + arangodb::aql::BindParameters::stripCollectionNames(keys.slice(), collection->name(), *bindVars.get()); bindVars->close(); std::string const queryString( diff --git a/js/server/modules/@arangodb/aql.js b/js/server/modules/@arangodb/aql.js index ae777712eee4..180c358fdb91 100644 --- a/js/server/modules/@arangodb/aql.js +++ b/js/server/modules/@arangodb/aql.js @@ -1179,7 +1179,7 @@ function GET_DOCUMENTS (collection, func) { return COLLECTION(collection, func).all().toArray(); } - return COLLECTION(collection, func).ALL(0, null).documents; + return COLLECTION(collection, func).ALL().documents; } // ////////////////////////////////////////////////////////////////////////////// diff --git a/js/server/modules/@arangodb/arango-collection.js b/js/server/modules/@arangodb/arango-collection.js index ae2fe22e3ea4..eac385e312cc 100644 --- a/js/server/modules/@arangodb/arango-collection.js +++ b/js/server/modules/@arangodb/arango-collection.js @@ -120,7 +120,7 @@ ArangoCollection.prototype.toArray = function () { return this.all().toArray(); } - return this.ALL(null, null).documents; + return this.ALL().documents; }; // //////////////////////////////////////////////////////////////////////////////