tursom/memgraph
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Extend property cache to the expression evaluator (#1432)

Browse Source 
* Add support for property cache in the produce
* Fix the previous implementation in the map literal
This commit is contained in:
Josipmrden 2023-10-29 04:32:58 +01:00 committed by GitHub
parent b1c3168308
commit 5b9802bd7b
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 185 additions and 37 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
src/query/context.hpp
View File

@ -51,8 +51,6 @@ struct EvaluationContext {
/// All counters generated by `counter` function, mutable because the function
/// modifies the values
mutable std::unordered_map<std::string, int64_t> counters{};
/// Property lookup cache ({symbol: {property_id: property_value, ...}, ...})
mutable std::unordered_map<int32_t, std::map<storage::PropertyId, storage::PropertyValue>> property_lookups_cache{};
};
inline std::vector<storage::PropertyId> NamesToProperties(const std::vector<std::string> &property_names,

52
src/query/frontend/semantic/symbol_generator.cpp
View File

@ -84,6 +84,9 @@ void SymbolGenerator::VisitReturnBody(ReturnBody &body, Where *where) {
for (auto &expr : body.named_expressions) {
expr->Accept(*this);
}
SetEvaluationModeOnPropertyLookups(body);
std::vector<Symbol> user_symbols;
if (body.all_identifiers) {
// Carry over user symbols because '*' appeared.
@ -400,26 +403,8 @@ SymbolGenerator::ReturnType SymbolGenerator::Visit(Identifier &ident) {
return true;
}
bool SymbolGenerator::PostVisit(MapLiteral &map_literal) {
std::unordered_map<int32_t, PropertyLookup *> property_lookups{};
for (const auto &pair : map_literal.elements_) {
if (pair.second->GetTypeInfo() != PropertyLookup::kType) continue;
auto *property_lookup = static_cast<PropertyLookup *>(pair.second);
if (property_lookup->expression_->GetTypeInfo() != Identifier::kType) continue;
auto symbol_pos = static_cast<Identifier *>(property_lookup->expression_)->symbol_pos_;
try {
auto *existing_property_lookup = property_lookups.at(symbol_pos);
// If already there (no exception), update the original and current PropertyLookups
existing_property_lookup->evaluation_mode_ = PropertyLookup::EvaluationMode::GET_ALL_PROPERTIES;
property_lookup->evaluation_mode_ = PropertyLookup::EvaluationMode::GET_ALL_PROPERTIES;
} catch (const std::out_of_range &) {
// Otherwise, add the PropertyLookup to the map
property_lookups.emplace(symbol_pos, property_lookup);
}
}
bool SymbolGenerator::PreVisit(MapLiteral &map_literal) {
SetEvaluationModeOnPropertyLookups(map_literal);
return true;
}
@ -741,4 +726,31 @@ bool SymbolGenerator::ConsumePredefinedIdentifier(const std::string &name) {
return true;
}
void PropertyLookupEvaluationModeVisitor::Visit(PropertyLookup &property_lookup) {
if (property_lookup.expression_->GetTypeInfo() != Identifier::kType) {
return;
}
auto identifier_symbol = static_cast<Identifier *>(property_lookup.expression_)->name_;
if (this->gather_property_lookup_counts) {
if (!property_lookup_counts_by_symbol.contains(identifier_symbol)) {
property_lookup_counts_by_symbol[identifier_symbol] = 0;
}
property_lookup_counts_by_symbol[identifier_symbol]++;
return;
}
if (this->assign_property_lookup_evaluations) {
if (property_lookup_counts_by_symbol.contains(identifier_symbol) &&
property_lookup_counts_by_symbol[identifier_symbol] > 1) {
property_lookup.evaluation_mode_ = PropertyLookup::EvaluationMode::GET_ALL_PROPERTIES;
}
return;
}
}
} // namespace memgraph::query

116
src/query/frontend/semantic/symbol_generator.hpp
View File

@ -72,8 +72,8 @@ class SymbolGenerator : public HierarchicalTreeVisitor {
// Expressions
ReturnType Visit(Identifier &) override;
ReturnType Visit(PrimitiveLiteral &) override { return true; }
bool PreVisit(MapLiteral &) override { return true; }
bool PostVisit(MapLiteral &) override;
bool PreVisit(MapLiteral &) override;
bool PostVisit(MapLiteral &) override { return true; };
ReturnType Visit(ParameterLookup &) override { return true; }
bool PreVisit(Aggregation &) override;
bool PostVisit(Aggregation &) override;
@ -177,6 +177,86 @@ class SymbolGenerator : public HierarchicalTreeVisitor {
std::vector<Scope> scopes_;
};
/// Visits the AST and assigns the evaluation mode for all the property lookups
/// If property lookup for one symbol is visited more times, it is better to fetch all properties
class PropertyLookupEvaluationModeVisitor : public ExpressionVisitor<void> {
public:
explicit PropertyLookupEvaluationModeVisitor() {}
using ExpressionVisitor<void>::Visit;
// Unary operators
void Visit(NotOperator &op) override { op.expression_->Accept(*this); }
void Visit(IsNullOperator &op) override { op.expression_->Accept(*this); };
void Visit(UnaryPlusOperator &op) override{};
void Visit(UnaryMinusOperator &op) override{};
void Visit(OrOperator &op) override {
op.expression1_->Accept(*this);
op.expression2_->Accept(*this);
}
void Visit(XorOperator &op) override {
op.expression1_->Accept(*this);
op.expression2_->Accept(*this);
}
void Visit(AndOperator &op) override {
op.expression1_->Accept(*this);
op.expression2_->Accept(*this);
}
void Visit(NotEqualOperator &op) override {
op.expression1_->Accept(*this);
op.expression2_->Accept(*this);
};
void Visit(EqualOperator &op) override {
op.expression1_->Accept(*this);
op.expression2_->Accept(*this);
};
void Visit(InListOperator &op) override {
op.expression1_->Accept(*this);
op.expression2_->Accept(*this);
};
void Visit(AdditionOperator &op) override{};
void Visit(SubtractionOperator &op) override{};
void Visit(MultiplicationOperator &op) override{};
void Visit(DivisionOperator &op) override{};
void Visit(ModOperator &op) override{};
void Visit(LessOperator &op) override{};
void Visit(GreaterOperator &op) override{};
void Visit(LessEqualOperator &op) override{};
void Visit(GreaterEqualOperator &op) override{};
void Visit(SubscriptOperator &op) override{};
void Visit(ListSlicingOperator &op) override{};
void Visit(IfOperator &op) override{};
void Visit(ListLiteral &op) override{};
void Visit(MapLiteral &op) override{};
void Visit(MapProjectionLiteral &op) override{};
void Visit(LabelsTest &op) override{};
void Visit(Aggregation &op) override{};
void Visit(Function &op) override{};
void Visit(Reduce &op) override{};
void Visit(Coalesce &op) override{};
void Visit(Extract &op) override{};
void Visit(Exists &op) override{};
void Visit(All &op) override{};
void Visit(Single &op) override{};
void Visit(Any &op) override{};
void Visit(None &op) override{};
void Visit(Identifier &op) override{};
void Visit(PrimitiveLiteral &op) override{};
void Visit(AllPropertiesLookup &op) override{};
void Visit(ParameterLookup &op) override{};
void Visit(NamedExpression &op) override { op.expression_->Accept(*this); };
void Visit(RegexMatch &op) override{};
void Visit(PropertyLookup & /*property_lookup*/) override;
bool gather_property_lookup_counts{false};
bool assign_property_lookup_evaluations{false};
private:
std::unordered_map<std::string, uint64_t> property_lookup_counts_by_symbol{};
};
inline SymbolTable MakeSymbolTable(CypherQuery *query, const std::vector<Identifier *> &predefined_identifiers = {}) {
SymbolTable symbol_table;
SymbolGenerator symbol_generator(&symbol_table, predefined_identifiers);
@ -187,4 +267,36 @@ inline SymbolTable MakeSymbolTable(CypherQuery *query, const std::vector<Identif
return symbol_table;
}
inline void SetEvaluationModeOnPropertyLookups(ReturnBody &body) {
PropertyLookupEvaluationModeVisitor visitor;
visitor.gather_property_lookup_counts = true;
for (auto *expr : body.named_expressions) {
expr->Accept(visitor);
}
visitor.gather_property_lookup_counts = false;
visitor.assign_property_lookup_evaluations = true;
for (auto *expr : body.named_expressions) {
expr->Accept(visitor);
}
visitor.assign_property_lookup_evaluations = false;
}
inline void SetEvaluationModeOnPropertyLookups(MapLiteral &map_literal) {
PropertyLookupEvaluationModeVisitor visitor;
visitor.gather_property_lookup_counts = true;
for (auto &pair : map_literal.elements_) {
pair.second->Accept(visitor);
}
visitor.gather_property_lookup_counts = false;
visitor.assign_property_lookup_evaluations = true;
for (auto &pair : map_literal.elements_) {
pair.second->Accept(visitor);
}
visitor.assign_property_lookup_evaluations = false;
}
} // namespace memgraph::query

22
src/query/interpret/eval.hpp
View File

@ -548,13 +548,13 @@ class ExpressionEvaluator : public ExpressionVisitor<TypedValue> {
case TypedValue::Type::Vertex:
if (property_lookup.evaluation_mode_ == PropertyLookup::EvaluationMode::GET_ALL_PROPERTIES) {
auto symbol_pos = static_cast<Identifier *>(property_lookup.expression_)->symbol_pos_;
if (!ctx_->property_lookups_cache.contains(symbol_pos)) {
ctx_->property_lookups_cache.emplace(symbol_pos, GetAllProperties(expression_result_ptr->ValueVertex()));
if (!property_lookup_cache_.contains(symbol_pos)) {
property_lookup_cache_.emplace(symbol_pos, GetAllProperties(expression_result_ptr->ValueVertex()));
}
auto property_id = ctx_->properties[property_lookup.property_.ix];
if (ctx_->property_lookups_cache[symbol_pos].contains(property_id)) {
return TypedValue(ctx_->property_lookups_cache[symbol_pos][property_id], ctx_->memory);
if (property_lookup_cache_[symbol_pos].contains(property_id)) {
return TypedValue(property_lookup_cache_[symbol_pos][property_id], ctx_->memory);
}
return TypedValue(ctx_->memory);
} else {
@ -563,13 +563,13 @@ class ExpressionEvaluator : public ExpressionVisitor<TypedValue> {
case TypedValue::Type::Edge:
if (property_lookup.evaluation_mode_ == PropertyLookup::EvaluationMode::GET_ALL_PROPERTIES) {
auto symbol_pos = static_cast<Identifier *>(property_lookup.expression_)->symbol_pos_;
if (!ctx_->property_lookups_cache.contains(symbol_pos)) {
ctx_->property_lookups_cache.emplace(symbol_pos, GetAllProperties(expression_result_ptr->ValueEdge()));
if (!property_lookup_cache_.contains(symbol_pos)) {
property_lookup_cache_.emplace(symbol_pos, GetAllProperties(expression_result_ptr->ValueEdge()));
}
auto property_id = ctx_->properties[property_lookup.property_.ix];
if (ctx_->property_lookups_cache[symbol_pos].contains(property_id)) {
return TypedValue(ctx_->property_lookups_cache[symbol_pos][property_id], ctx_->memory);
if (property_lookup_cache_[symbol_pos].contains(property_id)) {
return TypedValue(property_lookup_cache_[symbol_pos][property_id], ctx_->memory);
}
return TypedValue(ctx_->memory);
} else {
@ -784,10 +784,6 @@ class ExpressionEvaluator : public ExpressionVisitor<TypedValue> {
result.emplace(pair.first.name, pair.second->Accept(*this));
}
ctx_->property_lookups_cache.clear();
// TODO Don’t clear the cache if there are remaining MapLiterals with PropertyLookups that read the same properties
// from the same variable (symbol & value)
return TypedValue(result, ctx_->memory);
}
@ -1171,6 +1167,8 @@ class ExpressionEvaluator : public ExpressionVisitor<TypedValue> {
// which switching approach should be used when evaluating
storage::View view_;
FrameChangeCollector *frame_change_collector_;
/// Property lookup cache ({symbol: {property_id: property_value, ...}, ...})
mutable std::unordered_map<int32_t, std::map<storage::PropertyId, storage::PropertyValue>> property_lookup_cache_{};
}; // namespace memgraph::query
/// A helper function for evaluating an expression that's an int.

14
tests/gql_behave/tests/memgraph_V1/features/with.feature
View File

@ -250,3 +250,17 @@ Feature: With
| n |
| ({id: 0}) |
Scenario: With test 16:
Given an empty graph
And having executed:
"""
CREATE ({id: 0}) CREATE ({id:1});
"""
When executing query:
"""
MATCH (n) RETURN n.id AS id;
"""
Then the result should be:
| id |
| 0 |
| 1 |

15
tests/gql_behave/tests/memgraph_V1_on_disk/features/with.feature
View File

@ -249,3 +249,18 @@ Feature: With
Then the result should be:
| n |
| ({id: 0}) |
Scenario: With test 16:
Given an empty graph
And having executed:
"""
CREATE ({id: 0}) CREATE ({id:1});
"""
When executing query:
"""
MATCH (n) RETURN n.id AS id;
"""
Then the result should be:
| id |
| 0 |
| 1 |

1
tests/unit/query_plan_create_set_remove_delete.cpp
View File

@ -1763,7 +1763,6 @@ TYPED_TEST(QueryPlanTest, SetPropertiesFromMapWithCaching) {
expected_properties.emplace(dba.NameToProperty("prop2"), memgraph::storage::PropertyValue(44));
expected_properties.emplace(dba.NameToProperty("new_prop1"), memgraph::storage::PropertyValue(43));
expected_properties.emplace(dba.NameToProperty("new_prop2"), memgraph::storage::PropertyValue(44));
EXPECT_EQ(context.evaluation_context.property_lookups_cache.size(), 0);
EXPECT_EQ(new_properties.HasError(), false);
EXPECT_EQ(*new_properties, expected_properties);
}