From 3b79b6eda4b6b738caafbb166cd043bbc99b137b Mon Sep 17 00:00:00 2001 From: jsteemann Date: Thu, 9 Jun 2016 10:13:01 +0200 Subject: [PATCH] micro optimizations --- arangod/Aql/Condition.cpp | 46 ++++++++++++++----------- arangod/Aql/Condition.h | 5 +-- arangod/Aql/ShortestPathBlock.cpp | 14 +++++--- arangod/Aql/SortBlock.cpp | 4 ++- arangod/Aql/SortCondition.cpp | 4 ++- arangod/Aql/TraversalBlock.cpp | 20 +++++------ arangod/Indexes/HashIndex.cpp | 3 +- arangod/Indexes/SkiplistIndex.cpp | 57 ++++++++++++++++--------------- arangod/VocBase/transaction.cpp | 4 ++- arangod/Wal/CollectorThread.cpp | 4 +-- 10 files changed, 90 insertions(+), 71 deletions(-) diff --git a/arangod/Aql/Condition.cpp b/arangod/Aql/Condition.cpp index bef2702b21..193417e886 100644 --- a/arangod/Aql/Condition.cpp +++ b/arangod/Aql/Condition.cpp @@ -264,6 +264,13 @@ bool ConditionPart::isCoveredBy(ConditionPart const& other) const { return false; } +/// @brief clears the attribute access data +static inline void clearAttributeAccess( + std::pair>& parts) { + parts.first = nullptr; + parts.second.clear(); +} + /// @brief create the condition Condition::Condition(Ast* ast) : _ast(ast), _root(nullptr), _isNormalized(false), _isSorted(false) {} @@ -377,6 +384,7 @@ std::vector> Condition::getConstAtt return result; } + std::pair> parts; AstNode const* node = _root->getMember(0); n = node->numMembers(); @@ -384,7 +392,7 @@ std::vector> Condition::getConstAtt auto member = node->getMember(i); if (member->type == NODE_TYPE_OPERATOR_BINARY_EQ) { - std::pair> parts; + clearAttributeAccess(parts); auto lhs = member->getMember(0); auto rhs = member->getMember(1); @@ -470,6 +478,7 @@ AstNode* Condition::removeIndexCondition(Variable const* variable, TRI_ASSERT(andNode->type == NODE_TYPE_OPERATOR_NARY_AND); size_t const n = andNode->numMembers(); + std::pair> result; std::unordered_set toRemove; for (size_t i = 0; i < n; ++i) { @@ -482,8 +491,7 @@ AstNode* Condition::removeIndexCondition(Variable const* variable, auto rhs = operand->getMember(1); if (lhs->type == NODE_TYPE_ATTRIBUTE_ACCESS) { - std::pair> - result; + clearAttributeAccess(result); if (lhs->isAttributeAccessForVariable(result) && result.first == variable) { @@ -498,8 +506,7 @@ AstNode* Condition::removeIndexCondition(Variable const* variable, if (rhs->type == NODE_TYPE_ATTRIBUTE_ACCESS || rhs->type == NODE_TYPE_EXPANSION) { - std::pair> - result; + clearAttributeAccess(result); if (rhs->isAttributeAccessForVariable(result) && result.first == variable) { @@ -599,6 +606,8 @@ void Condition::optimize(ExecutionPlan* plan) { TRI_ASSERT(_root != nullptr); TRI_ASSERT(_root->type == NODE_TYPE_OPERATOR_NARY_OR); + + std::pair> varAccess; // handle sub nodes of top-level OR node size_t n = _root->numMembers(); @@ -673,11 +682,11 @@ void Condition::optimize(ExecutionPlan* plan) { auto rhs = operand->getMember(1); if (lhs->type == NODE_TYPE_ATTRIBUTE_ACCESS) { - storeAttributeAccess(variableUsage, lhs, j, ATTRIBUTE_LEFT); + storeAttributeAccess(varAccess, variableUsage, lhs, j, ATTRIBUTE_LEFT); } if (rhs->type == NODE_TYPE_ATTRIBUTE_ACCESS || rhs->type == NODE_TYPE_EXPANSION) { - storeAttributeAccess(variableUsage, rhs, j, ATTRIBUTE_RIGHT); + storeAttributeAccess(varAccess, variableUsage, rhs, j, ATTRIBUTE_RIGHT); } } } @@ -841,17 +850,17 @@ void Condition::optimize(ExecutionPlan* plan) { } /// @brief registers an attribute access for a particular (collection) variable -void Condition::storeAttributeAccess(VariableUsageType& variableUsage, - AstNode const* node, size_t position, - AttributeSideType side) { - std::pair> - result; +void Condition::storeAttributeAccess( + std::pair>& varAccess, + VariableUsageType& variableUsage, + AstNode const* node, size_t position, + AttributeSideType side) { - if (!node->isAttributeAccessForVariable(result)) { + if (!node->isAttributeAccessForVariable(varAccess)) { return; } - auto variable = result.first; + auto variable = varAccess.first; if (variable != nullptr) { auto it = variableUsage.find(variable); @@ -862,7 +871,7 @@ void Condition::storeAttributeAccess(VariableUsageType& variableUsage, } std::string attributeName; - TRI_AttributeNamesToString(result.second, attributeName, false); + TRI_AttributeNamesToString(varAccess.second, attributeName, false); auto it2 = (*it).second.find(attributeName); @@ -914,6 +923,7 @@ bool Condition::canRemove(ConditionPart const& me, TRI_ASSERT(otherCondition != nullptr); TRI_ASSERT(otherCondition->type == NODE_TYPE_OPERATOR_NARY_OR); + std::pair> result; auto andNode = otherCondition->getMemberUnchecked(0); TRI_ASSERT(andNode->type == NODE_TYPE_OPERATOR_NARY_AND); size_t const n = andNode->numMembers(); @@ -926,8 +936,7 @@ bool Condition::canRemove(ConditionPart const& me, auto rhs = operand->getMember(1); if (lhs->type == NODE_TYPE_ATTRIBUTE_ACCESS) { - std::pair> - result; + clearAttributeAccess(result); if (lhs->isAttributeAccessForVariable(result)) { if (rhs->isConstant()) { @@ -948,8 +957,7 @@ bool Condition::canRemove(ConditionPart const& me, if (rhs->type == NODE_TYPE_ATTRIBUTE_ACCESS || rhs->type == NODE_TYPE_EXPANSION) { - std::pair> - result; + clearAttributeAccess(result); if (rhs->isAttributeAccessForVariable(result)) { if (lhs->isConstant()) { diff --git a/arangod/Aql/Condition.h b/arangod/Aql/Condition.h index 81d2511d0c..d0ed62e4a5 100644 --- a/arangod/Aql/Condition.h +++ b/arangod/Aql/Condition.h @@ -252,8 +252,9 @@ class Condition { /// @brief registers an attribute access for a particular (collection) /// variable - void storeAttributeAccess(VariableUsageType&, AstNode const*, size_t, - AttributeSideType); + void storeAttributeAccess( + std::pair>& varAccess, + VariableUsageType&, AstNode const*, size_t, AttributeSideType); /// @brief validate the condition's AST #ifdef ARANGODB_ENABLE_MAINTAINER_MODE diff --git a/arangod/Aql/ShortestPathBlock.cpp b/arangod/Aql/ShortestPathBlock.cpp index ab594d236d..4f8f37c033 100644 --- a/arangod/Aql/ShortestPathBlock.cpp +++ b/arangod/Aql/ShortestPathBlock.cpp @@ -211,6 +211,7 @@ struct EdgeWeightExpanderLocal { std::vector& result) { std::vector cursor; std::shared_ptr edgeCursor; + std::unordered_map candidates; for (auto const& edgeCollection : _block->_collectionInfos) { TRI_ASSERT(edgeCollection != nullptr); if (_reverse) { @@ -218,7 +219,8 @@ struct EdgeWeightExpanderLocal { } else { edgeCursor = edgeCollection->getEdges(source); } - std::unordered_map candidates; + + candidates.clear(); auto inserter = [&](VPackSlice const& s, VPackSlice const& t, double currentWeight, VPackSlice edge) { @@ -279,6 +281,8 @@ struct EdgeWeightExpanderCluster { void operator()(VPackSlice const& source, std::vector& result) { int res = TRI_ERROR_NO_ERROR; + std::unordered_map candidates; + for (auto const& edgeCollection : _block->_collectionInfos) { TRI_ASSERT(edgeCollection != nullptr); VPackBuilder edgesBuilder; @@ -292,7 +296,7 @@ struct EdgeWeightExpanderCluster { THROW_ARANGO_EXCEPTION(res); } - std::unordered_map candidates; + candidates.clear(); auto inserter = [&](VPackSlice const& s, VPackSlice const& t, double currentWeight, VPackSlice const& edge) { @@ -300,7 +304,7 @@ struct EdgeWeightExpanderCluster { if (cand == candidates.end()) { // Add weight auto step = std::make_unique( - t, s, currentWeight, std::move(edge)); + t, s, currentWeight, edge); result.emplace_back(step.release()); candidates.emplace(t, result.size() - 1); } else { @@ -492,8 +496,7 @@ bool ShortestPathBlock::nextPath(AqlItemBlock const* items) { AqlValue const& in = items->getValueReference(_pos, _startReg); if (in.isObject()) { try { - std::string idString = _trx->extractIdString(in.slice()); - _opts.setStart(idString); + _opts.setStart(_trx->extractIdString(in.slice())); } catch (...) { // _id or _key not present... ignore this error and fall through @@ -610,6 +613,7 @@ AqlItemBlock* ShortestPathBlock::getSome(size_t, size_t atMost) { inheritRegisters(cur, res.get(), _pos); // TODO this might be optimized in favor of direct mptr. + // TODO: lease builder? VPackBuilder resultBuilder; for (size_t j = 0; j < toSend; j++) { if (j > 0) { diff --git a/arangod/Aql/SortBlock.cpp b/arangod/Aql/SortBlock.cpp index b35c585af2..3734172794 100644 --- a/arangod/Aql/SortBlock.cpp +++ b/arangod/Aql/SortBlock.cpp @@ -109,6 +109,8 @@ void SortBlock::doSorting() { count = 0; RegisterId const nrregs = _buffer.front()->getNrRegs(); + std::unordered_map cache; + // install the rearranged values from _buffer into newbuffer while (count < sum) { @@ -125,7 +127,7 @@ void SortBlock::doSorting() { throw; } - std::unordered_map cache; + cache.clear(); // only copy as much as needed! for (size_t i = 0; i < sizeNext; i++) { for (RegisterId j = 0; j < nrregs; j++) { diff --git a/arangod/Aql/SortCondition.cpp b/arangod/Aql/SortCondition.cpp index 0159681ced..81b1af2cb4 100644 --- a/arangod/Aql/SortCondition.cpp +++ b/arangod/Aql/SortCondition.cpp @@ -60,6 +60,7 @@ SortCondition::SortCondition( bool foundDirection = false; + std::vector fieldNames; size_t const n = sorts.size(); for (size_t i = 0; i < n; ++i) { @@ -73,7 +74,8 @@ SortCondition::SortCondition( auto node = (*it).second; if (node != nullptr && node->type == NODE_TYPE_ATTRIBUTE_ACCESS) { - std::vector fieldNames; + fieldNames.clear(); + while (node->type == NODE_TYPE_ATTRIBUTE_ACCESS) { fieldNames.emplace_back( arangodb::basics::AttributeName(node->getString())); diff --git a/arangod/Aql/TraversalBlock.cpp b/arangod/Aql/TraversalBlock.cpp index 2831964fd9..9196ca2098 100644 --- a/arangod/Aql/TraversalBlock.cpp +++ b/arangod/Aql/TraversalBlock.cpp @@ -269,7 +269,7 @@ bool TraversalBlock::morePaths(size_t hint) { return false; } - VPackBuilder tmp; + TransactionBuilderLeaser tmp(_trx); for (size_t j = 0; j < hint; ++j) { std::unique_ptr p(_traverser->next()); @@ -279,19 +279,19 @@ bool TraversalBlock::morePaths(size_t hint) { } if (usesVertexOutput()) { - tmp.clear(); - p->lastVertexToVelocyPack(_trx, tmp); - _vertices.emplace_back(tmp.slice()); + tmp->clear(); + p->lastVertexToVelocyPack(_trx, *tmp.builder()); + _vertices.emplace_back(tmp->slice()); } if (usesEdgeOutput()) { - tmp.clear(); - p->lastEdgeToVelocyPack(_trx, tmp); - _edges.emplace_back(tmp.slice()); + tmp->clear(); + p->lastEdgeToVelocyPack(_trx, *tmp.builder()); + _edges.emplace_back(tmp->slice()); } if (usesPathOutput()) { - tmp.clear(); - p->pathToVelocyPack(_trx, tmp); - _paths.emplace_back(tmp.slice()); + tmp->clear(); + p->pathToVelocyPack(_trx, *tmp.builder()); + _paths.emplace_back(tmp->slice()); } _engine->_stats.scannedIndex += p->getReadDocuments(); diff --git a/arangod/Indexes/HashIndex.cpp b/arangod/Indexes/HashIndex.cpp index 1fa6164366..8f042dc29c 100644 --- a/arangod/Indexes/HashIndex.cpp +++ b/arangod/Indexes/HashIndex.cpp @@ -50,7 +50,6 @@ LookupBuilder::LookupBuilder( std::vector> paramPair; std::vector storageOrder; - for (size_t i = 0; i < _coveredFields; ++i) { auto comp = node->getMemberUnchecked(i); auto attrNode = comp->getMember(0); @@ -629,6 +628,7 @@ int HashIndex::lookup(arangodb::Transaction* trx, return TRI_ERROR_NO_ERROR; } + // TODO: optimize this copying! std::vector results; try { _multiArray->_hashArray->lookupByKey(trx, &key, results); @@ -777,6 +777,7 @@ int HashIndex::batchInsertMulti( arangodb::Transaction* trx, std::vector const* documents, size_t numThreads) { std::vector elements; + elements.reserve(documents->size()); for (auto& doc : *documents) { int res = fillElement(elements, doc); diff --git a/arangod/Indexes/SkiplistIndex.cpp b/arangod/Indexes/SkiplistIndex.cpp index f4d569b466..3a6d4f4df8 100644 --- a/arangod/Indexes/SkiplistIndex.cpp +++ b/arangod/Indexes/SkiplistIndex.cpp @@ -347,7 +347,6 @@ SkiplistIterator* SkiplistIndex::lookup(arangodb::Transaction* trx, TRI_ASSERT(searchValues.isArray()); TRI_ASSERT(searchValues.length() <= _fields.size()); - VPackBuilder leftSearch; VPackSlice lastNonEq; leftSearch.openArray(); @@ -831,19 +830,21 @@ IndexIterator* SkiplistIndex::iteratorForCondition( arangodb::Transaction* trx, IndexIteratorContext* context, arangodb::aql::AstNode const* node, arangodb::aql::Variable const* reference, bool reverse) const { - VPackBuilder searchValues; - searchValues.openArray(); + + TransactionBuilderLeaser searchValues(trx); + searchValues->openArray(); bool needNormalize = false; if (node == nullptr) { // We only use this index for sort. Empty searchValue - VPackArrayBuilder guard(&searchValues); + searchValues->openArray(); + searchValues->close(); TRI_IF_FAILURE("SkiplistIndex::noSortIterator") { THROW_ARANGO_EXCEPTION(TRI_ERROR_DEBUG); } } else { // Create the search Values for the lookup - VPackArrayBuilder guard(&searchValues); + VPackArrayBuilder guard(searchValues.builder()); std::unordered_map> found; size_t unused = 0; @@ -896,30 +897,30 @@ IndexIterator* SkiplistIndex::iteratorForCondition( // We found an access for this field if (comp->type == arangodb::aql::NODE_TYPE_OPERATOR_BINARY_EQ) { - searchValues.openObject(); - searchValues.add(VPackValue(TRI_SLICE_KEY_EQUAL)); + searchValues->openObject(); + searchValues->add(VPackValue(TRI_SLICE_KEY_EQUAL)); TRI_IF_FAILURE("SkiplistIndex::permutationEQ") { THROW_ARANGO_EXCEPTION(TRI_ERROR_DEBUG); } } else if (comp->type == arangodb::aql::NODE_TYPE_OPERATOR_BINARY_IN) { if (isAttributeExpanded(usedFields)) { - searchValues.openObject(); - searchValues.add(VPackValue(TRI_SLICE_KEY_EQUAL)); + searchValues->openObject(); + searchValues->add(VPackValue(TRI_SLICE_KEY_EQUAL)); TRI_IF_FAILURE("SkiplistIndex::permutationArrayIN") { THROW_ARANGO_EXCEPTION(TRI_ERROR_DEBUG); } } else { needNormalize = true; - searchValues.openObject(); - searchValues.add(VPackValue(TRI_SLICE_KEY_IN)); + searchValues->openObject(); + searchValues->add(VPackValue(TRI_SLICE_KEY_IN)); } } else { // This is a one-sided range break; } // We have to add the value always, the key was added before - value->toVelocyPackValue(searchValues); - searchValues.close(); + value->toVelocyPackValue(*searchValues.builder()); + searchValues->close(); } // Now handle the next element, which might be a range @@ -928,7 +929,7 @@ IndexIterator* SkiplistIndex::iteratorForCondition( if (it != found.end()) { auto rangeConditions = it->second; TRI_ASSERT(rangeConditions.size() <= 2); - VPackObjectBuilder searchElement(&searchValues); + VPackObjectBuilder searchElement(searchValues.builder()); for (auto& comp : rangeConditions) { TRI_ASSERT(comp->numMembers() == 2); arangodb::aql::AstNode const* access = nullptr; @@ -938,30 +939,30 @@ IndexIterator* SkiplistIndex::iteratorForCondition( switch (comp->type) { case arangodb::aql::NODE_TYPE_OPERATOR_BINARY_LT: if (isReverseOrder) { - searchValues.add(VPackValue(TRI_SLICE_KEY_GT)); + searchValues->add(VPackValue(TRI_SLICE_KEY_GT)); } else { - searchValues.add(VPackValue(TRI_SLICE_KEY_LT)); + searchValues->add(VPackValue(TRI_SLICE_KEY_LT)); } break; case arangodb::aql::NODE_TYPE_OPERATOR_BINARY_LE: if (isReverseOrder) { - searchValues.add(VPackValue(TRI_SLICE_KEY_GE)); + searchValues->add(VPackValue(TRI_SLICE_KEY_GE)); } else { - searchValues.add(VPackValue(TRI_SLICE_KEY_LE)); + searchValues->add(VPackValue(TRI_SLICE_KEY_LE)); } break; case arangodb::aql::NODE_TYPE_OPERATOR_BINARY_GT: if (isReverseOrder) { - searchValues.add(VPackValue(TRI_SLICE_KEY_LT)); + searchValues->add(VPackValue(TRI_SLICE_KEY_LT)); } else { - searchValues.add(VPackValue(TRI_SLICE_KEY_GT)); + searchValues->add(VPackValue(TRI_SLICE_KEY_GT)); } break; case arangodb::aql::NODE_TYPE_OPERATOR_BINARY_GE: if (isReverseOrder) { - searchValues.add(VPackValue(TRI_SLICE_KEY_LE)); + searchValues->add(VPackValue(TRI_SLICE_KEY_LE)); } else { - searchValues.add(VPackValue(TRI_SLICE_KEY_GE)); + searchValues->add(VPackValue(TRI_SLICE_KEY_GE)); } break; default: @@ -970,21 +971,21 @@ IndexIterator* SkiplistIndex::iteratorForCondition( TRI_ASSERT(false); return nullptr; } - value->toVelocyPackValue(searchValues); + value->toVelocyPackValue(*searchValues.builder()); } } } } - searchValues.close(); + searchValues->close(); TRI_IF_FAILURE("SkiplistIndex::noIterator") { THROW_ARANGO_EXCEPTION(TRI_ERROR_DEBUG); } if (needNormalize) { - VPackBuilder expandedSearchValues; - expandInSearchValues(searchValues.slice(), expandedSearchValues); - VPackSlice expandedSlice = expandedSearchValues.slice(); + TransactionBuilderLeaser expandedSearchValues(trx); + expandInSearchValues(searchValues->slice(), *expandedSearchValues.builder()); + VPackSlice expandedSlice = expandedSearchValues->slice(); std::vector iterators; try { for (auto const& val : VPackArrayIterator(expandedSlice)) { @@ -1009,7 +1010,7 @@ IndexIterator* SkiplistIndex::iteratorForCondition( } return new MultiIndexIterator(iterators); } - VPackSlice searchSlice = searchValues.slice(); + VPackSlice searchSlice = searchValues->slice(); TRI_ASSERT(searchSlice.length() == 1); searchSlice = searchSlice.at(0); return lookup(trx, searchSlice, reverse); diff --git a/arangod/VocBase/transaction.cpp b/arangod/VocBase/transaction.cpp index 5c47e1f7d1..fa9778b054 100644 --- a/arangod/VocBase/transaction.cpp +++ b/arangod/VocBase/transaction.cpp @@ -176,6 +176,8 @@ static char const* StatusTransaction(const TRI_transaction_status_e status) { static void FreeOperations(TRI_transaction_t* trx) { bool const mustRollback = (trx->_status == TRI_TRANSACTION_ABORTED); bool const isSingleOperation = IsSingleOperationTransaction(trx); + + std::unordered_map> stats; for (auto& trxCollection : trx->_collections) { if (trxCollection->_operations == nullptr) { @@ -196,7 +198,7 @@ static void FreeOperations(TRI_transaction_t* trx) { } else { // update datafile statistics for all operations // pair (number of dead markers, size of dead markers) - std::unordered_map> stats; + stats.clear(); for (auto it = trxCollection->_operations->rbegin(); it != trxCollection->_operations->rend(); ++it) { diff --git a/arangod/Wal/CollectorThread.cpp b/arangod/Wal/CollectorThread.cpp index 24a1bfe091..7d64d773f5 100644 --- a/arangod/Wal/CollectorThread.cpp +++ b/arangod/Wal/CollectorThread.cpp @@ -973,9 +973,7 @@ int CollectorThread::queueOperations(arangodb::wal::Logfile* logfile, // it is only safe to access the queue if this flag is not set auto it = _operationsQueue.find(cid); if (it == _operationsQueue.end()) { - std::vector ops; - ops.push_back(cache); - _operationsQueue.emplace(cid, ops); + _operationsQueue.emplace(cid, std::vector({cache})); _logfileManager->increaseCollectQueueSize(logfile); } else { (*it).second.push_back(cache);