1
0
Fork 0

Bug fix/several optimizations (#4672)

This commit is contained in:
Jan 2018-02-26 15:19:01 +01:00 committed by GitHub
parent 586a66ebbf
commit d841b56c77
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 90 additions and 91 deletions

View File

@ -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!

View File

@ -270,8 +270,6 @@ AqlItemBlock* RemoveBlock::work(std::vector<AqlItemBlock*>& 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<AqlItemBlock*>& 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<AqlItemBlock*>& 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<AqlItemBlock*>& 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<AqlItemBlock*>& 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<AqlItemBlock*>& 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<AqlItemBlock*>& 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<AqlItemBlock*>& 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<AqlItemBlock*>& 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<AqlItemBlock*>& blocks) {
options.ignoreRevs = true;
options.isRestore = ep->getOptions().useIsRestore;
VPackBuilder keyBuilder;
VPackBuilder insertBuilder;
VPackBuilder updateBuilder;
@ -844,16 +840,16 @@ AqlItemBlock* UpsertBlock::work(std::vector<AqlItemBlock*>& 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<AqlItemBlock*>& 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<AqlItemBlock*>& 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 {

View File

@ -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 {

View File

@ -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 (...) {

View File

@ -218,24 +218,19 @@ void RestCursorHandler::processQuery(VPackSlice const& slice) {
Cursor* cursor = cursors->createFromQueryResult(
std::move(queryResult), batchSize, extra, ttl, count);
try {
VPackBuffer<uint8_t> buffer;
VPackBuilder result(buffer);
result.openObject();
result.add(StaticStrings::Error, VPackValue(false));
result.add(StaticStrings::Code, VPackValue(static_cast<int>(_response->responseCode())));
cursor->dump(result);
result.close();
TRI_DEFER(cursors->release(cursor));
VPackBuffer<uint8_t> buffer;
VPackBuilder result(buffer);
result.openObject();
result.add(StaticStrings::Error, VPackValue(false));
result.add(StaticStrings::Code, VPackValue(static_cast<int>(_response->responseCode())));
cursor->dump(result);
result.close();
_response->setContentType(rest::ContentType::JSON);
generateResult(_response->responseCode(), std::move(buffer),
static_cast<VelocyPackCursor*>(cursor)->result()->context);
cursors->release(cursor);
} catch (...) {
cursors->release(cursor);
throw;
}
_response->setContentType(rest::ContentType::JSON);
generateResult(_response->responseCode(), std::move(buffer),
static_cast<VelocyPackCursor*>(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<int>(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<VelocyPackCursor*>(cursor)->result()->context);
VPackBuilder builder;
builder.openObject();
builder.add(StaticStrings::Error, VPackValue(false));
builder.add(StaticStrings::Code, VPackValue(static_cast<int>(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<VelocyPackCursor*>(cursor)->result()->context);
}
////////////////////////////////////////////////////////////////////////////////

View File

@ -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<RocksDBCollection const*>(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<Index> 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<arangodb::Index> 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);

View File

@ -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<Result, rocksdb::SequenceNumber> serializeIndexEstimates(
rocksdb::Transaction*, rocksdb::SequenceNumber) const;
@ -273,8 +273,8 @@ class RocksDBCollection final : public PhysicalCollection {
std::atomic<uint64_t> _numberDocuments;
std::atomic<TRI_voc_rid_t> _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;

View File

@ -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
// -----------------------------------------------------------------------------
// -----------------------------------------------------------------------------