From d05739a068414e3ef0b433e2dc8c1b59bacc70f6 Mon Sep 17 00:00:00 2001 From: Thomas Goyne Date: Tue, 5 Mar 2024 10:57:12 -0800 Subject: [PATCH] Make Obj trivial and add a separate ObjCollectionParent type Giving Obj a virtual base class turns out to have a meaningful performance impact. --- CHANGELOG.md | 1 + src/realm/collection.hpp | 8 +- src/realm/collection_parent.cpp | 239 ++++++----------------- src/realm/collection_parent.hpp | 26 ++- src/realm/dictionary.cpp | 7 +- src/realm/list.cpp | 12 +- src/realm/list.hpp | 2 +- src/realm/obj.cpp | 90 ++++++++- src/realm/obj.hpp | 128 ++++++++---- src/realm/path.hpp | 39 ++-- src/realm/replication.cpp | 2 +- src/realm/set.hpp | 2 +- src/realm/transaction.cpp | 2 +- test/object-store/dictionary.cpp | 2 - test/object-store/nested_collections.cpp | 1 - test/object-store/object.cpp | 1 - test/object-store/primitive_list.cpp | 1 - test/test_list.cpp | 110 +++++++++++ test/test_parser.cpp | 4 +- 19 files changed, 384 insertions(+), 293 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f945ad0f210..4566df8aded 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ ### Fixed * ([#????](https://github.com/realm/realm-core/issues/????), since v?.?.?) * 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 * None. diff --git a/src/realm/collection.hpp b/src/realm/collection.hpp index 7cb7481e518..78469a182a4 100644 --- a/src/realm/collection.hpp +++ b/src/realm/collection.hpp @@ -571,7 +571,7 @@ 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; ColKey m_col_key; @@ -724,19 +724,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 diff --git a/src/realm/collection_parent.cpp b/src/realm/collection_parent.cpp index f852c7575cb..dc3ad806bc3 100644 --- a/src/realm/collection_parent.cpp +++ b/src/realm/collection_parent.cpp @@ -79,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 +template typename Collection, typename LinkCol> +std::unique_ptr create_collection(ColKey col_key, size_t level) { - 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 -{ - 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; @@ -307,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 29ffbe3a70b..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; @@ -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 @@ -129,20 +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; + /// Get the counter which is incremented whenever the root Obj is updated. 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 - 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); }; } // namespace realm diff --git a/src/realm/dictionary.cpp b/src/realm/dictionary.cpp index 561d63e3255..b4e5ac67d1e 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()); } } @@ -682,7 +678,6 @@ void Dictionary::ensure_created() { constexpr bool allow_create = true; if (do_update_if_needed(allow_create) == UpdateStatus::Detached) { - // FIXME: untested throw StaleAccessor("Dictionary no longer exists"); } } diff --git a/src/realm/list.cpp b/src/realm/list.cpp index d51adacd942..5a5482b70dc 100644 --- a/src/realm/list.cpp +++ b/src/realm/list.cpp @@ -534,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(); } @@ -560,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(); } diff --git a/src/realm/list.hpp b/src/realm/list.hpp index 945b3d53fd5..fec6505c030 100644 --- a/src/realm/list.hpp +++ b/src/realm/list.hpp @@ -629,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()); diff --git a/src/realm/obj.cpp b/src/realm/obj.cpp index 2a9a53c629d..afabea7a083 100644 --- a/src/realm/obj.cpp +++ b/src/realm/obj.cpp @@ -1107,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)); @@ -1965,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; } @@ -2031,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); @@ -2139,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; } @@ -2439,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())); @@ -2454,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; @@ -2465,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)); @@ -2474,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 cdb59f6f5ff..8d2d4938ef9 100644 --- a/src/realm/obj.hpp +++ b/src/realm/obj.hpp @@ -57,42 +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() = default; Obj(TableRef table, MemRef mem, ObjKey key, size_t row_ndx); - // Overriding members of CollectionParent: - UpdateStatus update_if_needed() 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; - 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; - uint32_t parent_version() const noexcept final - { - return m_version_counter; - } + 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; @@ -322,22 +310,18 @@ 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; @@ -417,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 : 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); 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.hpp b/src/realm/set.hpp index 2fe0a762f51..e3d7fac3d60 100644 --- a/src/realm/set.hpp +++ b/src/realm/set.hpp @@ -103,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()) { 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/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_list.cpp b/test/test_list.cpp index 2bfa0232873..4a7ed375556 100644 --- a/test/test_list.cpp +++ b/test/test_list.cpp @@ -1071,3 +1071,113 @@ 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"); + 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); + + // Update the row index of the parent object, forcing it to update + table->remove_object(table->begin()); + + // 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); + + 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); + + // 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); + + // 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); + + // 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); +} 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");