From 95c6efce8d573b00c97fa8cd447eabaceb3e6c75 Mon Sep 17 00:00:00 2001 From: Thomas Goyne Date: Thu, 14 Mar 2024 09:10:36 -0700 Subject: [PATCH] Improve performance of aggregate operations on empty dictionaries (#7418) --- CHANGELOG.md | 1 + src/realm/collection.hpp | 36 ++++++++++++++---------------- src/realm/dictionary.cpp | 8 +++++++ src/realm/dictionary.hpp | 2 ++ src/realm/object-store/results.cpp | 9 ++------ src/realm/table.cpp | 4 ++-- src/realm/table.hpp | 6 ++++- 7 files changed, 37 insertions(+), 29 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 16b9183f223..4f709eaefb7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ ### 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)). +* Improve performance of aggregate operations on Dictionaries of objects, particularly when the dictionaries are empty ([PR #7418](https://github.com/realm/realm-core/pull/7418)) ### 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)). diff --git a/src/realm/collection.hpp b/src/realm/collection.hpp index 6dbcebc7eaf..5b8ba7758d4 100644 --- a/src/realm/collection.hpp +++ b/src/realm/collection.hpp @@ -175,6 +175,12 @@ class CollectionBase : public Collection { return PathElement(ndx); } + // Clone this collection if it contains objects, and return nullptr otherwise + virtual LinkCollectionPtr clone_as_obj_list() const + { + return nullptr; + } + /// Return true if the collection has changed since the last call to /// `has_changed()`. Note that this function is not idempotent and updates /// the internal state of the accessor if it has changed. @@ -202,7 +208,7 @@ class CollectionBase : public Collection { } /// If this is a collection of links, get the target table. - virtual TableRef get_target_table() const final + TableRef get_target_table() const { return get_obj().get_target_table(get_col_key()); } @@ -815,24 +821,12 @@ void update_unresolved(std::vector& vec, const BPlusTree* tree); /// Clear the context flag on the tree if there are no more unresolved links. void check_for_last_unresolved(BPlusTree* tree); - -/// Proxy class needed because the ObjList interface clobbers method names from -/// CollectionBase. -struct ObjListProxy : ObjList { - virtual TableRef proxy_get_target_table() const = 0; - - TableRef get_target_table() const final - { - return proxy_get_target_table(); - } -}; - } // namespace _impl /// Base class for collections of objects, where unresolved links (tombstones) /// can occur. template -class ObjCollectionBase : public Interface, public _impl::ObjListProxy { +class ObjCollectionBase : public Interface, public ObjList { public: static_assert(std::is_base_of_v); @@ -868,7 +862,15 @@ class ObjCollectionBase : public Interface, public _impl::ObjListProxy { return m_unresolved.size() != 0; } - using Interface::get_target_table; + LinkCollectionPtr clone_as_obj_list() const final + { + return clone_obj_list(); + } + + TableRef get_target_table() const final + { + return Interface::get_target_table(); + } protected: ObjCollectionBase() = default; @@ -956,10 +958,6 @@ class ObjCollectionBase : public Interface, public _impl::ObjListProxy { // Sorted set of indices containing unresolved links. mutable std::vector m_unresolved; - TableRef proxy_get_target_table() const final - { - return Interface::get_target_table(); - } bool matches(const ObjList& other) const final { return get_owning_obj().get_key() == other.get_owning_obj().get_key() && diff --git a/src/realm/dictionary.cpp b/src/realm/dictionary.cpp index dbc9da3aba9..728442b79b2 100644 --- a/src/realm/dictionary.cpp +++ b/src/realm/dictionary.cpp @@ -1190,6 +1190,14 @@ void Dictionary::set_collection_ref(Index index, ref_type ref, CollectionType ty m_values->set(ndx, Mixed(ref, type)); } +LinkCollectionPtr Dictionary::clone_as_obj_list() const +{ + if (get_value_data_type() == type_Link) { + return std::make_unique(*this); + } + return nullptr; +} + /************************* DictionaryLinkValues *************************/ DictionaryLinkValues::DictionaryLinkValues(const Obj& obj, ColKey col_key) diff --git a/src/realm/dictionary.hpp b/src/realm/dictionary.hpp index ee313c9453a..e20efa39c70 100644 --- a/src/realm/dictionary.hpp +++ b/src/realm/dictionary.hpp @@ -226,6 +226,8 @@ class Dictionary final : public CollectionBaseImpl, public Colle void to_json(std::ostream&, JSONOutputMode, util::FunctionRef) const override; + LinkCollectionPtr clone_as_obj_list() const final; + private: using Base::set_collection; diff --git a/src/realm/object-store/results.cpp b/src/realm/object-store/results.cpp index 36cb62db80d..e9584d2b410 100644 --- a/src/realm/object-store/results.cpp +++ b/src/realm/object-store/results.cpp @@ -810,13 +810,8 @@ Query Results::do_get_query() const return Query(m_table, std::make_unique(m_table_view)); } case Mode::Collection: - if (auto list = dynamic_cast(m_collection.get())) { - return m_table->where(*list); - } - if (auto dict = dynamic_cast(m_collection.get())) { - if (dict->get_value_data_type() == type_Link) { - return m_table->where(*dict); - } + if (auto objlist = m_collection->clone_as_obj_list()) { + return m_table->where(std::move(objlist)); } return m_query; case Mode::Table: diff --git a/src/realm/table.cpp b/src/realm/table.cpp index 4314bd3a55c..81926077d0d 100644 --- a/src/realm/table.cpp +++ b/src/realm/table.cpp @@ -1082,9 +1082,9 @@ void Table::do_erase_root_column(ColKey col_key) bump_storage_version(); } -Query Table::where(const DictionaryLinkValues& dictionary_of_links) const +Query Table::where(const Dictionary& dict) const { - return Query(m_own_ref, dictionary_of_links); + return Query(m_own_ref, dict.clone_as_obj_list()); } void Table::set_table_type(Type table_type, bool handle_backlinks) diff --git a/src/realm/table.hpp b/src/realm/table.hpp index 66d793c3d91..6bdf33bc047 100644 --- a/src/realm/table.hpp +++ b/src/realm/table.hpp @@ -591,7 +591,11 @@ class Table { { return Query(m_own_ref, list); } - Query where(const DictionaryLinkValues& dictionary_of_links) const; + Query where(const Dictionary& dict) const; + Query where(LinkCollectionPtr&& list) const + { + return Query(m_own_ref, std::move(list)); + } Query query(const std::string& query_string, const std::vector>>& arguments = {}) const;