diff --git a/arangod/Aql/HashedCollectExecutor.cpp b/arangod/Aql/HashedCollectExecutor.cpp index 94585c2e40..cfd6757aa2 100644 --- a/arangod/Aql/HashedCollectExecutor.cpp +++ b/arangod/Aql/HashedCollectExecutor.cpp @@ -99,6 +99,7 @@ HashedCollectExecutor::HashedCollectExecutor(Fetcher& fetcher, Infos& infos) _aggregatorFactories(), _returnedGroups(0) { _aggregatorFactories = createAggregatorFactories(_infos); + _nextGroupValues.reserve(_infos.getGroupRegisters().size()); }; HashedCollectExecutor::~HashedCollectExecutor() { @@ -119,8 +120,7 @@ void HashedCollectExecutor::destroyAllGroupsAqlValues() { void HashedCollectExecutor::consumeInputRow(InputAqlItemRow& input) { TRI_ASSERT(input.isInitialized()); - decltype(_allGroups)::iterator currentGroupIt; - currentGroupIt = findOrEmplaceGroup(input); + decltype(_allGroups)::iterator currentGroupIt = findOrEmplaceGroup(input); // reduce the aggregates AggregateValuesType* aggregateValues = currentGroupIt->second.get(); @@ -209,7 +209,8 @@ ExecutionState HashedCollectExecutor::init() { // initialize group iterator for output _currentGroup = _allGroups.begin(); - + // The values within are not supposed to be used anymore. + _nextGroupValues.clear(); return ExecutionState::DONE; } @@ -242,58 +243,50 @@ std::pair HashedCollectExecutor::produceRows(OutputAqlI return {state, NoStats{}}; } -// if no group exists for the current row yet, this builds a new group. -std::pair, std::vector> -HashedCollectExecutor::buildNewGroup(InputAqlItemRow& input, size_t n) { - GroupKeyType group; - group.reserve(n); - - // copy the group values before they get invalidated - for (size_t i = 0; i < n; ++i) { - group.emplace_back(input.stealValue(_infos.getGroupRegisters()[i].second)); - } - - auto aggregateValues = std::make_unique(); - aggregateValues->reserve(_aggregatorFactories.size()); - - for (auto const& it : _aggregatorFactories) { - aggregateValues->emplace_back((*it)(_infos.getTransaction())); - } - return std::make_pair(std::move(aggregateValues), group); -} - // finds the group matching the current row, or emplaces it. in either case, // it returns an iterator to the group matching the current row in // _allGroups. additionally, .second is true iff a new group was emplaced. decltype(HashedCollectExecutor::_allGroups)::iterator HashedCollectExecutor::findOrEmplaceGroup( InputAqlItemRow& input) { - GroupKeyType groupValues; // TODO store groupValues locally - size_t const n = _infos.getGroupRegisters().size(); - groupValues.reserve(n); + _nextGroupValues.clear(); // for hashing simply re-use the aggregate registers, without cloning // their contents - for (size_t i = 0; i < n; ++i) { - groupValues.emplace_back(input.getValue(_infos.getGroupRegisters()[i].second)); + for (auto const& reg : _infos.getGroupRegisters()) { + _nextGroupValues.emplace_back(input.getValue(reg.second)); } - auto it = _allGroups.find(groupValues); + auto it = _allGroups.find(_nextGroupValues); if (it != _allGroups.end()) { // group already exists return it; } - // must create new group - GroupValueType aggregateValues; - GroupKeyType group; - std::tie(aggregateValues, group) = buildNewGroup(input, n); + _nextGroupValues.clear(); + // for inserting into group we need to clone the values + // and take over ownership + for (auto const& reg : _infos.getGroupRegisters()) { + _nextGroupValues.emplace_back(input.stealValue(reg.second)); + } + // this builds a new group with aggregate functions being prepared. + auto aggregateValues = std::make_unique(); + aggregateValues->reserve(_aggregatorFactories.size()); + auto trx = _infos.getTransaction(); + for (auto const& it : _aggregatorFactories) { + aggregateValues->emplace_back((*it)(trx)); + } // note: aggregateValues may be a nullptr! - auto emplaceResult = _allGroups.emplace(group, std::move(aggregateValues)); + auto emplaceResult = + _allGroups.emplace(std::move(_nextGroupValues), std::move(aggregateValues)); // emplace must not fail TRI_ASSERT(emplaceResult.second); + // Moving _nextGroupValues left us with an empty vector of minimum capacity. + // So in order to have correct capacity reserve again. + _nextGroupValues.reserve(_infos.getGroupRegisters().size()); + return emplaceResult.first; }; diff --git a/arangod/Aql/HashedCollectExecutor.h b/arangod/Aql/HashedCollectExecutor.h index 1e544878f4..16e1b3885b 100644 --- a/arangod/Aql/HashedCollectExecutor.h +++ b/arangod/Aql/HashedCollectExecutor.h @@ -158,8 +158,6 @@ class HashedCollectExecutor { static std::vector(transaction::Methods*)> const*> createAggregatorFactories(HashedCollectExecutor::Infos const& infos); - std::pair buildNewGroup(InputAqlItemRow& input, size_t n); - GroupMapType::iterator findOrEmplaceGroup(InputAqlItemRow& input); void consumeInputRow(InputAqlItemRow& input); @@ -185,6 +183,8 @@ class HashedCollectExecutor { std::vector(transaction::Methods*)> const*> _aggregatorFactories; size_t _returnedGroups; + + GroupKeyType _nextGroupValues; }; } // namespace aql