diff --git a/CHANGELOG.md b/CHANGELOG.md index aec0a598992..16b9183f223 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,10 +2,13 @@ ### Enhancements * Add support to synchronize collections embedded in Mixed properties and other collections (except sets) ([PR #7353](https://github.com/realm/realm-core/pull/7353)). +* Improve performance of change notifications on nested collections somewhat ([PR #7402](https://github.com/realm/realm-core/pull/7402)). ### Fixed * Fixed conflict resolution bug which may result in an crash when the AddInteger instruction on Mixed properties is merged against updates to a non-integer type ([PR #7353](https://github.com/realm/realm-core/pull/7353)). * Fix a spurious crash related to opening a Realm on background thread while the process was in the middle of exiting ([#7420](https://github.com/realm/realm-core/issues/7420jj)) +* Fix a data race in change notification delivery when running at debug log level ([PR #7402](https://github.com/realm/realm-core/pull/7402), since v14.0.0). +* Fix a 10-15% performance regression when reading data from the Realm resulting from Obj being made a non-trivial type ([PR #7402](https://github.com/realm/realm-core/pull/7402), since v14.0.0). ### Breaking changes * Remove `realm_scheduler_set_default_factory()` and `realm_scheduler_has_default_factory()`, and change the `Scheduler` factory function to a bare function pointer rather than a `UniqueFunction` so that it does not have a non-trivial destructor. diff --git a/src/realm/collection.cpp b/src/realm/collection.cpp index f43e6baf589..99553ec53ee 100644 --- a/src/realm/collection.cpp +++ b/src/realm/collection.cpp @@ -237,4 +237,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 20de7e448a4..6dbcebc7eaf 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; @@ -255,6 +255,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) @@ -492,7 +493,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; @@ -524,7 +525,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; @@ -563,6 +564,11 @@ class CollectionBaseImpl : public Interface, protected ArrayParent { m_content_version = 0; } + CollectionParent* get_owner() const noexcept + { + return m_parent; + } + void to_json(std::ostream&, JSONOutputMode, util::FunctionRef) const override; using Interface::get_owner_key; @@ -570,17 +576,15 @@ class CollectionBaseImpl : public Interface, protected ArrayParent { using Interface::get_target_table; protected: - Obj m_obj_mem; + ObjCollectionParent 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) @@ -650,13 +654,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; } } @@ -667,18 +672,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() @@ -728,19 +729,19 @@ class CollectionBaseImpl : public Interface, protected ArrayParent { void set_backlink(ColKey col_key, ObjLink new_link) const { check_parent(); - m_parent->set_backlink(col_key, new_link); + m_parent->get_object().set_backlink(col_key, new_link); } // Used when replacing a link, return true if CascadeState contains objects to remove bool replace_backlink(ColKey col_key, ObjLink old_link, ObjLink new_link, CascadeState& state) const { check_parent(); - return m_parent->replace_backlink(col_key, old_link, new_link, state); + return m_parent->get_object().replace_backlink(col_key, old_link, new_link, state); } // Used when removing a backlink, return true if CascadeState contains objects to remove bool remove_backlink(ColKey col_key, ObjLink old_link, CascadeState& state) const { check_parent(); - return m_parent->remove_backlink(col_key, old_link, state); + return m_parent->get_object().remove_backlink(col_key, old_link, state); } /// Reset the accessor's tracking of the content version. Derived classes @@ -796,7 +797,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 { @@ -884,7 +885,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.cpp b/src/realm/collection_parent.cpp index b54ac869e25..dc3ad806bc3 100644 --- a/src/realm/collection_parent.cpp +++ b/src/realm/collection_parent.cpp @@ -66,14 +66,7 @@ bool StablePath::is_prefix_of(const StablePath& other) const noexcept { if (size() > other.size()) return false; - - auto it = other.begin(); - for (auto& p : *this) { - if (!(p == *it)) - return false; - ++it; - } - return true; + return std::equal(begin(), end(), other.begin()); } /***************************** CollectionParent ******************************/ @@ -86,212 +79,81 @@ void CollectionParent::check_level() const throw LogicError(ErrorCodes::LimitExceeded, "Max nesting level reached"); } } -void CollectionParent::set_backlink(ColKey col_key, ObjLink new_link) const -{ - if (new_link && new_link.get_obj_key()) { - auto t = get_table(); - auto target_table = t->get_parent_group()->get_table(new_link.get_table_key()); - ColKey backlink_col_key; - auto type = col_key.get_type(); - if (type == col_type_TypedLink || type == col_type_Mixed || col_key.is_dictionary()) { - // This may modify the target table - backlink_col_key = target_table->find_or_add_backlink_column(col_key, t->get_key()); - // it is possible that this was a link to the same table and that adding a backlink column has - // caused the need to update this object as well. - update_if_needed(); - } - else { - backlink_col_key = t->get_opposite_column(col_key); - } - auto obj_key = new_link.get_obj_key(); - auto target_obj = obj_key.is_unresolved() ? target_table->try_get_tombstone(obj_key) - : target_table->try_get_object(obj_key); - if (!target_obj) { - throw InvalidArgument(ErrorCodes::KeyNotFound, "Target object not found"); - } - target_obj.add_backlink(backlink_col_key, get_object().get_key()); - } -} - -bool CollectionParent::replace_backlink(ColKey col_key, ObjLink old_link, ObjLink new_link, CascadeState& state) const -{ - bool recurse = remove_backlink(col_key, old_link, state); - set_backlink(col_key, new_link); - - return recurse; -} - -bool CollectionParent::remove_backlink(ColKey col_key, ObjLink old_link, CascadeState& state) const -{ - if (old_link && old_link.get_obj_key()) { - auto t = get_table(); - REALM_ASSERT(t->valid_column(col_key)); - ObjKey old_key = old_link.get_obj_key(); - auto target_obj = t->get_parent_group()->get_object(old_link); - TableRef target_table = target_obj.get_table(); - ColKey backlink_col_key; - auto type = col_key.get_type(); - if (type == col_type_TypedLink || type == col_type_Mixed || col_key.is_dictionary()) { - backlink_col_key = target_table->find_or_add_backlink_column(col_key, t->get_key()); - } - else { - backlink_col_key = t->get_opposite_column(col_key); - } - - bool strong_links = target_table->is_embedded(); - bool is_unres = old_key.is_unresolved(); - - bool last_removed = target_obj.remove_one_backlink(backlink_col_key, get_object().get_key()); // Throws - if (is_unres) { - if (last_removed) { - // Check is there are more backlinks - if (!target_obj.has_backlinks(false)) { - // Tombstones can be erased right away - there is no cascading effect - target_table->m_tombstones->erase(old_key, state); - } - } - } - else { - return state.enqueue_for_cascade(target_obj, strong_links, last_removed); - } - } - - return false; -} -LstBasePtr CollectionParent::get_listbase_ptr(ColKey col_key) const +template typename Collection, typename LinkCol> +std::unique_ptr create_collection(ColKey col_key, size_t level) { - auto table = get_table(); - auto attr = table->get_column_attr(col_key); - REALM_ASSERT(attr.test(col_attr_List) || attr.test(col_attr_Nullable)); - bool nullable = attr.test(col_attr_Nullable); - - switch (table->get_column_type(col_key)) { - case type_Int: { + bool nullable = col_key.get_attrs().test(col_attr_Nullable); + switch (col_key.get_type()) { + case col_type_Int: if (nullable) - return std::make_unique>>(col_key); - else - return std::make_unique>(col_key); - } - case type_Bool: { + return std::make_unique>>(col_key); + return std::make_unique>(col_key); + case col_type_Bool: if (nullable) - return std::make_unique>>(col_key); - else - return std::make_unique>(col_key); - } - case type_Float: { + return std::make_unique>>(col_key); + return std::make_unique>(col_key); + case col_type_Float: if (nullable) - return std::make_unique>>(col_key); - else - return std::make_unique>(col_key); - } - case type_Double: { + return std::make_unique>>(col_key); + return std::make_unique>(col_key); + case col_type_Double: if (nullable) - return std::make_unique>>(col_key); - else - return std::make_unique>(col_key); - } - case type_String: { - return std::make_unique>(col_key); - } - case type_Binary: { - return std::make_unique>(col_key); - } - case type_Timestamp: { - return std::make_unique>(col_key); - } - case type_Decimal: { - return std::make_unique>(col_key); - } - case type_ObjectId: { + return std::make_unique>>(col_key); + return std::make_unique>(col_key); + case col_type_String: + return std::make_unique>(col_key); + case col_type_Binary: + return std::make_unique>(col_key); + case col_type_Timestamp: + return std::make_unique>(col_key); + case col_type_Decimal: + return std::make_unique>(col_key); + case col_type_ObjectId: if (nullable) - return std::make_unique>>(col_key); - else - return std::make_unique>(col_key); - } - case type_UUID: { + return std::make_unique>>(col_key); + return std::make_unique>(col_key); + case col_type_UUID: if (nullable) - return std::make_unique>>(col_key); - else - return std::make_unique>(col_key); - } - case type_TypedLink: { - return std::make_unique>(col_key); - } - case type_Mixed: { - return std::make_unique>(col_key, get_level() + 1); - } - case type_Link: - return std::make_unique(col_key); + return std::make_unique>>(col_key); + return std::make_unique>(col_key); + case col_type_TypedLink: + return std::make_unique>(col_key); + case col_type_Mixed: + return std::make_unique>(col_key, level + 1); + case col_type_Link: + return std::make_unique(col_key); + default: + REALM_TERMINATE("Unsupported column type."); } - REALM_TERMINATE("Unsupported column type"); } -SetBasePtr CollectionParent::get_setbase_ptr(ColKey col_key) const +LstBasePtr CollectionParent::get_listbase_ptr(ColKey col_key, size_t level) { - auto table = get_table(); - auto attr = table->get_column_attr(col_key); - REALM_ASSERT(attr.test(col_attr_Set)); - bool nullable = attr.test(col_attr_Nullable); + REALM_ASSERT(col_key.get_attrs().test(col_attr_List) || col_key.get_type() == col_type_Mixed); + return create_collection(col_key, level); +} - switch (table->get_column_type(col_key)) { - case type_Int: - if (nullable) - return std::make_unique>>(col_key); - return std::make_unique>(col_key); - case type_Bool: - if (nullable) - return std::make_unique>>(col_key); - return std::make_unique>(col_key); - case type_Float: - if (nullable) - return std::make_unique>>(col_key); - return std::make_unique>(col_key); - case type_Double: - if (nullable) - return std::make_unique>>(col_key); - return std::make_unique>(col_key); - case type_String: - return std::make_unique>(col_key); - case type_Binary: - return std::make_unique>(col_key); - case type_Timestamp: - return std::make_unique>(col_key); - case type_Decimal: - return std::make_unique>(col_key); - case type_ObjectId: - if (nullable) - return std::make_unique>>(col_key); - return std::make_unique>(col_key); - case type_UUID: - if (nullable) - return std::make_unique>>(col_key); - return std::make_unique>(col_key); - case type_TypedLink: - return std::make_unique>(col_key); - case type_Mixed: - return std::make_unique>(col_key); - case type_Link: - return std::make_unique(col_key); - } - REALM_TERMINATE("Unsupported column type."); +SetBasePtr CollectionParent::get_setbase_ptr(ColKey col_key, size_t level) +{ + REALM_ASSERT(col_key.get_attrs().test(col_attr_Set)); + return create_collection(col_key, level); } -CollectionBasePtr CollectionParent::get_collection_ptr(ColKey col_key) const +CollectionBasePtr CollectionParent::get_collection_ptr(ColKey col_key, size_t level) { if (col_key.is_list()) { - return get_listbase_ptr(col_key); + return get_listbase_ptr(col_key, level); } else if (col_key.is_set()) { - return get_setbase_ptr(col_key); + return get_setbase_ptr(col_key, level); } else if (col_key.is_dictionary()) { - return std::make_unique(col_key, get_level() + 1); + return std::make_unique(col_key, level + 1); } return {}; } - int64_t CollectionParent::generate_key(size_t sz) { static std::mt19937 gen32; @@ -314,5 +176,13 @@ int64_t CollectionParent::generate_key(size_t sz) return key; } +void CollectionParent::set_key(BPlusTreeMixed& tree, size_t index) +{ + int64_t key = generate_key(tree.size()); + while (tree.find_key(key) != realm::not_found) { + key++; + } + tree.set_key(index, key); +} } // namespace realm diff --git a/src/realm/collection_parent.hpp b/src/realm/collection_parent.hpp index d7a6f9d1e06..53fe3ecb711 100644 --- a/src/realm/collection_parent.hpp +++ b/src/realm/collection_parent.hpp @@ -20,15 +20,16 @@ #define REALM_COLLECTION_PARENT_HPP #include -#include -#include #include +#include +#include namespace realm { class Obj; class Replication; class CascadeState; +class BPlusTreeMixed; class Collection; class CollectionBase; @@ -74,7 +75,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; } @@ -95,6 +96,13 @@ class CollectionParent : public std::enable_shared_from_this { /// Get table of owning object virtual TableRef get_table() const noexcept = 0; + static LstBasePtr get_listbase_ptr(ColKey col_key, size_t level); + static SetBasePtr get_setbase_ptr(ColKey col_key, size_t level); + static CollectionBasePtr get_collection_ptr(ColKey col_key, size_t level); + + static int64_t generate_key(size_t sz); + static void set_key(BPlusTreeMixed& tree, size_t index); + protected: friend class Collection; template @@ -106,10 +114,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 +124,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,18 +137,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; - // 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 - bool replace_backlink(ColKey col_key, ObjLink old_link, ObjLink new_link, CascadeState& state) const; - // Used when removing a backlink, return true if CascadeState contains objects to remove - bool remove_backlink(ColKey col_key, ObjLink old_link, CascadeState& state) const; - - LstBasePtr get_listbase_ptr(ColKey col_key) const; - SetBasePtr get_setbase_ptr(ColKey col_key) const; - CollectionBasePtr get_collection_ptr(ColKey col_key) const; - - static int64_t generate_key(size_t sz); + /// Get the counter which is incremented whenever the root Obj is updated. + virtual uint32_t parent_version() const noexcept = 0; }; } // namespace realm diff --git a/src/realm/dictionary.cpp b/src/realm/dictionary.cpp index 7c0c04f60ac..dbc9da3aba9 100644 --- a/src/realm/dictionary.cpp +++ b/src/realm/dictionary.cpp @@ -441,11 +441,7 @@ void Dictionary::insert_collection(const PathElement& path_elem, CollectionType if (!old_val || *old_val != new_val) { m_values->ensure_keys(); auto [it, inserted] = insert(path_elem.get_key(), new_val); - int64_t key = generate_key(size()); - while (m_values->find_key(key) != realm::not_found) { - key++; - } - m_values->set_key(it.index(), key); + set_key(*m_values, it.index()); } } @@ -651,10 +647,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; @@ -667,27 +662,23 @@ UpdateStatus Dictionary::update_if_needed_with_status() const // perform lazy initialization by treating it as an update. [[fallthrough]]; } - 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++; - return attached ? UpdateStatus::Updated : UpdateStatus::Detached; - } + case UpdateStatus::Updated: + return init_from_parent(allow_create); } 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) { + throw StaleAccessor("Dictionary no longer exists"); } } @@ -836,8 +827,9 @@ void Dictionary::clear() } } -bool Dictionary::init_from_parent(bool allow_create) const +UpdateStatus 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) { @@ -870,7 +862,7 @@ bool Dictionary::init_from_parent(bool allow_create) const // dictionary detached if (!allow_create) { m_dictionary_top.reset(); - return false; + return UpdateStatus::Detached; } // Create dictionary @@ -880,7 +872,7 @@ bool Dictionary::init_from_parent(bool allow_create) const m_dictionary_top->update_parent(); } - return true; + return UpdateStatus::Updated; } catch (...) { m_dictionary_top.reset(); @@ -1178,7 +1170,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 @@ -1199,15 +1190,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 93fc35eb9e5..ee313c9453a 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; @@ -241,7 +241,7 @@ class Dictionary final : public CollectionBaseImpl, public Colle Dictionary(Allocator& alloc, ColKey col_key, ref_type ref); - bool init_from_parent(bool allow_create) const; + UpdateStatus init_from_parent(bool allow_create) const; Mixed do_get(size_t ndx) const; void do_erase(size_t ndx, Mixed key); Mixed do_get_key(size_t ndx) const; @@ -263,12 +263,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 { @@ -461,7 +463,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..5a5482b70dc 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(); } @@ -553,11 +534,7 @@ void Lst::insert_collection(const PathElement& path_elem, CollectionType check_level(); m_tree->ensure_keys(); insert(path_elem.get_ndx(), Mixed(0, dict_or_list)); - int64_t key = generate_key(size()); - while (m_tree->find_key(key) != realm::not_found) { - key++; - } - m_tree->set_key(path_elem.get_ndx(), key); + set_key(*m_tree, path_elem.get_ndx()); bump_content_version(); } @@ -579,11 +556,7 @@ void Lst::set_collection(const PathElement& path_elem, CollectionType dic set(ndx, new_val); int64_t key = m_tree->get_key(ndx); if (key == 0) { - key = generate_key(size()); - while (m_tree->find_key(key) != realm::not_found) { - key++; - } - m_tree->set_key(ndx, key); + set_key(*m_tree, path_elem.get_ndx()); } bump_content_version(); } @@ -898,15 +871,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..fec6505c030 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 @@ -644,7 +629,7 @@ class LnkLst final : public ObjCollectionBase { } void add(const Obj& obj) { - if (get_target_table()->get_key() != obj.get_table_key()) { + if (get_target_table() != obj.get_table()) { throw InvalidArgument("LnkLst::add: Wrong object type"); } add(obj.get_key()); @@ -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 d49bf5f7fe9..d0e1c3c0bd2 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(); @@ -1099,7 +1107,7 @@ StablePath Obj::get_stable_path() const noexcept return {}; } -void Obj::add_index(Path& path, const Index& index) const +void Obj::add_index(Path& path, const CollectionParent::Index& index) const { if (path.empty()) { path.emplace_back(get_table()->get_column_key(index)); @@ -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(); @@ -1957,14 +1965,14 @@ void Obj::handle_multiple_backlinks_during_schema_migration() LstBasePtr Obj::get_listbase_ptr(ColKey col_key) const { - auto list = CollectionParent::get_listbase_ptr(col_key); + auto list = CollectionParent::get_listbase_ptr(col_key, 0); list->set_owner(*this, col_key); return list; } SetBasePtr Obj::get_setbase_ptr(ColKey col_key) const { - auto set = CollectionParent::get_setbase_ptr(col_key); + auto set = CollectionParent::get_setbase_ptr(col_key, 0); set->set_owner(*this, col_key); return set; } @@ -1972,7 +1980,7 @@ 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); } @@ -1983,7 +1991,7 @@ Obj& Obj::set_collection(ColKey col_key, CollectionType type) (col_key.is_list() && type == CollectionType::List)) { return *this; } - update_if_needed(); + checked_update_if_needed(); Mixed new_val(0, type); if (type == CollectionType::Set) { @@ -2023,7 +2031,7 @@ Obj& Obj::set_collection(ColKey col_key, CollectionType type) values.init_from_parent(); values.set(m_row_ndx, new_val); - values.set_key(m_row_ndx, generate_key(0x10)); + values.set_key(m_row_ndx, CollectionParent::generate_key(0x10)); sync(fields); @@ -2131,7 +2139,7 @@ CollectionPtr Obj::get_collection_by_stable_path(const StablePath& path) const CollectionBasePtr Obj::get_collection_ptr(ColKey col_key) const { if (col_key.is_collection()) { - auto collection = CollectionParent::get_collection_ptr(col_key); + auto collection = CollectionParent::get_collection_ptr(col_key, 0); collection->set_owner(*this, col_key); return collection; } @@ -2357,7 +2365,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()) { @@ -2431,7 +2439,7 @@ ref_type Obj::Internal::get_ref(const Obj& obj, ColKey col_key) return to_ref(obj._get(col_key.get_index())); } -ref_type Obj::get_collection_ref(Index index, CollectionType type) const +ref_type Obj::get_collection_ref(StableIndex index, CollectionType type) const { if (index.is_collection()) { return to_ref(_get(index.get_index())); @@ -2446,7 +2454,7 @@ ref_type Obj::get_collection_ref(Index index, CollectionType type) const throw StaleAccessor("This collection is no more"); } -bool Obj::check_collection_ref(Index index, CollectionType type) const noexcept +bool Obj::check_collection_ref(StableIndex index, CollectionType type) const noexcept { if (index.is_collection()) { return true; @@ -2457,7 +2465,7 @@ bool Obj::check_collection_ref(Index index, CollectionType type) const noexcept return false; } -void Obj::set_collection_ref(Index index, ref_type ref, CollectionType type) +void Obj::set_collection_ref(StableIndex index, ref_type ref, CollectionType type) { if (index.is_collection()) { set_int(index.get_index(), from_ref(ref)); @@ -2466,4 +2474,78 @@ void Obj::set_collection_ref(Index index, ref_type ref, CollectionType type) set_ref(index.get_index(), ref, type); } +void Obj::set_backlink(ColKey col_key, ObjLink new_link) const +{ + if (!new_link) { + return; + } + + auto target_table = m_table->get_parent_group()->get_table(new_link.get_table_key()); + ColKey backlink_col_key; + auto type = col_key.get_type(); + if (type == col_type_TypedLink || type == col_type_Mixed || col_key.is_dictionary()) { + // This may modify the target table + backlink_col_key = target_table->find_or_add_backlink_column(col_key, m_table->get_key()); + // it is possible that this was a link to the same table and that adding a backlink column has + // caused the need to update this object as well. + update_if_needed(); + } + else { + backlink_col_key = m_table->get_opposite_column(col_key); + } + auto obj_key = new_link.get_obj_key(); + auto target_obj = + obj_key.is_unresolved() ? target_table->try_get_tombstone(obj_key) : target_table->try_get_object(obj_key); + if (!target_obj) { + throw InvalidArgument(ErrorCodes::KeyNotFound, "Target object not found"); + } + target_obj.add_backlink(backlink_col_key, m_key); +} + +bool Obj::replace_backlink(ColKey col_key, ObjLink old_link, ObjLink new_link, CascadeState& state) const +{ + bool recurse = remove_backlink(col_key, old_link, state); + set_backlink(col_key, new_link); + return recurse; +} + +bool Obj::remove_backlink(ColKey col_key, ObjLink old_link, CascadeState& state) const +{ + if (!old_link) { + return false; + } + + REALM_ASSERT(m_table->valid_column(col_key)); + ObjKey old_key = old_link.get_obj_key(); + auto target_obj = m_table->get_parent_group()->get_object(old_link); + TableRef target_table = target_obj.get_table(); + ColKey backlink_col_key; + auto type = col_key.get_type(); + if (type == col_type_TypedLink || type == col_type_Mixed || col_key.is_dictionary()) { + backlink_col_key = target_table->find_or_add_backlink_column(col_key, m_table->get_key()); + } + else { + backlink_col_key = m_table->get_opposite_column(col_key); + } + + bool strong_links = target_table->is_embedded(); + bool is_unres = old_key.is_unresolved(); + + bool last_removed = target_obj.remove_one_backlink(backlink_col_key, m_key); // Throws + if (is_unres) { + if (last_removed) { + // Check is there are more backlinks + if (!target_obj.has_backlinks(false)) { + // Tombstones can be erased right away - there is no cascading effect + target_table->m_tombstones->erase(old_key, state); + } + } + } + else { + return state.enqueue_for_cascade(target_obj, strong_links, last_removed); + } + + return false; +} + } // namespace realm diff --git a/src/realm/obj.hpp b/src/realm/obj.hpp index edf9db3aa60..c7c25399500 100644 --- a/src/realm/obj.hpp +++ b/src/realm/obj.hpp @@ -57,45 +57,30 @@ class DeepChangeChecker; } // 'Object' would have been a better name, but it clashes with a class in ObjectStore -class Obj : public CollectionParent { +class Obj { 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; + // CollectionParent implementation + UpdateStatus update_if_needed() const; // 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). - FullPath get_path() const final; + FullPath get_path() const; std::string get_id() const; - Path get_short_path() const noexcept final; - ColKey get_col_key() const noexcept final; - StablePath get_stable_path() const noexcept final; - void add_index(Path& path, const Index& ndx) const final; - size_t find_index(const Index&) const final - { - return realm::npos; - } + Path get_short_path() const noexcept; + ColKey get_col_key() const noexcept; + StablePath get_stable_path() const noexcept; + void add_index(Path& path, const CollectionParent::Index& ndx) const; - bool update_if_needed() const final; - TableRef get_table() const noexcept final + TableRef get_table() const noexcept { return m_table.cast_away_const(); } - const Obj& get_object() const noexcept final - { - return *this; - } - 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; + ref_type get_collection_ref(CollectionParent::Index, CollectionType) const; + bool check_collection_ref(CollectionParent::Index, CollectionType) const noexcept; + void set_collection_ref(CollectionParent::Index, ref_type, CollectionType); StableIndex build_index(ColKey) const; bool check_index(StableIndex) const; @@ -325,29 +310,26 @@ class Obj : public CollectionParent { friend class ArrayBacklink; friend class CascadeState; friend class Cluster; + friend class CollectionParent; friend class ColumnListBase; - friend class CollectionBase; + friend class LinkCount; + friend class LinkMap; + friend class Lst; + friend class ObjCollectionParent; + friend class Table; friend class TableView; template friend class CollectionBaseImpl; template - friend class Lst; - friend class LnkLst; - friend class LinkCount; - friend class Dictionary; - friend class LinkMap; - template friend class Set; - friend class Table; - friend class Transaction; - friend class 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 +338,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; @@ -418,6 +401,82 @@ class Obj : public CollectionParent { bool compare_values(Mixed, Mixed, ColKey, Obj, StringData) const; bool compare_list_in_mixed(Lst&, Lst&, ColKey, Obj, StringData) const; bool compare_dict_in_mixed(Dictionary&, Dictionary&, ColKey, Obj, StringData) const; + + // 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 + bool replace_backlink(ColKey col_key, ObjLink old_link, ObjLink new_link, CascadeState& state) const; + // Used when removing a backlink, return true if CascadeState contains objects to remove + bool remove_backlink(ColKey col_key, ObjLink old_link, CascadeState& state) const; +}; +static_assert(std::is_trivially_destructible_v); + +class ObjCollectionParent final : public Obj, public CollectionParent { +public: + ObjCollectionParent() = default; + ObjCollectionParent(const Obj& obj) noexcept + : Obj(obj) + { + } + ObjCollectionParent& operator=(const Obj& obj) noexcept + { + static_cast(*this) = obj; + return *this; + } + +private: + FullPath get_path() const override + { + return Obj::get_path(); + } + Path get_short_path() const override + { + return Obj::get_short_path(); + } + ColKey get_col_key() const noexcept override + { + return Obj::get_col_key(); + } + StablePath get_stable_path() const override + { + return Obj::get_stable_path(); + } + void add_index(Path& path, const Index& ndx) const override + { + Obj::add_index(path, ndx); + } + size_t find_index(const Index&) const override + { + return realm::npos; + } + TableRef get_table() const noexcept override + { + return Obj::get_table(); + } + UpdateStatus update_if_needed() const override + { + return Obj::update_if_needed(); + } + const Obj& get_object() const noexcept override + { + return *this; + } + uint32_t parent_version() const noexcept override + { + return m_version_counter; + } + ref_type get_collection_ref(Index index, CollectionType type) const override + { + return Obj::get_collection_ref(index, type); + } + bool check_collection_ref(Index index, CollectionType type) const noexcept override + { + return Obj::check_collection_ref(index, type); + } + void set_collection_ref(Index index, ref_type ref, CollectionType type) override + { + Obj::set_collection_ref(index, ref, type); + } }; std::ostream& operator<<(std::ostream&, const Obj& obj); @@ -589,15 +648,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/object-store/collection_notifications.hpp b/src/realm/object-store/collection_notifications.hpp index 88648271e52..d484e73cd02 100644 --- a/src/realm/object-store/collection_notifications.hpp +++ b/src/realm/object-store/collection_notifications.hpp @@ -106,7 +106,7 @@ struct CollectionChangeSet { // Per-column version of `modifications` std::unordered_map columns; - std::set paths; + std::set paths; bool empty() const noexcept { diff --git a/src/realm/object-store/impl/collection_notifier.cpp b/src/realm/object-store/impl/collection_notifier.cpp index a4c9c21ae31..2dbf9c8d41e 100644 --- a/src/realm/object-store/impl/collection_notifier.cpp +++ b/src/realm/object-store/impl/collection_notifier.cpp @@ -307,8 +307,9 @@ void CollectionNotifier::prepare_handover() REALM_ASSERT(m_change.empty()); m_has_run = true; -#ifdef REALM_DEBUG util::CheckedLockGuard lock(m_callback_mutex); + m_run_time_point = std::chrono::steady_clock::now(); +#ifdef REALM_DEBUG for (auto& callback : m_callbacks) REALM_ASSERT(!callback.skip_next); #endif @@ -330,10 +331,59 @@ void CollectionNotifier::before_advance() }); } +static void log_changeset(util::Logger* logger, const CollectionChangeSet& changes, std::string_view description, + std::chrono::microseconds elapsed) +{ + if (!logger) { + return; + } + + logger->log(util::LogCategory::notification, util::Logger::Level::debug, + "Delivering notifications for %1 after %2 us", description, elapsed.count()); + if (!logger->would_log(util::Logger::Level::trace)) { + return; + } + if (changes.empty()) { + logger->log(util::LogCategory::notification, util::Logger::Level::trace, " No changes"); + } + else { + if (changes.collection_root_was_deleted) { + logger->log(util::LogCategory::notification, util::Logger::Level::trace, " collection deleted"); + } + else if (changes.collection_was_cleared) { + logger->log(util::LogCategory::notification, util::Logger::Level::trace, " collection cleared"); + } + else { + auto log = [logger](const char* change, const IndexSet& index_set) { + if (auto cnt = index_set.count()) { + std::ostringstream ostr; + bool first = true; + for (auto [a, b] : index_set) { + if (!first) + ostr << ','; + if (b > a + 1) { + ostr << '[' << a << ',' << b - 1 << ']'; + } + else { + ostr << a; + } + first = false; + } + logger->log(util::LogCategory::notification, util::Logger::Level::trace, " %1 %2: %3", cnt, + change, ostr.str().c_str()); + } + }; + log("deletions", changes.deletions); + log("insertions", changes.insertions); + log("modifications", changes.modifications); + } + } +} + void CollectionNotifier::after_advance() { using namespace std::chrono; - auto t1 = steady_clock::now(); + auto now = steady_clock::now(); for_each_callback([&](auto& lock, auto& callback) { if (callback.initial_delivered && callback.changes_to_deliver.empty()) { @@ -346,51 +396,9 @@ void CollectionNotifier::after_advance() // acquire a local reference to the callback so that removing the // callback from within it can't result in a dangling pointer auto cb = callback.fn; + auto elapsed = duration_cast(now - m_run_time_point); lock.unlock_unchecked(); - if (m_logger) { - m_logger->log(util::LogCategory::notification, util::Logger::Level::debug, - "Delivering notifications for %1 after %2 us", m_description, - duration_cast(t1 - m_run_time_point).count()); - if (m_logger->would_log(util::Logger::Level::trace)) { - if (changes.empty()) { - m_logger->log(util::LogCategory::notification, util::Logger::Level::trace, " No changes"); - } - else { - if (changes.collection_root_was_deleted) { - m_logger->log(util::LogCategory::notification, util::Logger::Level::trace, - " collection deleted"); - } - else if (changes.collection_was_cleared) { - m_logger->log(util::LogCategory::notification, util::Logger::Level::trace, - " collection cleared"); - } - else { - auto log = [this](const char* change, const IndexSet& index_set) { - if (auto cnt = index_set.count()) { - std::ostringstream ostr; - bool first = true; - for (auto [a, b] : index_set) { - if (!first) - ostr << ','; - if (b > a + 1) { - ostr << '[' << a << ',' << b - 1 << ']'; - } - else { - ostr << a; - } - first = false; - } - m_logger->log(util::LogCategory::notification, util::Logger::Level::trace, - " %1 %2: %3", cnt, change, ostr.str().c_str()); - } - }; - log("deletions", changes.deletions); - log("insertions", changes.insertions); - log("modifications", changes.modifications); - } - } - } - } + log_changeset(m_logger.get(), changes, m_description, elapsed); cb.after(changes); }); } @@ -529,3 +537,24 @@ void NotifierPackage::after_advance() for (auto& notifier : m_notifiers) notifier->after_advance(); } + +NotifierRunLogger::NotifierRunLogger(util::Logger* logger, std::string_view name, std::string_view description) + : m_logger(logger) + , m_name(name) + , m_description(description) +{ + if (logger && logger->would_log(util::Logger::Level::debug)) { + m_logger = logger; + m_start = std::chrono::steady_clock::now(); + } +} + +NotifierRunLogger::~NotifierRunLogger() +{ + using namespace std::chrono; + if (m_logger) { + auto now = steady_clock::now(); + m_logger->log(util::LogCategory::notification, util::Logger::Level::debug, "%1 %2 ran in %3 us", m_name, + m_description, duration_cast(now - m_start).count()); + } +} diff --git a/src/realm/object-store/impl/collection_notifier.hpp b/src/realm/object-store/impl/collection_notifier.hpp index 88e8b6f9cf3..c333203c645 100644 --- a/src/realm/object-store/impl/collection_notifier.hpp +++ b/src/realm/object-store/impl/collection_notifier.hpp @@ -381,6 +381,18 @@ class NotifierPackage { RealmCoordinator* m_coordinator = nullptr; }; +class NotifierRunLogger { +public: + NotifierRunLogger(util::Logger* logger, std::string_view name, std::string_view description); + ~NotifierRunLogger(); + +private: + util::Logger* m_logger; + std::string_view m_name; + std::string_view m_description; + std::chrono::steady_clock::time_point m_start; +}; + } // namespace realm::_impl #endif /* REALM_BACKGROUND_COLLECTION_HPP */ diff --git a/src/realm/object-store/impl/list_notifier.cpp b/src/realm/object-store/impl/list_notifier.cpp index 0de163fa7fa..ebcfb797d77 100644 --- a/src/realm/object-store/impl/list_notifier.cpp +++ b/src/realm/object-store/impl/list_notifier.cpp @@ -33,7 +33,7 @@ ListNotifier::ListNotifier(std::shared_ptr realm, CollectionBase const& l , m_prev_size(list.size()) { attach(list); - if (m_logger) { + if (m_logger && m_logger->would_log(util::Logger::Level::debug)) { auto path = m_list->get_short_path(); auto prop_name = m_list->get_table()->get_column_name(path[0].get_col_key()); path[0] = PathElement(prop_name); @@ -62,9 +62,11 @@ void ListNotifier::attach(CollectionBase const& src) if (auto obj = tr.get_table(src.get_table()->get_key())->try_get_object(src.get_owner_key())) { auto path = src.get_stable_path(); m_list = std::static_pointer_cast(obj.get_collection_by_stable_path(path)); + m_collection_parent = dynamic_cast(m_list.get()); } else { m_list = nullptr; + m_collection_parent = nullptr; } } @@ -73,14 +75,9 @@ bool ListNotifier::do_add_required_change_info(TransactionChangeInfo& info) if (!m_list || !m_list->is_attached()) return false; // origin row was deleted after the notification was added - // We need to have the collections with the shortest paths first StablePath this_path = m_list->get_stable_path(); - auto it = std::lower_bound(info.collections.begin(), info.collections.end(), this_path.size(), - [](const CollectionChangeInfo& info, size_t sz) { - return info.path.size() < sz; - }); - info.collections.insert( - it, {m_list->get_table()->get_key(), m_list->get_owner_key(), std::move(this_path), &m_change}); + info.collections.push_back( + {m_list->get_table()->get_key(), m_list->get_owner_key(), std::move(this_path), &m_change}); m_info = &info; @@ -97,16 +94,7 @@ bool ListNotifier::do_add_required_change_info(TransactionChangeInfo& info) void ListNotifier::run() { - using namespace std::chrono; - auto t1 = steady_clock::now(); - util::ScopeExit cleanup([&]() noexcept { - m_run_time_point = steady_clock::now(); - if (m_logger) { - m_logger->log(util::LogCategory::notification, util::Logger::Level::debug, - "ListNotifier %1 did run in %2 us", m_description, - duration_cast(m_run_time_point - t1).count()); - } - }); + NotifierRunLogger log(m_logger.get(), "ListNotifier", m_description); if (!m_list || !m_list->is_attached()) { // List was deleted, so report all of the rows being removed if this is @@ -142,14 +130,14 @@ void ListNotifier::run() } } + // Modifications to nested values in Mixed are recorded in replication as + // StableIndex and we have to look up the actual index afterwards if (m_change.paths.size()) { - if (auto coll = dynamic_cast(m_list.get())) { - for (auto& p : m_change.paths) { - // Report changes in substructure as modifications on this list - auto ndx = coll->find_index(p[0]); - if (ndx != realm::not_found) - m_change.modifications.add(ndx); // OK to insert same index again - } + REALM_ASSERT(m_collection_parent); + REALM_ASSERT(m_type == PropertyType::Mixed); + for (auto& p : m_change.paths) { + if (auto ndx = m_collection_parent->find_index(p); ndx != realm::not_found) + m_change.modifications.add(ndx); } } } diff --git a/src/realm/object-store/impl/list_notifier.hpp b/src/realm/object-store/impl/list_notifier.hpp index ec22e2c54fe..44f955cb7c1 100644 --- a/src/realm/object-store/impl/list_notifier.hpp +++ b/src/realm/object-store/impl/list_notifier.hpp @@ -34,6 +34,7 @@ class ListNotifier : public CollectionNotifier { private: PropertyType m_type; CollectionBasePtr m_list; + CollectionParent* m_collection_parent = nullptr; // The last-seen size of the collection so that when the parent of the collection // is deleted we can report each row as being deleted diff --git a/src/realm/object-store/impl/object_notifier.cpp b/src/realm/object-store/impl/object_notifier.cpp index 86feba916d0..676680017d8 100644 --- a/src/realm/object-store/impl/object_notifier.cpp +++ b/src/realm/object-store/impl/object_notifier.cpp @@ -64,16 +64,7 @@ void ObjectNotifier::run() { if (!m_table || !m_info) return; - using namespace std::chrono; - auto t1 = steady_clock::now(); - util::ScopeExit cleanup([&]() noexcept { - m_run_time_point = steady_clock::now(); - if (m_logger) { - m_logger->log(util::LogCategory::notification, util::Logger::Level::debug, - "ObjectNotifier %1 did run in %2 us", m_description, - duration_cast(m_run_time_point - t1).count()); - } - }); + NotifierRunLogger log(m_logger.get(), "ObjectNotifier", m_description); auto it = m_info->tables.find(m_table->get_key()); if (it != m_info->tables.end() && it->second.deletions_contains(m_obj_key)) { diff --git a/src/realm/object-store/impl/results_notifier.cpp b/src/realm/object-store/impl/results_notifier.cpp index 92ad8292f57..b60029e58cd 100644 --- a/src/realm/object-store/impl/results_notifier.cpp +++ b/src/realm/object-store/impl/results_notifier.cpp @@ -65,12 +65,12 @@ ResultsNotifier::ResultsNotifier(Results& target) , m_target_is_in_table_order(target.is_in_table_order()) { if (m_logger) { - m_description = std::string("'") + std::string(m_query->get_table()->get_class_name()) + std::string("'"); + m_description = "'" + std::string(m_query->get_table()->get_class_name()) + "'"; if (m_query->has_conditions()) { m_description += " where \""; m_description += m_query->get_description_safe() + "\""; } - m_logger->log(util::LogCategory::notification, util::Logger::Level::debug, "Creating ResultsNotifier for ", + m_logger->log(util::LogCategory::notification, util::Logger::Level::debug, "Creating ResultsNotifier for %1", m_description); } reattach(); @@ -151,20 +151,10 @@ void ResultsNotifier::calculate_changes() void ResultsNotifier::run() { - using namespace std::chrono; + NotifierRunLogger log(m_logger.get(), "ResultsNotifier", m_description); REALM_ASSERT(m_info || !has_run()); - auto t1 = steady_clock::now(); - util::ScopeExit cleanup([&]() noexcept { - m_run_time_point = steady_clock::now(); - if (m_logger) { - m_logger->log(util::LogCategory::notification, util::Logger::Level::debug, - "ResultsNotifier %1 did run in %2 us", m_description, - duration_cast(m_run_time_point - t1).count()); - } - }); - // Table's been deleted, so report all objects as deleted if (!m_query->get_table()) { m_change = {}; @@ -278,7 +268,7 @@ ListResultsNotifier::ListResultsNotifier(Results& target) auto path = m_list->get_short_path(); auto prop_name = m_list->get_table()->get_column_name(path[0].get_col_key()); path[0] = PathElement(prop_name); - std::string sort_order; + std::string_view sort_order = ""; if (m_sort_order) { sort_order = *m_sort_order ? " sorted ascending" : " sorted descending"; } @@ -358,17 +348,6 @@ void ListResultsNotifier::calculate_changes() void ListResultsNotifier::run() { - using namespace std::chrono; - auto t1 = steady_clock::now(); - util::ScopeExit cleanup([&]() noexcept { - m_run_time_point = steady_clock::now(); - if (m_logger) { - m_logger->log(util::LogCategory::notification, util::Logger::Level::debug, - "ListResultsNotifier %1 did run in %2 us", m_description, - duration_cast(m_run_time_point - t1).count()); - } - }); - if (!m_list || !m_list->is_attached()) { // List was deleted, so report all of the rows being removed m_change = {}; @@ -379,10 +358,11 @@ void ListResultsNotifier::run() } if (!need_to_run()) { - cleanup.cancel(); return; } + NotifierRunLogger log(m_logger.get(), "ListResultsNotifier", m_description); + m_run_indices = std::vector(); if (m_distinct) m_list->distinct(*m_run_indices, m_sort_order); @@ -393,13 +373,13 @@ void ListResultsNotifier::run() std::iota(m_run_indices->begin(), m_run_indices->end(), 0); } + // Modifications to nested values in Mixed are recorded in replication as + // StableIndex and we have to look up the actual index afterwards if (m_change.paths.size()) { if (auto coll = dynamic_cast(m_list.get())) { for (auto& p : m_change.paths) { - // Report changes in substructure as modifications on this list - auto ndx = coll->find_index(p[0]); - if (ndx != realm::not_found) - m_change.modifications.add(ndx); // OK to insert same index again + if (auto ndx = coll->find_index(p); ndx != realm::not_found) + m_change.modifications.add(ndx); } } } diff --git a/src/realm/object-store/impl/transact_log_handler.cpp b/src/realm/object-store/impl/transact_log_handler.cpp index be021dd2ec4..93377cfaa06 100644 --- a/src/realm/object-store/impl/transact_log_handler.cpp +++ b/src/realm/object-store/impl/transact_log_handler.cpp @@ -368,21 +368,19 @@ class TransactLogObserver : public TransactLogValidationMixin { { modify_object(col, obj); auto table = current_table(); + m_active_collection = nullptr; for (auto& c : m_info.collections) { - if (c.table_key == table && c.obj_key == obj && c.path.is_prefix_of(path)) { if (c.path.size() != path.size()) { - StablePath sub_path; - sub_path.insert(sub_path.begin(), path.begin() + c.path.size(), path.end()); - c.changes->paths.insert(std::move(sub_path)); + c.changes->paths.insert(path[c.path.size()]); } - else { + // If there are multiple exact matches for this collection we + // use the first and then propagate the data to the others later + else if (!m_active_collection) { m_active_collection = c.changes; - return true; } } } - m_active_collection = nullptr; return true; } diff --git a/src/realm/path.hpp b/src/realm/path.hpp index 08ead2dbae9..6124590271c 100644 --- a/src/realm/path.hpp +++ b/src/realm/path.hpp @@ -271,54 +271,45 @@ class ExtendedColumnKey { */ class StableIndex { public: - StableIndex() - { - value.raw = 0; - } + StableIndex() = default; StableIndex(ColKey col_key, int64_t salt) { - value.col_index = col_key.get_index().val; - value.is_collection = col_key.is_collection(); - value.is_column = true; - value.salt = int32_t(salt); + m_col_index = col_key.get_index().val; + m_is_collection = col_key.is_collection(); + m_is_column = true; + m_salt = int32_t(salt); } StableIndex(int64_t salt) { - value.raw = 0; - value.salt = int32_t(salt); + m_salt = int32_t(salt); } int64_t get_salt() const { - return value.salt; + return m_salt; } ColKey::Idx get_index() const noexcept { - return {unsigned(value.col_index)}; + return {unsigned(m_col_index)}; } bool is_collection() const noexcept { - return value.is_collection; + return m_is_collection; } bool operator==(const StableIndex& other) const noexcept { - return value.is_column ? value.col_index == other.value.col_index : value.salt == other.value.salt; + return m_is_column ? m_col_index == other.m_col_index : m_salt == other.m_salt; } bool operator<(const StableIndex& other) const noexcept { - return value.is_column ? value.col_index < other.value.col_index : value.salt < other.value.salt; + return m_is_column ? m_col_index < other.m_col_index : m_salt < other.m_salt; } private: - union { - struct { - bool is_column; - bool is_collection; - int16_t col_index; - int32_t salt; - }; - int64_t raw; - } value; + bool m_is_column = false; + bool m_is_collection = false; + int16_t m_col_index = 0; + int32_t m_salt = 0; }; static_assert(sizeof(StableIndex) == 8); diff --git a/src/realm/replication.cpp b/src/realm/replication.cpp index 35bd32d8b3a..cfc60c6580b 100644 --- a/src/realm/replication.cpp +++ b/src/realm/replication.cpp @@ -177,7 +177,7 @@ void Replication::remove_object(const Table* t, ObjKey key) m_encoder.remove_object(key); // Throws } -inline void Replication::select_obj(ObjKey key) +void Replication::select_obj(ObjKey key) { if (key == m_selected_obj) { return; 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..e3d7fac3d60 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); @@ -104,7 +103,7 @@ class Set final : public CollectionBaseImpl { this->set_owner(owner, col_key); } - Set(ColKey col_key) + Set(ColKey col_key, size_t = 0) : Base(col_key) { if (!col_key.is_set()) { @@ -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 diff --git a/src/realm/transaction.cpp b/src/realm/transaction.cpp index 6f246e36e2d..1cddd8fdce3 100644 --- a/src/realm/transaction.cpp +++ b/src/realm/transaction.cpp @@ -419,7 +419,7 @@ _impl::History* Transaction::get_history() const Obj Transaction::import_copy_of(const Obj& original) { if (bool(original) && original.is_valid()) { - TableKey tk = original.get_table_key(); + TableKey tk = original.get_table()->get_key(); ObjKey rk = original.get_key(); auto table = get_table(tk); if (table->is_valid(rk)) diff --git a/src/realm/util/logger.hpp b/src/realm/util/logger.hpp index a438cf60179..017cf8284f3 100644 --- a/src/realm/util/logger.hpp +++ b/src/realm/util/logger.hpp @@ -497,6 +497,12 @@ class LocalThresholdLogger : public Logger { std::shared_ptr m_chained_logger; }; +/// A logger that performs a noop when logging functions are called +class NullLogger : public Logger { + // Since we don't want to log anything, do_log() does nothing + void do_log(const LogCategory&, Level, const std::string&) override {} +}; + // Implementation diff --git a/test/object-store/audit.cpp b/test/object-store/audit.cpp index a0b5ffef2da..97886421c45 100644 --- a/test/object-store/audit.cpp +++ b/test/object-store/audit.cpp @@ -52,18 +52,11 @@ using namespace std::string_literals; using Catch::Matchers::StartsWith; using nlohmann::json; -namespace { -class NullLogger : public util::Logger { - // Since we don't want to log anything, do_log() does nothing - void do_log(const util::LogCategory&, Level, const std::string&) override {} -}; -} // namespace - static auto audit_logger = #ifdef AUDIT_LOG_LEVEL std::make_shared(AUDIT_LOG_LEVEL); #else - std::make_shared(); + std::make_shared(); #endif namespace { diff --git a/test/object-store/benchmarks/client_reset.cpp b/test/object-store/benchmarks/client_reset.cpp index b232eb1cf3d..3fea181b669 100644 --- a/test/object-store/benchmarks/client_reset.cpp +++ b/test/object-store/benchmarks/client_reset.cpp @@ -139,10 +139,7 @@ struct BenchmarkLocalClientReset : public reset_utils::TestClientReset { Transaction& wt_local = (Transaction&)m_local->read_group(); VersionID current_local_version = wt_local.get_version_of_current_transaction(); - class NullLogger : public util::Logger { - // Since we don't want to log anything, do_log() does nothing - void do_log(const util::LogCategory&, Level, const std::string&) override {} - } logger; + util::NullLogger logger; if (m_mode == ClientResyncMode::Recover) { auto history_local = dynamic_cast(wt_local.get_replication()->_get_history_write()); diff --git a/test/object-store/dictionary.cpp b/test/object-store/dictionary.cpp index f72791b6ed8..0ae18396843 100644 --- a/test/object-store/dictionary.cpp +++ b/test/object-store/dictionary.cpp @@ -63,9 +63,7 @@ struct StringMaker { namespace cf = realm::collection_fixtures; TEST_CASE("nested dictionary in mixed", "[dictionary]") { - InMemoryTestFile config; - config.cache = false; config.automatic_change_notifications = false; config.schema = Schema{{"any_collection", {{"any", PropertyType::Mixed | PropertyType::Nullable}}}}; diff --git a/test/object-store/nested_collections.cpp b/test/object-store/nested_collections.cpp index cd94a94b078..99962d25337 100644 --- a/test/object-store/nested_collections.cpp +++ b/test/object-store/nested_collections.cpp @@ -36,7 +36,6 @@ using namespace realm; TEST_CASE("nested-list-mixed", "[nested-collections]") { InMemoryTestFile config; - config.cache = false; config.automatic_change_notifications = false; auto r = Realm::get_shared_realm(config); r->update_schema({{ diff --git a/test/object-store/object.cpp b/test/object-store/object.cpp index b370234616a..ea779201724 100644 --- a/test/object-store/object.cpp +++ b/test/object-store/object.cpp @@ -160,7 +160,6 @@ TEST_CASE("object") { _impl::RealmCoordinator::assert_no_open_realms(); InMemoryTestFile config; - config.cache = false; config.automatic_change_notifications = false; config.schema_mode = SchemaMode::AdditiveExplicit; config.schema = Schema{ diff --git a/test/object-store/primitive_list.cpp b/test/object-store/primitive_list.cpp index 3160c88cc3c..9bab1b1deea 100644 --- a/test/object-store/primitive_list.cpp +++ b/test/object-store/primitive_list.cpp @@ -983,7 +983,6 @@ TEST_CASE("list of mixed links", "[primitives]") { TEST_CASE("list of strings - with index", "[primitives]") { InMemoryTestFile config; - config.cache = false; config.automatic_change_notifications = false; config.schema = Schema{ {"object", diff --git a/test/test_client_reset.cpp b/test/test_client_reset.cpp index f5e910b71c6..c6beed04601 100644 --- a/test/test_client_reset.cpp +++ b/test/test_client_reset.cpp @@ -1477,13 +1477,6 @@ TEST(ClientReset_Recover_RecoverableChangesOnListsAfterUnrecoverableAreNotDuplic CHECK_EQUAL(changes.size(), 1); } -namespace { -class NullLogger : public util::Logger { - // Since we don't want to log anything, do_log() does nothing - void do_log(const util::LogCategory&, Level, const std::string&) override {} -}; -} // namespace - // Apply uploaded changes in src to dst as if they had been exchanged by sync void apply_changes(DB& src, DB& dst) { @@ -1515,7 +1508,7 @@ void apply_changes(DB& src, DB& dst) dst_progress.download.server_version += remote_changesets.size(); dst_progress.latest_server_version.version += remote_changesets.size(); - NullLogger logger; + util::NullLogger logger; VersionInfo new_version; dst_history.integrate_server_changesets(dst_progress, nullptr, remote_changesets, new_version, DownloadBatchState::SteadyState, logger, dst.start_read()); diff --git a/test/test_list.cpp b/test/test_list.cpp index 2bfa0232873..2e88514780e 100644 --- a/test/test_list.cpp +++ b/test/test_list.cpp @@ -1071,3 +1071,177 @@ TEST(List_Nested_Replication) Dictionary dict3(*dict, dict2_index); CHECK_EQUAL(dict3.get_col_key(), col_any); } + +namespace realm { +static std::ostream& operator<<(std::ostream& os, UpdateStatus status) +{ + switch (status) { + case UpdateStatus::Detached: + os << "Detatched"; + break; + case UpdateStatus::Updated: + os << "Updated"; + break; + case UpdateStatus::NoChange: + os << "NoChange"; + break; + } + return os; +} +} // namespace realm + +TEST(List_UpdateIfNeeded) +{ + SHARED_GROUP_TEST_PATH(path); + DBRef db = DB::create(make_in_realm_history(), path); + auto tr = db->start_write(); + auto table = tr->add_table("table"); + auto col = table->add_column(type_Mixed, "mixed"); + auto col2 = table->add_column(type_Mixed, "col2"); + auto leading_obj = table->create_object(); + Obj obj = table->create_object(); + obj.set_collection(col, CollectionType::List); + + auto list_1 = obj.get_list(col); + auto list_2 = obj.get_list(col); + + // The underlying object starts out up-to-date + CHECK_EQUAL(list_1.get_obj().update_if_needed(), UpdateStatus::NoChange); + + // Attempt to initialize the accessor and fail because the list is empty, + // leaving it detached (only size() can be called on an empty list) + CHECK_EQUAL(list_1.update_if_needed(), UpdateStatus::Detached); + CHECK_EQUAL(list_2.update_if_needed(), UpdateStatus::Detached); + + list_1.add(Mixed()); + + // First accessor was used to create the list so it's already up to date, + // but the second is updated + CHECK_EQUAL(list_1.update_if_needed(), UpdateStatus::NoChange); + CHECK_EQUAL(list_2.update_if_needed(), UpdateStatus::Updated); + + // The list is now non-empty, so a new accessor can initialize + auto list_3 = obj.get_list(col); + CHECK_EQUAL(list_3.update_if_needed(), UpdateStatus::Updated); + CHECK_EQUAL(list_3.update_if_needed(), UpdateStatus::NoChange); + + // A copy of a list is lazily initialized, so it's updated on first call + // even if the source was up-to-date + auto list_4 = std::make_shared>(list_3); + CHECK_EQUAL(list_4->update_if_needed(), UpdateStatus::Updated); + + // Nested lists work the same way as top-level ones + list_4->insert_collection(1, CollectionType::List); + auto list_4_1 = list_4->get_list(1); + auto list_4_2 = list_4->get_list(1); + list_4_1->add(Mixed()); + // FIXME: this should be NoChange + CHECK_EQUAL(list_4_1->update_if_needed(), UpdateStatus::Updated); + CHECK_EQUAL(list_4_2->update_if_needed(), UpdateStatus::Updated); + + // Update the row index of the parent object, forcing it to update + leading_obj.remove(); + + // Updating the base object directly first doesn't change the result of + // updating the list + CHECK_EQUAL(list_1.get_obj().update_if_needed(), UpdateStatus::Updated); + CHECK_EQUAL(list_1.update_if_needed(), UpdateStatus::Updated); + + CHECK_EQUAL(list_2.update_if_needed(), UpdateStatus::Updated); + CHECK_EQUAL(list_3.update_if_needed(), UpdateStatus::Updated); + + // These two lists share the same parent, so the first updates due to the + // parent returning Updated, and the second updates due to seeing that the + // parent version has changed + CHECK_EQUAL(list_4_1->update_if_needed(), UpdateStatus::Updated); + CHECK_EQUAL(list_4_2->update_if_needed(), UpdateStatus::Updated); + + tr->commit_and_continue_as_read(); + + // Committing the write transaction changes the obj's ref, so everything + // has to update + CHECK_EQUAL(list_1.get_obj().update_if_needed(), UpdateStatus::Updated); + CHECK_EQUAL(list_1.update_if_needed(), UpdateStatus::Updated); + CHECK_EQUAL(list_2.update_if_needed(), UpdateStatus::Updated); + CHECK_EQUAL(list_3.update_if_needed(), UpdateStatus::Updated); + CHECK_EQUAL(list_4_1->update_if_needed(), UpdateStatus::Updated); + CHECK_EQUAL(list_4_2->update_if_needed(), UpdateStatus::Updated); + + // Perform a write which does not result in obj changing + { + auto tr2 = db->start_write(); + tr2->add_table("other table"); + tr2->commit(); + } + tr->advance_read(); + + // The obj's storage version has changed, but nothing needs to update + CHECK_EQUAL(list_1.get_obj().update_if_needed(), UpdateStatus::NoChange); + CHECK_EQUAL(list_1.update_if_needed(), UpdateStatus::NoChange); + CHECK_EQUAL(list_2.update_if_needed(), UpdateStatus::NoChange); + CHECK_EQUAL(list_3.update_if_needed(), UpdateStatus::NoChange); + CHECK_EQUAL(list_4_1->update_if_needed(), UpdateStatus::NoChange); + CHECK_EQUAL(list_4_2->update_if_needed(), UpdateStatus::NoChange); + + // Perform a write which does modify obj + { + auto tr2 = db->start_write(); + tr2->get_table("table")->get_object(obj.get_key()).set_any(col2, "value"); + tr2->commit(); + } + tr->advance_read(); + + // Everything needs to update even though the allocator's content version is unchanged + CHECK_EQUAL(list_1.get_obj().update_if_needed(), UpdateStatus::Updated); + CHECK_EQUAL(list_1.update_if_needed(), UpdateStatus::Updated); + CHECK_EQUAL(list_2.update_if_needed(), UpdateStatus::Updated); + CHECK_EQUAL(list_3.update_if_needed(), UpdateStatus::Updated); + CHECK_EQUAL(list_4_1->update_if_needed(), UpdateStatus::Updated); + CHECK_EQUAL(list_4_2->update_if_needed(), UpdateStatus::Updated); + + // Everything updates to detached when the object is removed + tr->promote_to_write(); + obj.remove(); + + CHECK_EQUAL(list_1.get_obj().update_if_needed(), UpdateStatus::Detached); + CHECK_EQUAL(list_1.update_if_needed(), UpdateStatus::Detached); + CHECK_EQUAL(list_2.update_if_needed(), UpdateStatus::Detached); + CHECK_EQUAL(list_3.update_if_needed(), UpdateStatus::Detached); + CHECK_EQUAL(list_4_1->update_if_needed(), UpdateStatus::Detached); + CHECK_EQUAL(list_4_2->update_if_needed(), UpdateStatus::Detached); +} + +TEST(List_AsCollectionParent) +{ + Group g; + auto table = g.add_table("table"); + auto col = table->add_column(type_Mixed, "mixed"); + + Obj obj = table->create_object(); + obj.set_collection(col, CollectionType::List); + auto list_1 = obj.get_list(col); + list_1.insert_collection(0, CollectionType::List); + + // list_1 is stack allocated, so we have to create a new object which can + // serve as the owner. This object is not reused for multiple calls. + auto list_1_1 = list_1.get_list(0); + auto list_1_2 = list_1.get_list(0); + CHECK_NOT_EQUAL(list_1_1->get_owner(), &list_1); + CHECK_NOT_EQUAL(list_1_1->get_owner(), list_1_2->get_owner()); + + // list_2 is heap allocated but not owned by a shared_ptr, so we have to + // create a new object which can serve as the owner. This object is not + // reused for multiple calls. + auto list_2 = obj.get_list_ptr(col); + auto list_2_1 = list_2->get_list(0); + auto list_2_2 = list_2->get_list(0); + CHECK_NOT_EQUAL(list_2_1->get_owner(), list_2.get()); + CHECK_NOT_EQUAL(list_2_1->get_owner(), list_2_2->get_owner()); + + // list_3 is owned by a shared_ptr, so we can just use it as the owner directly + auto list_3 = std::shared_ptr{std::move(list_2)}; + auto list_3_1 = list_3->get_list(0); + auto list_3_2 = list_3->get_list(0); + CHECK_EQUAL(list_3_1->get_owner(), list_3.get()); + CHECK_EQUAL(list_3_1->get_owner(), list_3_2->get_owner()); +} diff --git a/test/test_parser.cpp b/test/test_parser.cpp index 2fd69da5a6c..d60d5c1d215 100644 --- a/test/test_parser.cpp +++ b/test/test_parser.cpp @@ -5843,8 +5843,8 @@ TEST(Parser_CollectionLinks) Obj charlie = persons->create_object_with_primary_key("charlie"); Obj david = persons->create_object_with_primary_key("david"); - Obj elisabeth = persons->create_object_with_primary_key("elisabeth"); - Obj felix = persons->create_object_with_primary_key("felix"); + persons->create_object_with_primary_key("elisabeth"); + persons->create_object_with_primary_key("felix"); Obj gary = persons->create_object_with_primary_key("gary"); Obj hutch = persons->create_object_with_primary_key("hutch");