From c3bfd3004b765af9ddae72266e61257a1080536f Mon Sep 17 00:00:00 2001
From: Matej Ferencevic <matej.ferencevic@memgraph.io>
Date: Wed, 15 Jan 2020 10:13:48 +0100
Subject: [PATCH] Reduce memory allocations in indices
Reviewers: teon.banek
Reviewed By: teon.banek
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D2621
---
src/storage/v2/indices.cpp | 101 ++++++++++++++++++-------------------
src/storage/v2/indices.hpp | 7 ---
src/storage/v2/storage.hpp | 5 --
3 files changed, 50 insertions(+), 63 deletions(-)
diff --git a/src/storage/v2/indices.cpp b/src/storage/v2/indices.cpp
index a056d585f..cc4936deb 100644
--- a/src/storage/v2/indices.cpp
+++ b/src/storage/v2/indices.cpp
@@ -83,12 +83,11 @@ bool AnyVersionHasLabel(const Vertex &vertex, LabelId label,
/// Helper function for label-property index garbage collection. Returns true if
/// there's a reachable version of the vertex that has the given label and
/// property value.
-/// @throw std::bad_alloc if unable to copy the PropertyValue
bool AnyVersionHasLabelProperty(const Vertex &vertex, LabelId label,
PropertyId key, const PropertyValue &value,
uint64_t timestamp) {
bool has_label;
- PropertyValue current_value;
+ bool current_value_equal_to_value = value.IsNull();
bool deleted;
const Delta *delta;
{
@@ -96,20 +95,20 @@ bool AnyVersionHasLabelProperty(const Vertex &vertex, LabelId label,
has_label = utils::Contains(vertex.labels, label);
auto it = vertex.properties.find(key);
if (it != vertex.properties.end()) {
- current_value = it->second;
+ current_value_equal_to_value = it->second == value;
}
deleted = vertex.deleted;
delta = vertex.delta;
}
- if (!deleted && has_label && current_value == value) {
+ if (!deleted && has_label && current_value_equal_to_value) {
return true;
}
return AnyVersionSatisfiesPredicate(
timestamp, delta,
- [&has_label, ¤t_value, &deleted, label, key,
- value](const Delta &delta) {
+ [&has_label, ¤t_value_equal_to_value, &deleted, label, key,
+ &value](const Delta &delta) {
switch (delta.action) {
case Delta::Action::ADD_LABEL:
if (delta.label == label) {
@@ -125,7 +124,7 @@ bool AnyVersionHasLabelProperty(const Vertex &vertex, LabelId label,
break;
case Delta::Action::SET_PROPERTY:
if (delta.property.key == key) {
- current_value = delta.property.value;
+ current_value_equal_to_value = delta.property.value == value;
}
break;
case Delta::Action::RECREATE_OBJECT: {
@@ -144,7 +143,7 @@ bool AnyVersionHasLabelProperty(const Vertex &vertex, LabelId label,
case Delta::Action::REMOVE_OUT_EDGE:
break;
}
- return !deleted && has_label && current_value == value;
+ return !deleted && has_label && current_value_equal_to_value;
});
}
@@ -203,13 +202,12 @@ bool CurrentVersionHasLabel(const Vertex &vertex, LabelId label,
// Helper function for iterating through label-property index. Returns true if
// this transaction can see the given vertex, and the visible version has the
// given label and property.
-// @throw std::bad_alloc if unable to copy the PropertyValue
bool CurrentVersionHasLabelProperty(const Vertex &vertex, LabelId label,
PropertyId key, const PropertyValue &value,
Transaction *transaction, View view) {
bool deleted;
bool has_label;
- PropertyValue current_value;
+ bool current_value_equal_to_value = value.IsNull();
const Delta *delta;
{
std::lock_guard<utils::SpinLock> guard(vertex.lock);
@@ -217,50 +215,51 @@ bool CurrentVersionHasLabelProperty(const Vertex &vertex, LabelId label,
has_label = utils::Contains(vertex.labels, label);
auto it = vertex.properties.find(key);
if (it != vertex.properties.end()) {
- current_value = it->second;
+ current_value_equal_to_value = it->second == value;
}
delta = vertex.delta;
}
- ApplyDeltasForRead(
- transaction, delta, view,
- [&deleted, &has_label, ¤t_value, key, label](const Delta &delta) {
- switch (delta.action) {
- case Delta::Action::SET_PROPERTY: {
- if (delta.property.key == key) {
- current_value = delta.property.value;
- }
- break;
- }
- case Delta::Action::DELETE_OBJECT: {
- CHECK(!deleted) << "Invalid database state!";
- deleted = true;
- break;
- }
- case Delta::Action::RECREATE_OBJECT: {
- CHECK(deleted) << "Invalid database state!";
- deleted = false;
- break;
- }
- case Delta::Action::ADD_LABEL:
- if (delta.label == label) {
- CHECK(!has_label) << "Invalid database state!";
- has_label = true;
- }
- break;
- case Delta::Action::REMOVE_LABEL:
- if (delta.label == label) {
- CHECK(has_label) << "Invalid database state!";
- has_label = false;
- }
- break;
- case Delta::Action::ADD_IN_EDGE:
- case Delta::Action::ADD_OUT_EDGE:
- case Delta::Action::REMOVE_IN_EDGE:
- case Delta::Action::REMOVE_OUT_EDGE:
- break;
- }
- });
- return !deleted && has_label && current_value == value;
+ ApplyDeltasForRead(transaction, delta, view,
+ [&deleted, &has_label, ¤t_value_equal_to_value, key,
+ label, &value](const Delta &delta) {
+ switch (delta.action) {
+ case Delta::Action::SET_PROPERTY: {
+ if (delta.property.key == key) {
+ current_value_equal_to_value =
+ delta.property.value == value;
+ }
+ break;
+ }
+ case Delta::Action::DELETE_OBJECT: {
+ CHECK(!deleted) << "Invalid database state!";
+ deleted = true;
+ break;
+ }
+ case Delta::Action::RECREATE_OBJECT: {
+ CHECK(deleted) << "Invalid database state!";
+ deleted = false;
+ break;
+ }
+ case Delta::Action::ADD_LABEL:
+ if (delta.label == label) {
+ CHECK(!has_label) << "Invalid database state!";
+ has_label = true;
+ }
+ break;
+ case Delta::Action::REMOVE_LABEL:
+ if (delta.label == label) {
+ CHECK(has_label) << "Invalid database state!";
+ has_label = false;
+ }
+ break;
+ case Delta::Action::ADD_IN_EDGE:
+ case Delta::Action::ADD_OUT_EDGE:
+ case Delta::Action::REMOVE_IN_EDGE:
+ case Delta::Action::REMOVE_OUT_EDGE:
+ break;
+ }
+ });
+ return !deleted && has_label && current_value_equal_to_value;
}
} // namespace
diff --git a/src/storage/v2/indices.hpp b/src/storage/v2/indices.hpp
index 111b11bb8..7c312bd23 100644
--- a/src/storage/v2/indices.hpp
+++ b/src/storage/v2/indices.hpp
@@ -165,12 +165,10 @@ class LabelPropertyIndex {
std::vector<std::pair<LabelId, PropertyId>> ListIndices() const;
- /// @throw std::bad_alloc if unable to copy a PropertyValue
void RemoveObsoleteEntries(uint64_t oldest_active_start_timestamp);
class Iterable {
public:
- /// @throw std::bad_alloc if unable to copy a PropertyValue
Iterable(utils::SkipList<Entry>::Accessor index_accessor, LabelId label,
PropertyId property,
const std::optional<utils::Bound<PropertyValue>> &lower_bound,
@@ -180,7 +178,6 @@ class LabelPropertyIndex {
class Iterator {
public:
- /// @throw std::bad_alloc raised in AdvanceUntilValid
Iterator(Iterable *self, utils::SkipList<Entry>::Iterator index_iterator);
VertexAccessor operator*() const { return current_vertex_accessor_; }
@@ -192,11 +189,9 @@ class LabelPropertyIndex {
return index_iterator_ != other.index_iterator_;
}
- /// @throw std::bad_alloc raised in AdvanceUntilValid
Iterator &operator++();
private:
- /// @throw std::bad_alloc if unable to copy a PropertyValue
void AdvanceUntilValid();
Iterable *self_;
@@ -221,7 +216,6 @@ class LabelPropertyIndex {
Config::Items config_;
};
- /// @throw std::bad_alloc if unable to copy a PropertyValue
Iterable Vertices(
LabelId label, PropertyId property,
const std::optional<utils::Bound<PropertyValue>> &lower_bound,
@@ -276,7 +270,6 @@ struct Indices {
/// This function should be called from garbage collection to clean-up the
/// index.
-/// @throw std::bad_alloc raised in LabelPropertyIndex::RemoveObsoleteEntries
void RemoveObsoleteEntries(Indices *indices,
uint64_t oldest_active_start_timestamp);
diff --git a/src/storage/v2/storage.hpp b/src/storage/v2/storage.hpp
index 11f5804fb..dcdaae278 100644
--- a/src/storage/v2/storage.hpp
+++ b/src/storage/v2/storage.hpp
@@ -124,8 +124,6 @@ class VerticesIterable final {
VertexAccessor operator*() const;
- /// @throw std::bad_alloc raised in
- /// LabelPropertyIndex::Iterable::Iterator::operator++
Iterator &operator++();
bool operator==(const Iterator &other) const;
@@ -195,14 +193,11 @@ class Storage final {
VerticesIterable Vertices(LabelId label, View view);
- /// @throw std::bad_alloc raised in Index::Vertices
VerticesIterable Vertices(LabelId label, PropertyId property, View view);
- /// @throw std::bad_alloc raised in Index::Vertices
VerticesIterable Vertices(LabelId label, PropertyId property,
const PropertyValue &value, View view);
- /// @throw std::bad_alloc raised in Index::Vertices
VerticesIterable Vertices(
LabelId label, PropertyId property,
const std::optional<utils::Bound<PropertyValue>> &lower_bound,