From d79dd696079241aee45cad4a252daa3391be6f4f Mon Sep 17 00:00:00 2001
From: Antonio Filipovic <61245998+antoniofilipovic@users.noreply.github.com>
Date: Fri, 24 Feb 2023 15:40:35 +0100
Subject: [PATCH] Improve performance with props init on node|edge creation
(#788)
---
src/query/common.hpp | 34 +++++++++-
src/query/db_accessor.hpp | 10 ++-
src/query/plan/operator.cpp | 19 +++---
src/storage/v2/edge_accessor.cpp | 20 +++++-
src/storage/v2/edge_accessor.hpp | 7 ++-
src/storage/v2/property_store.cpp | 62 ++++++++++++++++++-
src/storage/v2/property_store.hpp | 8 ++-
src/storage/v2/vertex_accessor.cpp | 19 +++++-
src/storage/v2/vertex_accessor.hpp | 7 ++-
.../memgraph_V1/features/functions.feature | 4 +-
tests/unit/storage_v2_property_store.cpp | 25 +++++++-
11 files changed, 195 insertions(+), 20 deletions(-)
diff --git a/src/query/common.hpp b/src/query/common.hpp
index c51c34dee..6154900b5 100644
--- a/src/query/common.hpp
+++ b/src/query/common.hpp
@@ -1,4 +1,4 @@
-// Copyright 2022 Memgraph Ltd.
+// Copyright 2023 Memgraph Ltd.
//
// Use of this software is governed by the Business Source License
// included in the file licenses/BSL.txt; by using this file, you agree to be bound by the terms of the Business Source
@@ -107,5 +107,37 @@ storage::PropertyValue PropsSetChecked(T *record, const storage::PropertyId &key
}
}
+template <typename T>
+concept AccessorWithInitProperties = requires(T accessor,
+ const std::map<storage::PropertyId, storage::PropertyValue> &properties) {
+ { accessor.InitProperties(properties) } -> std::same_as<storage::Result<bool>>;
+};
+
+/// Set property `values` mapped with given `key` on a `record`.
+///
+/// @throw QueryRuntimeException if value cannot be set as a property value
+template <AccessorWithInitProperties T>
+bool MultiPropsInitChecked(T *record, std::map<storage::PropertyId, storage::PropertyValue> &properties) {
+ try {
+ auto maybe_values = record->InitProperties(properties);
+ if (maybe_values.HasError()) {
+ switch (maybe_values.GetError()) {
+ case storage::Error::SERIALIZATION_ERROR:
+ throw TransactionSerializationException();
+ case storage::Error::DELETED_OBJECT:
+ throw QueryRuntimeException("Trying to set properties on a deleted object.");
+ case storage::Error::PROPERTIES_DISABLED:
+ throw QueryRuntimeException("Can't set property because properties on edges are disabled.");
+ case storage::Error::VERTEX_HAS_EDGES:
+ case storage::Error::NONEXISTENT_OBJECT:
+ throw QueryRuntimeException("Unexpected error when setting a property.");
+ }
+ }
+ return std::move(*maybe_values);
+ } catch (const TypedValueException &) {
+ throw QueryRuntimeException("Cannot set properties.");
+ }
+}
+
int64_t QueryTimestamp();
} // namespace memgraph::query
diff --git a/src/query/db_accessor.hpp b/src/query/db_accessor.hpp
index 2c269a951..91c2ec721 100644
--- a/src/query/db_accessor.hpp
+++ b/src/query/db_accessor.hpp
@@ -1,4 +1,4 @@
-// Copyright 2022 Memgraph Ltd.
+// Copyright 2023 Memgraph Ltd.
//
// Use of this software is governed by the Business Source License
// included in the file licenses/BSL.txt; by using this file, you agree to be bound by the terms of the Business Source
@@ -71,6 +71,10 @@ class EdgeAccessor final {
return impl_.SetProperty(key, value);
}
+ storage::Result<bool> InitProperties(const std::map<storage::PropertyId, storage::PropertyValue> &properties) {
+ return impl_.InitProperties(properties);
+ }
+
storage::Result<storage::PropertyValue> RemoveProperty(storage::PropertyId key) {
return SetProperty(key, storage::PropertyValue());
}
@@ -125,6 +129,10 @@ class VertexAccessor final {
return impl_.SetProperty(key, value);
}
+ storage::Result<bool> InitProperties(const std::map<storage::PropertyId, storage::PropertyValue> &properties) {
+ return impl_.InitProperties(properties);
+ }
+
storage::Result<storage::PropertyValue> RemoveProperty(storage::PropertyId key) {
return SetProperty(key, storage::PropertyValue());
}
diff --git a/src/query/plan/operator.cpp b/src/query/plan/operator.cpp
index 645b0a463..846545721 100644
--- a/src/query/plan/operator.cpp
+++ b/src/query/plan/operator.cpp
@@ -25,6 +25,7 @@
#include <cppitertools/chain.hpp>
#include <cppitertools/imap.hpp>
+#include "query/common.hpp"
#include "spdlog/spdlog.h"
#include "license/license.hpp"
@@ -208,17 +209,18 @@ VertexAccessor &CreateLocalVertex(const NodeCreationInfo &node_info, Frame *fram
storage::View::NEW);
// TODO: PropsSetChecked allocates a PropertyValue, make it use context.memory
// when we update PropertyValue with custom allocator.
+ std::map<storage::PropertyId, storage::PropertyValue> properties;
if (const auto *node_info_properties = std::get_if<PropertiesMapList>(&node_info.properties)) {
for (const auto &[key, value_expression] : *node_info_properties) {
- PropsSetChecked(&new_node, key, value_expression->Accept(evaluator));
+ properties.emplace(key, value_expression->Accept(evaluator));
}
} else {
auto property_map = evaluator.Visit(*std::get<ParameterLookup *>(node_info.properties));
for (const auto &[key, value] : property_map.ValueMap()) {
- auto property_id = dba.NameToProperty(key);
- PropsSetChecked(&new_node, property_id, value);
+ properties.emplace(dba.NameToProperty(key), value);
}
}
+ MultiPropsInitChecked(&new_node, properties);
(*frame)[node_info.symbol] = new_node;
return (*frame)[node_info.symbol].ValueVertex();
@@ -299,17 +301,18 @@ EdgeAccessor CreateEdge(const EdgeCreationInfo &edge_info, DbAccessor *dba, Vert
auto maybe_edge = dba->InsertEdge(from, to, edge_info.edge_type);
if (maybe_edge.HasValue()) {
auto &edge = *maybe_edge;
- if (const auto *properties = std::get_if<PropertiesMapList>(&edge_info.properties)) {
- for (const auto &[key, value_expression] : *properties) {
- PropsSetChecked(&edge, key, value_expression->Accept(*evaluator));
+ std::map<storage::PropertyId, storage::PropertyValue> properties;
+ if (const auto *edge_info_properties = std::get_if<PropertiesMapList>(&edge_info.properties)) {
+ for (const auto &[key, value_expression] : *edge_info_properties) {
+ properties.emplace(key, value_expression->Accept(*evaluator));
}
} else {
auto property_map = evaluator->Visit(*std::get<ParameterLookup *>(edge_info.properties));
for (const auto &[key, value] : property_map.ValueMap()) {
- auto property_id = dba->NameToProperty(key);
- PropsSetChecked(&edge, property_id, value);
+ properties.emplace(dba->NameToProperty(key), value);
}
}
+ if (!properties.empty()) MultiPropsInitChecked(&edge, properties);
(*frame)[edge_info.symbol] = edge;
} else {
diff --git a/src/storage/v2/edge_accessor.cpp b/src/storage/v2/edge_accessor.cpp
index 2a15292a2..abfb67ff7 100644
--- a/src/storage/v2/edge_accessor.cpp
+++ b/src/storage/v2/edge_accessor.cpp
@@ -1,4 +1,4 @@
-// Copyright 2022 Memgraph Ltd.
+// Copyright 2023 Memgraph Ltd.
//
// Use of this software is governed by the Business Source License
// included in the file licenses/BSL.txt; by using this file, you agree to be bound by the terms of the Business Source
@@ -123,6 +123,24 @@ Result<storage::PropertyValue> EdgeAccessor::SetProperty(PropertyId property, co
return std::move(current_value);
}
+Result<bool> EdgeAccessor::InitProperties(const std::map<storage::PropertyId, storage::PropertyValue> &properties) {
+ utils::MemoryTracker::OutOfMemoryExceptionEnabler oom_exception;
+ if (!config_.properties_on_edges) return Error::PROPERTIES_DISABLED;
+
+ std::lock_guard<utils::SpinLock> guard(edge_.ptr->lock);
+
+ if (!PrepareForWrite(transaction_, edge_.ptr)) return Error::SERIALIZATION_ERROR;
+
+ if (edge_.ptr->deleted) return Error::DELETED_OBJECT;
+
+ if (!edge_.ptr->properties.InitProperties(properties)) return false;
+ for (const auto &[property, _] : properties) {
+ CreateAndLinkDelta(transaction_, edge_.ptr, Delta::SetPropertyTag(), property, PropertyValue());
+ }
+
+ return true;
+}
+
Result<std::map<PropertyId, PropertyValue>> EdgeAccessor::ClearProperties() {
if (!config_.properties_on_edges) return Error::PROPERTIES_DISABLED;
diff --git a/src/storage/v2/edge_accessor.hpp b/src/storage/v2/edge_accessor.hpp
index b0a1e1151..ee8176365 100644
--- a/src/storage/v2/edge_accessor.hpp
+++ b/src/storage/v2/edge_accessor.hpp
@@ -1,4 +1,4 @@
-// Copyright 2022 Memgraph Ltd.
+// Copyright 2023 Memgraph Ltd.
//
// Use of this software is governed by the Business Source License
// included in the file licenses/BSL.txt; by using this file, you agree to be bound by the terms of the Business Source
@@ -58,6 +58,11 @@ class EdgeAccessor final {
/// @throw std::bad_alloc
Result<storage::PropertyValue> SetProperty(PropertyId property, const PropertyValue &value);
+ /// Set property values only if property store is empty. Returns `true` if successully set all values,
+ /// `false` otherwise.
+ /// @throw std::bad_alloc
+ Result<bool> InitProperties(const std::map<storage::PropertyId, storage::PropertyValue> &properties);
+
/// Remove all properties and return old values for each removed property.
/// @throw std::bad_alloc
Result<std::map<PropertyId, PropertyValue>> ClearProperties();
diff --git a/src/storage/v2/property_store.cpp b/src/storage/v2/property_store.cpp
index db8982b6f..f689aa844 100644
--- a/src/storage/v2/property_store.cpp
+++ b/src/storage/v2/property_store.cpp
@@ -1,4 +1,4 @@
-// Copyright 2022 Memgraph Ltd.
+// Copyright 2023 Memgraph Ltd.
//
// Use of this software is governed by the Business Source License
// included in the file licenses/BSL.txt; by using this file, you agree to be bound by the terms of the Business Source
@@ -1053,7 +1053,7 @@ bool PropertyStore::SetProperty(PropertyId property, const PropertyValue &value)
in_local_buffer = true;
} else {
// Allocate a new external buffer.
- auto alloc_data = new uint8_t[property_size_to_power_of_8];
+ auto *alloc_data = new uint8_t[property_size_to_power_of_8];
auto alloc_size = property_size_to_power_of_8;
SetSizeData(buffer_, alloc_size, alloc_data);
@@ -1144,6 +1144,64 @@ bool PropertyStore::SetProperty(PropertyId property, const PropertyValue &value)
return !existed;
}
+bool PropertyStore::InitProperties(const std::map<storage::PropertyId, storage::PropertyValue> &properties) {
+ uint64_t size = 0;
+ uint8_t *data = nullptr;
+ std::tie(size, data) = GetSizeData(buffer_);
+ if (size != 0) {
+ return false;
+ }
+
+ uint64_t property_size = 0;
+ {
+ Writer writer;
+ for (const auto &[property, value] : properties) {
+ if (value.IsNull()) {
+ continue;
+ }
+ EncodeProperty(&writer, property, value);
+ property_size = writer.Written();
+ }
+ }
+
+ auto property_size_to_power_of_8 = ToPowerOf8(property_size);
+ if (property_size <= sizeof(buffer_) - 1) {
+ // Use the local buffer.
+ buffer_[0] = kUseLocalBuffer;
+ size = sizeof(buffer_) - 1;
+ data = &buffer_[1];
+ } else {
+ // Allocate a new external buffer.
+ auto *alloc_data = new uint8_t[property_size_to_power_of_8];
+ auto alloc_size = property_size_to_power_of_8;
+
+ SetSizeData(buffer_, alloc_size, alloc_data);
+
+ size = alloc_size;
+ data = alloc_data;
+ }
+
+ // Encode the property into the data buffer.
+ Writer writer(data, size);
+
+ for (const auto &[property, value] : properties) {
+ if (value.IsNull()) {
+ continue;
+ }
+ MG_ASSERT(EncodeProperty(&writer, property, value), "Invalid database state!");
+ writer.Written();
+ }
+
+ auto metadata = writer.WriteMetadata();
+ if (metadata) {
+ // If there is any space left in the buffer we add a tombstone to
+ // indicate that there are no more properties to be decoded.
+ metadata->Set({Type::EMPTY});
+ }
+
+ return true;
+}
+
bool PropertyStore::ClearProperties() {
bool in_local_buffer = false;
uint64_t size;
diff --git a/src/storage/v2/property_store.hpp b/src/storage/v2/property_store.hpp
index bd397285f..f0be30df7 100644
--- a/src/storage/v2/property_store.hpp
+++ b/src/storage/v2/property_store.hpp
@@ -1,4 +1,4 @@
-// Copyright 2022 Memgraph Ltd.
+// Copyright 2023 Memgraph Ltd.
//
// Use of this software is governed by the Business Source License
// included in the file licenses/BSL.txt; by using this file, you agree to be bound by the terms of the Business Source
@@ -59,6 +59,12 @@ class PropertyStore {
/// @throw std::bad_alloc
bool SetProperty(PropertyId property, const PropertyValue &value);
+ /// Init property values and return `true` if insertion took place. `false` is
+ /// returned if there exists property in property store and insertion couldn't take place. The time complexity of this
+ /// function is O(n).
+ /// @throw std::bad_alloc
+ bool InitProperties(const std::map<storage::PropertyId, storage::PropertyValue> &properties);
+
/// Remove all properties and return `true` if any removal took place.
/// `false` is returned if there were no properties to remove. The time
/// complexity of this function is O(1).
diff --git a/src/storage/v2/vertex_accessor.cpp b/src/storage/v2/vertex_accessor.cpp
index 05ba1ebcc..3b76080cb 100644
--- a/src/storage/v2/vertex_accessor.cpp
+++ b/src/storage/v2/vertex_accessor.cpp
@@ -1,4 +1,4 @@
-// Copyright 2022 Memgraph Ltd.
+// Copyright 2023 Memgraph Ltd.
//
// Use of this software is governed by the Business Source License
// included in the file licenses/BSL.txt; by using this file, you agree to be bound by the terms of the Business Source
@@ -230,6 +230,23 @@ Result<PropertyValue> VertexAccessor::SetProperty(PropertyId property, const Pro
return std::move(current_value);
}
+Result<bool> VertexAccessor::InitProperties(const std::map<storage::PropertyId, storage::PropertyValue> &properties) {
+ utils::MemoryTracker::OutOfMemoryExceptionEnabler oom_exception;
+ std::lock_guard<utils::SpinLock> guard(vertex_->lock);
+
+ if (!PrepareForWrite(transaction_, vertex_)) return Error::SERIALIZATION_ERROR;
+
+ if (vertex_->deleted) return Error::DELETED_OBJECT;
+
+ if (!vertex_->properties.InitProperties(properties)) return false;
+ for (const auto &[property, value] : properties) {
+ CreateAndLinkDelta(transaction_, vertex_, Delta::SetPropertyTag(), property, PropertyValue());
+ UpdateOnSetProperty(indices_, property, value, vertex_, *transaction_);
+ }
+
+ return true;
+}
+
Result<std::map<PropertyId, PropertyValue>> VertexAccessor::ClearProperties() {
std::lock_guard<utils::SpinLock> guard(vertex_->lock);
diff --git a/src/storage/v2/vertex_accessor.hpp b/src/storage/v2/vertex_accessor.hpp
index 840eec910..79a391708 100644
--- a/src/storage/v2/vertex_accessor.hpp
+++ b/src/storage/v2/vertex_accessor.hpp
@@ -1,4 +1,4 @@
-// Copyright 2022 Memgraph Ltd.
+// Copyright 2023 Memgraph Ltd.
//
// Use of this software is governed by the Business Source License
// included in the file licenses/BSL.txt; by using this file, you agree to be bound by the terms of the Business Source
@@ -68,6 +68,11 @@ class VertexAccessor final {
/// @throw std::bad_alloc
Result<PropertyValue> SetProperty(PropertyId property, const PropertyValue &value);
+ /// Set property values only if property store is empty. Returns `true` if successully set all values,
+ /// `false` otherwise.
+ /// @throw std::bad_alloc
+ Result<bool> InitProperties(const std::map<storage::PropertyId, storage::PropertyValue> &properties);
+
/// Remove all properties and return the values of the removed properties.
/// @throw std::bad_alloc
Result<std::map<PropertyId, PropertyValue>> ClearProperties();
diff --git a/tests/gql_behave/tests/memgraph_V1/features/functions.feature b/tests/gql_behave/tests/memgraph_V1/features/functions.feature
index 6df7e2e0c..169722f88 100644
--- a/tests/gql_behave/tests/memgraph_V1/features/functions.feature
+++ b/tests/gql_behave/tests/memgraph_V1/features/functions.feature
@@ -585,12 +585,12 @@ Feature: Functions
"""
When executing query:
"""
- MATCH ()-[r]->() RETURN PROPERTIES(r) AS p
+ MATCH ()-[r]->() RETURN PROPERTIES(r) AS p ORDER BY p.prop;
"""
Then the result should be:
| p |
- | {b: true} |
| {} |
+ | {b: true} |
| {c: 123} |
Scenario: Properties test2:
diff --git a/tests/unit/storage_v2_property_store.cpp b/tests/unit/storage_v2_property_store.cpp
index e247806d8..bf2a8f0a9 100644
--- a/tests/unit/storage_v2_property_store.cpp
+++ b/tests/unit/storage_v2_property_store.cpp
@@ -1,4 +1,4 @@
-// Copyright 2022 Memgraph Ltd.
+// Copyright 2023 Memgraph Ltd.
//
// Use of this software is governed by the Business Source License
// included in the file licenses/BSL.txt; by using this file, you agree to be bound by the terms of the Business Source
@@ -649,3 +649,26 @@ TEST(PropertyStore, IsPropertyEqualTemporalData) {
ASSERT_FALSE(props.IsPropertyEqual(prop, memgraph::storage::PropertyValue(memgraph::storage::TemporalData{
memgraph::storage::TemporalType::Date, 30})));
}
+
+TEST(PropertyStore, SetMultipleProperties) {
+ memgraph::storage::PropertyStore store;
+ std::vector<memgraph::storage::PropertyValue> vec{memgraph::storage::PropertyValue(true),
+ memgraph::storage::PropertyValue(123),
+ memgraph::storage::PropertyValue()};
+ std::map<std::string, memgraph::storage::PropertyValue> map{{"nandare", memgraph::storage::PropertyValue(false)}};
+ const memgraph::storage::TemporalData temporal{memgraph::storage::TemporalType::LocalDateTime, 23};
+ std::map<memgraph::storage::PropertyId, memgraph::storage::PropertyValue> data{
+ {memgraph::storage::PropertyId::FromInt(1), memgraph::storage::PropertyValue(true)},
+ {memgraph::storage::PropertyId::FromInt(2), memgraph::storage::PropertyValue(123)},
+ {memgraph::storage::PropertyId::FromInt(3), memgraph::storage::PropertyValue(123.5)},
+ {memgraph::storage::PropertyId::FromInt(4), memgraph::storage::PropertyValue("nandare")},
+ {memgraph::storage::PropertyId::FromInt(5), memgraph::storage::PropertyValue(vec)},
+ {memgraph::storage::PropertyId::FromInt(6), memgraph::storage::PropertyValue(map)},
+ {memgraph::storage::PropertyId::FromInt(7), memgraph::storage::PropertyValue(temporal)}};
+
+ store.InitProperties(data);
+
+ for (auto &[key, value] : data) {
+ ASSERT_TRUE(store.IsPropertyEqual(key, value));
+ }
+}