diff --git a/arangod/Aql/AqlValue.h b/arangod/Aql/AqlValue.h index 0eeec91061..e0050061b7 100644 --- a/arangod/Aql/AqlValue.h +++ b/arangod/Aql/AqlValue.h @@ -161,6 +161,7 @@ struct AqlValue final { /// RANGE: a managed range object. The memory is managed by the AqlValue private: union { + uint64_t words[2]; uint8_t internal[16]; uint8_t const* pointer; uint8_t* slice; @@ -172,10 +173,14 @@ struct AqlValue final { public: // construct an empty AqlValue // note: this is the default constructor and should be as cheap as possible - AqlValue() noexcept { + inline AqlValue() noexcept { // construct a slice of type None - _data.internal[0] = '\x00'; - setType(AqlValueType::VPACK_INLINE); + // we will simply zero-initialize the two 64 bit words + _data.words[0] = 0; + _data.words[1] = 0; + + // VPACK_INLINE must have a value of 0, and VPackSlice::None must be equal to a NUL byte too + static_assert(AqlValueType::VPACK_INLINE == 0, "invalid value for VPACK_INLINE"); } // construct from pointer, not copying! diff --git a/arangod/Aql/ModificationBlocks.cpp b/arangod/Aql/ModificationBlocks.cpp index 96c8cccfd4..8d652a633f 100644 --- a/arangod/Aql/ModificationBlocks.cpp +++ b/arangod/Aql/ModificationBlocks.cpp @@ -270,8 +270,6 @@ AqlItemBlock* RemoveBlock::work(std::vector& blocks) { result.reset(requestBlock(count, getPlanNode()->getRegisterPlan()->nrRegs[getPlanNode()->getDepth()])); - VPackBuilder keyBuilder; - OperationOptions options; options.silent = !producesOutput; options.waitForSync = ep->_options.waitForSync; @@ -288,10 +286,10 @@ AqlItemBlock* RemoveBlock::work(std::vector& blocks) { size_t const n = res->size(); bool isMultiple = (n > 1); - keyBuilder.clear(); + _tempBuilder.clear(); if (isMultiple) { // If we use multiple API we send an array - keyBuilder.openArray(); + _tempBuilder.openArray(); } std::string key; @@ -322,9 +320,9 @@ AqlItemBlock* RemoveBlock::work(std::vector& blocks) { if (errorCode == TRI_ERROR_NO_ERROR) { // no error. we expect to have a key // create a slice for the key - keyBuilder.openObject(); - keyBuilder.add(StaticStrings::KeyString, VPackValue(key)); - keyBuilder.close(); + _tempBuilder.openObject(); + _tempBuilder.add(StaticStrings::KeyString, VPackValue(key)); + _tempBuilder.close(); } else { // We have an error, handle it handleResult(errorCode, ep->_options.ignoreErrors, nullptr); @@ -334,9 +332,9 @@ AqlItemBlock* RemoveBlock::work(std::vector& blocks) { if (isMultiple) { // We have to close the array - keyBuilder.close(); + _tempBuilder.close(); } - VPackSlice toRemove = keyBuilder.slice(); + VPackSlice toRemove = _tempBuilder.slice(); if (!toRemove.isNone() && !(toRemove.isArray() && toRemove.length() == 0)) { @@ -432,12 +430,12 @@ AqlItemBlock* InsertBlock::work(std::vector& blocks) { result.reset(requestBlock(count, getPlanNode()->getRegisterPlan()->nrRegs[getPlanNode()->getDepth()])); OperationOptions options; + // use "silent" mode if we do not access the results later on options.silent = !producesOutput; options.waitForSync = ep->_options.waitForSync; options.returnNew = producesOutput; options.isRestore = ep->getOptions().useIsRestore; - - VPackBuilder babyBuilder; + // loop over all blocks size_t dstRow = 0; for (auto it = blocks.begin(); it != blocks.end(); ++it) { @@ -484,8 +482,8 @@ AqlItemBlock* InsertBlock::work(std::vector& blocks) { } // done with a block } else { - babyBuilder.clear(); - babyBuilder.openArray(); + _tempBuilder.clear(); + _tempBuilder.openArray(); for (size_t i = 0; i < n; ++i) { AqlValue const& a = res->getValueReference(i, registerId); @@ -494,12 +492,12 @@ AqlItemBlock* InsertBlock::work(std::vector& blocks) { // TODO This may be optimized with externals if (!ep->_options.consultAqlWriteFilter || !_collection->getCollection()->skipForAqlWrite(a.slice(), "")) { - babyBuilder.add(a.slice()); + _tempBuilder.add(a.slice()); } ++dstRow; } - babyBuilder.close(); - VPackSlice toSend = babyBuilder.slice(); + _tempBuilder.close(); + VPackSlice toSend = _tempBuilder.slice(); OperationResult opRes; if (toSend.length() > 0) { opRes = _trx->insert(_collection->name, toSend, options); @@ -565,7 +563,6 @@ AqlItemBlock* UpdateBlock::work(std::vector& blocks) { bool const hasKeyVariable = (ep->_inKeyVariable != nullptr); std::string errorMessage; - VPackBuilder keyBuilder; VPackBuilder object; if (hasKeyVariable) { @@ -635,13 +632,13 @@ AqlItemBlock* UpdateBlock::work(std::vector& blocks) { !_collection->getCollection()->skipForAqlWrite(a.slice(), key)) { wasTaken.push_back(true); if (hasKeyVariable) { - keyBuilder.clear(); - keyBuilder.openObject(); - keyBuilder.add(StaticStrings::KeyString, VPackValue(key)); - keyBuilder.close(); + _tempBuilder.clear(); + _tempBuilder.openObject(); + _tempBuilder.add(StaticStrings::KeyString, VPackValue(key)); + _tempBuilder.close(); VPackBuilder tmp = VPackCollection::merge( - a.slice(), keyBuilder.slice(), false, false); + a.slice(), _tempBuilder.slice(), false, false); if (isMultiple) { object.add(tmp.slice()); } else { @@ -786,7 +783,6 @@ AqlItemBlock* UpsertBlock::work(std::vector& blocks) { options.ignoreRevs = true; options.isRestore = ep->getOptions().useIsRestore; - VPackBuilder keyBuilder; VPackBuilder insertBuilder; VPackBuilder updateBuilder; @@ -844,16 +840,16 @@ AqlItemBlock* UpsertBlock::work(std::vector& blocks) { tookThis = true; VPackSlice toUpdate = updateDoc.slice(); - keyBuilder.clear(); - keyBuilder.openObject(); - keyBuilder.add(StaticStrings::KeyString, VPackValue(key)); - keyBuilder.close(); + _tempBuilder.clear(); + _tempBuilder.openObject(); + _tempBuilder.add(StaticStrings::KeyString, VPackValue(key)); + _tempBuilder.close(); if (isMultiple) { - VPackBuilder tmp = VPackCollection::merge(toUpdate, keyBuilder.slice(), false, false); + VPackBuilder tmp = VPackCollection::merge(toUpdate, _tempBuilder.slice(), false, false); updateBuilder.add(tmp.slice()); upRows.emplace_back(dstRow); } else { - updateBuilder = VPackCollection::merge(toUpdate, keyBuilder.slice(), false, false); + updateBuilder = VPackCollection::merge(toUpdate, _tempBuilder.slice(), false, false); } } else { errorCode = TRI_ERROR_ARANGO_DOCUMENT_TYPE_INVALID; @@ -1021,7 +1017,6 @@ AqlItemBlock* ReplaceBlock::work(std::vector& blocks) { bool const hasKeyVariable = (ep->_inKeyVariable != nullptr); std::string errorMessage; - VPackBuilder keyBuilder; VPackBuilder object; if (hasKeyVariable) { @@ -1091,12 +1086,12 @@ AqlItemBlock* ReplaceBlock::work(std::vector& blocks) { !_collection->getCollection()->skipForAqlWrite(a.slice(), key)) { wasTaken.push_back(true); if (hasKeyVariable) { - keyBuilder.clear(); - keyBuilder.openObject(); - keyBuilder.add(StaticStrings::KeyString, VPackValue(key)); - keyBuilder.close(); + _tempBuilder.clear(); + _tempBuilder.openObject(); + _tempBuilder.add(StaticStrings::KeyString, VPackValue(key)); + _tempBuilder.close(); VPackBuilder tmp = VPackCollection::merge( - a.slice(), keyBuilder.slice(), false, false); + a.slice(), _tempBuilder.slice(), false, false); if (isMultiple) { object.add(tmp.slice()); } else { diff --git a/arangod/Aql/ModificationBlocks.h b/arangod/Aql/ModificationBlocks.h index 07565353d7..3f11a95a40 100644 --- a/arangod/Aql/ModificationBlocks.h +++ b/arangod/Aql/ModificationBlocks.h @@ -73,6 +73,10 @@ class ModificationBlock : public ExecutionBlock { /// @brief whether or not the collection uses the default sharding attributes bool _usesDefaultSharding; + + protected: + /// @brief a Builder object, reused for various tasks to save a few memory allocations + velocypack::Builder _tempBuilder; }; class RemoveBlock : public ModificationBlock { diff --git a/arangod/Aql/Query.cpp b/arangod/Aql/Query.cpp index 9efa2faf7f..eb6334bfc8 100644 --- a/arangod/Aql/Query.cpp +++ b/arangod/Aql/Query.cpp @@ -615,8 +615,7 @@ QueryResult Query::execute(QueryRegistry* registry) { val.toVelocyPack(_trx, *resultBuilder, true); } } - delete value; - value = nullptr; + _engine->_itemBlockManager.returnBlock(value); } // must close result array here because it must be passed as a closed @@ -646,8 +645,7 @@ QueryResult Query::execute(QueryRegistry* registry) { val.toVelocyPack(_trx, *resultBuilder, false); } } - delete value; - value = nullptr; + _engine->_itemBlockManager.returnBlock(value); } // must close result array @@ -820,8 +818,7 @@ QueryResultV8 Query::executeV8(v8::Isolate* isolate, QueryRegistry* registry) { val.toVelocyPack(_trx, *builder, true); } } - delete value; - value = nullptr; + _engine->_itemBlockManager.returnBlock(value); } builder->close(); @@ -851,8 +848,7 @@ QueryResultV8 Query::executeV8(v8::Isolate* isolate, QueryRegistry* registry) { } } } - delete value; - value = nullptr; + _engine->_itemBlockManager.returnBlock(value); } } } catch (...) { diff --git a/arangod/RestHandler/RestCursorHandler.cpp b/arangod/RestHandler/RestCursorHandler.cpp index 3f7042ddf1..3879cc596f 100644 --- a/arangod/RestHandler/RestCursorHandler.cpp +++ b/arangod/RestHandler/RestCursorHandler.cpp @@ -218,24 +218,19 @@ void RestCursorHandler::processQuery(VPackSlice const& slice) { Cursor* cursor = cursors->createFromQueryResult( std::move(queryResult), batchSize, extra, ttl, count); - try { - VPackBuffer buffer; - VPackBuilder result(buffer); - result.openObject(); - result.add(StaticStrings::Error, VPackValue(false)); - result.add(StaticStrings::Code, VPackValue(static_cast(_response->responseCode()))); - cursor->dump(result); - result.close(); + TRI_DEFER(cursors->release(cursor)); + + VPackBuffer buffer; + VPackBuilder result(buffer); + result.openObject(); + result.add(StaticStrings::Error, VPackValue(false)); + result.add(StaticStrings::Code, VPackValue(static_cast(_response->responseCode()))); + cursor->dump(result); + result.close(); - _response->setContentType(rest::ContentType::JSON); - generateResult(_response->responseCode(), std::move(buffer), - static_cast(cursor)->result()->context); - - cursors->release(cursor); - } catch (...) { - cursors->release(cursor); - throw; - } + _response->setContentType(rest::ContentType::JSON); + generateResult(_response->responseCode(), std::move(buffer), + static_cast(cursor)->result()->context); } } @@ -466,23 +461,18 @@ void RestCursorHandler::modifyCursor() { return; } - try { - VPackBuilder builder; - builder.openObject(); - builder.add(StaticStrings::Error, VPackValue(false)); - builder.add(StaticStrings::Code, VPackValue(static_cast(ResponseCode::OK))); - cursor->dump(builder); - builder.close(); + TRI_DEFER(cursors->release(cursor)); - _response->setContentType(rest::ContentType::JSON); - generateResult(rest::ResponseCode::OK, builder.slice(), - static_cast(cursor)->result()->context); + VPackBuilder builder; + builder.openObject(); + builder.add(StaticStrings::Error, VPackValue(false)); + builder.add(StaticStrings::Code, VPackValue(static_cast(ResponseCode::OK))); + cursor->dump(builder); + builder.close(); - cursors->release(cursor); - } catch (...) { - cursors->release(cursor); - throw; - } + _response->setContentType(rest::ContentType::JSON); + generateResult(rest::ResponseCode::OK, builder.slice(), + static_cast(cursor)->result()->context); } //////////////////////////////////////////////////////////////////////////////// diff --git a/arangod/RocksDBEngine/RocksDBCollection.cpp b/arangod/RocksDBEngine/RocksDBCollection.cpp index dff669ec29..2f44e81104 100644 --- a/arangod/RocksDBEngine/RocksDBCollection.cpp +++ b/arangod/RocksDBEngine/RocksDBCollection.cpp @@ -78,7 +78,7 @@ RocksDBCollection::RocksDBCollection(LogicalCollection* collection, _objectId(basics::VelocyPackHelper::stringUInt64(info, "objectId")), _numberDocuments(0), _revisionId(0), - _hasGeoIndex(false), + _numberOfGeoIndexes(0), _primaryIndex(nullptr), _cache(nullptr), _cachePresent(false), @@ -106,7 +106,7 @@ RocksDBCollection::RocksDBCollection(LogicalCollection* collection, _objectId(static_cast(physical)->_objectId), _numberDocuments(0), _revisionId(0), - _hasGeoIndex(false), + _numberOfGeoIndexes(0), _primaryIndex(nullptr), _cache(nullptr), _cachePresent(false), @@ -257,7 +257,7 @@ void RocksDBCollection::open(bool ignoreErrors) { for (std::shared_ptr it : _indexes) { if (it->type() == Index::TRI_IDX_TYPE_GEO1_INDEX || it->type() == Index::TRI_IDX_TYPE_GEO2_INDEX) { - _hasGeoIndex = true; + ++_numberOfGeoIndexes; } } } @@ -626,6 +626,9 @@ bool RocksDBCollection::dropIndex(TRI_idx_iid_t iid) { // trigger compaction before deleting the object cindex->cleanup(); + bool isGeoIndex = (cindex->type() == Index::TRI_IDX_TYPE_GEO1_INDEX || + cindex->type() == Index::TRI_IDX_TYPE_GEO2_INDEX); + _indexes.erase(_indexes.begin() + i); events::DropIndex("", std::to_string(iid), TRI_ERROR_NO_ERROR); // toVelocyPackIgnore will take a read lock and we don't need the @@ -644,6 +647,13 @@ bool RocksDBCollection::dropIndex(TRI_idx_iid_t iid) { builder.slice(), RocksDBLogValue::IndexDrop(_logicalCollection->vocbase()->id(), _logicalCollection->cid(), iid)); + + if (isGeoIndex) { + // decrease total number of geo indexes by one + TRI_ASSERT(_numberOfGeoIndexes > 0); + --_numberOfGeoIndexes; + } + return res == TRI_ERROR_NO_ERROR; } @@ -1224,7 +1234,7 @@ void RocksDBCollection::addIndex(std::shared_ptr idx) { _indexes.emplace_back(idx); if (idx->type() == Index::TRI_IDX_TYPE_GEO1_INDEX || idx->type() == Index::TRI_IDX_TYPE_GEO2_INDEX) { - _hasGeoIndex = true; + ++_numberOfGeoIndexes; } if (idx->type() == Index::TRI_IDX_TYPE_PRIMARY_INDEX) { TRI_ASSERT(idx->id() == 0); diff --git a/arangod/RocksDBEngine/RocksDBCollection.h b/arangod/RocksDBEngine/RocksDBCollection.h index fdd5840089..487b853c2c 100644 --- a/arangod/RocksDBEngine/RocksDBCollection.h +++ b/arangod/RocksDBEngine/RocksDBCollection.h @@ -190,7 +190,7 @@ class RocksDBCollection final : public PhysicalCollection { void compact(); void estimateSize(velocypack::Builder& builder); - bool hasGeoIndex() { return _hasGeoIndex; } + bool hasGeoIndex() const { return _numberOfGeoIndexes > 0; } std::pair serializeIndexEstimates( rocksdb::Transaction*, rocksdb::SequenceNumber) const; @@ -273,8 +273,8 @@ class RocksDBCollection final : public PhysicalCollection { std::atomic _numberDocuments; std::atomic _revisionId; - /// upgrade write locks to exclusive locks if this flag is set - bool _hasGeoIndex; + /// upgrade write locks to exclusive locks if this is > 0 + uint32_t _numberOfGeoIndexes; /// cache the primary index for performance, do not delete RocksDBPrimaryIndex* _primaryIndex; diff --git a/arangod/Transaction/Methods.cpp b/arangod/Transaction/Methods.cpp index 2530b1e437..20fdce973e 100644 --- a/arangod/Transaction/Methods.cpp +++ b/arangod/Transaction/Methods.cpp @@ -1464,9 +1464,9 @@ OperationResult transaction::Methods::insertLocal( return res; } - TRI_ASSERT(!result.empty()); - if (!options.silent || _state->isDBServer()) { + TRI_ASSERT(!result.empty()); + StringRef keyString(transaction::helpers::extractKeyFromDocument( VPackSlice(result.vpack()))); @@ -1804,10 +1804,9 @@ OperationResult transaction::Methods::modifyLocal( return res; } - TRI_ASSERT(!result.empty()); - TRI_ASSERT(!previous.empty()); - if (!options.silent || _state->isDBServer()) { + TRI_ASSERT(!previous.empty()); + TRI_ASSERT(!result.empty()); StringRef key(newVal.get(StaticStrings::KeyString)); buildDocumentIdentity(collection, resultBuilder, cid, key, TRI_ExtractRevisionId(VPackSlice(result.vpack())), @@ -3179,4 +3178,4 @@ Result transaction::Methods::resolveId(char const* handle, size_t length, // ----------------------------------------------------------------------------- // --SECTION-- END-OF-FILE -// ----------------------------------------------------------------------------- \ No newline at end of file +// -----------------------------------------------------------------------------