From d997f73c296d02780ff594da33505803ab23f28e Mon Sep 17 00:00:00 2001 From: Michael Hackstein Date: Thu, 21 Apr 2016 17:46:51 +0200 Subject: [PATCH 1/2] Removed all id splits in favor of string.find which is way faster. --- arangod/V8Server/V8Traverser.cpp | 102 +++++++++++++++++++++---------- 1 file changed, 69 insertions(+), 33 deletions(-) diff --git a/arangod/V8Server/V8Traverser.cpp b/arangod/V8Server/V8Traverser.cpp index e147b6a515..8331b67d8f 100644 --- a/arangod/V8Server/V8Traverser.cpp +++ b/arangod/V8Server/V8Traverser.cpp @@ -46,17 +46,25 @@ static OperationResult FetchDocumentById(arangodb::Transaction* trx, std::string const& id, VPackBuilder& builder, OperationOptions& options) { - std::vector parts = - arangodb::basics::StringUtils::split(id, "/"); - TRI_ASSERT(parts.size() == 2); - trx->addCollectionAtRuntime(parts[0]); + size_t pos = id.find('/'); + if (pos == std::string::npos) { + TRI_ASSERT(false); + return OperationResult(TRI_ERROR_INTERNAL); + } + if (id.find('/', pos + 1) != std::string::npos) { + TRI_ASSERT(false); + return OperationResult(TRI_ERROR_INTERNAL); + } + + std::string col = id.substr(0, pos); + trx->addCollectionAtRuntime(col); builder.clear(); builder.openObject(); builder.add(VPackValue(TRI_VOC_ATTRIBUTE_KEY)); - builder.add(VPackValue(parts[1])); + builder.add(VPackValue(id.substr(pos + 1))); builder.close(); - OperationResult opRes = trx->document(parts[0], builder.slice(), options); + OperationResult opRes = trx->document(col, builder.slice(), options); if (opRes.failed() && opRes.code != TRI_ERROR_ARANGO_DOCUMENT_NOT_FOUND) { THROW_ARANGO_EXCEPTION(opRes.code); @@ -418,11 +426,20 @@ bool BasicOptions::matchesEdge(VPackSlice edge) const { return true; } - auto id = _trx->extractIdString(edge); - std::vector parts = arangodb::basics::StringUtils::split(id, "/"); - TRI_ASSERT(parts.size() == 2); // We have a real ID - auto it = _edgeFilter.find(parts[0]); - + std::string id = _trx->extractIdString(edge); + size_t pos = id.find('/'); + + if (pos == std::string::npos) { + // no / contained in _id! + return false; + } + if (id.find('/', pos + 1) != std::string::npos) { + // multiple / contained in _id! + return false; + } + + auto it = _edgeFilter.find(id.substr(0, pos)); + if (it == _edgeFilter.end()) { // This collection does not have any object that can match. // Short circuit. @@ -474,26 +491,35 @@ bool NeighborsOptions::matchesVertex(std::string const& id) const { return true; } - std::vector parts = - arangodb::basics::StringUtils::split(id, "/"); - TRI_ASSERT(parts.size() == 2); + size_t pos = id.find('/'); + if (pos == std::string::npos) { + TRI_ASSERT(false); + return false; + } + if (id.find('/', pos + 1) != std::string::npos) { + TRI_ASSERT(false); + return false; + } + + std::string col = id.substr(0, pos); // If there are explicitly marked collections check them. if (!_explicitCollections.empty()) { // If the current collection is not stored the result is invalid - if (_explicitCollections.find(parts[0]) == _explicitCollections.end()) { + if (_explicitCollections.find(col) == _explicitCollections.end()) { return false; } } + std::string key = id.substr(pos + 1); VPackBuilder tmp; tmp.openObject(); - tmp.add(TRI_VOC_ATTRIBUTE_KEY, VPackValue(parts[1])); + tmp.add(TRI_VOC_ATTRIBUTE_KEY, VPackValue(key)); tmp.close(); OperationOptions opOpts; - OperationResult opRes = _trx->document(parts[0], tmp.slice(), opOpts); + OperationResult opRes = _trx->document(col, tmp.slice(), opOpts); if (opRes.failed()) { return false; } - return BasicOptions::matchesVertex(parts[0], parts[1], opRes.slice()); + return BasicOptions::matchesVertex(col, key, opRes.slice()); } @@ -533,21 +559,31 @@ std::unique_ptr TRI_RunShortestPathSearch( auto edgeFilterClosure = [&opts](VPackSlice edge) -> bool { return opts.matchesEdge(edge); }; - auto vertexFilterClosure = - [&opts](std::string const& v) -> bool { - // TODO: this closure needs to be optimized - std::vector parts = arangodb::basics::StringUtils::split(v, "/"); - VPackBuilder tmp; - tmp.openObject(); - tmp.add(TRI_VOC_ATTRIBUTE_KEY, VPackValue(parts[1])); - tmp.close(); - OperationOptions opOpts; - OperationResult opRes = opts.trx()->document(parts[0], tmp.slice(), opOpts); - if (opRes.failed()) { - return false; - } - return opts.matchesVertex(parts[0], parts[1], opRes.slice()); - }; + auto vertexFilterClosure = [&opts](std::string const& v) -> bool { + size_t pos = v.find('/'); + + if (pos == std::string::npos) { + // no / contained in _id! + return false; + } + if (v.find('/', pos + 1) != std::string::npos) { + // multiple / contained in _id! + return false; + } + std::string col = v.substr(0, pos); + std::string key = v.substr(pos + 1); + + VPackBuilder tmp; + tmp.openObject(); + tmp.add(TRI_VOC_ATTRIBUTE_KEY, VPackValue(key)); + tmp.close(); + OperationOptions opOpts; + OperationResult opRes = opts.trx()->document(col, tmp.slice(), opOpts); + if (opRes.failed()) { + return false; + } + return opts.matchesVertex(col, key, opRes.slice()); + }; MultiCollectionEdgeExpander forwardExpander( forward, collectionInfos, edgeFilterClosure, vertexFilterClosure); From d58e3b502271daa22505a09b2e61768ebbf16623 Mon Sep 17 00:00:00 2001 From: Andreas Streichardt Date: Thu, 21 Apr 2016 17:55:21 +0200 Subject: [PATCH 2/2] Remove useless move --- arangod/Cluster/ClusterMethods.cpp | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/arangod/Cluster/ClusterMethods.cpp b/arangod/Cluster/ClusterMethods.cpp index 174b007ec0..75408eaf2e 100644 --- a/arangod/Cluster/ClusterMethods.cpp +++ b/arangod/Cluster/ClusterMethods.cpp @@ -828,14 +828,14 @@ int createDocumentOnCoordinator( TRI_ASSERT(it.second.size() == 1); auto idx = it.second.front(); if (idx.second.empty()) { - body = std::make_shared(std::move(slice.toJson())); + body = std::make_shared(slice.toJson()); } else { reqBuilder.clear(); reqBuilder.openObject(); TRI_SanitizeObject(slice, reqBuilder); reqBuilder.add(TRI_VOC_ATTRIBUTE_KEY, VPackValue(idx.second)); reqBuilder.close(); - body = std::make_shared(std::move(reqBuilder.slice().toJson())); + body = std::make_shared(reqBuilder.slice().toJson()); } } else { reqBuilder.clear(); @@ -851,7 +851,7 @@ int createDocumentOnCoordinator( } } reqBuilder.close(); - body = std::make_shared(std::move(reqBuilder.slice().toJson())); + body = std::make_shared(reqBuilder.slice().toJson()); } auto headersCopy = std::make_unique>(headers); @@ -998,7 +998,7 @@ int deleteDocumentOnCoordinator( for (auto const& it : shardMap) { if (!useMultiple) { TRI_ASSERT(it.second.size() == 1); - body = std::make_shared(std::move(slice.toJson())); + body = std::make_shared(slice.toJson()); } else { reqBuilder.clear(); reqBuilder.openArray(); @@ -1006,7 +1006,7 @@ int deleteDocumentOnCoordinator( reqBuilder.add(slice.at(idx)); } reqBuilder.close(); - body = std::make_shared(std::move(reqBuilder.slice().toJson())); + body = std::make_shared(reqBuilder.slice().toJson()); } auto headersCopy = std::make_unique>(*headers); @@ -1050,7 +1050,7 @@ int deleteDocumentOnCoordinator( // end // if (!skipped) => insert NOT_FOUND - auto body = std::make_shared(std::move(slice.toJson())); + auto body = std::make_shared(slice.toJson()); auto shardList = ci->getShardList(collid); for (auto const& shard : *shardList) { auto headersCopy = @@ -1277,7 +1277,7 @@ int getDocumentOnCoordinator( reqBuilder.add(slice.at(idx)); } reqBuilder.close(); - body = std::make_shared(std::move(reqBuilder.slice().toJson())); + body = std::make_shared(reqBuilder.slice().toJson()); // We send to Babies endpoint cc->asyncRequest("", coordTransactionID, "shard:" + it.first, reqType, baseUrl + StringUtils::urlEncode(it.first) + optsUrlPart, @@ -1332,7 +1332,7 @@ int getDocumentOnCoordinator( nullptr, headersCopy, nullptr, 60.0); } } else { - auto body = std::make_shared(std::move(slice.toJson())); + auto body = std::make_shared(slice.toJson()); for (auto const& shard : *shardList) { auto headersCopy = std::make_unique>(*headers); @@ -1795,7 +1795,7 @@ int modifyDocumentOnCoordinator( std::make_unique>(*headers); if (!useMultiple) { TRI_ASSERT(it.second.size() == 1); - body = std::make_shared(std::move(slice.toJson())); + body = std::make_shared(slice.toJson()); // We send to single endpoint cc->asyncRequest("", coordTransactionID, "shard:" + it.first, reqType, @@ -1810,7 +1810,7 @@ int modifyDocumentOnCoordinator( reqBuilder.add(slice.at(idx)); } reqBuilder.close(); - body = std::make_shared(std::move(reqBuilder.slice().toJson())); + body = std::make_shared(reqBuilder.slice().toJson()); // We send to Babies endpoint cc->asyncRequest("", coordTransactionID, "shard:" + it.first, reqType, baseUrl + StringUtils::urlEncode(it.first) + optsUrlPart, @@ -1848,7 +1848,7 @@ int modifyDocumentOnCoordinator( // Not all shard keys are known in all documents. // We contact all shards with the complete body and ignore NOT_FOUND - auto body = std::make_shared(std::move(slice.toJson())); + auto body = std::make_shared(slice.toJson()); auto shardList = ci->getShardList(collid); if (!useMultiple) { for (auto const& shard : *shardList) {