From bd9606a1152e115656a5e78092df95ca20ff9bae Mon Sep 17 00:00:00 2001 From: Thomas Goyne Date: Tue, 5 Mar 2024 10:26:16 -0800 Subject: [PATCH] Consolidate collection updating logic somewhat --- src/realm/collection.cpp | 21 +++++++++++++ src/realm/collection.hpp | 46 +++++++++++++--------------- src/realm/collection_parent.hpp | 14 ++++----- src/realm/dictionary.cpp | 37 +++++++++------------- src/realm/dictionary.hpp | 20 ++++++------ src/realm/list.cpp | 46 ++++++---------------------- src/realm/list.hpp | 43 +++++++++----------------- src/realm/obj.cpp | 54 +++++++++++++++++++-------------- src/realm/obj.hpp | 32 +++++++------------ src/realm/set.cpp | 26 ---------------- src/realm/set.hpp | 27 +++++++---------- 11 files changed, 149 insertions(+), 217 deletions(-) diff --git a/src/realm/collection.cpp b/src/realm/collection.cpp index 11829ac33b5..73a7ac1b47e 100644 --- a/src/realm/collection.cpp +++ b/src/realm/collection.cpp @@ -230,4 +230,25 @@ void Collection::get_any(QueryCtrlBlock& ctrl, Mixed val, size_t index) } } +UpdateStatus CollectionBase::do_init_from_parent(BPlusTreeBase* tree, ref_type ref, bool allow_create) +{ + if (ref) { + tree->init_from_ref(ref); + } + else { + if (tree->init_from_parent()) { + // All is well + return UpdateStatus::Updated; + } + if (!allow_create) { + tree->detach(); + return UpdateStatus::Detached; + } + // The ref in the column was NULL, create the tree in place. + tree->create(); + REALM_ASSERT(tree->is_attached()); + } + return UpdateStatus::Updated; +} + } // namespace realm diff --git a/src/realm/collection.hpp b/src/realm/collection.hpp index f2b5c8f56c7..87f530352ca 100644 --- a/src/realm/collection.hpp +++ b/src/realm/collection.hpp @@ -53,18 +53,18 @@ class DummyParent : public CollectionParent { { return m_obj; } + uint32_t parent_version() const noexcept final + { + return 0; + } protected: Obj m_obj; ref_type m_ref; - UpdateStatus update_if_needed_with_status() const final + UpdateStatus update_if_needed() const final { return UpdateStatus::Updated; } - bool update_if_needed() const final - { - return true; - } ref_type get_collection_ref(Index, CollectionType) const final { return m_ref; @@ -262,6 +262,7 @@ class CollectionBase : public Collection { CollectionBase& operator=(CollectionBase&&) noexcept = default; void validate_index(const char* msg, size_t index, size_t size) const; + static UpdateStatus do_init_from_parent(BPlusTreeBase* tree, ref_type ref, bool allow_create); }; inline std::string_view collection_type_name(CollectionType col_type, bool uppercase = false) @@ -499,7 +500,7 @@ class CollectionBaseImpl : public Interface, protected ArrayParent { if (m_parent) { try { // Update the parent. Will throw if parent is not existing. - switch (m_parent->update_if_needed_with_status()) { + switch (m_parent->update_if_needed()) { case UpdateStatus::Updated: // Make sure to update next time around m_content_version = 0; @@ -531,7 +532,7 @@ class CollectionBaseImpl : public Interface, protected ArrayParent { { try { // `has_changed()` sneakily modifies internal state. - update_if_needed_with_status(); + update_if_needed(); if (m_last_content_version != m_content_version) { m_last_content_version = m_content_version; return true; @@ -580,14 +581,12 @@ class CollectionBaseImpl : public Interface, protected ArrayParent { Obj m_obj_mem; std::shared_ptr m_col_parent; CollectionParent::Index m_index; - mutable size_t m_my_version = 0; ColKey m_col_key; - bool m_nullable = false; - mutable uint_fast64_t m_content_version = 0; - // Content version used by `has_changed()`. mutable uint_fast64_t m_last_content_version = 0; + mutable uint32_t m_parent_version = 0; + bool m_nullable = false; CollectionBaseImpl() = default; CollectionBaseImpl(const CollectionBaseImpl& other) @@ -657,13 +656,14 @@ class CollectionBaseImpl : public Interface, protected ArrayParent { UpdateStatus get_update_status() const { - UpdateStatus status = m_parent ? m_parent->update_if_needed_with_status() : UpdateStatus::Detached; + UpdateStatus status = m_parent ? m_parent->update_if_needed() : UpdateStatus::Detached; if (status != UpdateStatus::Detached) { auto content_version = m_alloc->get_content_version(); - if (content_version != m_content_version || m_my_version != m_parent->m_parent_version) { + auto parent_version = m_parent->parent_version(); + if (content_version != m_content_version || m_parent_version != parent_version) { m_content_version = content_version; - m_my_version = m_parent->m_parent_version; + m_parent_version = parent_version; status = UpdateStatus::Updated; } } @@ -674,18 +674,14 @@ class CollectionBaseImpl : public Interface, protected ArrayParent { /// Refresh the parent object (if needed) and compare version numbers. /// Return true if the collection should initialize from parent /// Throws if the owning object no longer exists. - bool should_update() + bool should_update() const { check_parent(); - bool changed = m_parent->update_if_needed(); // Throws if the object does not exist. - auto content_version = m_alloc->get_content_version(); - - if (changed || content_version != m_content_version || m_my_version != m_parent->m_parent_version) { - m_content_version = content_version; - m_my_version = m_parent->m_parent_version; - return true; + auto status = get_update_status(); + if (status == UpdateStatus::Detached) { + throw StaleAccessor("Parent no longer exists"); } - return false; + return status == UpdateStatus::Updated; } void bump_content_version() @@ -803,7 +799,7 @@ class CollectionBaseImpl : public Interface, protected ArrayParent { /// /// If no change has happened to the data, this function returns /// `UpdateStatus::NoChange`, and the caller is allowed to not do anything. - virtual UpdateStatus update_if_needed_with_status() const = 0; + virtual UpdateStatus update_if_needed() const = 0; }; namespace _impl { @@ -891,7 +887,7 @@ class ObjCollectionBase : public Interface, public _impl::ObjListProxy { /// `BPlusTree`. virtual BPlusTree* get_mutable_tree() const = 0; - /// Implements update_if_needed() in a way that ensures the consistency of + /// Implements `update_if_needed()` in a way that ensures the consistency of /// the unresolved list. Derived classes should call this instead of calling /// `update_if_needed()` on their inner accessor. UpdateStatus update_if_needed() const diff --git a/src/realm/collection_parent.hpp b/src/realm/collection_parent.hpp index d7a6f9d1e06..29ffbe3a70b 100644 --- a/src/realm/collection_parent.hpp +++ b/src/realm/collection_parent.hpp @@ -74,7 +74,7 @@ class CollectionParent : public std::enable_shared_from_this { using Index = StableIndex; // Return the nesting level of the parent - size_t get_level() const noexcept + uint8_t get_level() const noexcept { return m_level; } @@ -106,10 +106,9 @@ class CollectionParent : public std::enable_shared_from_this { #else static constexpr size_t s_max_level = 100; #endif - size_t m_level = 0; - mutable size_t m_parent_version = 0; + uint8_t m_level = 0; - constexpr CollectionParent(size_t level = 0) + constexpr CollectionParent(uint8_t level = 0) : m_level(level) { } @@ -117,10 +116,7 @@ class CollectionParent : public std::enable_shared_from_this { virtual ~CollectionParent(); /// Update the accessor (and return `UpdateStatus::Detached` if the // collection is not initialized. - virtual UpdateStatus update_if_needed_with_status() const = 0; - /// Check if the storage version has changed and update if it has - /// Return true if the object was updated - virtual bool update_if_needed() const = 0; + virtual UpdateStatus update_if_needed() const = 0; /// Get owning object virtual const Obj& get_object() const noexcept = 0; /// Get the top ref from pareht @@ -133,6 +129,8 @@ class CollectionParent : public std::enable_shared_from_this { /// Set the top ref in parent virtual void set_collection_ref(Index, ref_type ref, CollectionType) = 0; + virtual uint32_t parent_version() const noexcept = 0; + // Used when inserting a new link. You will not remove existing links in this process void set_backlink(ColKey col_key, ObjLink new_link) const; // Used when replacing a link, return true if CascadeState contains objects to remove diff --git a/src/realm/dictionary.cpp b/src/realm/dictionary.cpp index 854d51cfc6f..4da6fb45ebb 100644 --- a/src/realm/dictionary.cpp +++ b/src/realm/dictionary.cpp @@ -651,10 +651,9 @@ size_t Dictionary::find_index(const Index& index) const return m_values->find_key(index.get_salt()); } -UpdateStatus Dictionary::update_if_needed_with_status() const +UpdateStatus Dictionary::do_update_if_needed(bool allow_create) const { - auto status = Base::get_update_status(); - switch (status) { + switch (get_update_status()) { case UpdateStatus::Detached: { m_dictionary_top.reset(); return UpdateStatus::Detached; @@ -670,24 +669,25 @@ UpdateStatus Dictionary::update_if_needed_with_status() const case UpdateStatus::Updated: { // Try to initialize. If the dictionary is not initialized // the function will return false; - bool attached = init_from_parent(false); - Base::update_content_version(); - CollectionParent::m_parent_version++; + bool attached = init_from_parent(allow_create); return attached ? UpdateStatus::Updated : UpdateStatus::Detached; } } REALM_UNREACHABLE(); } +UpdateStatus Dictionary::update_if_needed() const +{ + constexpr bool allow_create = false; + return do_update_if_needed(allow_create); +} + void Dictionary::ensure_created() { - if (Base::should_update() || !(m_dictionary_top && m_dictionary_top->is_attached())) { - // When allow_create is true, init_from_parent will always succeed - // In case of errors, an exception is thrown. - constexpr bool allow_create = true; - init_from_parent(allow_create); // Throws - CollectionParent::m_parent_version++; - Base::update_content_version(); + constexpr bool allow_create = true; + if (do_update_if_needed(allow_create) == UpdateStatus::Detached) { + // FIXME: untested + throw StaleAccessor("Dictionary no longer exists"); } } @@ -839,6 +839,7 @@ void Dictionary::clear() bool Dictionary::init_from_parent(bool allow_create) const { + Base::update_content_version(); try { auto ref = Base::get_collection_ref(); if ((ref || allow_create) && !m_dictionary_top) { @@ -1179,7 +1180,6 @@ ref_type Dictionary::get_collection_ref(Index index, CollectionType type) const throw realm::IllegalOperation(util::format("Not a %1", type)); } throw StaleAccessor("This collection is no more"); - return 0; } bool Dictionary::check_collection_ref(Index index, CollectionType type) const noexcept @@ -1200,15 +1200,6 @@ void Dictionary::set_collection_ref(Index index, ref_type ref, CollectionType ty m_values->set(ndx, Mixed(ref, type)); } -bool Dictionary::update_if_needed() const -{ - auto status = update_if_needed_with_status(); - if (status == UpdateStatus::Detached) { - throw StaleAccessor("CollectionList no longer exists"); - } - return status == UpdateStatus::Updated; -} - /************************* DictionaryLinkValues *************************/ DictionaryLinkValues::DictionaryLinkValues(const Obj& obj, ColKey col_key) diff --git a/src/realm/dictionary.hpp b/src/realm/dictionary.hpp index 3cf08e9b91c..2c1c876890e 100644 --- a/src/realm/dictionary.hpp +++ b/src/realm/dictionary.hpp @@ -39,10 +39,7 @@ class Dictionary final : public CollectionBaseImpl, public Colle using Base = CollectionBaseImpl; class Iterator; - Dictionary() - : CollectionParent(0) - { - } + Dictionary() = default; ~Dictionary(); Dictionary(const Obj& obj, ColKey col_key) @@ -213,12 +210,15 @@ class Dictionary final : public CollectionBaseImpl, public Colle { return get_obj().get_table(); } - UpdateStatus update_if_needed_with_status() const override; - bool update_if_needed() const override; + UpdateStatus update_if_needed() const override; const Obj& get_object() const noexcept override { return get_obj(); } + uint32_t parent_version() const noexcept override + { + return m_parent_version; + } ref_type get_collection_ref(Index, CollectionType) const override; bool check_collection_ref(Index, CollectionType) const noexcept override; void set_collection_ref(Index, ref_type ref, CollectionType) override; @@ -261,12 +261,14 @@ class Dictionary final : public CollectionBaseImpl, public Colle void do_accumulate(size_t* return_ndx, AggregateType& agg) const; void ensure_created(); - inline bool update() const + bool update() const { - return update_if_needed_with_status() != UpdateStatus::Detached; + return update_if_needed() != UpdateStatus::Detached; } void verify() const; void get_key_type(); + + UpdateStatus do_update_if_needed(bool allow_create) const; }; class Dictionary::Iterator { @@ -459,7 +461,7 @@ class DictionaryLinkValues final : public ObjCollectionBase { // Overrides of ObjCollectionBase: UpdateStatus do_update_if_needed() const final { - return m_source.update_if_needed_with_status(); + return m_source.update_if_needed(); } BPlusTree* get_mutable_tree() const final { diff --git a/src/realm/list.cpp b/src/realm/list.cpp index 2bd3ffff183..d51adacd942 100644 --- a/src/realm/list.cpp +++ b/src/realm/list.cpp @@ -347,45 +347,30 @@ void Lst::do_remove(size_t ndx) /******************************** Lst *********************************/ -bool Lst::init_from_parent(bool allow_create) const +UpdateStatus Lst::init_from_parent(bool allow_create) const { + Base::update_content_version(); + if (!m_tree) { m_tree.reset(new BPlusTreeMixed(get_alloc())); const ArrayParent* parent = this; m_tree->set_parent(const_cast(parent), 0); } try { - auto ref = Base::get_collection_ref(); - if (ref) { - m_tree->init_from_ref(ref); - } - else { - if (!allow_create) { - m_tree->detach(); - return false; - } - - // The ref in the column was NULL, create the tree in place. - m_tree->create(); - REALM_ASSERT(m_tree->is_attached()); - } + return do_init_from_parent(m_tree.get(), Base::get_collection_ref(), allow_create); } catch (...) { m_tree->detach(); throw; } - - return true; } -UpdateStatus Lst::update_if_needed_with_status() const +UpdateStatus Lst::update_if_needed() const { - auto status = Base::get_update_status(); - switch (status) { - case UpdateStatus::Detached: { + switch (get_update_status()) { + case UpdateStatus::Detached: m_tree.reset(); return UpdateStatus::Detached; - } case UpdateStatus::NoChange: if (m_tree && m_tree->is_attached()) { return UpdateStatus::NoChange; @@ -393,12 +378,8 @@ UpdateStatus Lst::update_if_needed_with_status() const // The tree has not been initialized yet for this accessor, so // perform lazy initialization by treating it as an update. [[fallthrough]]; - case UpdateStatus::Updated: { - bool attached = init_from_parent(false); - Base::update_content_version(); - CollectionParent::m_parent_version++; - return attached ? UpdateStatus::Updated : UpdateStatus::Detached; - } + case UpdateStatus::Updated: + return init_from_parent(false); } REALM_UNREACHABLE(); } @@ -898,15 +879,6 @@ bool Lst::remove_backlinks(CascadeState& state) const return recurse; } -bool Lst::update_if_needed() const -{ - auto status = update_if_needed_with_status(); - if (status == UpdateStatus::Detached) { - throw StaleAccessor("CollectionList no longer exists"); - } - return status == UpdateStatus::Updated; -} - /********************************** LnkLst ***********************************/ Obj LnkLst::create_and_insert_linked_object(size_t ndx) diff --git a/src/realm/list.hpp b/src/realm/list.hpp index d3bca568116..945b3d53fd5 100644 --- a/src/realm/list.hpp +++ b/src/realm/list.hpp @@ -183,7 +183,7 @@ class Lst final : public CollectionBaseImpl { return *m_tree; } - UpdateStatus update_if_needed_with_status() const final + UpdateStatus update_if_needed() const final { auto status = Base::get_update_status(); switch (status) { @@ -199,9 +199,7 @@ class Lst final : public CollectionBaseImpl { // perform lazy initialization by treating it as an update. [[fallthrough]]; case UpdateStatus::Updated: { - bool attached = init_from_parent(false); - Base::update_content_version(); - return attached ? UpdateStatus::Updated : UpdateStatus::Detached; + return init_from_parent(false); } } REALM_UNREACHABLE(); @@ -214,14 +212,13 @@ class Lst final : public CollectionBaseImpl { // In case of errors, an exception is thrown. constexpr bool allow_create = true; init_from_parent(allow_create); // Throws - Base::update_content_version(); } } /// Update the accessor and return true if it is attached after the update. inline bool update() const { - return update_if_needed_with_status() != UpdateStatus::Detached; + return update_if_needed() != UpdateStatus::Detached; } size_t translate_index(size_t ndx) const noexcept override @@ -255,28 +252,15 @@ class Lst final : public CollectionBaseImpl { using Base::m_col_key; using Base::m_nullable; - bool init_from_parent(bool allow_create) const + UpdateStatus init_from_parent(bool allow_create) const { if (!m_tree) { m_tree.reset(new BPlusTree(get_alloc())); const ArrayParent* parent = this; m_tree->set_parent(const_cast(parent), 0); } - - if (m_tree->init_from_parent()) { - // All is well - return true; - } - - if (!allow_create) { - m_tree->detach(); - return false; - } - - // The ref in the column was NULL, create the tree in place. - m_tree->create(); - REALM_ASSERT(m_tree->is_attached()); - return true; + Base::update_content_version(); + return do_init_from_parent(m_tree.get(), 0, allow_create); } template @@ -448,7 +432,7 @@ class Lst final : public CollectionBaseImpl, public CollectionPa return *m_tree; } - UpdateStatus update_if_needed_with_status() const final; + UpdateStatus update_if_needed() const final; void ensure_created() { @@ -457,15 +441,13 @@ class Lst final : public CollectionBaseImpl, public CollectionPa // In case of errors, an exception is thrown. constexpr bool allow_create = true; init_from_parent(allow_create); // Throws - Base::update_content_version(); - CollectionParent::m_parent_version++; } } /// Update the accessor and return true if it is attached after the update. inline bool update() const { - return update_if_needed_with_status() != UpdateStatus::Detached; + return update_if_needed() != UpdateStatus::Detached; } // Overriding members in CollectionParent @@ -500,11 +482,14 @@ class Lst final : public CollectionBaseImpl, public CollectionPa { return get_obj().get_table(); } - bool update_if_needed() const override; const Obj& get_object() const noexcept override { return get_obj(); } + uint32_t parent_version() const noexcept override + { + return m_parent_version; + } ref_type get_collection_ref(Index, CollectionType) const override; bool check_collection_ref(Index, CollectionType) const noexcept override; void set_collection_ref(Index, ref_type ref, CollectionType) override; @@ -527,7 +512,7 @@ class Lst final : public CollectionBaseImpl, public CollectionPa using Base::m_col_key; using Base::m_nullable; - bool init_from_parent(bool allow_create) const; + UpdateStatus init_from_parent(bool allow_create) const; template void find_all_mixed_unresolved_links(Func&& func) const @@ -800,7 +785,7 @@ class LnkLst final : public ObjCollectionBase { UpdateStatus do_update_if_needed() const final { - return m_list.update_if_needed_with_status(); + return m_list.update_if_needed(); } BPlusTree* get_mutable_tree() const final diff --git a/src/realm/obj.cpp b/src/realm/obj.cpp index 4d482988076..9b267b56f6c 100644 --- a/src/realm/obj.cpp +++ b/src/realm/obj.cpp @@ -385,7 +385,7 @@ bool Obj::update() const if (changes) { m_mem = new_obj.m_mem; m_row_ndx = new_obj.m_row_ndx; - CollectionParent::m_parent_version++; + ++m_version_counter; } // Always update versions m_storage_version = new_obj.m_storage_version; @@ -402,14 +402,14 @@ inline bool Obj::_update_if_needed() const return false; } -UpdateStatus Obj::update_if_needed_with_status() const +UpdateStatus Obj::update_if_needed() const { if (!m_table) { // Table deleted return UpdateStatus::Detached; } - auto current_version = get_alloc().get_storage_version(); + auto current_version = _get_alloc().get_storage_version(); if (current_version != m_storage_version) { ClusterNode::State state = get_tree_top()->try_get(m_key); @@ -423,13 +423,21 @@ UpdateStatus Obj::update_if_needed_with_status() const if ((m_mem.get_addr() != state.mem.get_addr()) || (m_row_ndx != state.index)) { m_mem = state.mem; m_row_ndx = state.index; - CollectionParent::m_parent_version++; + ++m_version_counter; return UpdateStatus::Updated; } } return UpdateStatus::NoChange; } +void Obj::checked_update_if_needed() const +{ + if (update_if_needed() == UpdateStatus::Detached) { + m_table.check(); + get_tree_top()->get(m_key); // should always throw + } +} + template T Obj::get(ColKey col_key) const { @@ -704,7 +712,7 @@ Obj Obj::_get_linked_object(ColKey link_col_key, Mixed link) const Obj Obj::get_parent_object() const { Obj obj; - update_if_needed(); + checked_update_if_needed(); if (!m_table->is_embedded()) { throw LogicError(ErrorCodes::TopLevelObject, "Object is not embedded"); @@ -747,7 +755,7 @@ size_t Obj::get_link_count(ColKey col_key) const bool Obj::is_null(ColKey col_key) const { - update_if_needed(); + checked_update_if_needed(); ColumnAttrMask attr = col_key.get_attrs(); ColKey::Idx col_ndx = col_key.get_index(); if (attr.test(col_attr_Nullable) && !attr.test(col_attr_Collection)) { @@ -802,7 +810,7 @@ bool Obj::has_backlinks(bool only_strong_links) const size_t Obj::get_backlink_count() const { - update_if_needed(); + checked_update_if_needed(); size_t cnt = 0; m_table->for_each_backlink_column([&](ColKey backlink_col_key) { @@ -814,7 +822,7 @@ size_t Obj::get_backlink_count() const size_t Obj::get_backlink_count(const Table& origin, ColKey origin_col_key) const { - update_if_needed(); + checked_update_if_needed(); size_t cnt = 0; if (TableKey origin_table_key = origin.get_key()) { @@ -867,7 +875,7 @@ ObjKey Obj::get_backlink(ColKey backlink_col, size_t backlink_ndx) const std::vector Obj::get_all_backlinks(ColKey backlink_col) const { - update_if_needed(); + checked_update_if_needed(); get_table()->check_column(backlink_col); Allocator& alloc = get_alloc(); @@ -1151,7 +1159,7 @@ REALM_FORCEINLINE void Obj::sync(Node& arr) template <> Obj& Obj::set(ColKey col_key, Mixed value, bool is_default) { - update_if_needed(); + checked_update_if_needed(); get_table()->check_column(col_key); auto type = col_key.get_type(); auto col_ndx = col_key.get_index(); @@ -1284,7 +1292,7 @@ Obj& Obj::set_any(ColKey col_key, Mixed value, bool is_default) template <> Obj& Obj::set(ColKey col_key, int64_t value, bool is_default) { - update_if_needed(); + checked_update_if_needed(); get_table()->check_column(col_key); auto col_ndx = col_key.get_index(); @@ -1328,7 +1336,7 @@ Obj& Obj::set(ColKey col_key, int64_t value, bool is_default) Obj& Obj::add_int(ColKey col_key, int64_t value) { - update_if_needed(); + checked_update_if_needed(); get_table()->check_column(col_key); auto col_ndx = col_key.get_index(); @@ -1406,7 +1414,7 @@ Obj& Obj::add_int(ColKey col_key, int64_t value) template <> Obj& Obj::set(ColKey col_key, ObjKey target_key, bool is_default) { - update_if_needed(); + checked_update_if_needed(); get_table()->check_column(col_key); ColKey::Idx col_ndx = col_key.get_index(); ColumnType type = col_key.get_type(); @@ -1460,7 +1468,7 @@ Obj& Obj::set(ColKey col_key, ObjKey target_key, bool is_default) template <> Obj& Obj::set(ColKey col_key, ObjLink target_link, bool is_default) { - update_if_needed(); + checked_update_if_needed(); get_table()->check_column(col_key); ColKey::Idx col_ndx = col_key.get_index(); ColumnType type = col_key.get_type(); @@ -1504,7 +1512,7 @@ Obj& Obj::set(ColKey col_key, ObjLink target_link, bool is_default) Obj Obj::create_and_set_linked_object(ColKey col_key, bool is_default) { - update_if_needed(); + checked_update_if_needed(); get_table()->check_column(col_key); ColKey::Idx col_ndx = col_key.get_index(); ColumnType type = col_key.get_type(); @@ -1601,7 +1609,7 @@ inline void Obj::set_spec(ArrayString& values, ColKey col_key) template <> Obj& Obj::set(ColKey col_key, Geospatial value, bool) { - update_if_needed(); + checked_update_if_needed(); get_table()->check_column(col_key); auto type = col_key.get_type(); @@ -1621,7 +1629,7 @@ Obj& Obj::set(ColKey col_key, Geospatial value, bool) template <> Obj& Obj::set(ColKey col_key, std::optional value, bool) { - update_if_needed(); + checked_update_if_needed(); auto table = get_table(); table->check_column(col_key); auto type = col_key.get_type(); @@ -1652,7 +1660,7 @@ Obj& Obj::set(ColKey col_key, std::optional value, bool) template Obj& Obj::set(ColKey col_key, T value, bool is_default) { - update_if_needed(); + checked_update_if_needed(); get_table()->check_column(col_key); auto type = col_key.get_type(); auto attrs = col_key.get_attrs(); @@ -1705,7 +1713,7 @@ INSTANTIATE_OBJ_SET(UUID); void Obj::set_int(ColKey::Idx col_ndx, int64_t value) { - update_if_needed(); + checked_update_if_needed(); Allocator& alloc = get_alloc(); alloc.bump_content_version(); @@ -1722,7 +1730,7 @@ void Obj::set_int(ColKey::Idx col_ndx, int64_t value) void Obj::set_ref(ColKey::Idx col_ndx, ref_type value, CollectionType type) { - update_if_needed(); + checked_update_if_needed(); Allocator& alloc = get_alloc(); alloc.bump_content_version(); @@ -1972,14 +1980,14 @@ SetBasePtr Obj::get_setbase_ptr(ColKey col_key) const Dictionary Obj::get_dictionary(ColKey col_key) const { REALM_ASSERT(col_key.is_dictionary() || col_key.get_type() == col_type_Mixed); - update_if_needed(); + checked_update_if_needed(); return Dictionary(Obj(*this), col_key); } Obj& Obj::set_collection(ColKey col_key, CollectionType type) { REALM_ASSERT(col_key.get_type() == col_type_Mixed); - update_if_needed(); + checked_update_if_needed(); Mixed new_val(0, type); if (type == CollectionType::Set) { @@ -2353,7 +2361,7 @@ Obj& Obj::set_null(ColKey col_key, bool is_default) m_table->get_column_name(col_key)); } - update_if_needed(); + checked_update_if_needed(); SearchIndex* index = m_table->get_search_index(col_key); if (index && !m_key.is_unresolved()) { diff --git a/src/realm/obj.hpp b/src/realm/obj.hpp index edf9db3aa60..cdb59f6f5ff 100644 --- a/src/realm/obj.hpp +++ b/src/realm/obj.hpp @@ -59,17 +59,11 @@ class DeepChangeChecker; // 'Object' would have been a better name, but it clashes with a class in ObjectStore class Obj : public CollectionParent { public: - constexpr Obj() - : m_table(nullptr) - , m_row_ndx(size_t(-1)) - , m_storage_version(-1) - , m_valid(false) - { - } + constexpr Obj() = default; Obj(TableRef table, MemRef mem, ObjKey key, size_t row_ndx); // Overriding members of CollectionParent: - UpdateStatus update_if_needed_with_status() const final; + UpdateStatus update_if_needed() const final; // Get the path in a minimal format without including object accessors. // If you need to obtain additional information for each object in the path, // you should use get_fat_path() or traverse_path() instead (see below). @@ -84,7 +78,6 @@ class Obj : public CollectionParent { return realm::npos; } - bool update_if_needed() const final; TableRef get_table() const noexcept final { return m_table.cast_away_const(); @@ -96,6 +89,10 @@ class Obj : public CollectionParent { ref_type get_collection_ref(Index, CollectionType) const final; bool check_collection_ref(Index, CollectionType) const noexcept final; void set_collection_ref(Index, ref_type, CollectionType) final; + uint32_t parent_version() const noexcept final + { + return m_version_counter; + } StableIndex build_index(ColKey) const; bool check_index(StableIndex) const; @@ -345,9 +342,10 @@ class Obj : public CollectionParent { mutable TableRef m_table; ObjKey m_key; mutable MemRef m_mem; - mutable size_t m_row_ndx; - mutable uint64_t m_storage_version; - mutable bool m_valid; + mutable size_t m_row_ndx = -1; + mutable uint64_t m_storage_version = -1; + mutable uint32_t m_version_counter = 0; + mutable bool m_valid = false; Allocator& _get_alloc() const noexcept; @@ -356,6 +354,7 @@ class Obj : public CollectionParent { /// reflect new changes to the underlying state. bool update() const; bool _update_if_needed() const; // no check, use only when already checked + void checked_update_if_needed() const; template bool do_is_null(ColKey::Idx col_ndx) const; @@ -589,15 +588,6 @@ inline Obj& Obj::set_all(Head v, Tail... tail) return _set_all(start_index, v, tail...); } -inline bool Obj::update_if_needed() const -{ - auto current_version = get_alloc().get_storage_version(); - if (current_version != m_storage_version) { - return update(); - } - return false; -} - inline int_fast64_t Obj::bump_content_version() { Allocator& alloc = get_alloc(); diff --git a/src/realm/set.cpp b/src/realm/set.cpp index 7ef6b07d7b1..74d4e51c0f6 100644 --- a/src/realm/set.cpp +++ b/src/realm/set.cpp @@ -273,32 +273,6 @@ void CollectionBaseImpl::to_json(std::ostream& out, JSONOutputMode outp } } -bool SetBase::do_init_from_parent(ref_type ref, bool allow_create) const -{ - try { - if (ref) { - m_tree->init_from_ref(ref); - } - else { - if (m_tree->init_from_parent()) { - // All is well - return true; - } - if (!allow_create) { - return false; - } - // The ref in the column was NULL, create the tree in place. - m_tree->create(); - REALM_ASSERT(m_tree->is_attached()); - } - } - catch (...) { - m_tree->detach(); - throw; - } - return true; -} - void SetBase::resort_range(size_t start, size_t end) { if (end > size()) { diff --git a/src/realm/set.hpp b/src/realm/set.hpp index d9d989ea1ed..2fe0a762f51 100644 --- a/src/realm/set.hpp +++ b/src/realm/set.hpp @@ -57,7 +57,6 @@ class SetBase : public CollectionBase { void erase_repl(Replication* repl, size_t index, Mixed value) const; void clear_repl(Replication* repl) const; static std::vector convert_to_mixed_set(const CollectionBase& rhs); - bool do_init_from_parent(ref_type ref, bool allow_create) const; void resort_range(size_t from, size_t to); @@ -195,7 +194,7 @@ class Set final : public CollectionBaseImpl { return tree(); } - UpdateStatus update_if_needed_with_status() const final; + UpdateStatus update_if_needed() const final; void ensure_created(); void migrate(); @@ -216,12 +215,12 @@ class Set final : public CollectionBaseImpl { return static_cast&>(*m_tree); } - bool init_from_parent(bool allow_create) const; + UpdateStatus init_from_parent(bool allow_create) const; /// Update the accessor and return true if it is attached after the update. inline bool update() const { - return update_if_needed_with_status() != UpdateStatus::Detached; + return update_if_needed() != UpdateStatus::Detached; } // `do_` methods here perform the action after preconditions have been @@ -377,7 +376,7 @@ class LnkSet final : public ObjCollectionBase { // Overriding members of ObjCollectionBase: UpdateStatus do_update_if_needed() const final { - return m_set.update_if_needed_with_status(); + return m_set.update_if_needed(); } BPlusTree* get_mutable_tree() const final @@ -494,10 +493,9 @@ inline Set& Set::operator=(Set&& other) noexcept } template -UpdateStatus Set::update_if_needed_with_status() const +UpdateStatus Set::update_if_needed() const { - auto status = Base::get_update_status(); - switch (status) { + switch (get_update_status()) { case UpdateStatus::Detached: { m_tree.reset(); return UpdateStatus::Detached; @@ -509,11 +507,8 @@ UpdateStatus Set::update_if_needed_with_status() const // The tree has not been initialized yet for this accessor, so // perform lazy initialization by treating it as an update. [[fallthrough]]; - case UpdateStatus::Updated: { - bool attached = init_from_parent(false); - Base::update_content_version(); - return attached ? UpdateStatus::Updated : UpdateStatus::Detached; - } + case UpdateStatus::Updated: + return init_from_parent(false); } REALM_UNREACHABLE(); } @@ -526,19 +521,19 @@ void Set::ensure_created() // In case of errors, an exception is thrown. constexpr bool allow_create = true; init_from_parent(allow_create); // Throws - Base::update_content_version(); } } template -bool Set::init_from_parent(bool allow_create) const +UpdateStatus Set::init_from_parent(bool allow_create) const { + Base::update_content_version(); if (!m_tree) { m_tree.reset(new BPlusTree(get_alloc())); const ArrayParent* parent = this; m_tree->set_parent(const_cast(parent), 0); } - return do_init_from_parent(Base::get_collection_ref(), allow_create); + return do_init_from_parent(m_tree.get(), Base::get_collection_ref(), allow_create); } template