-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix VertexHolder::getDefaultProp performance issue. #2249
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -926,7 +926,7 @@ void GoExecutor::fetchVertexProps(std::vector<VertexID> ids) { | |
auto returns = status.value(); | ||
auto future = ectx()->getStorageClient()->getVertexProps(spaceId, ids, returns); | ||
auto *runner = ectx()->rctx()->runner(); | ||
auto cb = [this] (auto &&result) mutable { | ||
auto cb = [this, ectx = ectx()] (auto &&result) mutable { | ||
auto completeness = result.completeness(); | ||
if (completeness == 0) { | ||
doError(Status::Error("Get dest props failed")); | ||
|
@@ -939,7 +939,7 @@ void GoExecutor::fetchVertexProps(std::vector<VertexID> ids) { | |
} | ||
} | ||
if (vertexHolder_ == nullptr) { | ||
vertexHolder_ = std::make_unique<VertexHolder>(); | ||
vertexHolder_ = std::make_unique<VertexHolder>(ectx); | ||
} | ||
for (auto &resp : result.responses()) { | ||
vertexHolder_->add(resp); | ||
|
@@ -1222,7 +1222,7 @@ bool GoExecutor::processFinalResult(Callback cb) const { | |
return index_->getColumnWithRow(*inputRow, prop); | ||
}; | ||
|
||
std::unique_ptr<RowReader> reader; | ||
RowReader reader = RowReader::getEmptyRowReader(); | ||
if (currEdgeSchema) { | ||
reader = RowReader::getRowReader(edge.props, currEdgeSchema); | ||
} | ||
|
@@ -1315,28 +1315,24 @@ bool GoExecutor::processFinalResult(Callback cb) const { | |
return true; | ||
} | ||
|
||
OptVariantType GoExecutor::VertexHolder::getDefaultProp(TagID tid, const std::string &prop) const { | ||
for (auto it = data_.cbegin(); it != data_.cend(); ++it) { | ||
auto it2 = it->second.find(tid); | ||
if (it2 != it->second.cend()) { | ||
return RowReader::getDefaultProp(std::get<0>(it2->second).get(), prop); | ||
} | ||
OptVariantType GoExecutor::VertexHolder::getDefaultProp( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Actually, we don't need "getDefaultProp" and "getDefaultPropType" inside VertexHolder, You could use RowReader::getDefaultProp directly. FYI. You could pass schemaManager into VertexHolder directly. |
||
TagID tagId, const std::string &prop) const { | ||
auto space = ectx_->rctx()->session()->space(); | ||
auto schema = ectx_->schemaManager()->getTagSchema(space, tagId); | ||
if (schema == nullptr) { | ||
return Status::Error("No tag schema for tagId %d", tagId); | ||
} | ||
|
||
|
||
return Status::Error("Unknown property: `%s'", prop.c_str()); | ||
return RowReader::getDefaultProp(schema.get(), prop); | ||
} | ||
|
||
SupportedType GoExecutor::VertexHolder::getDefaultPropType(TagID tid, | ||
const std::string &prop) const { | ||
for (auto it = data_.cbegin(); it != data_.cend(); ++it) { | ||
auto it2 = it->second.find(tid); | ||
if (it2 != it->second.cend()) { | ||
return std::get<0>(it2->second)->getFieldType(prop).type; | ||
} | ||
SupportedType GoExecutor::VertexHolder::getDefaultPropType( | ||
TagID tagId, const std::string &prop) const { | ||
auto space = ectx_->rctx()->session()->space(); | ||
auto schema = ectx_->schemaManager()->getTagSchema(space, tagId); | ||
if (schema == nullptr) { | ||
return nebula::cpp2::SupportedType::UNKNOWN; | ||
} | ||
|
||
return nebula::cpp2::SupportedType::UNKNOWN; | ||
return schema->getFieldType(prop).type; | ||
} | ||
|
||
OptVariantType GoExecutor::VertexHolder::get(VertexID id, TagID tid, | ||
|
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -87,7 +87,7 @@ std::string AddEdgesProcessor::addEdges(int64_t version, PartitionID partId, | |||||||||||||||||||||||||||||||
}); | ||||||||||||||||||||||||||||||||
for (auto& e : newEdges) { | ||||||||||||||||||||||||||||||||
std::string val; | ||||||||||||||||||||||||||||||||
std::unique_ptr<RowReader> nReader; | ||||||||||||||||||||||||||||||||
RowReader nReader = RowReader::getEmptyRowReader(); | ||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You'd better use pointer of RowReader this place. Because there are lots of check null for rowReader in the code. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nebula/src/dataman/RowReader.h Lines 299 to 313 in e5cefb2
Solve it with CPP magic. This can really help to improve performance, the total batch computing costs reduces from 13min to 5min, for about 2.6X improvement. Access data set from the stack is much faster than from heap, for CPU hardware cache optimization. |
||||||||||||||||||||||||||||||||
auto edgeType = NebulaKeyUtils::getEdgeType(e.first); | ||||||||||||||||||||||||||||||||
for (auto& index : indexes_) { | ||||||||||||||||||||||||||||||||
if (edgeType == index->get_schema_id().get_edge_type()) { | ||||||||||||||||||||||||||||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems we dont need reset any more. But never mind, leave it here.