From 976e6ff0a6fd4adcc459c4742e4c5af73657f84d Mon Sep 17 00:00:00 2001 From: jbajic <jure.bajic@memgraph.com> Date: Wed, 30 Nov 2022 14:11:22 +0100 Subject: [PATCH 01/19] Replace skiplist with std::set --- src/storage/v3/CMakeLists.txt | 2 - src/storage/v3/indices.cpp | 11 ++- src/storage/v3/indices.hpp | 6 +- src/storage/v3/key_store.cpp | 43 ------------ src/storage/v3/key_store.hpp | 41 ----------- .../v3/lexicographically_ordered_vertex.cpp | 14 ---- .../v3/lexicographically_ordered_vertex.hpp | 42 ----------- src/storage/v3/property_store.cpp | 11 +++ src/storage/v3/shard.cpp | 69 +++++++++---------- src/storage/v3/shard.hpp | 17 +++-- src/storage/v3/vertex.hpp | 4 +- src/storage/v3/vertex_accessor.cpp | 10 +-- ...s_skip_list.hpp => vertices_container.hpp} | 8 ++- 13 files changed, 70 insertions(+), 208 deletions(-) delete mode 100644 src/storage/v3/key_store.cpp delete mode 100644 src/storage/v3/lexicographically_ordered_vertex.cpp delete mode 100644 src/storage/v3/lexicographically_ordered_vertex.hpp rename src/storage/v3/{vertices_skip_list.hpp => vertices_container.hpp} (79%) diff --git a/src/storage/v3/CMakeLists.txt b/src/storage/v3/CMakeLists.txt index 2b3a7c861..f371f7b7b 100644 --- a/src/storage/v3/CMakeLists.txt +++ b/src/storage/v3/CMakeLists.txt @@ -9,8 +9,6 @@ set(storage_v3_src_files temporal.cpp edge_accessor.cpp indices.cpp - key_store.cpp - lexicographically_ordered_vertex.cpp property_store.cpp vertex_accessor.cpp schemas.cpp diff --git a/src/storage/v3/indices.cpp b/src/storage/v3/indices.cpp index b6a01b81a..fbe004d60 100644 --- a/src/storage/v3/indices.cpp +++ b/src/storage/v3/indices.cpp @@ -17,6 +17,7 @@ #include "storage/v3/mvcc.hpp" #include "storage/v3/property_value.hpp" #include "storage/v3/schemas.hpp" +#include "storage/v3/vertices_container.hpp" #include "utils/bound.hpp" #include "utils/logging.hpp" #include "utils/memory_tracker.hpp" @@ -269,7 +270,7 @@ void LabelIndex::UpdateOnAddLabel(LabelId label, Vertex *vertex, const Transacti acc.insert(Entry{vertex, tx.start_timestamp.logical_id}); } -bool LabelIndex::CreateIndex(LabelId label, VerticesSkipList::Accessor vertices) { +bool LabelIndex::CreateIndex(LabelId label, VertexContainer vertices) { utils::MemoryTracker::OutOfMemoryExceptionEnabler oom_exception; auto [it, emplaced] = index_.emplace(std::piecewise_construct, std::forward_as_tuple(label), std::forward_as_tuple()); if (!emplaced) { @@ -278,8 +279,7 @@ bool LabelIndex::CreateIndex(LabelId label, VerticesSkipList::Accessor vertices) } try { auto acc = it->second.access(); - for (auto &lgo_vertex : vertices) { - auto &vertex = lgo_vertex.vertex; + for ([[maybe_unused]] auto &[pk, vertex] : vertices) { if (vertex.deleted || !utils::Contains(vertex.labels, label)) { continue; } @@ -416,7 +416,7 @@ void LabelPropertyIndex::UpdateOnSetProperty(PropertyId property, const Property } } -bool LabelPropertyIndex::CreateIndex(LabelId label, PropertyId property, VerticesSkipList::Accessor vertices) { +bool LabelPropertyIndex::CreateIndex(LabelId label, PropertyId property, VertexContainer vertices) { utils::MemoryTracker::OutOfMemoryExceptionEnabler oom_exception; auto [it, emplaced] = index_.emplace(std::piecewise_construct, std::forward_as_tuple(label, property), std::forward_as_tuple()); @@ -426,8 +426,7 @@ bool LabelPropertyIndex::CreateIndex(LabelId label, PropertyId property, Vertice } try { auto acc = it->second.access(); - for (auto &lgo_vertex : vertices) { - auto &vertex = lgo_vertex.vertex; + for ([[maybe_unused]] auto &[pk, vertex] : vertices) { if (vertex.deleted || !utils::Contains(vertex.labels, label)) { continue; } diff --git a/src/storage/v3/indices.hpp b/src/storage/v3/indices.hpp index 0dd09ce53..02d8f362b 100644 --- a/src/storage/v3/indices.hpp +++ b/src/storage/v3/indices.hpp @@ -20,7 +20,7 @@ #include "storage/v3/property_value.hpp" #include "storage/v3/transaction.hpp" #include "storage/v3/vertex_accessor.hpp" -#include "storage/v3/vertices_skip_list.hpp" +#include "storage/v3/vertices_container.hpp" #include "utils/bound.hpp" #include "utils/logging.hpp" #include "utils/skip_list.hpp" @@ -59,7 +59,7 @@ class LabelIndex { void UpdateOnAddLabel(LabelId label, Vertex *vertex, const Transaction &tx); /// @throw std::bad_alloc - bool CreateIndex(LabelId label, VerticesSkipList::Accessor vertices); + bool CreateIndex(LabelId label, VertexContainer vertices); /// Returns false if there was no index to drop bool DropIndex(LabelId label) { return index_.erase(label) > 0; } @@ -157,7 +157,7 @@ class LabelPropertyIndex { void UpdateOnSetProperty(PropertyId property, const PropertyValue &value, Vertex *vertex, const Transaction &tx); /// @throw std::bad_alloc - bool CreateIndex(LabelId label, PropertyId property, VerticesSkipList::Accessor vertices); + bool CreateIndex(LabelId label, PropertyId property, VertexContainer vertices); bool DropIndex(LabelId label, PropertyId property) { return index_.erase({label, property}) > 0; } diff --git a/src/storage/v3/key_store.cpp b/src/storage/v3/key_store.cpp deleted file mode 100644 index 4e65f5fd1..000000000 --- a/src/storage/v3/key_store.cpp +++ /dev/null @@ -1,43 +0,0 @@ -// Copyright 2022 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 -// License, and you may not use this file except in compliance with the Business Source License. -// -// As of the Change Date specified in that file, in accordance with -// the Business Source License, use of this software will be governed -// by the Apache License, Version 2.0, included in the file -// licenses/APL.txt. - -#include <algorithm> -#include <iterator> -#include <ranges> - -#include "storage/v3/id_types.hpp" -#include "storage/v3/key_store.hpp" -#include "storage/v3/property_value.hpp" - -namespace memgraph::storage::v3 { - -KeyStore::KeyStore(const PrimaryKey &key_values) { - for (auto i = 0; i < key_values.size(); ++i) { - MG_ASSERT(!key_values[i].IsNull()); - store_.SetProperty(PropertyId::FromInt(i), key_values[i]); - } -} - -PropertyValue KeyStore::GetKey(const size_t index) const { return store_.GetProperty(PropertyId::FromUint(index)); } - -PropertyValue KeyStore::GetKey(const PropertyId property_id) const { return store_.GetProperty(property_id); } - -PrimaryKey KeyStore::Keys() const { - auto keys_map = store_.Properties(); - PrimaryKey keys; - keys.reserve(keys_map.size()); - std::ranges::transform( - keys_map, std::back_inserter(keys), - [](std::pair<const PropertyId, PropertyValue> &id_and_value) { return std::move(id_and_value.second); }); - return keys; -} - -} // namespace memgraph::storage::v3 diff --git a/src/storage/v3/key_store.hpp b/src/storage/v3/key_store.hpp index 4bc3c25e3..5c6e97426 100644 --- a/src/storage/v3/key_store.hpp +++ b/src/storage/v3/key_store.hpp @@ -11,12 +11,6 @@ #pragma once -#include <algorithm> -#include <compare> -#include <functional> - -#include "storage/v3/id_types.hpp" -#include "storage/v3/property_store.hpp" #include "storage/v3/property_value.hpp" namespace memgraph::storage::v3 { @@ -24,39 +18,4 @@ namespace memgraph::storage::v3 { // Primary key is a collection of primary properties. using PrimaryKey = std::vector<PropertyValue>; -class KeyStore { - public: - explicit KeyStore(const PrimaryKey &key_values); - - KeyStore(const KeyStore &) = delete; - KeyStore(KeyStore &&other) noexcept = default; - KeyStore &operator=(const KeyStore &) = delete; - KeyStore &operator=(KeyStore &&other) noexcept = default; - - ~KeyStore() = default; - - PropertyValue GetKey(size_t index) const; - - PropertyValue GetKey(PropertyId property) const; - - PrimaryKey Keys() const; - - friend bool operator<(const KeyStore &lhs, const KeyStore &rhs) { - return std::ranges::lexicographical_compare(lhs.Keys(), rhs.Keys(), std::less<PropertyValue>{}); - } - - friend bool operator==(const KeyStore &lhs, const KeyStore &rhs) { - return std::ranges::equal(lhs.Keys(), rhs.Keys()); - } - - friend bool operator<(const KeyStore &lhs, const PrimaryKey &rhs) { - return std::ranges::lexicographical_compare(lhs.Keys(), rhs, std::less<PropertyValue>{}); - } - - friend bool operator==(const KeyStore &lhs, const PrimaryKey &rhs) { return std::ranges::equal(lhs.Keys(), rhs); } - - private: - PropertyStore store_; -}; - } // namespace memgraph::storage::v3 diff --git a/src/storage/v3/lexicographically_ordered_vertex.cpp b/src/storage/v3/lexicographically_ordered_vertex.cpp deleted file mode 100644 index 04687f6e6..000000000 --- a/src/storage/v3/lexicographically_ordered_vertex.cpp +++ /dev/null @@ -1,14 +0,0 @@ -// Copyright 2022 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 -// License, and you may not use this file except in compliance with the Business Source License. -// -// As of the Change Date specified in that file, in accordance with -// the Business Source License, use of this software will be governed -// by the Apache License, Version 2.0, included in the file -// licenses/APL.txt. - -#include "storage/v3/lexicographically_ordered_vertex.hpp" - -namespace memgraph::storage::v3 {} // namespace memgraph::storage::v3 diff --git a/src/storage/v3/lexicographically_ordered_vertex.hpp b/src/storage/v3/lexicographically_ordered_vertex.hpp deleted file mode 100644 index e039a2687..000000000 --- a/src/storage/v3/lexicographically_ordered_vertex.hpp +++ /dev/null @@ -1,42 +0,0 @@ -// Copyright 2022 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 -// License, and you may not use this file except in compliance with the Business Source License. -// -// As of the Change Date specified in that file, in accordance with -// the Business Source License, use of this software will be governed -// by the Apache License, Version 2.0, included in the file -// licenses/APL.txt. - -#pragma once - -#include <concepts> -#include <type_traits> - -#include "storage/v3/vertex.hpp" -#include "utils/concepts.hpp" - -namespace memgraph::storage::v3 { - -struct LexicographicallyOrderedVertex { - Vertex vertex; - - friend bool operator==(const LexicographicallyOrderedVertex &lhs, const LexicographicallyOrderedVertex &rhs) { - return lhs.vertex.keys == rhs.vertex.keys; - } - - friend bool operator<(const LexicographicallyOrderedVertex &lhs, const LexicographicallyOrderedVertex &rhs) { - return lhs.vertex.keys < rhs.vertex.keys; - } - - // TODO(antaljanosbenjamin): maybe it worth to overload this for std::array to avoid heap construction of the vector - friend bool operator==(const LexicographicallyOrderedVertex &lhs, const std::vector<PropertyValue> &rhs) { - return lhs.vertex.keys == rhs; - } - - friend bool operator<(const LexicographicallyOrderedVertex &lhs, const std::vector<PropertyValue> &rhs) { - return lhs.vertex.keys < rhs; - } -}; -} // namespace memgraph::storage::v3 diff --git a/src/storage/v3/property_store.cpp b/src/storage/v3/property_store.cpp index fc7bd6984..2f8eeafe2 100644 --- a/src/storage/v3/property_store.cpp +++ b/src/storage/v3/property_store.cpp @@ -928,11 +928,22 @@ void SetSizeData(uint8_t *buffer, uint64_t size, uint8_t *data) { PropertyStore::PropertyStore() { memset(buffer_, 0, sizeof(buffer_)); } +// PropertyStore::PropertyStore(const PropertyStore &other) { memcpy(buffer_, other.buffer_, sizeof(buffer_)); } + PropertyStore::PropertyStore(PropertyStore &&other) noexcept { memcpy(buffer_, other.buffer_, sizeof(buffer_)); memset(other.buffer_, 0, sizeof(other.buffer_)); } +// PropertyStore &PropertyStore::operator=(const PropertyStore &other) { +// if (buffer_ == other.buffer_) [[unlikely]] { +// return *this; +// } +// memcpy(buffer_, other.buffer_, sizeof(buffer_)); + +// return *this; +// } + PropertyStore &PropertyStore::operator=(PropertyStore &&other) noexcept { uint64_t size{0}; uint8_t *data{nullptr}; diff --git a/src/storage/v3/shard.cpp b/src/storage/v3/shard.cpp index 5dd8e7256..21bace471 100644 --- a/src/storage/v3/shard.cpp +++ b/src/storage/v3/shard.cpp @@ -38,7 +38,7 @@ #include "storage/v3/transaction.hpp" #include "storage/v3/vertex.hpp" #include "storage/v3/vertex_accessor.hpp" -#include "storage/v3/vertices_skip_list.hpp" +#include "storage/v3/vertices_container.hpp" #include "utils/exceptions.hpp" #include "utils/file.hpp" #include "utils/logging.hpp" @@ -75,11 +75,11 @@ uint64_t GetCleanupBeforeTimestamp(const std::map<uint64_t, std::unique_ptr<Tran } // namespace -auto AdvanceToVisibleVertex(VerticesSkipList::Iterator it, VerticesSkipList::Iterator end, +auto AdvanceToVisibleVertex(VertexContainer::iterator it, VertexContainer::iterator end, std::optional<VertexAccessor> *vertex, Transaction *tx, View view, Indices *indices, Config::Items config, const VertexValidator &vertex_validator) { while (it != end) { - *vertex = VertexAccessor::Create(&it->vertex, tx, indices, config, vertex_validator, view); + *vertex = VertexAccessor::Create(&it->second, tx, indices, config, vertex_validator, view); if (!*vertex) { ++it; continue; @@ -89,7 +89,7 @@ auto AdvanceToVisibleVertex(VerticesSkipList::Iterator it, VerticesSkipList::Ite return it; } -AllVerticesIterable::Iterator::Iterator(AllVerticesIterable *self, VerticesSkipList::Iterator it) +AllVerticesIterable::Iterator::Iterator(AllVerticesIterable *self, VertexContainer::iterator it) : self_(self), it_(AdvanceToVisibleVertex(it, self->vertices_accessor_.end(), &self->vertex_, self->transaction_, self->view_, self->indices_, self->config_, *self_->vertex_validator_)) {} @@ -357,16 +357,15 @@ ShardResult<VertexAccessor> Shard::Accessor::CreateVertexAndValidate( return {std::move(maybe_schema_violation.GetError())}; } - auto acc = shard_->vertices_.access(); auto *delta = CreateDeleteObjectDelta(transaction_); - auto [it, inserted] = acc.insert({Vertex{delta, primary_properties}}); - delta->prev.Set(&it->vertex); + auto [it, inserted] = shard_->vertices_.emplace(primary_properties, Vertex(delta, primary_properties)); + delta->prev.Set(&it->second); - VertexAccessor vertex_acc{&it->vertex, transaction_, &shard_->indices_, config_, shard_->vertex_validator_}; + VertexAccessor vertex_acc{&it->second, transaction_, &shard_->indices_, config_, shard_->vertex_validator_}; if (!inserted) { return SHARD_ERROR(ErrorCode::VERTEX_ALREADY_INSERTED); } - MG_ASSERT(it != acc.end(), "Invalid Vertex accessor!"); + MG_ASSERT(it != shard_->vertices_.end(), "Invalid Vertex accessor!"); // TODO(jbajic) Improve, maybe delay index update for (const auto &[property_id, property_value] : properties) { @@ -386,13 +385,11 @@ ShardResult<VertexAccessor> Shard::Accessor::CreateVertexAndValidate( } std::optional<VertexAccessor> Shard::Accessor::FindVertex(std::vector<PropertyValue> primary_key, View view) { - auto acc = shard_->vertices_.access(); - // Later on use label space - auto it = acc.find(primary_key); - if (it == acc.end()) { + auto it = shard_->vertices_.find(primary_key); + if (it == shard_->vertices_.end()) { return std::nullopt; } - return VertexAccessor::Create(&it->vertex, transaction_, &shard_->indices_, config_, shard_->vertex_validator_, view); + return VertexAccessor::Create(&it->second, transaction_, &shard_->indices_, config_, shard_->vertex_validator_, view); } ShardResult<std::optional<VertexAccessor>> Shard::Accessor::DeleteVertex(VertexAccessor *vertex) { @@ -438,7 +435,7 @@ ShardResult<std::optional<std::pair<VertexAccessor, std::vector<EdgeAccessor>>>> } std::vector<EdgeAccessor> deleted_edges; - const VertexId vertex_id{shard_->primary_label_, vertex_ptr->keys.Keys()}; + const VertexId vertex_id{shard_->primary_label_, vertex_ptr->keys}; for (const auto &item : in_edges) { auto [edge_type, from_vertex, edge] = item; EdgeAccessor e(edge, edge_type, from_vertex, vertex_id, transaction_, &shard_->indices_, config_); @@ -488,22 +485,22 @@ ShardResult<EdgeAccessor> Shard::Accessor::CreateEdge(VertexId from_vertex_id, V Vertex *from_vertex{nullptr}; Vertex *to_vertex{nullptr}; - auto acc = shard_->vertices_.access(); + auto &vertices = shard_->vertices_; const auto from_is_local = shard_->IsVertexBelongToShard(from_vertex_id); const auto to_is_local = shard_->IsVertexBelongToShard(to_vertex_id); MG_ASSERT(from_is_local || to_is_local, "Trying to create an edge without having a local vertex"); if (from_is_local) { - auto it = acc.find(from_vertex_id.primary_key); - MG_ASSERT(it != acc.end(), "Cannot find local vertex"); - from_vertex = &it->vertex; + auto it = vertices.find(from_vertex_id.primary_key); + MG_ASSERT(it != vertices.end(), "Cannot find local vertex"); + from_vertex = &it->second; } if (to_is_local) { - auto it = acc.find(to_vertex_id.primary_key); - MG_ASSERT(it != acc.end(), "Cannot find local vertex"); - to_vertex = &it->vertex; + auto it = vertices.find(to_vertex_id.primary_key); + MG_ASSERT(it != vertices.end(), "Cannot find local vertex"); + to_vertex = &it->second; } if (from_is_local) { @@ -546,21 +543,21 @@ ShardResult<std::optional<EdgeAccessor>> Shard::Accessor::DeleteEdge(VertexId fr Vertex *from_vertex{nullptr}; Vertex *to_vertex{nullptr}; - auto acc = shard_->vertices_.access(); + auto &vertices = shard_->vertices_; const auto from_is_local = shard_->IsVertexBelongToShard(from_vertex_id); const auto to_is_local = shard_->IsVertexBelongToShard(to_vertex_id); if (from_is_local) { - auto it = acc.find(from_vertex_id.primary_key); - MG_ASSERT(it != acc.end(), "Cannot find local vertex"); - from_vertex = &it->vertex; + auto it = vertices.find(from_vertex_id.primary_key); + MG_ASSERT(it != vertices.end(), "Cannot find local vertex"); + from_vertex = &it->second; } if (to_is_local) { - auto it = acc.find(to_vertex_id.primary_key); - MG_ASSERT(it != acc.end(), "Cannot find local vertex"); - to_vertex = &it->vertex; + auto it = vertices.find(to_vertex_id.primary_key); + MG_ASSERT(it != vertices.end(), "Cannot find local vertex"); + to_vertex = &it->second; } MG_ASSERT(from_is_local || to_is_local, "Trying to delete an edge without having a local vertex"); @@ -762,7 +759,7 @@ void Shard::Accessor::Abort() { } case Delta::Action::DELETE_OBJECT: { vertex->deleted = true; - shard_->deleted_vertices_.push_back(vertex->keys.Keys()); + shard_->deleted_vertices_.push_back(vertex->keys); break; } case Delta::Action::RECREATE_OBJECT: { @@ -851,10 +848,7 @@ const std::string &Shard::EdgeTypeToName(EdgeTypeId edge_type) const { bool Shard::CreateIndex(LabelId label, const std::optional<uint64_t> /*desired_commit_timestamp*/) { // TODO(jbajic) response should be different when label == primary_label - if (label == primary_label_ || !indices_.label_index.CreateIndex(label, vertices_.access())) { - return false; - } - return true; + return !(label == primary_label_ || !indices_.label_index.CreateIndex(label, vertices_)); } bool Shard::CreateIndex(LabelId label, PropertyId property, @@ -865,7 +859,7 @@ bool Shard::CreateIndex(LabelId label, PropertyId property, // Index already exists on primary key return false; } - if (!indices_.label_property_index.CreateIndex(label, property, vertices_.access())) { + if (!indices_.label_property_index.CreateIndex(label, property, vertices_)) { return false; } return true; @@ -975,7 +969,7 @@ void Shard::CollectGarbage(const io::Time current_time) { Vertex *vertex = prev.vertex; vertex->delta = nullptr; if (vertex->deleted) { - deleted_vertices_.push_back(vertex->keys.Keys()); + deleted_vertices_.push_back(vertex->keys); } break; } @@ -1022,9 +1016,8 @@ void Shard::CollectGarbage(const io::Time current_time) { RemoveObsoleteEntries(&indices_, clean_up_before_timestamp); } - auto vertex_acc = vertices_.access(); for (const auto &vertex : deleted_vertices_) { - MG_ASSERT(vertex_acc.remove(vertex), "Invalid database state!"); + MG_ASSERT(vertices_.erase(vertex), "Invalid database state!"); } deleted_vertices_.clear(); { diff --git a/src/storage/v3/shard.hpp b/src/storage/v3/shard.hpp index 5c025ae57..9928c6336 100644 --- a/src/storage/v3/shard.hpp +++ b/src/storage/v3/shard.hpp @@ -31,7 +31,6 @@ #include "storage/v3/indices.hpp" #include "storage/v3/isolation_level.hpp" #include "storage/v3/key_store.hpp" -#include "storage/v3/lexicographically_ordered_vertex.hpp" #include "storage/v3/mvcc.hpp" #include "storage/v3/name_id_mapper.hpp" #include "storage/v3/property_value.hpp" @@ -42,7 +41,7 @@ #include "storage/v3/vertex.hpp" #include "storage/v3/vertex_accessor.hpp" #include "storage/v3/vertex_id.hpp" -#include "storage/v3/vertices_skip_list.hpp" +#include "storage/v3/vertices_container.hpp" #include "storage/v3/view.hpp" #include "utils/exceptions.hpp" #include "utils/file_locker.hpp" @@ -65,7 +64,7 @@ namespace memgraph::storage::v3 { /// An instance of this will be usually be wrapped inside VerticesIterable for /// generic, public use. class AllVerticesIterable final { - VerticesSkipList::Accessor vertices_accessor_; + VertexContainer vertices_accessor_; Transaction *transaction_; View view_; Indices *indices_; @@ -76,10 +75,10 @@ class AllVerticesIterable final { public: class Iterator final { AllVerticesIterable *self_; - VerticesSkipList::Iterator it_; + VertexContainer::iterator it_; public: - Iterator(AllVerticesIterable *self, VerticesSkipList::Iterator it); + Iterator(AllVerticesIterable *self, VertexContainer::iterator it); VertexAccessor operator*() const; @@ -90,8 +89,8 @@ class AllVerticesIterable final { bool operator!=(const Iterator &other) const { return !(*this == other); } }; - AllVerticesIterable(VerticesSkipList::Accessor vertices_accessor, Transaction *transaction, View view, - Indices *indices, Config::Items config, const VertexValidator &vertex_validator) + AllVerticesIterable(VertexContainer vertices_accessor, Transaction *transaction, View view, Indices *indices, + Config::Items config, const VertexValidator &vertex_validator) : vertices_accessor_(std::move(vertices_accessor)), transaction_(transaction), view_(view), @@ -213,7 +212,7 @@ class Shard final { std::optional<VertexAccessor> FindVertex(std::vector<PropertyValue> primary_key, View view); VerticesIterable Vertices(View view) { - return VerticesIterable(AllVerticesIterable(shard_->vertices_.access(), transaction_, view, &shard_->indices_, + return VerticesIterable(AllVerticesIterable(shard_->vertices_, transaction_, view, &shard_->indices_, shard_->config_.items, shard_->vertex_validator_)); } @@ -377,7 +376,7 @@ class Shard final { // The shard's range is [min, max) PrimaryKey min_primary_key_; std::optional<PrimaryKey> max_primary_key_; - VerticesSkipList vertices_; + VertexContainer vertices_; utils::SkipList<Edge> edges_; // Even though the edge count is already kept in the `edges_` SkipList, the // list is used only when properties are enabled for edges. Because of that we diff --git a/src/storage/v3/vertex.hpp b/src/storage/v3/vertex.hpp index 2ed4c6e4c..565e39d1a 100644 --- a/src/storage/v3/vertex.hpp +++ b/src/storage/v3/vertex.hpp @@ -31,14 +31,14 @@ namespace memgraph::storage::v3 { struct Vertex { using EdgeLink = std::tuple<EdgeTypeId, VertexId, EdgeRef>; - Vertex(Delta *delta, const std::vector<PropertyValue> &primary_properties) : keys{primary_properties}, delta{delta} { + Vertex(Delta *delta, PrimaryKey primary_properties) : keys{std::move(primary_properties)}, delta{delta} { MG_ASSERT(delta == nullptr || delta->action == Delta::Action::DELETE_OBJECT, "Vertex must be created with an initial DELETE_OBJECT delta!"); } friend bool operator==(const Vertex &vertex, const PrimaryKey &primary_key) { return vertex.keys == primary_key; } - KeyStore keys; + PrimaryKey keys; std::vector<LabelId> labels; PropertyStore properties; diff --git a/src/storage/v3/vertex_accessor.cpp b/src/storage/v3/vertex_accessor.cpp index 1b0ba6340..72ea4d7fb 100644 --- a/src/storage/v3/vertex_accessor.cpp +++ b/src/storage/v3/vertex_accessor.cpp @@ -215,14 +215,14 @@ ShardResult<PrimaryKey> VertexAccessor::PrimaryKey(const View view) const { if (const auto result = CheckVertexExistence(view); result.HasError()) { return result.GetError(); } - return vertex_->keys.Keys(); + return vertex_->keys; } ShardResult<VertexId> VertexAccessor::Id(View view) const { if (const auto result = CheckVertexExistence(view); result.HasError()) { return result.GetError(); } - return VertexId{vertex_validator_->primary_label_, vertex_->keys.Keys()}; + return VertexId{vertex_validator_->primary_label_, vertex_->keys}; }; ShardResult<std::vector<LabelId>> VertexAccessor::Labels(View view) const { @@ -401,7 +401,7 @@ PropertyValue VertexAccessor::GetPropertyValue(PropertyId property, View view) c } } - value = vertex_->keys.GetKey(property_index); + value = vertex_->keys[property_index]; if (value.IsNull()) { value = vertex_->properties.GetProperty(property); } @@ -572,7 +572,7 @@ ShardResult<std::vector<EdgeAccessor>> VertexAccessor::InEdges(View view, const return ret; } ret.reserve(in_edges.size()); - const auto id = VertexId{vertex_validator_->primary_label_, vertex_->keys.Keys()}; + const auto id = VertexId{vertex_validator_->primary_label_, vertex_->keys}; for (const auto &item : in_edges) { const auto &[edge_type, from_vertex, edge] = item; ret.emplace_back(edge, edge_type, from_vertex, id, transaction_, indices_, config_); @@ -652,7 +652,7 @@ ShardResult<std::vector<EdgeAccessor>> VertexAccessor::OutEdges(View view, const return ret; } ret.reserve(out_edges.size()); - const auto id = VertexId{vertex_validator_->primary_label_, vertex_->keys.Keys()}; + const auto id = VertexId{vertex_validator_->primary_label_, vertex_->keys}; for (const auto &item : out_edges) { const auto &[edge_type, to_vertex, edge] = item; ret.emplace_back(edge, edge_type, id, to_vertex, transaction_, indices_, config_); diff --git a/src/storage/v3/vertices_skip_list.hpp b/src/storage/v3/vertices_container.hpp similarity index 79% rename from src/storage/v3/vertices_skip_list.hpp rename to src/storage/v3/vertices_container.hpp index 283dd5aeb..c67f78efd 100644 --- a/src/storage/v3/vertices_skip_list.hpp +++ b/src/storage/v3/vertices_container.hpp @@ -11,9 +11,11 @@ #pragma once -#include "storage/v3/lexicographically_ordered_vertex.hpp" -#include "utils/skip_list.hpp" +#include <map> + +#include "storage/v3/key_store.hpp" +#include "storage/v3/vertex.hpp" namespace memgraph::storage::v3 { -using VerticesSkipList = utils::SkipList<LexicographicallyOrderedVertex>; +using VertexContainer = std::map<PrimaryKey, Vertex>; } // namespace memgraph::storage::v3 From 9af20c295c985a2a4b0c18e93e69d057f8cf9f10 Mon Sep 17 00:00:00 2001 From: jbajic <jure.bajic@memgraph.com> Date: Wed, 30 Nov 2022 14:39:52 +0100 Subject: [PATCH 02/19] Fix VerticesContainer being passed by value --- src/storage/v3/indices.cpp | 4 ++-- src/storage/v3/indices.hpp | 4 ++-- src/storage/v3/property_store.cpp | 11 ----------- src/storage/v3/shard.cpp | 6 +++--- src/storage/v3/shard.hpp | 10 +++++----- 5 files changed, 12 insertions(+), 23 deletions(-) diff --git a/src/storage/v3/indices.cpp b/src/storage/v3/indices.cpp index fbe004d60..a7a898f82 100644 --- a/src/storage/v3/indices.cpp +++ b/src/storage/v3/indices.cpp @@ -270,7 +270,7 @@ void LabelIndex::UpdateOnAddLabel(LabelId label, Vertex *vertex, const Transacti acc.insert(Entry{vertex, tx.start_timestamp.logical_id}); } -bool LabelIndex::CreateIndex(LabelId label, VertexContainer vertices) { +bool LabelIndex::CreateIndex(LabelId label, VertexContainer &vertices) { utils::MemoryTracker::OutOfMemoryExceptionEnabler oom_exception; auto [it, emplaced] = index_.emplace(std::piecewise_construct, std::forward_as_tuple(label), std::forward_as_tuple()); if (!emplaced) { @@ -416,7 +416,7 @@ void LabelPropertyIndex::UpdateOnSetProperty(PropertyId property, const Property } } -bool LabelPropertyIndex::CreateIndex(LabelId label, PropertyId property, VertexContainer vertices) { +bool LabelPropertyIndex::CreateIndex(LabelId label, PropertyId property, VertexContainer &vertices) { utils::MemoryTracker::OutOfMemoryExceptionEnabler oom_exception; auto [it, emplaced] = index_.emplace(std::piecewise_construct, std::forward_as_tuple(label, property), std::forward_as_tuple()); diff --git a/src/storage/v3/indices.hpp b/src/storage/v3/indices.hpp index 02d8f362b..6182e789c 100644 --- a/src/storage/v3/indices.hpp +++ b/src/storage/v3/indices.hpp @@ -59,7 +59,7 @@ class LabelIndex { void UpdateOnAddLabel(LabelId label, Vertex *vertex, const Transaction &tx); /// @throw std::bad_alloc - bool CreateIndex(LabelId label, VertexContainer vertices); + bool CreateIndex(LabelId label, VertexContainer &vertices); /// Returns false if there was no index to drop bool DropIndex(LabelId label) { return index_.erase(label) > 0; } @@ -157,7 +157,7 @@ class LabelPropertyIndex { void UpdateOnSetProperty(PropertyId property, const PropertyValue &value, Vertex *vertex, const Transaction &tx); /// @throw std::bad_alloc - bool CreateIndex(LabelId label, PropertyId property, VertexContainer vertices); + bool CreateIndex(LabelId label, PropertyId property, VertexContainer &vertices); bool DropIndex(LabelId label, PropertyId property) { return index_.erase({label, property}) > 0; } diff --git a/src/storage/v3/property_store.cpp b/src/storage/v3/property_store.cpp index 2f8eeafe2..fc7bd6984 100644 --- a/src/storage/v3/property_store.cpp +++ b/src/storage/v3/property_store.cpp @@ -928,22 +928,11 @@ void SetSizeData(uint8_t *buffer, uint64_t size, uint8_t *data) { PropertyStore::PropertyStore() { memset(buffer_, 0, sizeof(buffer_)); } -// PropertyStore::PropertyStore(const PropertyStore &other) { memcpy(buffer_, other.buffer_, sizeof(buffer_)); } - PropertyStore::PropertyStore(PropertyStore &&other) noexcept { memcpy(buffer_, other.buffer_, sizeof(buffer_)); memset(other.buffer_, 0, sizeof(other.buffer_)); } -// PropertyStore &PropertyStore::operator=(const PropertyStore &other) { -// if (buffer_ == other.buffer_) [[unlikely]] { -// return *this; -// } -// memcpy(buffer_, other.buffer_, sizeof(buffer_)); - -// return *this; -// } - PropertyStore &PropertyStore::operator=(PropertyStore &&other) noexcept { uint64_t size{0}; uint8_t *data{nullptr}; diff --git a/src/storage/v3/shard.cpp b/src/storage/v3/shard.cpp index 21bace471..cde51739a 100644 --- a/src/storage/v3/shard.cpp +++ b/src/storage/v3/shard.cpp @@ -91,15 +91,15 @@ auto AdvanceToVisibleVertex(VertexContainer::iterator it, VertexContainer::itera AllVerticesIterable::Iterator::Iterator(AllVerticesIterable *self, VertexContainer::iterator it) : self_(self), - it_(AdvanceToVisibleVertex(it, self->vertices_accessor_.end(), &self->vertex_, self->transaction_, self->view_, + it_(AdvanceToVisibleVertex(it, self->vertices_accessor_->end(), &self->vertex_, self->transaction_, self->view_, self->indices_, self->config_, *self_->vertex_validator_)) {} VertexAccessor AllVerticesIterable::Iterator::operator*() const { return *self_->vertex_; } AllVerticesIterable::Iterator &AllVerticesIterable::Iterator::operator++() { ++it_; - it_ = AdvanceToVisibleVertex(it_, self_->vertices_accessor_.end(), &self_->vertex_, self_->transaction_, self_->view_, - self_->indices_, self_->config_, *self_->vertex_validator_); + it_ = AdvanceToVisibleVertex(it_, self_->vertices_accessor_->end(), &self_->vertex_, self_->transaction_, + self_->view_, self_->indices_, self_->config_, *self_->vertex_validator_); return *this; } diff --git a/src/storage/v3/shard.hpp b/src/storage/v3/shard.hpp index 9928c6336..8a5bdd268 100644 --- a/src/storage/v3/shard.hpp +++ b/src/storage/v3/shard.hpp @@ -64,7 +64,7 @@ namespace memgraph::storage::v3 { /// An instance of this will be usually be wrapped inside VerticesIterable for /// generic, public use. class AllVerticesIterable final { - VertexContainer vertices_accessor_; + VertexContainer *vertices_accessor_; Transaction *transaction_; View view_; Indices *indices_; @@ -89,17 +89,17 @@ class AllVerticesIterable final { bool operator!=(const Iterator &other) const { return !(*this == other); } }; - AllVerticesIterable(VertexContainer vertices_accessor, Transaction *transaction, View view, Indices *indices, + AllVerticesIterable(VertexContainer &vertices_accessor, Transaction *transaction, View view, Indices *indices, Config::Items config, const VertexValidator &vertex_validator) - : vertices_accessor_(std::move(vertices_accessor)), + : vertices_accessor_(&vertices_accessor), transaction_(transaction), view_(view), indices_(indices), config_(config), vertex_validator_{&vertex_validator} {} - Iterator begin() { return {this, vertices_accessor_.begin()}; } - Iterator end() { return {this, vertices_accessor_.end()}; } + Iterator begin() { return {this, vertices_accessor_->begin()}; } + Iterator end() { return {this, vertices_accessor_->end()}; } }; /// Generic access to different kinds of vertex iterations. From 31f907cb536eeea4c0546e4e99fe2f7bbe5817c8 Mon Sep 17 00:00:00 2001 From: jbajic <jure.bajic@memgraph.com> Date: Wed, 30 Nov 2022 14:40:12 +0100 Subject: [PATCH 03/19] Remove keystore --- tests/unit/CMakeLists.txt | 3 -- tests/unit/storage_v3_key_store.cpp | 46 ----------------------------- 2 files changed, 49 deletions(-) delete mode 100644 tests/unit/storage_v3_key_store.cpp diff --git a/tests/unit/CMakeLists.txt b/tests/unit/CMakeLists.txt index 188c6c1b0..5bfa26afd 100644 --- a/tests/unit/CMakeLists.txt +++ b/tests/unit/CMakeLists.txt @@ -328,9 +328,6 @@ target_link_libraries(${test_prefix}query_v2_dummy_test mg-query-v2) add_unit_test(storage_v3_property_store.cpp) target_link_libraries(${test_prefix}storage_v3_property_store mg-storage-v3 fmt) -add_unit_test(storage_v3_key_store.cpp) -target_link_libraries(${test_prefix}storage_v3_key_store mg-storage-v3 rapidcheck rapidcheck_gtest) - add_unit_test(storage_v3_indices.cpp) target_link_libraries(${test_prefix}storage_v3_indices mg-storage-v3) diff --git a/tests/unit/storage_v3_key_store.cpp b/tests/unit/storage_v3_key_store.cpp deleted file mode 100644 index 4e9ae4da1..000000000 --- a/tests/unit/storage_v3_key_store.cpp +++ /dev/null @@ -1,46 +0,0 @@ -// Copyright 2022 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 -// License, and you may not use this file except in compliance with the Business Source License. -// -// As of the Change Date specified in that file, in accordance with -// the Business Source License, use of this software will be governed -// by the Apache License, Version 2.0, included in the file -// licenses/APL.txt. - -#include <algorithm> -#include <string> -#include <vector> - -/** - * gtest/gtest.h must be included before rapidcheck/gtest.h! - */ -#include <gtest/gtest.h> -#include <rapidcheck.h> -#include <rapidcheck/gtest.h> - -#include "storage/v3/id_types.hpp" -#include "storage/v3/key_store.hpp" -#include "storage/v3/property_value.hpp" - -namespace memgraph::storage::v3::test { - -RC_GTEST_PROP(KeyStore, KeyStore, (std::vector<std::string> values)) { - RC_PRE(!values.empty()); - - std::vector<PropertyValue> property_values; - property_values.reserve(values.size()); - std::transform(values.begin(), values.end(), std::back_inserter(property_values), - [](std::string &value) { return PropertyValue{std::move(value)}; }); - - KeyStore key_store{property_values}; - - const auto keys = key_store.Keys(); - RC_ASSERT(keys.size() == property_values.size()); - for (int i = 0; i < keys.size(); ++i) { - RC_ASSERT(keys[i] == property_values[i]); - RC_ASSERT(key_store.GetKey(i) == property_values[i]); - } -} -} // namespace memgraph::storage::v3::test From d4bdedd9e8d45b99d0cf7212e20506a6e696fec6 Mon Sep 17 00:00:00 2001 From: jbajic <jure.bajic@memgraph.com> Date: Wed, 30 Nov 2022 17:12:09 +0100 Subject: [PATCH 04/19] Fix GetProperty --- src/storage/v3/shard.hpp | 3 --- src/storage/v3/vertex_accessor.cpp | 19 +++++++++---------- 2 files changed, 9 insertions(+), 13 deletions(-) diff --git a/src/storage/v3/shard.hpp b/src/storage/v3/shard.hpp index 8a5bdd268..58e4a42ae 100644 --- a/src/storage/v3/shard.hpp +++ b/src/storage/v3/shard.hpp @@ -308,9 +308,6 @@ class Shard final { void Abort(); private: - /// @throw std::bad_alloc - VertexAccessor CreateVertex(Gid gid, LabelId primary_label); - Shard *shard_; Transaction *transaction_; Config::Items config_; diff --git a/src/storage/v3/vertex_accessor.cpp b/src/storage/v3/vertex_accessor.cpp index 72ea4d7fb..3f3d01efb 100644 --- a/src/storage/v3/vertex_accessor.cpp +++ b/src/storage/v3/vertex_accessor.cpp @@ -394,18 +394,13 @@ PropertyValue VertexAccessor::GetPropertyValue(PropertyId property, View view) c return value; } // Find PropertyId index in keystore - size_t property_index{0}; - for (; property_index < schema->second.size(); ++property_index) { + for (size_t property_index{0}; property_index < schema->second.size(); ++property_index) { if (schema->second[property_index].property_id == property) { - break; + return vertex_->keys[property_index]; } } - value = vertex_->keys[property_index]; - if (value.IsNull()) { - value = vertex_->properties.GetProperty(property); - } - return value; + return value = vertex_->properties.GetProperty(property); } ShardResult<PropertyValue> VertexAccessor::GetProperty(PropertyId property, View view) const { @@ -443,8 +438,12 @@ ShardResult<PropertyValue> VertexAccessor::GetProperty(PropertyId property, View break; } }); - if (!exists) return SHARD_ERROR(ErrorCode::NONEXISTENT_OBJECT); - if (!for_deleted_ && deleted) return SHARD_ERROR(ErrorCode::DELETED_OBJECT); + if (!exists) { + return SHARD_ERROR(ErrorCode::NONEXISTENT_OBJECT); + } + if (!for_deleted_ && deleted) { + return SHARD_ERROR(ErrorCode::DELETED_OBJECT); + } return std::move(value); } From 4e7d8c3ba2bac14b16c9b0f74628cf195e0df89f Mon Sep 17 00:00:00 2001 From: jbajic <jure.bajic@memgraph.com> Date: Thu, 1 Dec 2022 12:06:01 +0100 Subject: [PATCH 05/19] Replace LabelPropertyIndex skiplist with std::map --- src/storage/v3/indices.cpp | 114 ++++++++++++++++-------------- src/storage/v3/indices.hpp | 28 ++++---- src/storage/v3/property_value.hpp | 30 ++++++++ src/storage/v3/shard.hpp | 6 +- 4 files changed, 106 insertions(+), 72 deletions(-) diff --git a/src/storage/v3/indices.cpp b/src/storage/v3/indices.cpp index a7a898f82..503972b42 100644 --- a/src/storage/v3/indices.cpp +++ b/src/storage/v3/indices.cpp @@ -11,8 +11,12 @@ #include "indices.hpp" +#include <cstddef> +#include <cstdint> +#include <functional> #include <limits> +#include "storage/v3/delta.hpp" #include "storage/v3/id_types.hpp" #include "storage/v3/mvcc.hpp" #include "storage/v3/property_value.hpp" @@ -388,14 +392,13 @@ bool LabelPropertyIndex::Entry::operator<(const PropertyValue &rhs) const { retu bool LabelPropertyIndex::Entry::operator==(const PropertyValue &rhs) const { return value == rhs; } void LabelPropertyIndex::UpdateOnAddLabel(LabelId label, Vertex *vertex, const Transaction &tx) { - for (auto &[label_prop, storage] : index_) { + for (auto &[label_prop, container] : index_) { if (label_prop.first != label) { continue; } auto prop_value = vertex->properties.GetProperty(label_prop.second); if (!prop_value.IsNull()) { - auto acc = storage.access(); - acc.insert(Entry{std::move(prop_value), vertex, tx.start_timestamp.logical_id}); + container.emplace(vertex->keys, Entry{std::move(prop_value), vertex, tx.start_timestamp.logical_id}); } } } @@ -405,13 +408,12 @@ void LabelPropertyIndex::UpdateOnSetProperty(PropertyId property, const Property if (value.IsNull()) { return; } - for (auto &[label_prop, storage] : index_) { + for (auto &[label_prop, container] : index_) { if (label_prop.second != property) { continue; } if (utils::Contains(vertex->labels, label_prop.first)) { - auto acc = storage.access(); - acc.insert(Entry{value, vertex, tx.start_timestamp.logical_id}); + container.emplace(vertex->keys, Entry{value, vertex, tx.start_timestamp.logical_id}); } } } @@ -425,7 +427,6 @@ bool LabelPropertyIndex::CreateIndex(LabelId label, PropertyId property, VertexC return false; } try { - auto acc = it->second.access(); for ([[maybe_unused]] auto &[pk, vertex] : vertices) { if (vertex.deleted || !utils::Contains(vertex.labels, label)) { continue; @@ -434,7 +435,7 @@ bool LabelPropertyIndex::CreateIndex(LabelId label, PropertyId property, VertexC if (value.IsNull()) { continue; } - acc.insert(Entry{std::move(value), &vertex, 0}); + it->second.emplace(vertex.keys, Entry{std::move(value), &vertex, 0}); } } catch (const utils::OutOfMemoryException &) { utils::MemoryTracker::OutOfMemoryExceptionBlocker oom_exception_blocker; @@ -454,28 +455,28 @@ std::vector<std::pair<LabelId, PropertyId>> LabelPropertyIndex::ListIndices() co } void LabelPropertyIndex::RemoveObsoleteEntries(const uint64_t clean_up_before_timestamp) { - for (auto &[label_property, index] : index_) { - auto index_acc = index.access(); - for (auto it = index_acc.begin(); it != index_acc.end();) { + for (auto &[label_property, container] : index_) { + for (auto it = container.begin(); it != container.end();) { auto next_it = it; ++next_it; - if (it->timestamp >= clean_up_before_timestamp) { + if (it->second.timestamp >= clean_up_before_timestamp) { it = next_it; continue; } - if ((next_it != index_acc.end() && it->vertex == next_it->vertex && it->value == next_it->value) || - !AnyVersionHasLabelProperty(*it->vertex, label_property.first, label_property.second, it->value, + if (auto &entry = it->second; + (next_it != container.end() && entry.vertex == next_it->second.vertex && entry.value == entry.value) || + !AnyVersionHasLabelProperty(*entry.vertex, label_property.first, label_property.second, entry.value, clean_up_before_timestamp)) { - index_acc.remove(*it); + container.erase(it->first); } it = next_it; } } } -LabelPropertyIndex::Iterable::Iterator::Iterator(Iterable *self, utils::SkipList<Entry>::Iterator index_iterator) +LabelPropertyIndex::Iterable::Iterator::Iterator(Iterable *self, LabelPropertyIndexContainer::iterator index_iterator) : self_(self), index_iterator_(index_iterator), current_vertex_accessor_(nullptr, nullptr, nullptr, self_->config_, *self_->vertex_validator_), @@ -490,33 +491,33 @@ LabelPropertyIndex::Iterable::Iterator &LabelPropertyIndex::Iterable::Iterator:: } void LabelPropertyIndex::Iterable::Iterator::AdvanceUntilValid() { - for (; index_iterator_ != self_->index_accessor_.end(); ++index_iterator_) { - if (index_iterator_->vertex == current_vertex_) { + for (; index_iterator_ != self_->index_accessor_->end(); ++index_iterator_) { + if (index_iterator_->second.vertex == current_vertex_) { continue; } if (self_->lower_bound_) { - if (index_iterator_->value < self_->lower_bound_->value()) { + if (index_iterator_->second.value < self_->lower_bound_->value()) { continue; } - if (!self_->lower_bound_->IsInclusive() && index_iterator_->value == self_->lower_bound_->value()) { + if (!self_->lower_bound_->IsInclusive() && index_iterator_->second.value == self_->lower_bound_->value()) { continue; } } if (self_->upper_bound_) { - if (self_->upper_bound_->value() < index_iterator_->value) { - index_iterator_ = self_->index_accessor_.end(); + if (self_->upper_bound_->value() < index_iterator_->second.value) { + index_iterator_ = self_->index_accessor_->end(); break; } - if (!self_->upper_bound_->IsInclusive() && index_iterator_->value == self_->upper_bound_->value()) { - index_iterator_ = self_->index_accessor_.end(); + if (!self_->upper_bound_->IsInclusive() && index_iterator_->second.value == self_->upper_bound_->value()) { + index_iterator_ = self_->index_accessor_->end(); break; } } - if (CurrentVersionHasLabelProperty(*index_iterator_->vertex, self_->label_, self_->property_, - index_iterator_->value, self_->transaction_, self_->view_)) { - current_vertex_ = index_iterator_->vertex; + if (CurrentVersionHasLabelProperty(*index_iterator_->second.vertex, self_->label_, self_->property_, + index_iterator_->second.value, self_->transaction_, self_->view_)) { + current_vertex_ = index_iterator_->second.vertex; current_vertex_accessor_ = VertexAccessor(current_vertex_, self_->transaction_, self_->indices_, self_->config_, *self_->vertex_validator_); break; @@ -536,13 +537,12 @@ const PropertyValue kSmallestMap = PropertyValue(std::map<std::string, PropertyV const PropertyValue kSmallestTemporalData = PropertyValue(TemporalData{static_cast<TemporalType>(0), std::numeric_limits<int64_t>::min()}); -LabelPropertyIndex::Iterable::Iterable(utils::SkipList<Entry>::Accessor index_accessor, LabelId label, - PropertyId property, +LabelPropertyIndex::Iterable::Iterable(LabelPropertyIndexContainer &index_accessor, LabelId label, PropertyId property, const std::optional<utils::Bound<PropertyValue>> &lower_bound, const std::optional<utils::Bound<PropertyValue>> &upper_bound, View view, Transaction *transaction, Indices *indices, Config::Items config, const VertexValidator &vertex_validator) - : index_accessor_(std::move(index_accessor)), + : index_accessor_(&index_accessor), label_(label), property_(property), lower_bound_(lower_bound), @@ -650,49 +650,53 @@ LabelPropertyIndex::Iterable::Iterator LabelPropertyIndex::Iterable::begin() { // If the bounds are set and don't have comparable types we don't yield any // items from the index. if (!bounds_valid_) { - return {this, index_accessor_.end()}; + return {this, index_accessor_->end()}; } - auto index_iterator = index_accessor_.begin(); + auto index_iterator = index_accessor_->begin(); if (lower_bound_) { - index_iterator = index_accessor_.find_equal_or_greater(lower_bound_->value()); + index_iterator = std::ranges::find_if(*index_accessor_, [lower_bound = lower_bound_->value()](const auto &pair) { + return pair.second.value >= lower_bound; + }); } return {this, index_iterator}; } -LabelPropertyIndex::Iterable::Iterator LabelPropertyIndex::Iterable::end() { return {this, index_accessor_.end()}; } +LabelPropertyIndex::Iterable::Iterator LabelPropertyIndex::Iterable::end() { return {this, index_accessor_->end()}; } -int64_t LabelPropertyIndex::ApproximateVertexCount(LabelId label, PropertyId property, - const PropertyValue &value) const { +int64_t LabelPropertyIndex::VertexCount(LabelId label, PropertyId property, const PropertyValue &value) const { auto it = index_.find({label, property}); MG_ASSERT(it != index_.end(), "Index for label {} and property {} doesn't exist", label.AsUint(), property.AsUint()); - auto acc = it->second.access(); if (!value.IsNull()) { - return static_cast<int64_t>( - acc.estimate_count(value, static_cast<int>(utils::SkipListLayerForCountEstimation(acc.size())))); + return static_cast<int64_t>(it->second.count(value)); } - // The value `Null` won't ever appear in the index because it indicates that - // the property shouldn't exist. Instead, this value is used as an indicator - // to estimate the average number of equal elements in the list (for any - // given value). - return static_cast<int64_t>(acc.estimate_average_number_of_equals( - [](const auto &first, const auto &second) { return first.value == second.value; }, - static_cast<int>(utils::SkipListLayerForAverageEqualsEstimation(acc.size())))); + return static_cast<int64_t>(it->second.count(value)); } -int64_t LabelPropertyIndex::ApproximateVertexCount(LabelId label, PropertyId property, - const std::optional<utils::Bound<PropertyValue>> &lower, - const std::optional<utils::Bound<PropertyValue>> &upper) const { +int64_t LabelPropertyIndex::VertexCount(LabelId label, PropertyId property, + const std::optional<utils::Bound<PropertyValue>> &lower, + const std::optional<utils::Bound<PropertyValue>> &upper) const { auto it = index_.find({label, property}); MG_ASSERT(it != index_.end(), "Index for label {} and property {} doesn't exist", label.AsUint(), property.AsUint()); - auto acc = it->second.access(); - return static_cast<int64_t>( - acc.estimate_range_count(lower, upper, static_cast<int>(utils::SkipListLayerForCountEstimation(acc.size())))); + const auto get_iter = [&it](const auto &value, const auto def) { + if (value) { + auto found_it = it->second.find(value->value()); + if (value->IsInclusive()) { + return found_it; + } + return ++found_it; + } + return def; + }; + const auto lower_it = get_iter(lower, it->second.begin()); + const auto upper_it = get_iter(upper, it->second.end()); + return static_cast<int64_t>(std::distance(lower_it, upper_it)); } void LabelPropertyIndex::RunGC() { - for (auto &index_entry : index_) { - index_entry.second.run_gc(); - } + // TODO What to do? + // for (auto &index_entry : index_) { + // index_entry.second.run_gc(); + // } } void RemoveObsoleteEntries(Indices *indices, const uint64_t clean_up_before_timestamp) { diff --git a/src/storage/v3/indices.hpp b/src/storage/v3/indices.hpp index 6182e789c..7f159acb2 100644 --- a/src/storage/v3/indices.hpp +++ b/src/storage/v3/indices.hpp @@ -133,7 +133,7 @@ class LabelIndex { }; class LabelPropertyIndex { - private: + public: struct Entry { PropertyValue value; Vertex *vertex; @@ -146,7 +146,8 @@ class LabelPropertyIndex { bool operator==(const PropertyValue &rhs) const; }; - public: + using LabelPropertyIndexContainer = std::map<PropertyValue, Entry>; + LabelPropertyIndex(Indices *indices, Config::Items config, const VertexValidator &vertex_validator) : indices_(indices), config_(config), vertex_validator_{&vertex_validator} {} @@ -169,14 +170,14 @@ class LabelPropertyIndex { class Iterable { public: - Iterable(utils::SkipList<Entry>::Accessor index_accessor, LabelId label, PropertyId property, + Iterable(LabelPropertyIndexContainer &index_accessor, LabelId label, PropertyId property, const std::optional<utils::Bound<PropertyValue>> &lower_bound, const std::optional<utils::Bound<PropertyValue>> &upper_bound, View view, Transaction *transaction, Indices *indices, Config::Items config, const VertexValidator &vertex_validator); class Iterator { public: - Iterator(Iterable *self, utils::SkipList<Entry>::Iterator index_iterator); + Iterator(Iterable *self, LabelPropertyIndexContainer::iterator index_iterator); VertexAccessor operator*() const { return current_vertex_accessor_; } @@ -189,7 +190,7 @@ class LabelPropertyIndex { void AdvanceUntilValid(); Iterable *self_; - utils::SkipList<Entry>::Iterator index_iterator_; + LabelPropertyIndexContainer::iterator index_iterator_; VertexAccessor current_vertex_accessor_; Vertex *current_vertex_; }; @@ -198,7 +199,7 @@ class LabelPropertyIndex { Iterator end(); private: - utils::SkipList<Entry>::Accessor index_accessor_; + LabelPropertyIndexContainer *index_accessor_; LabelId label_; PropertyId property_; std::optional<utils::Bound<PropertyValue>> lower_bound_; @@ -217,11 +218,11 @@ class LabelPropertyIndex { auto it = index_.find({label, property}); MG_ASSERT(it != index_.end(), "Index for label {} and property {} doesn't exist", label.AsUint(), property.AsUint()); - return {it->second.access(), label, property, lower_bound, upper_bound, view, - transaction, indices_, config_, *vertex_validator_}; + return {it->second, label, property, lower_bound, upper_bound, + view, transaction, indices_, config_, *vertex_validator_}; } - int64_t ApproximateVertexCount(LabelId label, PropertyId property) const { + int64_t VertexCount(LabelId label, PropertyId property) const { auto it = index_.find({label, property}); MG_ASSERT(it != index_.end(), "Index for label {} and property {} doesn't exist", label.AsUint(), property.AsUint()); @@ -232,18 +233,17 @@ class LabelPropertyIndex { /// an estimated count of nodes which have their property's value set to /// `value`. If the `value` specified is `Null`, then an average number of /// equal elements is returned. - int64_t ApproximateVertexCount(LabelId label, PropertyId property, const PropertyValue &value) const; + int64_t VertexCount(LabelId label, PropertyId property, const PropertyValue &value) const; - int64_t ApproximateVertexCount(LabelId label, PropertyId property, - const std::optional<utils::Bound<PropertyValue>> &lower, - const std::optional<utils::Bound<PropertyValue>> &upper) const; + int64_t VertexCount(LabelId label, PropertyId property, const std::optional<utils::Bound<PropertyValue>> &lower, + const std::optional<utils::Bound<PropertyValue>> &upper) const; void Clear() { index_.clear(); } void RunGC(); private: - std::map<std::pair<LabelId, PropertyId>, utils::SkipList<Entry>> index_; + std::map<std::pair<LabelId, PropertyId>, LabelPropertyIndexContainer> index_; Indices *indices_; Config::Items config_; const VertexValidator *vertex_validator_; diff --git a/src/storage/v3/property_value.hpp b/src/storage/v3/property_value.hpp index 56902bed5..4b81d90b4 100644 --- a/src/storage/v3/property_value.hpp +++ b/src/storage/v3/property_value.hpp @@ -330,6 +330,36 @@ inline bool operator<(const PropertyValue &first, const PropertyValue &second) { } } +inline bool operator>=(const PropertyValue &first, const PropertyValue &second) { + if (!PropertyValue::AreComparableTypes(first.type(), second.type())) return first.type() < second.type(); + switch (first.type()) { + case PropertyValue::Type::Null: + return false; + case PropertyValue::Type::Bool: + return first.ValueBool() >= second.ValueBool(); + case PropertyValue::Type::Int: + if (second.type() == PropertyValue::Type::Double) { + return static_cast<double>(first.ValueInt()) >= second.ValueDouble(); + } else { + return first.ValueInt() >= second.ValueInt(); + } + case PropertyValue::Type::Double: + if (second.type() == PropertyValue::Type::Double) { + return first.ValueDouble() >= second.ValueDouble(); + } else { + return first.ValueDouble() < static_cast<double>(second.ValueInt()); + } + case PropertyValue::Type::String: + return first.ValueString() >= second.ValueString(); + case PropertyValue::Type::List: + return first.ValueList() >= second.ValueList(); + case PropertyValue::Type::Map: + return first.ValueMap() >= second.ValueMap(); + case PropertyValue::Type::TemporalData: + return first.ValueTemporalData() >= second.ValueTemporalData(); + } +} + inline PropertyValue::PropertyValue(const PropertyValue &other) : type_(other.type_) { switch (other.type_) { case Type::Null: diff --git a/src/storage/v3/shard.hpp b/src/storage/v3/shard.hpp index 58e4a42ae..858dd351e 100644 --- a/src/storage/v3/shard.hpp +++ b/src/storage/v3/shard.hpp @@ -239,14 +239,14 @@ class Shard final { /// Return approximate number of vertices with the given label and property. /// Note that this is always an over-estimate and never an under-estimate. int64_t ApproximateVertexCount(LabelId label, PropertyId property) const { - return shard_->indices_.label_property_index.ApproximateVertexCount(label, property); + return shard_->indices_.label_property_index.VertexCount(label, property); } /// Return approximate number of vertices with the given label and the given /// value for the given property. Note that this is always an over-estimate /// and never an under-estimate. int64_t ApproximateVertexCount(LabelId label, PropertyId property, const PropertyValue &value) const { - return shard_->indices_.label_property_index.ApproximateVertexCount(label, property, value); + return shard_->indices_.label_property_index.VertexCount(label, property, value); } /// Return approximate number of vertices with the given label and value for @@ -255,7 +255,7 @@ class Shard final { int64_t ApproximateVertexCount(LabelId label, PropertyId property, const std::optional<utils::Bound<PropertyValue>> &lower, const std::optional<utils::Bound<PropertyValue>> &upper) const { - return shard_->indices_.label_property_index.ApproximateVertexCount(label, property, lower, upper); + return shard_->indices_.label_property_index.VertexCount(label, property, lower, upper); } /// @return Accessor to the deleted vertex if a deletion took place, std::nullopt otherwise From 730bac6b74b4bc948c3a5b3975b7a0cbc4b76b69 Mon Sep 17 00:00:00 2001 From: jbajic <jure.bajic@memgraph.com> Date: Thu, 1 Dec 2022 14:32:33 +0100 Subject: [PATCH 06/19] Replace edges skiplist --- ...{vertices_container.hpp => containers.hpp} | 3 +++ src/storage/v3/shard.cpp | 21 ++++++++----------- src/storage/v3/shard.hpp | 4 ++-- 3 files changed, 14 insertions(+), 14 deletions(-) rename src/storage/v3/{vertices_container.hpp => containers.hpp} (87%) diff --git a/src/storage/v3/vertices_container.hpp b/src/storage/v3/containers.hpp similarity index 87% rename from src/storage/v3/vertices_container.hpp rename to src/storage/v3/containers.hpp index c67f78efd..918cf3568 100644 --- a/src/storage/v3/vertices_container.hpp +++ b/src/storage/v3/containers.hpp @@ -13,9 +13,12 @@ #include <map> +#include "storage/v3/edge.hpp" +#include "storage/v3/id_types.hpp" #include "storage/v3/key_store.hpp" #include "storage/v3/vertex.hpp" namespace memgraph::storage::v3 { using VertexContainer = std::map<PrimaryKey, Vertex>; +using EdgeContainer = std::map<Gid, Edge>; } // namespace memgraph::storage::v3 diff --git a/src/storage/v3/shard.cpp b/src/storage/v3/shard.cpp index cde51739a..a77f42a76 100644 --- a/src/storage/v3/shard.cpp +++ b/src/storage/v3/shard.cpp @@ -26,6 +26,7 @@ #include "io/network/endpoint.hpp" #include "io/time.hpp" +#include "storage/v3/containers.hpp" #include "storage/v3/edge_accessor.hpp" #include "storage/v3/id_types.hpp" #include "storage/v3/indices.hpp" @@ -38,7 +39,6 @@ #include "storage/v3/transaction.hpp" #include "storage/v3/vertex.hpp" #include "storage/v3/vertex_accessor.hpp" -#include "storage/v3/vertices_container.hpp" #include "utils/exceptions.hpp" #include "utils/file.hpp" #include "utils/logging.hpp" @@ -514,13 +514,12 @@ ShardResult<EdgeAccessor> Shard::Accessor::CreateEdge(VertexId from_vertex_id, V EdgeRef edge(gid); if (config_.properties_on_edges) { - auto acc = shard_->edges_.access(); auto *delta = CreateDeleteObjectDelta(transaction_); - auto [it, inserted] = acc.insert(Edge(gid, delta)); + auto [it, inserted] = shard_->edges_.emplace(gid, Edge{gid, delta}); MG_ASSERT(inserted, "The edge must be inserted here!"); - MG_ASSERT(it != acc.end(), "Invalid Edge accessor!"); - edge = EdgeRef(&*it); - delta->prev.Set(&*it); + MG_ASSERT(it != shard_->edges_.end(), "Invalid Edge accessor!"); + edge = EdgeRef(&it->second); + delta->prev.Set(&it->second); } if (from_is_local) { @@ -579,10 +578,9 @@ ShardResult<std::optional<EdgeAccessor>> Shard::Accessor::DeleteEdge(VertexId fr if (!config_.properties_on_edges) { return EdgeRef(edge_id); } - auto edge_acc = shard_->edges_.access(); - auto res = edge_acc.find(edge_id); - MG_ASSERT(res != edge_acc.end(), "Cannot find edge"); - return EdgeRef(&*res); + auto res = shard_->edges_.find(edge_id); + MG_ASSERT(res != shard_->edges_.end(), "Cannot find edge"); + return EdgeRef(&res->second); }); std::optional<EdgeTypeId> edge_type{}; @@ -1021,9 +1019,8 @@ void Shard::CollectGarbage(const io::Time current_time) { } deleted_vertices_.clear(); { - auto edge_acc = edges_.access(); for (auto edge : deleted_edges_) { - MG_ASSERT(edge_acc.remove(edge), "Invalid database state!"); + MG_ASSERT(edges_.erase(edge), "Invalid database state!"); } } deleted_edges_.clear(); diff --git a/src/storage/v3/shard.hpp b/src/storage/v3/shard.hpp index 858dd351e..d41ca3105 100644 --- a/src/storage/v3/shard.hpp +++ b/src/storage/v3/shard.hpp @@ -25,6 +25,7 @@ #include "io/time.hpp" #include "kvstore/kvstore.hpp" #include "storage/v3/config.hpp" +#include "storage/v3/containers.hpp" #include "storage/v3/edge.hpp" #include "storage/v3/edge_accessor.hpp" #include "storage/v3/id_types.hpp" @@ -41,7 +42,6 @@ #include "storage/v3/vertex.hpp" #include "storage/v3/vertex_accessor.hpp" #include "storage/v3/vertex_id.hpp" -#include "storage/v3/vertices_container.hpp" #include "storage/v3/view.hpp" #include "utils/exceptions.hpp" #include "utils/file_locker.hpp" @@ -374,7 +374,7 @@ class Shard final { PrimaryKey min_primary_key_; std::optional<PrimaryKey> max_primary_key_; VertexContainer vertices_; - utils::SkipList<Edge> edges_; + EdgeContainer edges_; // Even though the edge count is already kept in the `edges_` SkipList, the // list is used only when properties are enabled for edges. Because of that we // keep a separate count of edges that is always updated. From 4a3f950cf94d5eb3e411b2757bc891f9464fb19a Mon Sep 17 00:00:00 2001 From: jbajic <jure.bajic@memgraph.com> Date: Thu, 1 Dec 2022 15:00:23 +0100 Subject: [PATCH 07/19] Fix indices --- src/storage/v3/indices.cpp | 23 +++++++++++------------ src/storage/v3/indices.hpp | 4 ++-- src/storage/v3/property_value.hpp | 4 ++-- 3 files changed, 15 insertions(+), 16 deletions(-) diff --git a/src/storage/v3/indices.cpp b/src/storage/v3/indices.cpp index 503972b42..0dbb13481 100644 --- a/src/storage/v3/indices.cpp +++ b/src/storage/v3/indices.cpp @@ -21,7 +21,6 @@ #include "storage/v3/mvcc.hpp" #include "storage/v3/property_value.hpp" #include "storage/v3/schemas.hpp" -#include "storage/v3/vertices_container.hpp" #include "utils/bound.hpp" #include "utils/logging.hpp" #include "utils/memory_tracker.hpp" @@ -392,13 +391,13 @@ bool LabelPropertyIndex::Entry::operator<(const PropertyValue &rhs) const { retu bool LabelPropertyIndex::Entry::operator==(const PropertyValue &rhs) const { return value == rhs; } void LabelPropertyIndex::UpdateOnAddLabel(LabelId label, Vertex *vertex, const Transaction &tx) { - for (auto &[label_prop, container] : index_) { + for (auto &[label_prop, index] : index_) { if (label_prop.first != label) { continue; } auto prop_value = vertex->properties.GetProperty(label_prop.second); if (!prop_value.IsNull()) { - container.emplace(vertex->keys, Entry{std::move(prop_value), vertex, tx.start_timestamp.logical_id}); + index.emplace(prop_value, Entry{prop_value, vertex, tx.start_timestamp.logical_id}); } } } @@ -408,12 +407,12 @@ void LabelPropertyIndex::UpdateOnSetProperty(PropertyId property, const Property if (value.IsNull()) { return; } - for (auto &[label_prop, container] : index_) { + for (auto &[label_prop, index] : index_) { if (label_prop.second != property) { continue; } if (utils::Contains(vertex->labels, label_prop.first)) { - container.emplace(vertex->keys, Entry{value, vertex, tx.start_timestamp.logical_id}); + index.emplace(value, Entry{value, vertex, tx.start_timestamp.logical_id}); } } } @@ -435,7 +434,7 @@ bool LabelPropertyIndex::CreateIndex(LabelId label, PropertyId property, VertexC if (value.IsNull()) { continue; } - it->second.emplace(vertex.keys, Entry{std::move(value), &vertex, 0}); + it->second.emplace(value, Entry{value, &vertex, 0}); } } catch (const utils::OutOfMemoryException &) { utils::MemoryTracker::OutOfMemoryExceptionBlocker oom_exception_blocker; @@ -455,8 +454,8 @@ std::vector<std::pair<LabelId, PropertyId>> LabelPropertyIndex::ListIndices() co } void LabelPropertyIndex::RemoveObsoleteEntries(const uint64_t clean_up_before_timestamp) { - for (auto &[label_property, container] : index_) { - for (auto it = container.begin(); it != container.end();) { + for (auto &[label_property, index] : index_) { + for (auto it = index.begin(); it != index.end();) { auto next_it = it; ++next_it; @@ -465,11 +464,11 @@ void LabelPropertyIndex::RemoveObsoleteEntries(const uint64_t clean_up_before_ti continue; } - if (auto &entry = it->second; - (next_it != container.end() && entry.vertex == next_it->second.vertex && entry.value == entry.value) || + if (auto &entry = it->second, &next_entry = next_it->second; + (next_it != index.end() && entry.vertex == next_entry.vertex && entry.value == next_entry.value) || !AnyVersionHasLabelProperty(*entry.vertex, label_property.first, label_property.second, entry.value, clean_up_before_timestamp)) { - container.erase(it->first); + index.erase(it->first); } it = next_it; } @@ -683,7 +682,7 @@ int64_t LabelPropertyIndex::VertexCount(LabelId label, PropertyId property, if (value->IsInclusive()) { return found_it; } - return ++found_it; + return found_it != it->second.end() ? ++found_it : found_it; } return def; }; diff --git a/src/storage/v3/indices.hpp b/src/storage/v3/indices.hpp index 7f159acb2..d1e5ba737 100644 --- a/src/storage/v3/indices.hpp +++ b/src/storage/v3/indices.hpp @@ -17,10 +17,10 @@ #include <utility> #include "storage/v3/config.hpp" +#include "storage/v3/containers.hpp" #include "storage/v3/property_value.hpp" #include "storage/v3/transaction.hpp" #include "storage/v3/vertex_accessor.hpp" -#include "storage/v3/vertices_container.hpp" #include "utils/bound.hpp" #include "utils/logging.hpp" #include "utils/skip_list.hpp" @@ -133,7 +133,6 @@ class LabelIndex { }; class LabelPropertyIndex { - public: struct Entry { PropertyValue value; Vertex *vertex; @@ -146,6 +145,7 @@ class LabelPropertyIndex { bool operator==(const PropertyValue &rhs) const; }; + public: using LabelPropertyIndexContainer = std::map<PropertyValue, Entry>; LabelPropertyIndex(Indices *indices, Config::Items config, const VertexValidator &vertex_validator) diff --git a/src/storage/v3/property_value.hpp b/src/storage/v3/property_value.hpp index 4b81d90b4..e1e4b4d67 100644 --- a/src/storage/v3/property_value.hpp +++ b/src/storage/v3/property_value.hpp @@ -331,7 +331,7 @@ inline bool operator<(const PropertyValue &first, const PropertyValue &second) { } inline bool operator>=(const PropertyValue &first, const PropertyValue &second) { - if (!PropertyValue::AreComparableTypes(first.type(), second.type())) return first.type() < second.type(); + if (!PropertyValue::AreComparableTypes(first.type(), second.type())) return first.type() >= second.type(); switch (first.type()) { case PropertyValue::Type::Null: return false; @@ -347,7 +347,7 @@ inline bool operator>=(const PropertyValue &first, const PropertyValue &second) if (second.type() == PropertyValue::Type::Double) { return first.ValueDouble() >= second.ValueDouble(); } else { - return first.ValueDouble() < static_cast<double>(second.ValueInt()); + return first.ValueDouble() >= static_cast<double>(second.ValueInt()); } case PropertyValue::Type::String: return first.ValueString() >= second.ValueString(); From 65e9ceb779aef64da89689fae8388e26dc8dc24c Mon Sep 17 00:00:00 2001 From: jbajic <jure.bajic@memgraph.com> Date: Mon, 5 Dec 2022 14:52:51 +0100 Subject: [PATCH 08/19] Use multimap as index structure --- src/storage/v3/indices.hpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/storage/v3/indices.hpp b/src/storage/v3/indices.hpp index d1e5ba737..d46fb7f38 100644 --- a/src/storage/v3/indices.hpp +++ b/src/storage/v3/indices.hpp @@ -12,6 +12,7 @@ #pragma once #include <cstdint> +#include <map> #include <optional> #include <tuple> #include <utility> @@ -146,7 +147,7 @@ class LabelPropertyIndex { }; public: - using LabelPropertyIndexContainer = std::map<PropertyValue, Entry>; + using LabelPropertyIndexContainer = std::multimap<PropertyValue, Entry>; LabelPropertyIndex(Indices *indices, Config::Items config, const VertexValidator &vertex_validator) : indices_(indices), config_(config), vertex_validator_{&vertex_validator} {} From a20edf2b7499fa4f0243d1cfc79ff1ca027fc2f9 Mon Sep 17 00:00:00 2001 From: jbajic <jure.bajic@memgraph.com> Date: Mon, 5 Dec 2022 15:30:10 +0100 Subject: [PATCH 09/19] Fix bounds --- src/storage/v3/indices.cpp | 36 +++++++++++++++++++++++------------- 1 file changed, 23 insertions(+), 13 deletions(-) diff --git a/src/storage/v3/indices.cpp b/src/storage/v3/indices.cpp index 0dbb13481..fc546b594 100644 --- a/src/storage/v3/indices.cpp +++ b/src/storage/v3/indices.cpp @@ -676,23 +676,33 @@ int64_t LabelPropertyIndex::VertexCount(LabelId label, PropertyId property, const std::optional<utils::Bound<PropertyValue>> &upper) const { auto it = index_.find({label, property}); MG_ASSERT(it != index_.end(), "Index for label {} and property {} doesn't exist", label.AsUint(), property.AsUint()); - const auto get_iter = [&it](const auto &value, const auto def) { - if (value) { - auto found_it = it->second.find(value->value()); - if (value->IsInclusive()) { - return found_it; - } - return found_it != it->second.end() ? ++found_it : found_it; - } - return def; - }; - const auto lower_it = get_iter(lower, it->second.begin()); - const auto upper_it = get_iter(upper, it->second.end()); + const auto lower_it = std::invoke( + [&index = it->second](const auto value, const auto def) { + if (value) { + if (value->IsInclusive()) { + return index.lower_bound(value->value()); + } + return index.upper_bound(value->value()); + } + return def; + }, + lower, it->second.begin()); + const auto upper_it = std::invoke( + [&index = it->second](const auto value, const auto def) { + if (value) { + if (value->IsInclusive()) { + return index.upper_bound(value->value()); + } + return index.lower_bound(value->value()); + } + return def; + }, + upper, it->second.end()); return static_cast<int64_t>(std::distance(lower_it, upper_it)); } void LabelPropertyIndex::RunGC() { - // TODO What to do? + // TODO(jbajic) What to do? // for (auto &index_entry : index_) { // index_entry.second.run_gc(); // } From 2488895362099a8242d0c920035d5a8083713c5f Mon Sep 17 00:00:00 2001 From: jbajic <jure.bajic@memgraph.com> Date: Mon, 5 Dec 2022 15:37:10 +0100 Subject: [PATCH 10/19] Rename ApproximateVertexCount to VertexCount --- src/storage/v3/bindings/db_accessor.hpp | 8 ++++---- src/storage/v3/shard.hpp | 23 +++++++++-------------- tests/unit/storage_v3_indices.cpp | 8 ++++---- 3 files changed, 17 insertions(+), 22 deletions(-) diff --git a/src/storage/v3/bindings/db_accessor.hpp b/src/storage/v3/bindings/db_accessor.hpp index 6392ed18f..a8afdcffa 100644 --- a/src/storage/v3/bindings/db_accessor.hpp +++ b/src/storage/v3/bindings/db_accessor.hpp @@ -162,23 +162,23 @@ class DbAccessor final { return accessor_->LabelPropertyIndexExists(label, prop); } - int64_t VerticesCount() const { return accessor_->ApproximateVertexCount(); } + int64_t VerticesCount() const { return accessor_->VertexCount(); } int64_t VerticesCount(storage::v3::LabelId label) const { return accessor_->ApproximateVertexCount(label); } int64_t VerticesCount(storage::v3::LabelId label, storage::v3::PropertyId property) const { - return accessor_->ApproximateVertexCount(label, property); + return accessor_->VertexCount(label, property); } int64_t VerticesCount(storage::v3::LabelId label, storage::v3::PropertyId property, const storage::v3::PropertyValue &value) const { - return accessor_->ApproximateVertexCount(label, property, value); + return accessor_->VertexCount(label, property, value); } int64_t VerticesCount(storage::v3::LabelId label, storage::v3::PropertyId property, const std::optional<utils::Bound<storage::v3::PropertyValue>> &lower, const std::optional<utils::Bound<storage::v3::PropertyValue>> &upper) const { - return accessor_->ApproximateVertexCount(label, property, lower, upper); + return accessor_->VertexCount(label, property, lower, upper); } storage::v3::IndicesInfo ListAllIndices() const { return accessor_->ListAllIndices(); } diff --git a/src/storage/v3/shard.hpp b/src/storage/v3/shard.hpp index d41ca3105..b998c06cc 100644 --- a/src/storage/v3/shard.hpp +++ b/src/storage/v3/shard.hpp @@ -226,9 +226,8 @@ class Shard final { const std::optional<utils::Bound<PropertyValue>> &lower_bound, const std::optional<utils::Bound<PropertyValue>> &upper_bound, View view); - /// Return approximate number of all vertices in the database. - /// Note that this is always an over-estimate and never an under-estimate. - int64_t ApproximateVertexCount() const { return static_cast<int64_t>(shard_->vertices_.size()); } + /// Return number of all vertices in the database. + int64_t VertexCount() const { return static_cast<int64_t>(shard_->vertices_.size()); } /// Return approximate number of vertices with the given label. /// Note that this is always an over-estimate and never an under-estimate. @@ -236,25 +235,21 @@ class Shard final { return shard_->indices_.label_index.ApproximateVertexCount(label); } - /// Return approximate number of vertices with the given label and property. - /// Note that this is always an over-estimate and never an under-estimate. - int64_t ApproximateVertexCount(LabelId label, PropertyId property) const { + /// Return number of vertices with the given label and property. + int64_t VertexCount(LabelId label, PropertyId property) const { return shard_->indices_.label_property_index.VertexCount(label, property); } - /// Return approximate number of vertices with the given label and the given - /// value for the given property. Note that this is always an over-estimate - /// and never an under-estimate. - int64_t ApproximateVertexCount(LabelId label, PropertyId property, const PropertyValue &value) const { + /// Return number of vertices with the given label and the given + int64_t VertexCount(LabelId label, PropertyId property, const PropertyValue &value) const { return shard_->indices_.label_property_index.VertexCount(label, property, value); } - /// Return approximate number of vertices with the given label and value for + /// Return number of vertices with the given label and value for /// the given property in the range defined by provided upper and lower /// bounds. - int64_t ApproximateVertexCount(LabelId label, PropertyId property, - const std::optional<utils::Bound<PropertyValue>> &lower, - const std::optional<utils::Bound<PropertyValue>> &upper) const { + int64_t VertexCount(LabelId label, PropertyId property, const std::optional<utils::Bound<PropertyValue>> &lower, + const std::optional<utils::Bound<PropertyValue>> &upper) const { return shard_->indices_.label_property_index.VertexCount(label, property, lower, upper); } diff --git a/tests/unit/storage_v3_indices.cpp b/tests/unit/storage_v3_indices.cpp index 620878fcf..13f528d5a 100644 --- a/tests/unit/storage_v3_indices.cpp +++ b/tests/unit/storage_v3_indices.cpp @@ -652,13 +652,13 @@ TEST_F(IndexTest, LabelPropertyIndexCountEstimate) { } } - EXPECT_EQ(acc.ApproximateVertexCount(label1, prop_val), 55); + EXPECT_EQ(acc.VertexCount(label1, prop_val), 55); for (int i = 1; i <= 10; ++i) { - EXPECT_EQ(acc.ApproximateVertexCount(label1, prop_val, PropertyValue(i)), i); + EXPECT_EQ(acc.VertexCount(label1, prop_val, PropertyValue(i)), i); } - EXPECT_EQ(acc.ApproximateVertexCount(label1, prop_val, memgraph::utils::MakeBoundInclusive(PropertyValue(2)), - memgraph::utils::MakeBoundInclusive(PropertyValue(6))), + EXPECT_EQ(acc.VertexCount(label1, prop_val, memgraph::utils::MakeBoundInclusive(PropertyValue(2)), + memgraph::utils::MakeBoundInclusive(PropertyValue(6))), 2 + 3 + 4 + 5 + 6); } From 0cf440519ea339ed244c0f5be4d5de31b4aaeb88 Mon Sep 17 00:00:00 2001 From: jbajic <jure.bajic@memgraph.com> Date: Mon, 12 Dec 2022 12:39:49 +0100 Subject: [PATCH 11/19] Introduce VertexData --- src/storage/v3/CMakeLists.txt | 1 + src/storage/v3/delta.hpp | 2 +- src/storage/v3/edge.hpp | 6 +- src/storage/v3/edge_accessor.hpp | 1 - src/storage/v3/indices.cpp | 43 ++--- src/storage/v3/indices.hpp | 1 - src/storage/v3/mvcc.hpp | 32 +++- src/storage/v3/shard.cpp | 159 +++++++++--------- src/storage/v3/shard.hpp | 3 +- src/storage/v3/{containers.hpp => vertex.cpp} | 24 ++- src/storage/v3/vertex.hpp | 21 +-- src/storage/v3/vertex_accessor.cpp | 132 ++++++++------- 12 files changed, 226 insertions(+), 199 deletions(-) rename src/storage/v3/{containers.hpp => vertex.cpp} (59%) diff --git a/src/storage/v3/CMakeLists.txt b/src/storage/v3/CMakeLists.txt index f371f7b7b..b3a3d68a9 100644 --- a/src/storage/v3/CMakeLists.txt +++ b/src/storage/v3/CMakeLists.txt @@ -17,6 +17,7 @@ set(storage_v3_src_files shard_rsm.cpp bindings/typed_value.cpp expr.cpp + vertex.cpp request_helper.cpp) # ###################### diff --git a/src/storage/v3/delta.hpp b/src/storage/v3/delta.hpp index 9548be146..1c6e57d2b 100644 --- a/src/storage/v3/delta.hpp +++ b/src/storage/v3/delta.hpp @@ -15,13 +15,13 @@ #include "storage/v3/edge_ref.hpp" #include "storage/v3/id_types.hpp" #include "storage/v3/property_value.hpp" +#include "storage/v3/vertex.hpp" #include "storage/v3/vertex_id.hpp" #include "utils/logging.hpp" namespace memgraph::storage::v3 { // Forward declarations because we only store pointers here. -struct Vertex; struct Edge; struct Delta; struct CommitInfo; diff --git a/src/storage/v3/edge.hpp b/src/storage/v3/edge.hpp index df47b8416..c5cf6c6f7 100644 --- a/src/storage/v3/edge.hpp +++ b/src/storage/v3/edge.hpp @@ -21,7 +21,7 @@ namespace memgraph::storage::v3 { -struct Vertex; +using EdgeContainer = std::map<Gid, Edge>; struct Edge { Edge(Gid gid, Delta *delta) : gid(gid), deleted(false), delta(delta) { @@ -34,13 +34,13 @@ struct Edge { PropertyStore properties; bool deleted; - // uint8_t PAD; + uint8_t PAD; // uint16_t PAD; Delta *delta; }; -static_assert(alignof(Edge) >= 8, "The Edge should be aligned to at least 8!"); +// static_assert(alignof(Edge) >= 8, "The Edge should be aligned to at least 8!"); inline bool operator==(const Edge &first, const Edge &second) { return first.gid == second.gid; } inline bool operator<(const Edge &first, const Edge &second) { return first.gid < second.gid; } diff --git a/src/storage/v3/edge_accessor.hpp b/src/storage/v3/edge_accessor.hpp index bc143c1e3..b57beeb96 100644 --- a/src/storage/v3/edge_accessor.hpp +++ b/src/storage/v3/edge_accessor.hpp @@ -25,7 +25,6 @@ namespace memgraph::storage::v3 { -struct Vertex; class VertexAccessor; struct Indices; diff --git a/src/storage/v3/indices.cpp b/src/storage/v3/indices.cpp index fc546b594..84097a26a 100644 --- a/src/storage/v3/indices.cpp +++ b/src/storage/v3/indices.cpp @@ -21,6 +21,7 @@ #include "storage/v3/mvcc.hpp" #include "storage/v3/property_value.hpp" #include "storage/v3/schemas.hpp" +#include "storage/v3/vertex.hpp" #include "utils/bound.hpp" #include "utils/logging.hpp" #include "utils/memory_tracker.hpp" @@ -57,9 +58,9 @@ bool AnyVersionHasLabel(const Vertex &vertex, LabelId label, uint64_t timestamp) bool deleted{false}; const Delta *delta{nullptr}; { - has_label = utils::Contains(vertex.labels, label); - deleted = vertex.deleted; - delta = vertex.delta; + has_label = utils::Contains(vertex.second.labels, label); + deleted = vertex.second.deleted; + delta = vertex.second.delta; } if (!deleted && has_label) { return true; @@ -109,10 +110,10 @@ bool AnyVersionHasLabelProperty(const Vertex &vertex, LabelId label, PropertyId bool deleted{false}; const Delta *delta{nullptr}; { - has_label = utils::Contains(vertex.labels, label); - current_value_equal_to_value = vertex.properties.IsPropertyEqual(key, value); - deleted = vertex.deleted; - delta = vertex.delta; + has_label = utils::Contains(vertex.second.labels, label); + current_value_equal_to_value = vertex.second.properties.IsPropertyEqual(key, value); + deleted = vertex.second.deleted; + delta = vertex.second.delta; } if (!deleted && has_label && current_value_equal_to_value) { @@ -167,9 +168,9 @@ bool CurrentVersionHasLabel(const Vertex &vertex, LabelId label, Transaction *tr bool has_label{false}; const Delta *delta{nullptr}; { - deleted = vertex.deleted; - has_label = utils::Contains(vertex.labels, label); - delta = vertex.delta; + deleted = vertex.second.deleted; + has_label = utils::Contains(vertex.second.labels, label); + delta = vertex.second.delta; } ApplyDeltasForRead(transaction, delta, view, [&deleted, &has_label, label](const Delta &delta) { switch (delta.action) { @@ -218,10 +219,10 @@ bool CurrentVersionHasLabelProperty(const Vertex &vertex, LabelId label, Propert bool current_value_equal_to_value = value.IsNull(); const Delta *delta{nullptr}; { - deleted = vertex.deleted; - has_label = utils::Contains(vertex.labels, label); - current_value_equal_to_value = vertex.properties.IsPropertyEqual(key, value); - delta = vertex.delta; + deleted = vertex.second.deleted; + has_label = utils::Contains(vertex.second.labels, label); + current_value_equal_to_value = vertex.second.properties.IsPropertyEqual(key, value); + delta = vertex.second.delta; } ApplyDeltasForRead(transaction, delta, view, [&deleted, &has_label, ¤t_value_equal_to_value, key, label, &value](const Delta &delta) { @@ -282,8 +283,8 @@ bool LabelIndex::CreateIndex(LabelId label, VertexContainer &vertices) { } try { auto acc = it->second.access(); - for ([[maybe_unused]] auto &[pk, vertex] : vertices) { - if (vertex.deleted || !utils::Contains(vertex.labels, label)) { + for ([[maybe_unused]] auto &vertex : vertices) { + if (vertex.second.deleted || !VertexHasLabel(vertex, label)) { continue; } acc.insert(Entry{&vertex, 0}); @@ -395,7 +396,7 @@ void LabelPropertyIndex::UpdateOnAddLabel(LabelId label, Vertex *vertex, const T if (label_prop.first != label) { continue; } - auto prop_value = vertex->properties.GetProperty(label_prop.second); + auto prop_value = vertex->second.properties.GetProperty(label_prop.second); if (!prop_value.IsNull()) { index.emplace(prop_value, Entry{prop_value, vertex, tx.start_timestamp.logical_id}); } @@ -411,7 +412,7 @@ void LabelPropertyIndex::UpdateOnSetProperty(PropertyId property, const Property if (label_prop.second != property) { continue; } - if (utils::Contains(vertex->labels, label_prop.first)) { + if (VertexHasLabel(*vertex, label_prop.first)) { index.emplace(value, Entry{value, vertex, tx.start_timestamp.logical_id}); } } @@ -426,11 +427,11 @@ bool LabelPropertyIndex::CreateIndex(LabelId label, PropertyId property, VertexC return false; } try { - for ([[maybe_unused]] auto &[pk, vertex] : vertices) { - if (vertex.deleted || !utils::Contains(vertex.labels, label)) { + for ([[maybe_unused]] auto &vertex : vertices) { + if (vertex.second.deleted || !VertexHasLabel(vertex, label)) { continue; } - auto value = vertex.properties.GetProperty(property); + auto value = vertex.second.properties.GetProperty(property); if (value.IsNull()) { continue; } diff --git a/src/storage/v3/indices.hpp b/src/storage/v3/indices.hpp index d46fb7f38..425c9da13 100644 --- a/src/storage/v3/indices.hpp +++ b/src/storage/v3/indices.hpp @@ -18,7 +18,6 @@ #include <utility> #include "storage/v3/config.hpp" -#include "storage/v3/containers.hpp" #include "storage/v3/property_value.hpp" #include "storage/v3/transaction.hpp" #include "storage/v3/vertex_accessor.hpp" diff --git a/src/storage/v3/mvcc.hpp b/src/storage/v3/mvcc.hpp index 219491635..001f57003 100644 --- a/src/storage/v3/mvcc.hpp +++ b/src/storage/v3/mvcc.hpp @@ -11,9 +11,13 @@ #pragma once +#include <type_traits> +#include "storage/v3/edge.hpp" #include "storage/v3/property_value.hpp" #include "storage/v3/transaction.hpp" +#include "storage/v3/vertex.hpp" #include "storage/v3/view.hpp" +#include "utils/concepts.hpp" namespace memgraph::storage::v3 { @@ -77,10 +81,18 @@ inline void ApplyDeltasForRead(Transaction *transaction, const Delta *delta, Vie /// transaction) and returns a `bool` value indicating whether the caller can /// proceed with a write operation. template <typename TObj> +requires utils::SameAsAnyOf<TObj, Edge, Vertex> inline bool PrepareForWrite(Transaction *transaction, TObj *object) { - if (object->delta == nullptr) return true; + auto *delta_holder = std::invoke([object]() -> auto * { + if constexpr (std::is_same_v<TObj, Vertex>) { + return &object->second; + } else { + return object; + } + }); + if (delta_holder->delta == nullptr) return true; - const auto &delta_commit_info = *object->delta->commit_info; + const auto &delta_commit_info = *delta_holder->delta->commit_info; if (delta_commit_info.start_or_commit_timestamp == transaction->commit_info->start_or_commit_timestamp || (delta_commit_info.is_locally_committed && delta_commit_info.start_or_commit_timestamp < transaction->start_timestamp)) { @@ -105,9 +117,17 @@ inline Delta *CreateDeleteObjectDelta(Transaction *transaction) { /// the delta into the object's delta list. /// @throw std::bad_alloc template <typename TObj, class... Args> +requires utils::SameAsAnyOf<TObj, Edge, Vertex> inline void CreateAndLinkDelta(Transaction *transaction, TObj *object, Args &&...args) { auto delta = &transaction->deltas.emplace_back(std::forward<Args>(args)..., transaction->commit_info.get(), transaction->command_id); + auto *delta_holder = std::invoke([object]() -> auto * { + if constexpr (std::is_same_v<TObj, Vertex>) { + return &object->second; + } else { + return object; + } + }); // The operations are written in such order so that both `next` and `prev` // chains are valid at all times. The chains must be valid at all times @@ -118,21 +138,21 @@ inline void CreateAndLinkDelta(Transaction *transaction, TObj *object, Args &&.. // TODO(antaljanosbenjamin): clang-tidy detects (in my opinion a false positive) issue in // `Shard::Accessor::CreateEdge`. // NOLINTNEXTLINE(clang-analyzer-core.NullDereference) - delta->next = object->delta; + delta->next = delta_holder->delta; // 2. We need to set the previous delta of the new delta to the object. delta->prev.Set(object); // 3. We need to set the previous delta of the existing delta to the new // delta. After this point the garbage collector will be able to see the new // delta but won't modify it until we are done with all of our modifications. - if (object->delta) { - object->delta->prev.Set(delta); + if (delta_holder->delta) { + delta_holder->delta->prev.Set(delta); } // 4. Finally, we need to set the object's delta to the new delta. The garbage // collector and other transactions will acquire the object lock to read the // delta from the object. Because the lock is held during the whole time this // modification is being done, everybody else will wait until we are fully // done with our modification before they read the object's delta value. - object->delta = delta; + delta_holder->delta = delta; } } // namespace memgraph::storage::v3 diff --git a/src/storage/v3/shard.cpp b/src/storage/v3/shard.cpp index a77f42a76..2573110c4 100644 --- a/src/storage/v3/shard.cpp +++ b/src/storage/v3/shard.cpp @@ -26,7 +26,6 @@ #include "io/network/endpoint.hpp" #include "io/time.hpp" -#include "storage/v3/containers.hpp" #include "storage/v3/edge_accessor.hpp" #include "storage/v3/id_types.hpp" #include "storage/v3/indices.hpp" @@ -39,6 +38,7 @@ #include "storage/v3/transaction.hpp" #include "storage/v3/vertex.hpp" #include "storage/v3/vertex_accessor.hpp" +#include "storage/v3/view.hpp" #include "utils/exceptions.hpp" #include "utils/file.hpp" #include "utils/logging.hpp" @@ -79,7 +79,7 @@ auto AdvanceToVisibleVertex(VertexContainer::iterator it, VertexContainer::itera std::optional<VertexAccessor> *vertex, Transaction *tx, View view, Indices *indices, Config::Items config, const VertexValidator &vertex_validator) { while (it != end) { - *vertex = VertexAccessor::Create(&it->second, tx, indices, config, vertex_validator, view); + *vertex = VertexAccessor::Create(&*it, tx, indices, config, vertex_validator, view); if (!*vertex) { ++it; continue; @@ -346,7 +346,7 @@ Shard::Accessor::Accessor(Shard &shard, Transaction &transaction) : shard_(&shard), transaction_(&transaction), config_(shard_->config_.items) {} ShardResult<VertexAccessor> Shard::Accessor::CreateVertexAndValidate( - const std::vector<LabelId> &labels, const std::vector<PropertyValue> &primary_properties, + const std::vector<LabelId> &labels, const PrimaryKey &primary_properties, const std::vector<std::pair<PropertyId, PropertyValue>> &properties) { OOMExceptionEnabler oom_exception; const auto schema = shard_->GetSchema(shard_->primary_label_)->second; @@ -358,10 +358,10 @@ ShardResult<VertexAccessor> Shard::Accessor::CreateVertexAndValidate( } auto *delta = CreateDeleteObjectDelta(transaction_); - auto [it, inserted] = shard_->vertices_.emplace(primary_properties, Vertex(delta, primary_properties)); - delta->prev.Set(&it->second); + auto [it, inserted] = shard_->vertices_.emplace(primary_properties, VertexData{delta}); + delta->prev.Set(&*it); - VertexAccessor vertex_acc{&it->second, transaction_, &shard_->indices_, config_, shard_->vertex_validator_}; + VertexAccessor vertex_acc{&*it, transaction_, &shard_->indices_, config_, shard_->vertex_validator_}; if (!inserted) { return SHARD_ERROR(ErrorCode::VERTEX_ALREADY_INSERTED); } @@ -389,7 +389,7 @@ std::optional<VertexAccessor> Shard::Accessor::FindVertex(std::vector<PropertyVa if (it == shard_->vertices_.end()) { return std::nullopt; } - return VertexAccessor::Create(&it->second, transaction_, &shard_->indices_, config_, shard_->vertex_validator_, view); + return VertexAccessor::Create(&*it, transaction_, &shard_->indices_, config_, shard_->vertex_validator_, view); } ShardResult<std::optional<VertexAccessor>> Shard::Accessor::DeleteVertex(VertexAccessor *vertex) { @@ -400,14 +400,15 @@ ShardResult<std::optional<VertexAccessor>> Shard::Accessor::DeleteVertex(VertexA if (!PrepareForWrite(transaction_, vertex_ptr)) return SHARD_ERROR(ErrorCode::SERIALIZATION_ERROR); - if (vertex_ptr->deleted) { + if (vertex_ptr->second.deleted) { return std::optional<VertexAccessor>{}; } - if (!vertex_ptr->in_edges.empty() || !vertex_ptr->out_edges.empty()) return SHARD_ERROR(ErrorCode::VERTEX_HAS_EDGES); + if (!vertex_ptr->second.in_edges.empty() || !vertex_ptr->second.out_edges.empty()) + return SHARD_ERROR(ErrorCode::VERTEX_HAS_EDGES); CreateAndLinkDelta(transaction_, vertex_ptr, Delta::RecreateObjectTag()); - vertex_ptr->deleted = true; + vertex_ptr->second.deleted = true; return std::make_optional<VertexAccessor>(vertex_ptr, transaction_, &shard_->indices_, config_, shard_->vertex_validator_, true); @@ -422,20 +423,20 @@ ShardResult<std::optional<std::pair<VertexAccessor, std::vector<EdgeAccessor>>>> "accessor when deleting a vertex!"); auto *vertex_ptr = vertex->vertex_; - std::vector<Vertex::EdgeLink> in_edges; - std::vector<Vertex::EdgeLink> out_edges; + std::vector<VertexData::EdgeLink> in_edges; + std::vector<VertexData::EdgeLink> out_edges; { if (!PrepareForWrite(transaction_, vertex_ptr)) return SHARD_ERROR(ErrorCode::SERIALIZATION_ERROR); - if (vertex_ptr->deleted) return std::optional<ReturnType>{}; + if (vertex_ptr->second.deleted) return std::optional<ReturnType>{}; - in_edges = vertex_ptr->in_edges; - out_edges = vertex_ptr->out_edges; + in_edges = vertex_ptr->second.in_edges; + out_edges = vertex_ptr->second.out_edges; } std::vector<EdgeAccessor> deleted_edges; - const VertexId vertex_id{shard_->primary_label_, vertex_ptr->keys}; + const VertexId vertex_id{shard_->primary_label_, *vertex->PrimaryKey(View::OLD)}; // TODO Replace for (const auto &item : in_edges) { auto [edge_type, from_vertex, edge] = item; EdgeAccessor e(edge, edge_type, from_vertex, vertex_id, transaction_, &shard_->indices_, config_); @@ -469,10 +470,10 @@ ShardResult<std::optional<std::pair<VertexAccessor, std::vector<EdgeAccessor>>>> if (!PrepareForWrite(transaction_, vertex_ptr)) return SHARD_ERROR(ErrorCode::SERIALIZATION_ERROR); - MG_ASSERT(!vertex_ptr->deleted, "Invalid database state!"); + MG_ASSERT(!vertex_ptr->second.deleted, "Invalid database state!"); CreateAndLinkDelta(transaction_, vertex_ptr, Delta::RecreateObjectTag()); - vertex_ptr->deleted = true; + vertex_ptr->second.deleted = true; return std::make_optional<ReturnType>( VertexAccessor{vertex_ptr, transaction_, &shard_->indices_, config_, shard_->vertex_validator_, true}, @@ -494,22 +495,22 @@ ShardResult<EdgeAccessor> Shard::Accessor::CreateEdge(VertexId from_vertex_id, V if (from_is_local) { auto it = vertices.find(from_vertex_id.primary_key); MG_ASSERT(it != vertices.end(), "Cannot find local vertex"); - from_vertex = &it->second; + from_vertex = &*it; } if (to_is_local) { auto it = vertices.find(to_vertex_id.primary_key); MG_ASSERT(it != vertices.end(), "Cannot find local vertex"); - to_vertex = &it->second; + to_vertex = &*it; } if (from_is_local) { if (!PrepareForWrite(transaction_, from_vertex)) return SHARD_ERROR(ErrorCode::SERIALIZATION_ERROR); - if (from_vertex->deleted) return SHARD_ERROR(ErrorCode::DELETED_OBJECT); + if (from_vertex->second.deleted) return SHARD_ERROR(ErrorCode::DELETED_OBJECT); } if (to_is_local && to_vertex != from_vertex) { if (!PrepareForWrite(transaction_, to_vertex)) return SHARD_ERROR(ErrorCode::SERIALIZATION_ERROR); - if (to_vertex->deleted) return SHARD_ERROR(ErrorCode::DELETED_OBJECT); + if (to_vertex->second.deleted) return SHARD_ERROR(ErrorCode::DELETED_OBJECT); } EdgeRef edge(gid); @@ -524,11 +525,11 @@ ShardResult<EdgeAccessor> Shard::Accessor::CreateEdge(VertexId from_vertex_id, V if (from_is_local) { CreateAndLinkDelta(transaction_, from_vertex, Delta::RemoveOutEdgeTag(), edge_type, to_vertex_id, edge); - from_vertex->out_edges.emplace_back(edge_type, to_vertex_id, edge); + from_vertex->second.out_edges.emplace_back(edge_type, to_vertex_id, edge); } if (to_is_local) { CreateAndLinkDelta(transaction_, to_vertex, Delta::RemoveInEdgeTag(), edge_type, from_vertex_id, edge); - to_vertex->in_edges.emplace_back(edge_type, from_vertex_id, edge); + from_vertex->second.in_edges.emplace_back(edge_type, from_vertex_id, edge); } // Increment edge count. ++shard_->edge_count_; @@ -539,8 +540,8 @@ ShardResult<EdgeAccessor> Shard::Accessor::CreateEdge(VertexId from_vertex_id, V ShardResult<std::optional<EdgeAccessor>> Shard::Accessor::DeleteEdge(VertexId from_vertex_id, VertexId to_vertex_id, const Gid edge_id) { - Vertex *from_vertex{nullptr}; - Vertex *to_vertex{nullptr}; + VertexContainer::value_type *from_vertex{nullptr}; + VertexContainer::value_type *to_vertex{nullptr}; auto &vertices = shard_->vertices_; @@ -550,13 +551,13 @@ ShardResult<std::optional<EdgeAccessor>> Shard::Accessor::DeleteEdge(VertexId fr if (from_is_local) { auto it = vertices.find(from_vertex_id.primary_key); MG_ASSERT(it != vertices.end(), "Cannot find local vertex"); - from_vertex = &it->second; + from_vertex = &*it; } if (to_is_local) { auto it = vertices.find(to_vertex_id.primary_key); MG_ASSERT(it != vertices.end(), "Cannot find local vertex"); - to_vertex = &it->second; + to_vertex = &*it; } MG_ASSERT(from_is_local || to_is_local, "Trying to delete an edge without having a local vertex"); @@ -565,13 +566,13 @@ ShardResult<std::optional<EdgeAccessor>> Shard::Accessor::DeleteEdge(VertexId fr if (!PrepareForWrite(transaction_, from_vertex)) { return SHARD_ERROR(ErrorCode::SERIALIZATION_ERROR); } - MG_ASSERT(!from_vertex->deleted, "Invalid database state!"); + MG_ASSERT(!from_vertex->second.deleted, "Invalid database state!"); } if (to_is_local && to_vertex != from_vertex) { if (!PrepareForWrite(transaction_, to_vertex)) { return SHARD_ERROR(ErrorCode::SERIALIZATION_ERROR); } - MG_ASSERT(!to_vertex->deleted, "Invalid database state!"); + MG_ASSERT(!to_vertex->second.deleted, "Invalid database state!"); } const auto edge_ref = std::invoke([edge_id, this]() -> EdgeRef { @@ -584,9 +585,9 @@ ShardResult<std::optional<EdgeAccessor>> Shard::Accessor::DeleteEdge(VertexId fr }); std::optional<EdgeTypeId> edge_type{}; - auto delete_edge_from_storage = [&edge_type, &edge_ref, this](std::vector<Vertex::EdgeLink> &edges) mutable { + auto delete_edge_from_storage = [&edge_type, &edge_ref, this](std::vector<VertexData::EdgeLink> &edges) mutable { auto it = std::find_if(edges.begin(), edges.end(), - [&edge_ref](const Vertex::EdgeLink &link) { return std::get<2>(link) == edge_ref; }); + [&edge_ref](const VertexData::EdgeLink &link) { return std::get<2>(link) == edge_ref; }); if (config_.properties_on_edges) { MG_ASSERT(it != edges.end(), "Invalid database state!"); } else if (it == edges.end()) { @@ -598,8 +599,8 @@ ShardResult<std::optional<EdgeAccessor>> Shard::Accessor::DeleteEdge(VertexId fr return true; }; // NOLINTNEXTLINE(clang-analyzer-core.NonNullParamChecker) - auto success_on_to = to_is_local ? delete_edge_from_storage(to_vertex->in_edges) : false; - auto success_on_from = from_is_local ? delete_edge_from_storage(from_vertex->out_edges) : false; + auto success_on_to = to_is_local ? delete_edge_from_storage(to_vertex->second.in_edges) : false; + auto success_on_from = from_is_local ? delete_edge_from_storage(from_vertex->second.out_edges) : false; if (config_.properties_on_edges) { // Because of the check above, we are sure that the vertex exists. @@ -625,10 +626,10 @@ ShardResult<std::optional<EdgeAccessor>> Shard::Accessor::DeleteEdge(VertexId fr MG_ASSERT(edge_type.has_value(), "Edge type is not determined"); if (from_is_local) { - CreateAndLinkDelta(transaction_, from_vertex, Delta::AddOutEdgeTag(), *edge_type, to_vertex_id, edge_ref); + CreateAndLinkDelta(transaction_, &*from_vertex, Delta::AddOutEdgeTag(), *edge_type, to_vertex_id, edge_ref); } if (to_is_local) { - CreateAndLinkDelta(transaction_, to_vertex, Delta::AddInEdgeTag(), *edge_type, from_vertex_id, edge_ref); + CreateAndLinkDelta(transaction_, &*to_vertex, Delta::AddInEdgeTag(), *edge_type, from_vertex_id, edge_ref); } // Decrement edge count. @@ -690,41 +691,41 @@ void Shard::Accessor::Abort() { auto prev = delta.prev.Get(); switch (prev.type) { case PreviousPtr::Type::VERTEX: { - auto *vertex = prev.vertex; - Delta *current = vertex->delta; + auto &[pk, vertex] = *prev.vertex; + Delta *current = vertex.delta; while (current != nullptr && current->commit_info->start_or_commit_timestamp == transaction_->start_timestamp) { switch (current->action) { case Delta::Action::REMOVE_LABEL: { - auto it = std::find(vertex->labels.begin(), vertex->labels.end(), current->label); - MG_ASSERT(it != vertex->labels.end(), "Invalid database state!"); - std::swap(*it, *vertex->labels.rbegin()); - vertex->labels.pop_back(); + auto it = std::find(vertex.labels.begin(), vertex.labels.end(), current->label); + MG_ASSERT(it != vertex.labels.end(), "Invalid database state!"); + std::swap(*it, *vertex.labels.rbegin()); + vertex.labels.pop_back(); break; } case Delta::Action::ADD_LABEL: { - auto it = std::find(vertex->labels.begin(), vertex->labels.end(), current->label); - MG_ASSERT(it == vertex->labels.end(), "Invalid database state!"); - vertex->labels.push_back(current->label); + auto it = std::find(vertex.labels.begin(), vertex.labels.end(), current->label); + MG_ASSERT(it == vertex.labels.end(), "Invalid database state!"); + vertex.labels.push_back(current->label); break; } case Delta::Action::SET_PROPERTY: { - vertex->properties.SetProperty(current->property.key, current->property.value); + vertex.properties.SetProperty(current->property.key, current->property.value); break; } case Delta::Action::ADD_IN_EDGE: { - Vertex::EdgeLink link{current->vertex_edge.edge_type, current->vertex_edge.vertex_id, - current->vertex_edge.edge}; - auto it = std::find(vertex->in_edges.begin(), vertex->in_edges.end(), link); - MG_ASSERT(it == vertex->in_edges.end(), "Invalid database state!"); - vertex->in_edges.push_back(link); + VertexData::EdgeLink link{current->vertex_edge.edge_type, current->vertex_edge.vertex_id, + current->vertex_edge.edge}; + auto it = std::find(vertex.in_edges.begin(), vertex.in_edges.end(), link); + MG_ASSERT(it == vertex.in_edges.end(), "Invalid database state!"); + vertex.in_edges.push_back(link); break; } case Delta::Action::ADD_OUT_EDGE: { - Vertex::EdgeLink link{current->vertex_edge.edge_type, current->vertex_edge.vertex_id, - current->vertex_edge.edge}; - auto it = std::find(vertex->out_edges.begin(), vertex->out_edges.end(), link); - MG_ASSERT(it == vertex->out_edges.end(), "Invalid database state!"); - vertex->out_edges.push_back(link); + VertexData::EdgeLink link{current->vertex_edge.edge_type, current->vertex_edge.vertex_id, + current->vertex_edge.edge}; + auto it = std::find(vertex.out_edges.begin(), vertex.out_edges.end(), link); + MG_ASSERT(it == vertex.out_edges.end(), "Invalid database state!"); + vertex.out_edges.push_back(link); // Increment edge count. We only increment the count here because // the information in `ADD_IN_EDGE` and `Edge/RECREATE_OBJECT` is // redundant. Also, `Edge/RECREATE_OBJECT` isn't available when @@ -733,21 +734,21 @@ void Shard::Accessor::Abort() { break; } case Delta::Action::REMOVE_IN_EDGE: { - Vertex::EdgeLink link{current->vertex_edge.edge_type, current->vertex_edge.vertex_id, - current->vertex_edge.edge}; - auto it = std::find(vertex->in_edges.begin(), vertex->in_edges.end(), link); - MG_ASSERT(it != vertex->in_edges.end(), "Invalid database state!"); - std::swap(*it, *vertex->in_edges.rbegin()); - vertex->in_edges.pop_back(); + VertexData::EdgeLink link{current->vertex_edge.edge_type, current->vertex_edge.vertex_id, + current->vertex_edge.edge}; + auto it = std::find(vertex.in_edges.begin(), vertex.in_edges.end(), link); + MG_ASSERT(it != vertex.in_edges.end(), "Invalid database state!"); + std::swap(*it, *vertex.in_edges.rbegin()); + vertex.in_edges.pop_back(); break; } case Delta::Action::REMOVE_OUT_EDGE: { - Vertex::EdgeLink link{current->vertex_edge.edge_type, current->vertex_edge.vertex_id, - current->vertex_edge.edge}; - auto it = std::find(vertex->out_edges.begin(), vertex->out_edges.end(), link); - MG_ASSERT(it != vertex->out_edges.end(), "Invalid database state!"); - std::swap(*it, *vertex->out_edges.rbegin()); - vertex->out_edges.pop_back(); + VertexData::EdgeLink link{current->vertex_edge.edge_type, current->vertex_edge.vertex_id, + current->vertex_edge.edge}; + auto it = std::find(vertex.out_edges.begin(), vertex.out_edges.end(), link); + MG_ASSERT(it != vertex.out_edges.end(), "Invalid database state!"); + std::swap(*it, *vertex.out_edges.rbegin()); + vertex.out_edges.pop_back(); // Decrement edge count. We only decrement the count here because // the information in `REMOVE_IN_EDGE` and `Edge/DELETE_OBJECT` is // redundant. Also, `Edge/DELETE_OBJECT` isn't available when edge @@ -756,20 +757,20 @@ void Shard::Accessor::Abort() { break; } case Delta::Action::DELETE_OBJECT: { - vertex->deleted = true; - shard_->deleted_vertices_.push_back(vertex->keys); + vertex.deleted = true; + shard_->deleted_vertices_.push_back(&pk); break; } case Delta::Action::RECREATE_OBJECT: { - vertex->deleted = false; + vertex.deleted = false; break; } } current = current->next; } - vertex->delta = current; + vertex.delta = current; if (current != nullptr) { - current->prev.Set(vertex); + current->prev.Set(prev.vertex); } break; @@ -964,10 +965,12 @@ void Shard::CollectGarbage(const io::Time current_time) { auto prev = delta.prev.Get(); switch (prev.type) { case PreviousPtr::Type::VERTEX: { - Vertex *vertex = prev.vertex; - vertex->delta = nullptr; - if (vertex->deleted) { - deleted_vertices_.push_back(vertex->keys); + // Here we need to get pk from prev pointer, and that is why change + // to the PrevPtr so it points to pair of pk and vertex + auto &[pk, vertex] = *prev.vertex; + vertex.delta = nullptr; + if (vertex.deleted) { + deleted_vertices_.push_back(&pk); } break; } @@ -1015,7 +1018,7 @@ void Shard::CollectGarbage(const io::Time current_time) { } for (const auto &vertex : deleted_vertices_) { - MG_ASSERT(vertices_.erase(vertex), "Invalid database state!"); + MG_ASSERT(vertices_.erase(*vertex), "Invalid database state!"); } deleted_vertices_.clear(); { diff --git a/src/storage/v3/shard.hpp b/src/storage/v3/shard.hpp index b998c06cc..8542d47eb 100644 --- a/src/storage/v3/shard.hpp +++ b/src/storage/v3/shard.hpp @@ -25,7 +25,6 @@ #include "io/time.hpp" #include "kvstore/kvstore.hpp" #include "storage/v3/config.hpp" -#include "storage/v3/containers.hpp" #include "storage/v3/edge.hpp" #include "storage/v3/edge_accessor.hpp" #include "storage/v3/id_types.hpp" @@ -387,7 +386,7 @@ class Shard final { // Vertices that are logically deleted but still have to be removed from // indices before removing them from the main storage. - std::list<PrimaryKey> deleted_vertices_; + std::list<const PrimaryKey *> deleted_vertices_; // Edges that are logically deleted and wait to be removed from the main // storage. diff --git a/src/storage/v3/containers.hpp b/src/storage/v3/vertex.cpp similarity index 59% rename from src/storage/v3/containers.hpp rename to src/storage/v3/vertex.cpp index 918cf3568..0dc80ad65 100644 --- a/src/storage/v3/containers.hpp +++ b/src/storage/v3/vertex.cpp @@ -9,16 +9,22 @@ // by the Apache License, Version 2.0, included in the file // licenses/APL.txt. -#pragma once - -#include <map> - -#include "storage/v3/edge.hpp" -#include "storage/v3/id_types.hpp" -#include "storage/v3/key_store.hpp" #include "storage/v3/vertex.hpp" +#include <limits> +#include <tuple> +#include <type_traits> +#include <vector> + +#include "storage/v3/delta.hpp" + namespace memgraph::storage::v3 { -using VertexContainer = std::map<PrimaryKey, Vertex>; -using EdgeContainer = std::map<Gid, Edge>; + +VertexData::VertexData(Delta *delta) : delta{delta} { + MG_ASSERT(delta == nullptr || delta->action == Delta::Action::DELETE_OBJECT, + "Vertex must be created with an initial DELETE_OBJECT delta!"); +} + +bool VertexHasLabel(const Vertex &vertex, const LabelId label) { return utils::Contains(vertex.second.labels, label); } + } // namespace memgraph::storage::v3 diff --git a/src/storage/v3/vertex.hpp b/src/storage/v3/vertex.hpp index 565e39d1a..f8caaa399 100644 --- a/src/storage/v3/vertex.hpp +++ b/src/storage/v3/vertex.hpp @@ -16,7 +16,6 @@ #include <type_traits> #include <vector> -#include "storage/v3/delta.hpp" #include "storage/v3/edge_ref.hpp" #include "storage/v3/id_types.hpp" #include "storage/v3/key_store.hpp" @@ -28,17 +27,12 @@ namespace memgraph::storage::v3 { -struct Vertex { +struct Delta; + +struct VertexData { using EdgeLink = std::tuple<EdgeTypeId, VertexId, EdgeRef>; - Vertex(Delta *delta, PrimaryKey primary_properties) : keys{std::move(primary_properties)}, delta{delta} { - MG_ASSERT(delta == nullptr || delta->action == Delta::Action::DELETE_OBJECT, - "Vertex must be created with an initial DELETE_OBJECT delta!"); - } - - friend bool operator==(const Vertex &vertex, const PrimaryKey &primary_key) { return vertex.keys == primary_key; } - - PrimaryKey keys; + explicit VertexData(Delta *delta); std::vector<LabelId> labels; PropertyStore properties; @@ -52,8 +46,11 @@ struct Vertex { Delta *delta; }; -static_assert(alignof(Vertex) >= 8, "The Vertex should be aligned to at least 8!"); +// static_assert(alignof(Vertex) >= 8, "The Vertex should be aligned to at least 8!"); -inline bool VertexHasLabel(const Vertex &vertex, const LabelId label) { return utils::Contains(vertex.labels, label); } +using VertexContainer = std::map<PrimaryKey, VertexData>; +using Vertex = VertexContainer::value_type; + +bool VertexHasLabel(const Vertex &vertex, LabelId label); } // namespace memgraph::storage::v3 diff --git a/src/storage/v3/vertex_accessor.cpp b/src/storage/v3/vertex_accessor.cpp index 3f3d01efb..95e0524ea 100644 --- a/src/storage/v3/vertex_accessor.cpp +++ b/src/storage/v3/vertex_accessor.cpp @@ -36,8 +36,8 @@ std::pair<bool, bool> IsVisible(Vertex *vertex, Transaction *transaction, View v bool deleted = false; Delta *delta = nullptr; { - deleted = vertex->deleted; - delta = vertex->delta; + deleted = vertex->second.deleted; + delta = vertex->second.delta; } ApplyDeltasForRead(transaction, delta, view, [&](const Delta &delta) { switch (delta.action) { @@ -85,13 +85,14 @@ ShardResult<bool> VertexAccessor::AddLabel(LabelId label) { if (!PrepareForWrite(transaction_, vertex_)) return SHARD_ERROR(ErrorCode::SERIALIZATION_ERROR); - if (vertex_->deleted) return SHARD_ERROR(ErrorCode::DELETED_OBJECT); + if (vertex_->second.deleted) return SHARD_ERROR(ErrorCode::DELETED_OBJECT); - if (std::find(vertex_->labels.begin(), vertex_->labels.end(), label) != vertex_->labels.end()) return false; + if (std::find(vertex_->second.labels.begin(), vertex_->second.labels.end(), label) != vertex_->second.labels.end()) + return false; CreateAndLinkDelta(transaction_, vertex_, Delta::RemoveLabelTag(), label); - vertex_->labels.push_back(label); + vertex_->second.labels.push_back(label); UpdateOnAddLabel(indices_, label, vertex_, *transaction_); @@ -106,13 +107,14 @@ ShardResult<bool> VertexAccessor::AddLabelAndValidate(LabelId label) { if (!PrepareForWrite(transaction_, vertex_)) return SHARD_ERROR(ErrorCode::SERIALIZATION_ERROR); - if (vertex_->deleted) return SHARD_ERROR(ErrorCode::DELETED_OBJECT); + if (vertex_->second.deleted) return SHARD_ERROR(ErrorCode::DELETED_OBJECT); - if (std::find(vertex_->labels.begin(), vertex_->labels.end(), label) != vertex_->labels.end()) return false; + if (std::find(vertex_->second.labels.begin(), vertex_->second.labels.end(), label) != vertex_->second.labels.end()) + return false; CreateAndLinkDelta(transaction_, vertex_, Delta::RemoveLabelTag(), label); - vertex_->labels.push_back(label); + vertex_->second.labels.push_back(label); UpdateOnAddLabel(indices_, label, vertex_, *transaction_); @@ -122,15 +124,15 @@ ShardResult<bool> VertexAccessor::AddLabelAndValidate(LabelId label) { ShardResult<bool> VertexAccessor::RemoveLabel(LabelId label) { if (!PrepareForWrite(transaction_, vertex_)) return SHARD_ERROR(ErrorCode::SERIALIZATION_ERROR); - if (vertex_->deleted) return SHARD_ERROR(ErrorCode::DELETED_OBJECT); + if (vertex_->second.deleted) return SHARD_ERROR(ErrorCode::DELETED_OBJECT); - auto it = std::find(vertex_->labels.begin(), vertex_->labels.end(), label); - if (it == vertex_->labels.end()) return false; + auto it = std::find(vertex_->second.labels.begin(), vertex_->second.labels.end(), label); + if (it == vertex_->second.labels.end()) return false; CreateAndLinkDelta(transaction_, vertex_, Delta::AddLabelTag(), label); - std::swap(*it, *vertex_->labels.rbegin()); - vertex_->labels.pop_back(); + std::swap(*it, *vertex_->second.labels.rbegin()); + vertex_->second.labels.pop_back(); return true; } @@ -142,15 +144,15 @@ ShardResult<bool> VertexAccessor::RemoveLabelAndValidate(LabelId label) { if (!PrepareForWrite(transaction_, vertex_)) return SHARD_ERROR(ErrorCode::SERIALIZATION_ERROR); - if (vertex_->deleted) return SHARD_ERROR(ErrorCode::DELETED_OBJECT); + if (vertex_->second.deleted) return SHARD_ERROR(ErrorCode::DELETED_OBJECT); - auto it = std::find(vertex_->labels.begin(), vertex_->labels.end(), label); - if (it == vertex_->labels.end()) return false; + auto it = std::find(vertex_->second.labels.begin(), vertex_->second.labels.end(), label); + if (it == vertex_->second.labels.end()) return false; CreateAndLinkDelta(transaction_, vertex_, Delta::AddLabelTag(), label); - std::swap(*it, *vertex_->labels.rbegin()); - vertex_->labels.pop_back(); + std::swap(*it, *vertex_->second.labels.rbegin()); + vertex_->second.labels.pop_back(); return true; } @@ -162,9 +164,9 @@ ShardResult<bool> VertexAccessor::HasLabel(LabelId label, View view) const { bool has_label = false; Delta *delta = nullptr; { - deleted = vertex_->deleted; + deleted = vertex_->second.deleted; has_label = label == vertex_validator_->primary_label_ || VertexHasLabel(*vertex_, label); - delta = vertex_->delta; + delta = vertex_->second.delta; } ApplyDeltasForRead(transaction_, delta, view, [&exists, &deleted, &has_label, label](const Delta &delta) { switch (delta.action) { @@ -215,14 +217,14 @@ ShardResult<PrimaryKey> VertexAccessor::PrimaryKey(const View view) const { if (const auto result = CheckVertexExistence(view); result.HasError()) { return result.GetError(); } - return vertex_->keys; + return vertex_->first; } ShardResult<VertexId> VertexAccessor::Id(View view) const { if (const auto result = CheckVertexExistence(view); result.HasError()) { return result.GetError(); } - return VertexId{vertex_validator_->primary_label_, vertex_->keys}; + return VertexId{vertex_validator_->primary_label_, vertex_->first}; }; ShardResult<std::vector<LabelId>> VertexAccessor::Labels(View view) const { @@ -231,9 +233,9 @@ ShardResult<std::vector<LabelId>> VertexAccessor::Labels(View view) const { std::vector<LabelId> labels; Delta *delta = nullptr; { - deleted = vertex_->deleted; - labels = vertex_->labels; - delta = vertex_->delta; + deleted = vertex_->second.deleted; + labels = vertex_->second.labels; + delta = vertex_->second.delta; } ApplyDeltasForRead(transaction_, delta, view, [&exists, &deleted, &labels](const Delta &delta) { switch (delta.action) { @@ -278,9 +280,9 @@ ShardResult<PropertyValue> VertexAccessor::SetProperty(PropertyId property, cons if (!PrepareForWrite(transaction_, vertex_)) return SHARD_ERROR(ErrorCode::SERIALIZATION_ERROR); - if (vertex_->deleted) return SHARD_ERROR(ErrorCode::DELETED_OBJECT); + if (vertex_->second.deleted) return SHARD_ERROR(ErrorCode::DELETED_OBJECT); - auto current_value = vertex_->properties.GetProperty(property); + auto current_value = vertex_->second.properties.GetProperty(property); // We could skip setting the value if the previous one is the same to the new // one. This would save some memory as a delta would not be created as well as // avoid copying the value. The reason we are not doing that is because the @@ -288,7 +290,7 @@ ShardResult<PropertyValue> VertexAccessor::SetProperty(PropertyId property, cons // "modify in-place". Additionally, the created delta will make other // transactions get a SERIALIZATION_ERROR. CreateAndLinkDelta(transaction_, vertex_, Delta::SetPropertyTag(), property, current_value); - vertex_->properties.SetProperty(property, value); + vertex_->second.properties.SetProperty(property, value); UpdateOnSetProperty(indices_, property, value, vertex_, *transaction_); @@ -300,8 +302,8 @@ ShardResult<void> VertexAccessor::CheckVertexExistence(View view) const { bool deleted = false; Delta *delta = nullptr; { - deleted = vertex_->deleted; - delta = vertex_->delta; + deleted = vertex_->second.deleted; + delta = vertex_->second.delta; } ApplyDeltasForRead(transaction_, delta, view, [&exists, &deleted](const Delta &delta) { switch (delta.action) { @@ -343,11 +345,11 @@ ShardResult<PropertyValue> VertexAccessor::SetPropertyAndValidate(PropertyId pro return SHARD_ERROR(ErrorCode::SERIALIZATION_ERROR); } - if (vertex_->deleted) { + if (vertex_->second.deleted) { return SHARD_ERROR(ErrorCode::DELETED_OBJECT); } - auto current_value = vertex_->properties.GetProperty(property); + auto current_value = vertex_->second.properties.GetProperty(property); // We could skip setting the value if the previous one is the same to the new // one. This would save some memory as a delta would not be created as well as // avoid copying the value. The reason we are not doing that is because the @@ -355,7 +357,7 @@ ShardResult<PropertyValue> VertexAccessor::SetPropertyAndValidate(PropertyId pro // "modify in-place". Additionally, the created delta will make other // transactions get a SERIALIZATION_ERROR. CreateAndLinkDelta(transaction_, vertex_, Delta::SetPropertyTag(), property, current_value); - vertex_->properties.SetProperty(property, value); + vertex_->second.properties.SetProperty(property, value); UpdateOnSetProperty(indices_, property, value, vertex_, *transaction_); @@ -365,15 +367,15 @@ ShardResult<PropertyValue> VertexAccessor::SetPropertyAndValidate(PropertyId pro ShardResult<std::map<PropertyId, PropertyValue>> VertexAccessor::ClearProperties() { if (!PrepareForWrite(transaction_, vertex_)) return SHARD_ERROR(ErrorCode::SERIALIZATION_ERROR); - if (vertex_->deleted) return SHARD_ERROR(ErrorCode::DELETED_OBJECT); + if (vertex_->second.deleted) return SHARD_ERROR(ErrorCode::DELETED_OBJECT); - auto properties = vertex_->properties.Properties(); + auto properties = vertex_->second.properties.Properties(); for (const auto &property : properties) { CreateAndLinkDelta(transaction_, vertex_, Delta::SetPropertyTag(), property.first, property.second); UpdateOnSetProperty(indices_, property.first, PropertyValue(), vertex_, *transaction_); } - vertex_->properties.ClearProperties(); + vertex_->second.properties.ClearProperties(); return std::move(properties); } @@ -396,11 +398,11 @@ PropertyValue VertexAccessor::GetPropertyValue(PropertyId property, View view) c // Find PropertyId index in keystore for (size_t property_index{0}; property_index < schema->second.size(); ++property_index) { if (schema->second[property_index].property_id == property) { - return vertex_->keys[property_index]; + return vertex_->first[property_index]; } } - return value = vertex_->properties.GetProperty(property); + return value = vertex_->second.properties.GetProperty(property); } ShardResult<PropertyValue> VertexAccessor::GetProperty(PropertyId property, View view) const { @@ -409,9 +411,9 @@ ShardResult<PropertyValue> VertexAccessor::GetProperty(PropertyId property, View PropertyValue value; Delta *delta = nullptr; { - deleted = vertex_->deleted; + deleted = vertex_->second.deleted; value = GetPropertyValue(property, view); - delta = vertex_->delta; + delta = vertex_->second.delta; } ApplyDeltasForRead(transaction_, delta, view, [&exists, &deleted, &value, property](const Delta &delta) { switch (delta.action) { @@ -453,10 +455,10 @@ ShardResult<std::map<PropertyId, PropertyValue>> VertexAccessor::Properties(View std::map<PropertyId, PropertyValue> properties; Delta *delta = nullptr; { - deleted = vertex_->deleted; + deleted = vertex_->second.deleted; // TODO(antaljanosbenjamin): Should this also return the primary key? - properties = vertex_->properties.Properties(); - delta = vertex_->delta; + properties = vertex_->second.properties.Properties(); + delta = vertex_->second.delta; } ApplyDeltasForRead(transaction_, delta, view, [&exists, &deleted, &properties](const Delta &delta) { switch (delta.action) { @@ -501,14 +503,14 @@ ShardResult<std::vector<EdgeAccessor>> VertexAccessor::InEdges(View view, const const VertexId *destination_id) const { bool exists = true; bool deleted = false; - std::vector<Vertex::EdgeLink> in_edges; + std::vector<VertexData::EdgeLink> in_edges; Delta *delta = nullptr; { - deleted = vertex_->deleted; + deleted = vertex_->second.deleted; if (edge_types.empty() && nullptr == destination_id) { - in_edges = vertex_->in_edges; + in_edges = vertex_->second.in_edges; } else { - for (const auto &item : vertex_->in_edges) { + for (const auto &item : vertex_->second.in_edges) { const auto &[edge_type, from_vertex, edge] = item; if (nullptr != destination_id && from_vertex != *destination_id) { continue; @@ -518,7 +520,7 @@ ShardResult<std::vector<EdgeAccessor>> VertexAccessor::InEdges(View view, const in_edges.push_back(item); } } - delta = vertex_->delta; + delta = vertex_->second.delta; } ApplyDeltasForRead( transaction_, delta, view, [&exists, &deleted, &in_edges, &edge_types, destination_id](const Delta &delta) { @@ -529,7 +531,7 @@ ShardResult<std::vector<EdgeAccessor>> VertexAccessor::InEdges(View view, const std::find(edge_types.begin(), edge_types.end(), delta.vertex_edge.edge_type) == edge_types.end()) break; // Add the edge because we don't see the removal. - Vertex::EdgeLink link{delta.vertex_edge.edge_type, delta.vertex_edge.vertex_id, delta.vertex_edge.edge}; + VertexData::EdgeLink link{delta.vertex_edge.edge_type, delta.vertex_edge.vertex_id, delta.vertex_edge.edge}; auto it = std::find(in_edges.begin(), in_edges.end(), link); MG_ASSERT(it == in_edges.end(), "Invalid database state!"); in_edges.push_back(link); @@ -541,7 +543,7 @@ ShardResult<std::vector<EdgeAccessor>> VertexAccessor::InEdges(View view, const std::find(edge_types.begin(), edge_types.end(), delta.vertex_edge.edge_type) == edge_types.end()) break; // Remove the label because we don't see the addition. - Vertex::EdgeLink link{delta.vertex_edge.edge_type, delta.vertex_edge.vertex_id, delta.vertex_edge.edge}; + VertexData::EdgeLink link{delta.vertex_edge.edge_type, delta.vertex_edge.vertex_id, delta.vertex_edge.edge}; auto it = std::find(in_edges.begin(), in_edges.end(), link); MG_ASSERT(it != in_edges.end(), "Invalid database state!"); std::swap(*it, *in_edges.rbegin()); @@ -571,7 +573,7 @@ ShardResult<std::vector<EdgeAccessor>> VertexAccessor::InEdges(View view, const return ret; } ret.reserve(in_edges.size()); - const auto id = VertexId{vertex_validator_->primary_label_, vertex_->keys}; + const auto id = VertexId{vertex_validator_->primary_label_, vertex_->first}; for (const auto &item : in_edges) { const auto &[edge_type, from_vertex, edge] = item; ret.emplace_back(edge, edge_type, from_vertex, id, transaction_, indices_, config_); @@ -583,14 +585,14 @@ ShardResult<std::vector<EdgeAccessor>> VertexAccessor::OutEdges(View view, const const VertexId *destination_id) const { bool exists = true; bool deleted = false; - std::vector<Vertex::EdgeLink> out_edges; + std::vector<VertexData::EdgeLink> out_edges; Delta *delta = nullptr; { - deleted = vertex_->deleted; + deleted = vertex_->second.deleted; if (edge_types.empty() && nullptr == destination_id) { - out_edges = vertex_->out_edges; + out_edges = vertex_->second.out_edges; } else { - for (const auto &item : vertex_->out_edges) { + for (const auto &item : vertex_->second.out_edges) { const auto &[edge_type, to_vertex, edge] = item; if (nullptr != destination_id && to_vertex != *destination_id) continue; if (!edge_types.empty() && std::find(edge_types.begin(), edge_types.end(), edge_type) == edge_types.end()) @@ -598,7 +600,7 @@ ShardResult<std::vector<EdgeAccessor>> VertexAccessor::OutEdges(View view, const out_edges.push_back(item); } } - delta = vertex_->delta; + delta = vertex_->second.delta; } ApplyDeltasForRead( transaction_, delta, view, [&exists, &deleted, &out_edges, &edge_types, destination_id](const Delta &delta) { @@ -609,7 +611,7 @@ ShardResult<std::vector<EdgeAccessor>> VertexAccessor::OutEdges(View view, const std::find(edge_types.begin(), edge_types.end(), delta.vertex_edge.edge_type) == edge_types.end()) break; // Add the edge because we don't see the removal. - Vertex::EdgeLink link{delta.vertex_edge.edge_type, delta.vertex_edge.vertex_id, delta.vertex_edge.edge}; + VertexData::EdgeLink link{delta.vertex_edge.edge_type, delta.vertex_edge.vertex_id, delta.vertex_edge.edge}; auto it = std::find(out_edges.begin(), out_edges.end(), link); MG_ASSERT(it == out_edges.end(), "Invalid database state!"); out_edges.push_back(link); @@ -621,7 +623,7 @@ ShardResult<std::vector<EdgeAccessor>> VertexAccessor::OutEdges(View view, const std::find(edge_types.begin(), edge_types.end(), delta.vertex_edge.edge_type) == edge_types.end()) break; // Remove the label because we don't see the addition. - Vertex::EdgeLink link{delta.vertex_edge.edge_type, delta.vertex_edge.vertex_id, delta.vertex_edge.edge}; + VertexData::EdgeLink link{delta.vertex_edge.edge_type, delta.vertex_edge.vertex_id, delta.vertex_edge.edge}; auto it = std::find(out_edges.begin(), out_edges.end(), link); MG_ASSERT(it != out_edges.end(), "Invalid database state!"); std::swap(*it, *out_edges.rbegin()); @@ -651,7 +653,7 @@ ShardResult<std::vector<EdgeAccessor>> VertexAccessor::OutEdges(View view, const return ret; } ret.reserve(out_edges.size()); - const auto id = VertexId{vertex_validator_->primary_label_, vertex_->keys}; + const auto id = VertexId{vertex_validator_->primary_label_, vertex_->first}; for (const auto &item : out_edges) { const auto &[edge_type, to_vertex, edge] = item; ret.emplace_back(edge, edge_type, id, to_vertex, transaction_, indices_, config_); @@ -665,9 +667,9 @@ ShardResult<size_t> VertexAccessor::InDegree(View view) const { size_t degree = 0; Delta *delta = nullptr; { - deleted = vertex_->deleted; - degree = vertex_->in_edges.size(); - delta = vertex_->delta; + deleted = vertex_->second.deleted; + degree = vertex_->second.in_edges.size(); + delta = vertex_->second.delta; } ApplyDeltasForRead(transaction_, delta, view, [&exists, &deleted, °ree](const Delta &delta) { switch (delta.action) { @@ -702,9 +704,9 @@ ShardResult<size_t> VertexAccessor::OutDegree(View view) const { size_t degree = 0; Delta *delta = nullptr; { - deleted = vertex_->deleted; - degree = vertex_->out_edges.size(); - delta = vertex_->delta; + deleted = vertex_->second.deleted; + degree = vertex_->second.out_edges.size(); + delta = vertex_->second.delta; } ApplyDeltasForRead(transaction_, delta, view, [&exists, &deleted, °ree](const Delta &delta) { switch (delta.action) { From a90a2d86c918351fb57036c47e4405a82fb83765 Mon Sep 17 00:00:00 2001 From: jbajic <jure.bajic@memgraph.com> Date: Mon, 12 Dec 2022 12:46:13 +0100 Subject: [PATCH 12/19] Fix edge test --- src/storage/v3/shard.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/storage/v3/shard.cpp b/src/storage/v3/shard.cpp index 2573110c4..0d8b21402 100644 --- a/src/storage/v3/shard.cpp +++ b/src/storage/v3/shard.cpp @@ -529,7 +529,7 @@ ShardResult<EdgeAccessor> Shard::Accessor::CreateEdge(VertexId from_vertex_id, V } if (to_is_local) { CreateAndLinkDelta(transaction_, to_vertex, Delta::RemoveInEdgeTag(), edge_type, from_vertex_id, edge); - from_vertex->second.in_edges.emplace_back(edge_type, from_vertex_id, edge); + to_vertex->second.in_edges.emplace_back(edge_type, from_vertex_id, edge); } // Increment edge count. ++shard_->edge_count_; From c3e19498daf3031b9cf7f1d6326c60a43555e341 Mon Sep 17 00:00:00 2001 From: jbajic <jure.bajic@memgraph.com> Date: Mon, 12 Dec 2022 14:44:58 +0100 Subject: [PATCH 13/19] Replace LabelPropertyIndex with std::set --- src/storage/v3/indices.cpp | 53 +++++++++++++++++++++----------------- src/storage/v3/indices.hpp | 4 +-- 2 files changed, 32 insertions(+), 25 deletions(-) diff --git a/src/storage/v3/indices.cpp b/src/storage/v3/indices.cpp index 84097a26a..93573d275 100644 --- a/src/storage/v3/indices.cpp +++ b/src/storage/v3/indices.cpp @@ -11,6 +11,7 @@ #include "indices.hpp" +#include <algorithm> #include <cstddef> #include <cstdint> #include <functional> @@ -398,7 +399,7 @@ void LabelPropertyIndex::UpdateOnAddLabel(LabelId label, Vertex *vertex, const T } auto prop_value = vertex->second.properties.GetProperty(label_prop.second); if (!prop_value.IsNull()) { - index.emplace(prop_value, Entry{prop_value, vertex, tx.start_timestamp.logical_id}); + index.emplace(Entry{prop_value, vertex, tx.start_timestamp.logical_id}); } } } @@ -413,7 +414,7 @@ void LabelPropertyIndex::UpdateOnSetProperty(PropertyId property, const Property continue; } if (VertexHasLabel(*vertex, label_prop.first)) { - index.emplace(value, Entry{value, vertex, tx.start_timestamp.logical_id}); + index.emplace(Entry{value, vertex, tx.start_timestamp.logical_id}); } } } @@ -435,7 +436,7 @@ bool LabelPropertyIndex::CreateIndex(LabelId label, PropertyId property, VertexC if (value.IsNull()) { continue; } - it->second.emplace(value, Entry{value, &vertex, 0}); + it->second.emplace(Entry{value, &vertex, 0}); } } catch (const utils::OutOfMemoryException &) { utils::MemoryTracker::OutOfMemoryExceptionBlocker oom_exception_blocker; @@ -460,16 +461,15 @@ void LabelPropertyIndex::RemoveObsoleteEntries(const uint64_t clean_up_before_ti auto next_it = it; ++next_it; - if (it->second.timestamp >= clean_up_before_timestamp) { + if (it->timestamp >= clean_up_before_timestamp) { it = next_it; continue; } - if (auto &entry = it->second, &next_entry = next_it->second; - (next_it != index.end() && entry.vertex == next_entry.vertex && entry.value == next_entry.value) || - !AnyVersionHasLabelProperty(*entry.vertex, label_property.first, label_property.second, entry.value, + if ((next_it != index.end() && it->vertex == next_it->vertex && it->value == next_it->value) || + !AnyVersionHasLabelProperty(*it->vertex, label_property.first, label_property.second, it->value, clean_up_before_timestamp)) { - index.erase(it->first); + index.erase(it); } it = next_it; } @@ -492,32 +492,32 @@ LabelPropertyIndex::Iterable::Iterator &LabelPropertyIndex::Iterable::Iterator:: void LabelPropertyIndex::Iterable::Iterator::AdvanceUntilValid() { for (; index_iterator_ != self_->index_accessor_->end(); ++index_iterator_) { - if (index_iterator_->second.vertex == current_vertex_) { + if (index_iterator_->vertex == current_vertex_) { continue; } if (self_->lower_bound_) { - if (index_iterator_->second.value < self_->lower_bound_->value()) { + if (index_iterator_->value < self_->lower_bound_->value()) { continue; } - if (!self_->lower_bound_->IsInclusive() && index_iterator_->second.value == self_->lower_bound_->value()) { + if (!self_->lower_bound_->IsInclusive() && index_iterator_->value == self_->lower_bound_->value()) { continue; } } if (self_->upper_bound_) { - if (self_->upper_bound_->value() < index_iterator_->second.value) { + if (self_->upper_bound_->value() < index_iterator_->value) { index_iterator_ = self_->index_accessor_->end(); break; } - if (!self_->upper_bound_->IsInclusive() && index_iterator_->second.value == self_->upper_bound_->value()) { + if (!self_->upper_bound_->IsInclusive() && index_iterator_->value == self_->upper_bound_->value()) { index_iterator_ = self_->index_accessor_->end(); break; } } - if (CurrentVersionHasLabelProperty(*index_iterator_->second.vertex, self_->label_, self_->property_, - index_iterator_->second.value, self_->transaction_, self_->view_)) { - current_vertex_ = index_iterator_->second.vertex; + if (CurrentVersionHasLabelProperty(*index_iterator_->vertex, self_->label_, self_->property_, + index_iterator_->value, self_->transaction_, self_->view_)) { + current_vertex_ = index_iterator_->vertex; current_vertex_accessor_ = VertexAccessor(current_vertex_, self_->transaction_, self_->indices_, self_->config_, *self_->vertex_validator_); break; @@ -655,7 +655,7 @@ LabelPropertyIndex::Iterable::Iterator LabelPropertyIndex::Iterable::begin() { auto index_iterator = index_accessor_->begin(); if (lower_bound_) { index_iterator = std::ranges::find_if(*index_accessor_, [lower_bound = lower_bound_->value()](const auto &pair) { - return pair.second.value >= lower_bound; + return pair.value >= lower_bound; }); } return {this, index_iterator}; @@ -667,9 +667,12 @@ int64_t LabelPropertyIndex::VertexCount(LabelId label, PropertyId property, cons auto it = index_.find({label, property}); MG_ASSERT(it != index_.end(), "Index for label {} and property {} doesn't exist", label.AsUint(), property.AsUint()); if (!value.IsNull()) { - return static_cast<int64_t>(it->second.count(value)); + return static_cast<int64_t>( + std::ranges::count_if(it->second, [&value](const auto &elem) { return elem.value == value; })); } - return static_cast<int64_t>(it->second.count(value)); + // TODO Do check this + return static_cast<int64_t>( + std::ranges::count_if(it->second, [&value](const auto &elem) { return elem.value == value; })); } int64_t LabelPropertyIndex::VertexCount(LabelId label, PropertyId property, @@ -681,9 +684,11 @@ int64_t LabelPropertyIndex::VertexCount(LabelId label, PropertyId property, [&index = it->second](const auto value, const auto def) { if (value) { if (value->IsInclusive()) { - return index.lower_bound(value->value()); + return std::lower_bound(index.begin(), index.end(), value->value(), + [](const Entry &elem, const PropertyValue &val) { return elem.value < val; }); } - return index.upper_bound(value->value()); + return std::upper_bound(index.begin(), index.end(), value->value(), + [](const PropertyValue &val, const Entry &elem) { return val < elem.value; }); } return def; }, @@ -692,9 +697,11 @@ int64_t LabelPropertyIndex::VertexCount(LabelId label, PropertyId property, [&index = it->second](const auto value, const auto def) { if (value) { if (value->IsInclusive()) { - return index.upper_bound(value->value()); + return std::upper_bound(index.begin(), index.end(), value->value(), + [](const PropertyValue &val, const Entry &elem) { return val < elem.value; }); } - return index.lower_bound(value->value()); + return std::lower_bound(index.begin(), index.end(), value->value(), + [](const Entry &elem, const PropertyValue &val) { return elem.value < val; }); } return def; }, diff --git a/src/storage/v3/indices.hpp b/src/storage/v3/indices.hpp index 425c9da13..4dfacabc2 100644 --- a/src/storage/v3/indices.hpp +++ b/src/storage/v3/indices.hpp @@ -12,8 +12,8 @@ #pragma once #include <cstdint> -#include <map> #include <optional> +#include <set> #include <tuple> #include <utility> @@ -146,7 +146,7 @@ class LabelPropertyIndex { }; public: - using LabelPropertyIndexContainer = std::multimap<PropertyValue, Entry>; + using LabelPropertyIndexContainer = std::set<Entry>; LabelPropertyIndex(Indices *indices, Config::Items config, const VertexValidator &vertex_validator) : indices_(indices), config_(config), vertex_validator_{&vertex_validator} {} From b0c454428701c317eee36b3254f10d673bff9793 Mon Sep 17 00:00:00 2001 From: jbajic <jure.bajic@memgraph.com> Date: Mon, 12 Dec 2022 15:46:41 +0100 Subject: [PATCH 14/19] Add asserts --- src/storage/v3/edge.hpp | 4 ++-- src/storage/v3/indices.cpp | 13 ------------- src/storage/v3/indices.hpp | 4 ---- src/storage/v3/vertex.hpp | 2 +- 4 files changed, 3 insertions(+), 20 deletions(-) diff --git a/src/storage/v3/edge.hpp b/src/storage/v3/edge.hpp index c5cf6c6f7..db2964b68 100644 --- a/src/storage/v3/edge.hpp +++ b/src/storage/v3/edge.hpp @@ -34,13 +34,13 @@ struct Edge { PropertyStore properties; bool deleted; - uint8_t PAD; + // uint8_t PAD; // uint16_t PAD; Delta *delta; }; -// static_assert(alignof(Edge) >= 8, "The Edge should be aligned to at least 8!"); +static_assert(alignof(Edge) >= 8, "The Edge should be aligned to at least 8!"); inline bool operator==(const Edge &first, const Edge &second) { return first.gid == second.gid; } inline bool operator<(const Edge &first, const Edge &second) { return first.gid < second.gid; } diff --git a/src/storage/v3/indices.cpp b/src/storage/v3/indices.cpp index 93573d275..c3dbe6c97 100644 --- a/src/storage/v3/indices.cpp +++ b/src/storage/v3/indices.cpp @@ -368,12 +368,6 @@ LabelIndex::Iterable::Iterable(utils::SkipList<Entry>::Accessor index_accessor, config_(config), vertex_validator_(&vertex_validator) {} -void LabelIndex::RunGC() { - for (auto &index_entry : index_) { - index_entry.second.run_gc(); - } -} - bool LabelPropertyIndex::Entry::operator<(const Entry &rhs) const { if (value < rhs.value) { return true; @@ -709,13 +703,6 @@ int64_t LabelPropertyIndex::VertexCount(LabelId label, PropertyId property, return static_cast<int64_t>(std::distance(lower_it, upper_it)); } -void LabelPropertyIndex::RunGC() { - // TODO(jbajic) What to do? - // for (auto &index_entry : index_) { - // index_entry.second.run_gc(); - // } -} - void RemoveObsoleteEntries(Indices *indices, const uint64_t clean_up_before_timestamp) { indices->label_index.RemoveObsoleteEntries(clean_up_before_timestamp); indices->label_property_index.RemoveObsoleteEntries(clean_up_before_timestamp); diff --git a/src/storage/v3/indices.hpp b/src/storage/v3/indices.hpp index 4dfacabc2..d8b730a87 100644 --- a/src/storage/v3/indices.hpp +++ b/src/storage/v3/indices.hpp @@ -123,8 +123,6 @@ class LabelIndex { void Clear() { index_.clear(); } - void RunGC(); - private: std::map<LabelId, utils::SkipList<Entry>> index_; Indices *indices_; @@ -240,8 +238,6 @@ class LabelPropertyIndex { void Clear() { index_.clear(); } - void RunGC(); - private: std::map<std::pair<LabelId, PropertyId>, LabelPropertyIndexContainer> index_; Indices *indices_; diff --git a/src/storage/v3/vertex.hpp b/src/storage/v3/vertex.hpp index f8caaa399..66d7a38e8 100644 --- a/src/storage/v3/vertex.hpp +++ b/src/storage/v3/vertex.hpp @@ -46,7 +46,7 @@ struct VertexData { Delta *delta; }; -// static_assert(alignof(Vertex) >= 8, "The Vertex should be aligned to at least 8!"); +static_assert(alignof(VertexData) >= 8, "The Vertex should be aligned to at least 8!"); using VertexContainer = std::map<PrimaryKey, VertexData>; using Vertex = VertexContainer::value_type; From edb122cb33585803e709ac867f6058009eede244 Mon Sep 17 00:00:00 2001 From: jbajic <jure.bajic@memgraph.com> Date: Mon, 12 Dec 2022 16:01:20 +0100 Subject: [PATCH 15/19] Fix benchmark tests --- tests/benchmark/data_structures_common.hpp | 6 ++---- tests/benchmark/data_structures_contains.cpp | 3 +-- tests/benchmark/data_structures_find.cpp | 3 +-- tests/benchmark/data_structures_insert.cpp | 7 ++----- tests/benchmark/data_structures_remove.cpp | 3 +-- 5 files changed, 7 insertions(+), 15 deletions(-) diff --git a/tests/benchmark/data_structures_common.hpp b/tests/benchmark/data_structures_common.hpp index 23fe394ee..4c2d41fc0 100644 --- a/tests/benchmark/data_structures_common.hpp +++ b/tests/benchmark/data_structures_common.hpp @@ -17,9 +17,9 @@ #include "coordinator/hybrid_logical_clock.hpp" #include "storage/v3/key_store.hpp" -#include "storage/v3/lexicographically_ordered_vertex.hpp" #include "storage/v3/mvcc.hpp" #include "storage/v3/transaction.hpp" +#include "storage/v3/vertex.hpp" #include "utils/skip_list.hpp" namespace memgraph::benchmark { @@ -40,9 +40,7 @@ inline void PrepareData(std::map<TKey, TValue> &std_map, const int64_t num_eleme storage::v3::Transaction transaction{start_timestamp, storage::v3::IsolationLevel::SNAPSHOT_ISOLATION}; auto *delta = storage::v3::CreateDeleteObjectDelta(&transaction); for (auto i{0}; i < num_elements; ++i) { - std_map.insert({storage::v3::PrimaryKey{storage::v3::PropertyValue{i}}, - storage::v3::LexicographicallyOrderedVertex{storage::v3::Vertex{ - delta, std::vector<storage::v3::PropertyValue>{storage::v3::PropertyValue{true}}}}}); + std_map.insert({storage::v3::PrimaryKey{storage::v3::PropertyValue{i}}, storage::v3::VertexData{delta}}); } } diff --git a/tests/benchmark/data_structures_contains.cpp b/tests/benchmark/data_structures_contains.cpp index 47af24a1b..679adee1f 100644 --- a/tests/benchmark/data_structures_contains.cpp +++ b/tests/benchmark/data_structures_contains.cpp @@ -25,7 +25,6 @@ #include "data_structures_common.hpp" #include "storage/v3/key_store.hpp" -#include "storage/v3/lexicographically_ordered_vertex.hpp" #include "storage/v3/mvcc.hpp" #include "storage/v3/property_value.hpp" #include "storage/v3/transaction.hpp" @@ -57,7 +56,7 @@ static void BM_BenchmarkContainsSkipList(::benchmark::State &state) { } static void BM_BenchmarkContainsStdMap(::benchmark::State &state) { - std::map<storage::v3::PrimaryKey, storage::v3::LexicographicallyOrderedVertex> std_map; + std::map<storage::v3::PrimaryKey, storage::v3::VertexData> std_map; PrepareData(std_map, state.range(0)); // So we can also have elements that does don't exist diff --git a/tests/benchmark/data_structures_find.cpp b/tests/benchmark/data_structures_find.cpp index d51771be5..fc0473909 100644 --- a/tests/benchmark/data_structures_find.cpp +++ b/tests/benchmark/data_structures_find.cpp @@ -25,7 +25,6 @@ #include "data_structures_common.hpp" #include "storage/v3/key_store.hpp" -#include "storage/v3/lexicographically_ordered_vertex.hpp" #include "storage/v3/mvcc.hpp" #include "storage/v3/property_value.hpp" #include "storage/v3/transaction.hpp" @@ -57,7 +56,7 @@ static void BM_BenchmarkFindSkipList(::benchmark::State &state) { } static void BM_BenchmarkFindStdMap(::benchmark::State &state) { - std::map<storage::v3::PrimaryKey, storage::v3::LexicographicallyOrderedVertex> std_map; + std::map<storage::v3::PrimaryKey, storage::v3::VertexData> std_map; PrepareData(std_map, state.range(0)); // So we can also have elements that does don't exist std::mt19937 i_generator(std::random_device{}()); diff --git a/tests/benchmark/data_structures_insert.cpp b/tests/benchmark/data_structures_insert.cpp index d83cbfe23..b5dc99f38 100644 --- a/tests/benchmark/data_structures_insert.cpp +++ b/tests/benchmark/data_structures_insert.cpp @@ -24,7 +24,6 @@ #include <gflags/gflags.h> #include "storage/v3/key_store.hpp" -#include "storage/v3/lexicographically_ordered_vertex.hpp" #include "storage/v3/mvcc.hpp" #include "storage/v3/property_value.hpp" #include "storage/v3/transaction.hpp" @@ -50,16 +49,14 @@ static void BM_BenchmarkInsertSkipList(::benchmark::State &state) { } static void BM_BenchmarkInsertStdMap(::benchmark::State &state) { - std::map<storage::v3::PrimaryKey, storage::v3::LexicographicallyOrderedVertex> std_map; + std::map<storage::v3::PrimaryKey, storage::v3::VertexData> std_map; coordinator::Hlc start_timestamp; storage::v3::Transaction transaction{start_timestamp, storage::v3::IsolationLevel::SNAPSHOT_ISOLATION}; auto *delta = storage::v3::CreateDeleteObjectDelta(&transaction); for (auto _ : state) { for (auto i{0}; i < state.range(0); ++i) { - std_map.insert({storage::v3::PrimaryKey{storage::v3::PropertyValue{i}}, - storage::v3::LexicographicallyOrderedVertex{storage::v3::Vertex{ - delta, std::vector<storage::v3::PropertyValue>{storage::v3::PropertyValue{true}}}}}); + std_map.insert({storage::v3::PrimaryKey{storage::v3::PropertyValue{i}}, storage::v3::VertexData{delta}}); } } } diff --git a/tests/benchmark/data_structures_remove.cpp b/tests/benchmark/data_structures_remove.cpp index 641ad9453..2d060961d 100644 --- a/tests/benchmark/data_structures_remove.cpp +++ b/tests/benchmark/data_structures_remove.cpp @@ -25,7 +25,6 @@ #include "data_structures_common.hpp" #include "storage/v3/key_store.hpp" -#include "storage/v3/lexicographically_ordered_vertex.hpp" #include "storage/v3/mvcc.hpp" #include "storage/v3/property_value.hpp" #include "storage/v3/transaction.hpp" @@ -58,7 +57,7 @@ static void BM_BenchmarkRemoveSkipList(::benchmark::State &state) { } static void BM_BenchmarkRemoveStdMap(::benchmark::State &state) { - std::map<storage::v3::PrimaryKey, storage::v3::LexicographicallyOrderedVertex> std_map; + std::map<storage::v3::PrimaryKey, storage::v3::VertexData> std_map; PrepareData(std_map, state.range(0)); // So we can also have elements that does don't exist From 76dcf3ad0f418b679a93fe7a096ce62ee2d6f94a Mon Sep 17 00:00:00 2001 From: jbajic <jure.bajic@memgraph.com> Date: Tue, 13 Dec 2022 13:21:52 +0100 Subject: [PATCH 16/19] Use std::set in LabelIndex --- src/storage/v3/indices.cpp | 25 +++++++++++-------------- src/storage/v3/indices.hpp | 31 +++++++++++-------------------- 2 files changed, 22 insertions(+), 34 deletions(-) diff --git a/src/storage/v3/indices.cpp b/src/storage/v3/indices.cpp index c3dbe6c97..aeb451128 100644 --- a/src/storage/v3/indices.cpp +++ b/src/storage/v3/indices.cpp @@ -271,8 +271,7 @@ bool CurrentVersionHasLabelProperty(const Vertex &vertex, LabelId label, Propert void LabelIndex::UpdateOnAddLabel(LabelId label, Vertex *vertex, const Transaction &tx) { auto it = index_.find(label); if (it == index_.end()) return; - auto acc = it->second.access(); - acc.insert(Entry{vertex, tx.start_timestamp.logical_id}); + it->second.insert(Entry{vertex, tx.start_timestamp.logical_id}); } bool LabelIndex::CreateIndex(LabelId label, VertexContainer &vertices) { @@ -283,12 +282,11 @@ bool LabelIndex::CreateIndex(LabelId label, VertexContainer &vertices) { return false; } try { - auto acc = it->second.access(); - for ([[maybe_unused]] auto &vertex : vertices) { + for (auto &vertex : vertices) { if (vertex.second.deleted || !VertexHasLabel(vertex, label)) { continue; } - acc.insert(Entry{&vertex, 0}); + it->second.insert(Entry{&vertex, 0}); } } catch (const utils::OutOfMemoryException &) { utils::MemoryTracker::OutOfMemoryExceptionBlocker oom_exception_blocker; @@ -309,7 +307,7 @@ std::vector<LabelId> LabelIndex::ListIndices() const { void LabelIndex::RemoveObsoleteEntries(const uint64_t clean_up_before_timestamp) { for (auto &label_storage : index_) { - auto vertices_acc = label_storage.second.access(); + auto &vertices_acc = label_storage.second; for (auto it = vertices_acc.begin(); it != vertices_acc.end();) { auto next_it = it; ++next_it; @@ -321,7 +319,7 @@ void LabelIndex::RemoveObsoleteEntries(const uint64_t clean_up_before_timestamp) if ((next_it != vertices_acc.end() && it->vertex == next_it->vertex) || !AnyVersionHasLabel(*it->vertex, label_storage.first, clean_up_before_timestamp)) { - vertices_acc.remove(*it); + vertices_acc.erase(*it); } it = next_it; @@ -329,7 +327,7 @@ void LabelIndex::RemoveObsoleteEntries(const uint64_t clean_up_before_timestamp) } } -LabelIndex::Iterable::Iterator::Iterator(Iterable *self, utils::SkipList<Entry>::Iterator index_iterator) +LabelIndex::Iterable::Iterator::Iterator(Iterable *self, LabelIndexContainer::iterator index_iterator) : self_(self), index_iterator_(index_iterator), current_vertex_accessor_(nullptr, nullptr, nullptr, self_->config_, *self_->vertex_validator_), @@ -344,7 +342,7 @@ LabelIndex::Iterable::Iterator &LabelIndex::Iterable::Iterator::operator++() { } void LabelIndex::Iterable::Iterator::AdvanceUntilValid() { - for (; index_iterator_ != self_->index_accessor_.end(); ++index_iterator_) { + for (; index_iterator_ != self_->index_accessor_->end(); ++index_iterator_) { if (index_iterator_->vertex == current_vertex_) { continue; } @@ -357,10 +355,9 @@ void LabelIndex::Iterable::Iterator::AdvanceUntilValid() { } } -LabelIndex::Iterable::Iterable(utils::SkipList<Entry>::Accessor index_accessor, LabelId label, View view, - Transaction *transaction, Indices *indices, Config::Items config, - const VertexValidator &vertex_validator) - : index_accessor_(std::move(index_accessor)), +LabelIndex::Iterable::Iterable(LabelIndexContainer &index_accessor, LabelId label, View view, Transaction *transaction, + Indices *indices, Config::Items config, const VertexValidator &vertex_validator) + : index_accessor_(&index_accessor), label_(label), view_(view), transaction_(transaction), @@ -422,7 +419,7 @@ bool LabelPropertyIndex::CreateIndex(LabelId label, PropertyId property, VertexC return false; } try { - for ([[maybe_unused]] auto &vertex : vertices) { + for (auto &vertex : vertices) { if (vertex.second.deleted || !VertexHasLabel(vertex, label)) { continue; } diff --git a/src/storage/v3/indices.hpp b/src/storage/v3/indices.hpp index d8b730a87..ef80625f2 100644 --- a/src/storage/v3/indices.hpp +++ b/src/storage/v3/indices.hpp @@ -30,7 +30,6 @@ namespace memgraph::storage::v3 { struct Indices; class LabelIndex { - private: struct Entry { Vertex *vertex; uint64_t timestamp; @@ -41,17 +40,9 @@ class LabelIndex { bool operator==(const Entry &rhs) const { return vertex == rhs.vertex && timestamp == rhs.timestamp; } }; - struct LabelStorage { - LabelId label; - utils::SkipList<Entry> vertices; - - bool operator<(const LabelStorage &rhs) const { return label < rhs.label; } - bool operator<(LabelId rhs) const { return label < rhs; } - bool operator==(const LabelStorage &rhs) const { return label == rhs.label; } - bool operator==(LabelId rhs) const { return label == rhs; } - }; - public: + using LabelIndexContainer = std::set<Entry>; + LabelIndex(Indices *indices, Config::Items config, const VertexValidator &vertex_validator) : indices_(indices), config_(config), vertex_validator_{&vertex_validator} {} @@ -72,12 +63,12 @@ class LabelIndex { class Iterable { public: - Iterable(utils::SkipList<Entry>::Accessor index_accessor, LabelId label, View view, Transaction *transaction, - Indices *indices, Config::Items config, const VertexValidator &vertex_validator); + Iterable(LabelIndexContainer &index_accessor, LabelId label, View view, Transaction *transaction, Indices *indices, + Config::Items config, const VertexValidator &vertex_validator); class Iterator { public: - Iterator(Iterable *self, utils::SkipList<Entry>::Iterator index_iterator); + Iterator(Iterable *self, LabelIndexContainer::iterator index_iterator); VertexAccessor operator*() const { return current_vertex_accessor_; } @@ -90,16 +81,16 @@ class LabelIndex { void AdvanceUntilValid(); Iterable *self_; - utils::SkipList<Entry>::Iterator index_iterator_; + LabelIndexContainer::iterator index_iterator_; VertexAccessor current_vertex_accessor_; Vertex *current_vertex_; }; - Iterator begin() { return {this, index_accessor_.begin()}; } - Iterator end() { return {this, index_accessor_.end()}; } + Iterator begin() { return {this, index_accessor_->begin()}; } + Iterator end() { return {this, index_accessor_->end()}; } private: - utils::SkipList<Entry>::Accessor index_accessor_; + LabelIndexContainer *index_accessor_; LabelId label_; View view_; Transaction *transaction_; @@ -112,7 +103,7 @@ class LabelIndex { Iterable Vertices(LabelId label, View view, Transaction *transaction) { auto it = index_.find(label); MG_ASSERT(it != index_.end(), "Index for label {} doesn't exist", label.AsUint()); - return {it->second.access(), label, view, transaction, indices_, config_, *vertex_validator_}; + return {it->second, label, view, transaction, indices_, config_, *vertex_validator_}; } int64_t ApproximateVertexCount(LabelId label) { @@ -124,7 +115,7 @@ class LabelIndex { void Clear() { index_.clear(); } private: - std::map<LabelId, utils::SkipList<Entry>> index_; + std::map<LabelId, LabelIndexContainer> index_; Indices *indices_; Config::Items config_; const VertexValidator *vertex_validator_; From 817433d342de9574fc99c24b02ea2bc78d50b783 Mon Sep 17 00:00:00 2001 From: jbajic <jure.bajic@memgraph.com> Date: Tue, 13 Dec 2022 13:32:14 +0100 Subject: [PATCH 17/19] Revert to approximate --- src/storage/v3/bindings/db_accessor.hpp | 8 ++++---- src/storage/v3/shard.hpp | 24 ++++++++++++++---------- tests/unit/storage_v3_indices.cpp | 8 ++++---- 3 files changed, 22 insertions(+), 18 deletions(-) diff --git a/src/storage/v3/bindings/db_accessor.hpp b/src/storage/v3/bindings/db_accessor.hpp index a8afdcffa..6392ed18f 100644 --- a/src/storage/v3/bindings/db_accessor.hpp +++ b/src/storage/v3/bindings/db_accessor.hpp @@ -162,23 +162,23 @@ class DbAccessor final { return accessor_->LabelPropertyIndexExists(label, prop); } - int64_t VerticesCount() const { return accessor_->VertexCount(); } + int64_t VerticesCount() const { return accessor_->ApproximateVertexCount(); } int64_t VerticesCount(storage::v3::LabelId label) const { return accessor_->ApproximateVertexCount(label); } int64_t VerticesCount(storage::v3::LabelId label, storage::v3::PropertyId property) const { - return accessor_->VertexCount(label, property); + return accessor_->ApproximateVertexCount(label, property); } int64_t VerticesCount(storage::v3::LabelId label, storage::v3::PropertyId property, const storage::v3::PropertyValue &value) const { - return accessor_->VertexCount(label, property, value); + return accessor_->ApproximateVertexCount(label, property, value); } int64_t VerticesCount(storage::v3::LabelId label, storage::v3::PropertyId property, const std::optional<utils::Bound<storage::v3::PropertyValue>> &lower, const std::optional<utils::Bound<storage::v3::PropertyValue>> &upper) const { - return accessor_->VertexCount(label, property, lower, upper); + return accessor_->ApproximateVertexCount(label, property, lower, upper); } storage::v3::IndicesInfo ListAllIndices() const { return accessor_->ListAllIndices(); } diff --git a/src/storage/v3/shard.hpp b/src/storage/v3/shard.hpp index 8542d47eb..16e2bf6be 100644 --- a/src/storage/v3/shard.hpp +++ b/src/storage/v3/shard.hpp @@ -225,30 +225,34 @@ class Shard final { const std::optional<utils::Bound<PropertyValue>> &lower_bound, const std::optional<utils::Bound<PropertyValue>> &upper_bound, View view); - /// Return number of all vertices in the database. - int64_t VertexCount() const { return static_cast<int64_t>(shard_->vertices_.size()); } + /// Return approximate number of all vertices in the database. + /// Note that this is always an over-estimate and never an under-estimate. + int64_t ApproximateVertexCount() const { return static_cast<int64_t>(shard_->vertices_.size()); } /// Return approximate number of vertices with the given label. /// Note that this is always an over-estimate and never an under-estimate. int64_t ApproximateVertexCount(LabelId label) const { return shard_->indices_.label_index.ApproximateVertexCount(label); } - - /// Return number of vertices with the given label and property. - int64_t VertexCount(LabelId label, PropertyId property) const { + /// Return approximate number of vertices with the given label and property. + /// Note that this is always an over-estimate and never an under-estimate. + int64_t ApproximateVertexCount(LabelId label, PropertyId property) const { return shard_->indices_.label_property_index.VertexCount(label, property); } - /// Return number of vertices with the given label and the given - int64_t VertexCount(LabelId label, PropertyId property, const PropertyValue &value) const { + /// Return approximate number of vertices with the given label and the given + /// value for the given property. + /// Note that this is always an over-estimate and never an under-estimate. + int64_t ApproximateVertexCount(LabelId label, PropertyId property, const PropertyValue &value) const { return shard_->indices_.label_property_index.VertexCount(label, property, value); } - /// Return number of vertices with the given label and value for + /// Return approximate number of vertices with the given label and value for /// the given property in the range defined by provided upper and lower /// bounds. - int64_t VertexCount(LabelId label, PropertyId property, const std::optional<utils::Bound<PropertyValue>> &lower, - const std::optional<utils::Bound<PropertyValue>> &upper) const { + int64_t ApproximateVertexCount(LabelId label, PropertyId property, + const std::optional<utils::Bound<PropertyValue>> &lower, + const std::optional<utils::Bound<PropertyValue>> &upper) const { return shard_->indices_.label_property_index.VertexCount(label, property, lower, upper); } diff --git a/tests/unit/storage_v3_indices.cpp b/tests/unit/storage_v3_indices.cpp index 13f528d5a..620878fcf 100644 --- a/tests/unit/storage_v3_indices.cpp +++ b/tests/unit/storage_v3_indices.cpp @@ -652,13 +652,13 @@ TEST_F(IndexTest, LabelPropertyIndexCountEstimate) { } } - EXPECT_EQ(acc.VertexCount(label1, prop_val), 55); + EXPECT_EQ(acc.ApproximateVertexCount(label1, prop_val), 55); for (int i = 1; i <= 10; ++i) { - EXPECT_EQ(acc.VertexCount(label1, prop_val, PropertyValue(i)), i); + EXPECT_EQ(acc.ApproximateVertexCount(label1, prop_val, PropertyValue(i)), i); } - EXPECT_EQ(acc.VertexCount(label1, prop_val, memgraph::utils::MakeBoundInclusive(PropertyValue(2)), - memgraph::utils::MakeBoundInclusive(PropertyValue(6))), + EXPECT_EQ(acc.ApproximateVertexCount(label1, prop_val, memgraph::utils::MakeBoundInclusive(PropertyValue(2)), + memgraph::utils::MakeBoundInclusive(PropertyValue(6))), 2 + 3 + 4 + 5 + 6); } From a0e0791aa15d88658c66636f5333a78f42533df6 Mon Sep 17 00:00:00 2001 From: jbajic <jure.bajic@memgraph.com> Date: Thu, 15 Dec 2022 11:25:37 +0100 Subject: [PATCH 18/19] Address review comments --- src/storage/v3/delta.hpp | 3 +- src/storage/v3/indices.cpp | 51 ++++++++++++------------------- src/storage/v3/indices.hpp | 12 ++++---- src/storage/v3/mvcc.hpp | 21 +++++-------- src/storage/v3/property_value.hpp | 30 ------------------ src/storage/v3/shard.cpp | 2 +- 6 files changed, 36 insertions(+), 83 deletions(-) diff --git a/src/storage/v3/delta.hpp b/src/storage/v3/delta.hpp index 1c6e57d2b..0bc58f5ef 100644 --- a/src/storage/v3/delta.hpp +++ b/src/storage/v3/delta.hpp @@ -11,6 +11,7 @@ #pragma once +#include <cstdint> #include <memory> #include "storage/v3/edge_ref.hpp" #include "storage/v3/id_types.hpp" @@ -129,7 +130,7 @@ inline bool operator==(const PreviousPtr::Pointer &a, const PreviousPtr::Pointer inline bool operator!=(const PreviousPtr::Pointer &a, const PreviousPtr::Pointer &b) { return !(a == b); } struct Delta { - enum class Action { + enum class Action : uint8_t { // Used for both Vertex and Edge DELETE_OBJECT, RECREATE_OBJECT, diff --git a/src/storage/v3/indices.cpp b/src/storage/v3/indices.cpp index aeb451128..dd2bdf75e 100644 --- a/src/storage/v3/indices.cpp +++ b/src/storage/v3/indices.cpp @@ -12,8 +12,6 @@ #include "indices.hpp" #include <algorithm> -#include <cstddef> -#include <cstdint> #include <functional> #include <limits> @@ -342,7 +340,7 @@ LabelIndex::Iterable::Iterator &LabelIndex::Iterable::Iterator::operator++() { } void LabelIndex::Iterable::Iterator::AdvanceUntilValid() { - for (; index_iterator_ != self_->index_accessor_->end(); ++index_iterator_) { + for (; index_iterator_ != self_->index_container_->end(); ++index_iterator_) { if (index_iterator_->vertex == current_vertex_) { continue; } @@ -355,9 +353,9 @@ void LabelIndex::Iterable::Iterator::AdvanceUntilValid() { } } -LabelIndex::Iterable::Iterable(LabelIndexContainer &index_accessor, LabelId label, View view, Transaction *transaction, +LabelIndex::Iterable::Iterable(LabelIndexContainer &index_container, LabelId label, View view, Transaction *transaction, Indices *indices, Config::Items config, const VertexValidator &vertex_validator) - : index_accessor_(&index_accessor), + : index_container_(&index_container), label_(label), view_(view), transaction_(transaction), @@ -482,7 +480,7 @@ LabelPropertyIndex::Iterable::Iterator &LabelPropertyIndex::Iterable::Iterator:: } void LabelPropertyIndex::Iterable::Iterator::AdvanceUntilValid() { - for (; index_iterator_ != self_->index_accessor_->end(); ++index_iterator_) { + for (; index_iterator_ != self_->index_container_->end(); ++index_iterator_) { if (index_iterator_->vertex == current_vertex_) { continue; } @@ -497,11 +495,11 @@ void LabelPropertyIndex::Iterable::Iterator::AdvanceUntilValid() { } if (self_->upper_bound_) { if (self_->upper_bound_->value() < index_iterator_->value) { - index_iterator_ = self_->index_accessor_->end(); + index_iterator_ = self_->index_container_->end(); break; } if (!self_->upper_bound_->IsInclusive() && index_iterator_->value == self_->upper_bound_->value()) { - index_iterator_ = self_->index_accessor_->end(); + index_iterator_ = self_->index_container_->end(); break; } } @@ -528,12 +526,12 @@ const PropertyValue kSmallestMap = PropertyValue(std::map<std::string, PropertyV const PropertyValue kSmallestTemporalData = PropertyValue(TemporalData{static_cast<TemporalType>(0), std::numeric_limits<int64_t>::min()}); -LabelPropertyIndex::Iterable::Iterable(LabelPropertyIndexContainer &index_accessor, LabelId label, PropertyId property, +LabelPropertyIndex::Iterable::Iterable(LabelPropertyIndexContainer &index_container, LabelId label, PropertyId property, const std::optional<utils::Bound<PropertyValue>> &lower_bound, const std::optional<utils::Bound<PropertyValue>> &upper_bound, View view, Transaction *transaction, Indices *indices, Config::Items config, const VertexValidator &vertex_validator) - : index_accessor_(&index_accessor), + : index_container_(&index_container), label_(label), property_(property), lower_bound_(lower_bound), @@ -641,29 +639,24 @@ LabelPropertyIndex::Iterable::Iterator LabelPropertyIndex::Iterable::begin() { // If the bounds are set and don't have comparable types we don't yield any // items from the index. if (!bounds_valid_) { - return {this, index_accessor_->end()}; + return {this, index_container_->end()}; } - auto index_iterator = index_accessor_->begin(); if (lower_bound_) { - index_iterator = std::ranges::find_if(*index_accessor_, [lower_bound = lower_bound_->value()](const auto &pair) { - return pair.value >= lower_bound; - }); + return {this, std::ranges::lower_bound(*index_container_, lower_bound_->value(), std::less{}, &Entry::value)}; } - return {this, index_iterator}; + return {this, index_container_->begin()}; } -LabelPropertyIndex::Iterable::Iterator LabelPropertyIndex::Iterable::end() { return {this, index_accessor_->end()}; } +LabelPropertyIndex::Iterable::Iterator LabelPropertyIndex::Iterable::end() { return {this, index_container_->end()}; } int64_t LabelPropertyIndex::VertexCount(LabelId label, PropertyId property, const PropertyValue &value) const { auto it = index_.find({label, property}); MG_ASSERT(it != index_.end(), "Index for label {} and property {} doesn't exist", label.AsUint(), property.AsUint()); - if (!value.IsNull()) { - return static_cast<int64_t>( - std::ranges::count_if(it->second, [&value](const auto &elem) { return elem.value == value; })); - } - // TODO Do check this + MG_ASSERT(!value.IsNull(), "Null is not supported!"); + + auto start_it = std::ranges::lower_bound(it->second, value, std::less{}, &Entry::value); return static_cast<int64_t>( - std::ranges::count_if(it->second, [&value](const auto &elem) { return elem.value == value; })); + std::ranges::count_if(start_it, it->second.end(), [&value](const auto &elem) { return elem.value == value; })); } int64_t LabelPropertyIndex::VertexCount(LabelId label, PropertyId property, @@ -675,11 +668,9 @@ int64_t LabelPropertyIndex::VertexCount(LabelId label, PropertyId property, [&index = it->second](const auto value, const auto def) { if (value) { if (value->IsInclusive()) { - return std::lower_bound(index.begin(), index.end(), value->value(), - [](const Entry &elem, const PropertyValue &val) { return elem.value < val; }); + return std::ranges::lower_bound(index, value->value(), std::less{}, &Entry::value); } - return std::upper_bound(index.begin(), index.end(), value->value(), - [](const PropertyValue &val, const Entry &elem) { return val < elem.value; }); + return std::ranges::upper_bound(index, value->value(), std::less{}, &Entry::value); } return def; }, @@ -688,11 +679,9 @@ int64_t LabelPropertyIndex::VertexCount(LabelId label, PropertyId property, [&index = it->second](const auto value, const auto def) { if (value) { if (value->IsInclusive()) { - return std::upper_bound(index.begin(), index.end(), value->value(), - [](const PropertyValue &val, const Entry &elem) { return val < elem.value; }); + return std::ranges::upper_bound(index, value->value(), std::less{}, &Entry::value); } - return std::lower_bound(index.begin(), index.end(), value->value(), - [](const Entry &elem, const PropertyValue &val) { return elem.value < val; }); + return std::ranges::lower_bound(index, value->value(), std::less{}, &Entry::value); } return def; }, diff --git a/src/storage/v3/indices.hpp b/src/storage/v3/indices.hpp index ef80625f2..d8e491470 100644 --- a/src/storage/v3/indices.hpp +++ b/src/storage/v3/indices.hpp @@ -63,7 +63,7 @@ class LabelIndex { class Iterable { public: - Iterable(LabelIndexContainer &index_accessor, LabelId label, View view, Transaction *transaction, Indices *indices, + Iterable(LabelIndexContainer &index_container, LabelId label, View view, Transaction *transaction, Indices *indices, Config::Items config, const VertexValidator &vertex_validator); class Iterator { @@ -86,11 +86,11 @@ class LabelIndex { Vertex *current_vertex_; }; - Iterator begin() { return {this, index_accessor_->begin()}; } - Iterator end() { return {this, index_accessor_->end()}; } + Iterator begin() { return {this, index_container_->begin()}; } + Iterator end() { return {this, index_container_->end()}; } private: - LabelIndexContainer *index_accessor_; + LabelIndexContainer *index_container_; LabelId label_; View view_; Transaction *transaction_; @@ -159,7 +159,7 @@ class LabelPropertyIndex { class Iterable { public: - Iterable(LabelPropertyIndexContainer &index_accessor, LabelId label, PropertyId property, + Iterable(LabelPropertyIndexContainer &index_container, LabelId label, PropertyId property, const std::optional<utils::Bound<PropertyValue>> &lower_bound, const std::optional<utils::Bound<PropertyValue>> &upper_bound, View view, Transaction *transaction, Indices *indices, Config::Items config, const VertexValidator &vertex_validator); @@ -188,7 +188,7 @@ class LabelPropertyIndex { Iterator end(); private: - LabelPropertyIndexContainer *index_accessor_; + LabelPropertyIndexContainer *index_container_; LabelId label_; PropertyId property_; std::optional<utils::Bound<PropertyValue>> lower_bound_; diff --git a/src/storage/v3/mvcc.hpp b/src/storage/v3/mvcc.hpp index 001f57003..6ce058d62 100644 --- a/src/storage/v3/mvcc.hpp +++ b/src/storage/v3/mvcc.hpp @@ -12,6 +12,7 @@ #pragma once #include <type_traits> + #include "storage/v3/edge.hpp" #include "storage/v3/property_value.hpp" #include "storage/v3/transaction.hpp" @@ -21,6 +22,10 @@ namespace memgraph::storage::v3 { +inline VertexData *GetDeltaHolder(Vertex *vertex) { return &vertex->second; } + +inline Edge *GetDeltaHolder(Edge *edge) { return edge; } + /// This function iterates through the undo buffers from an object (starting /// from the supplied delta) and determines what deltas should be applied to get /// the currently visible version of the object. When the function finds a delta @@ -83,13 +88,7 @@ inline void ApplyDeltasForRead(Transaction *transaction, const Delta *delta, Vie template <typename TObj> requires utils::SameAsAnyOf<TObj, Edge, Vertex> inline bool PrepareForWrite(Transaction *transaction, TObj *object) { - auto *delta_holder = std::invoke([object]() -> auto * { - if constexpr (std::is_same_v<TObj, Vertex>) { - return &object->second; - } else { - return object; - } - }); + auto *delta_holder = GetDeltaHolder(object); if (delta_holder->delta == nullptr) return true; const auto &delta_commit_info = *delta_holder->delta->commit_info; @@ -121,13 +120,7 @@ requires utils::SameAsAnyOf<TObj, Edge, Vertex> inline void CreateAndLinkDelta(Transaction *transaction, TObj *object, Args &&...args) { auto delta = &transaction->deltas.emplace_back(std::forward<Args>(args)..., transaction->commit_info.get(), transaction->command_id); - auto *delta_holder = std::invoke([object]() -> auto * { - if constexpr (std::is_same_v<TObj, Vertex>) { - return &object->second; - } else { - return object; - } - }); + auto *delta_holder = GetDeltaHolder(object); // The operations are written in such order so that both `next` and `prev` // chains are valid at all times. The chains must be valid at all times diff --git a/src/storage/v3/property_value.hpp b/src/storage/v3/property_value.hpp index e1e4b4d67..56902bed5 100644 --- a/src/storage/v3/property_value.hpp +++ b/src/storage/v3/property_value.hpp @@ -330,36 +330,6 @@ inline bool operator<(const PropertyValue &first, const PropertyValue &second) { } } -inline bool operator>=(const PropertyValue &first, const PropertyValue &second) { - if (!PropertyValue::AreComparableTypes(first.type(), second.type())) return first.type() >= second.type(); - switch (first.type()) { - case PropertyValue::Type::Null: - return false; - case PropertyValue::Type::Bool: - return first.ValueBool() >= second.ValueBool(); - case PropertyValue::Type::Int: - if (second.type() == PropertyValue::Type::Double) { - return static_cast<double>(first.ValueInt()) >= second.ValueDouble(); - } else { - return first.ValueInt() >= second.ValueInt(); - } - case PropertyValue::Type::Double: - if (second.type() == PropertyValue::Type::Double) { - return first.ValueDouble() >= second.ValueDouble(); - } else { - return first.ValueDouble() >= static_cast<double>(second.ValueInt()); - } - case PropertyValue::Type::String: - return first.ValueString() >= second.ValueString(); - case PropertyValue::Type::List: - return first.ValueList() >= second.ValueList(); - case PropertyValue::Type::Map: - return first.ValueMap() >= second.ValueMap(); - case PropertyValue::Type::TemporalData: - return first.ValueTemporalData() >= second.ValueTemporalData(); - } -} - inline PropertyValue::PropertyValue(const PropertyValue &other) : type_(other.type_) { switch (other.type_) { case Type::Null: diff --git a/src/storage/v3/shard.cpp b/src/storage/v3/shard.cpp index 0d8b21402..5e16209d9 100644 --- a/src/storage/v3/shard.cpp +++ b/src/storage/v3/shard.cpp @@ -1017,7 +1017,7 @@ void Shard::CollectGarbage(const io::Time current_time) { RemoveObsoleteEntries(&indices_, clean_up_before_timestamp); } - for (const auto &vertex : deleted_vertices_) { + for (const auto *vertex : deleted_vertices_) { MG_ASSERT(vertices_.erase(*vertex), "Invalid database state!"); } deleted_vertices_.clear(); From e82955895a519379cee39c308d1960baaf8720c0 Mon Sep 17 00:00:00 2001 From: jbajic <jure.bajic@memgraph.com> Date: Tue, 20 Dec 2022 16:14:01 +0100 Subject: [PATCH 19/19] Leave a TODO --- src/storage/v3/indices.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/storage/v3/indices.cpp b/src/storage/v3/indices.cpp index dd2bdf75e..2f11a35ec 100644 --- a/src/storage/v3/indices.cpp +++ b/src/storage/v3/indices.cpp @@ -654,6 +654,7 @@ int64_t LabelPropertyIndex::VertexCount(LabelId label, PropertyId property, cons MG_ASSERT(it != index_.end(), "Index for label {} and property {} doesn't exist", label.AsUint(), property.AsUint()); MG_ASSERT(!value.IsNull(), "Null is not supported!"); + // TODO(jbajic) This can be improved by exiting early auto start_it = std::ranges::lower_bound(it->second, value, std::less{}, &Entry::value); return static_cast<int64_t>( std::ranges::count_if(start_it, it->second.end(), [&value](const auto &elem) { return elem.value == value; }));