Improve performance with very large numbers of notifiers

CHANGELOG.md

@@ -3,6 +3,8 @@
 ### Enhancements
 * <New feature description> (PR [#????](https://github.com/realm/realm-core/pull/????))
 * Added ability to get current log level via C API (PR [#7419](https://github.com/realm/realm-core/pull/7419))
+* Improve performance of object notifiers with complex schemas and very simple changes to process by as much as 20% ([PR #7424](https://github.com/realm/realm-core/pull/7424)).
+* Improve performance with very large number of notifiers as much as 75% ([PR #7424](https://github.com/realm/realm-core/pull/7424)).
 
 ### Fixed
 * <How do the end-user experience this issue? what was the impact?> ([#????](https://github.com/realm/realm-core/issues/????), since v?.?.?)

src/realm/group.hpp

@@ -807,7 +807,7 @@ class Group : public ArrayParent {
     Table* get_table_unchecked(TableKey);
     size_t find_table_index(StringData name) const noexcept;
     TableKey ndx2key(size_t ndx) const;
-    size_t key2ndx(TableKey key) const;
+    static size_t key2ndx(TableKey key);
     size_t key2ndx_checked(TableKey key) const;
     void set_size() const noexcept;
     std::map<TableRef, ColKey> get_primary_key_columns_from_pk_table(TableRef pk_table);
@@ -822,15 +822,16 @@ class Group : public ArrayParent {
             throw StaleAccessor("Stale transaction");
     }
 
-    friend class Table;
-    friend class GroupWriter;
-    friend class GroupCommitter;
-    friend class DB;
-    friend class _impl::GroupFriend;
-    friend class Transaction;
-    friend class TableKeyIterator;
     friend class CascadeState;
+    friend class DB;
+    friend class GroupCommitter;
+    friend class GroupWriter;
     friend class SlabAlloc;
+    friend class Table;
+    friend class TableKeyIterator;
+    friend class Transaction;
+    friend class _impl::DeepChangeChecker;
+    friend class _impl::GroupFriend;
 };
 
 class TableKeyIterator {
@@ -904,7 +905,7 @@ inline bool Group::is_empty() const noexcept
     return size() == 0;
 }
 
-inline size_t Group::key2ndx(TableKey key) const
+inline size_t Group::key2ndx(TableKey key)
 {
     size_t idx = key.value & 0xFFFF;
     return idx;

src/realm/object-store/impl/collection_notifier.cpp

@@ -259,14 +259,16 @@ std::vector<NotificationCallback>::iterator CollectionNotifier::find_callback(ui
 
 void CollectionNotifier::unregister() noexcept
 {
-    std::lock_guard<std::mutex> lock(m_realm_mutex);
-    m_realm = nullptr;
+    {
+        std::lock_guard lock(m_realm_mutex);
+        m_realm = nullptr;
+    }
+    m_is_alive.store(false, std::memory_order_release);
 }
 
 bool CollectionNotifier::is_alive() const noexcept
 {
-    std::lock_guard<std::mutex> lock(m_realm_mutex);
-    return m_realm != nullptr;
+    return m_is_alive.load(std::memory_order_acquire);
 }
 
 std::unique_lock<std::mutex> CollectionNotifier::lock_target()

src/realm/object-store/impl/collection_notifier.hpp

@@ -252,6 +252,7 @@ class CollectionNotifier {
 
     mutable std::mutex m_realm_mutex;
     std::shared_ptr<Realm> m_realm;
+    std::atomic<bool> m_is_alive = true;
 
     std::shared_ptr<Transaction> m_transaction;
 

src/realm/object-store/impl/deep_change_checker.cpp

@@ -40,7 +40,6 @@ void DeepChangeChecker::find_related_tables(std::vector<RelatedTable>& related_t
     struct LinkInfo {
         std::vector<ColKey> forward_links;
         std::vector<TableKey> forward_tables;
-
         std::vector<TableKey> backlink_tables;
         bool processed_table = false;
     };
@@ -56,24 +55,35 @@ void DeepChangeChecker::find_related_tables(std::vector<RelatedTable>& related_t
     // so we rely on the fact that if there are any TypedLinks from a
     // Mixed value, there will be a corresponding backlink column
     // created at the destination table.
-    std::unordered_map<TableKey, LinkInfo> complete_mapping;
     Group* group = table.get_parent_group();
     REALM_ASSERT(group);
+    std::vector<LinkInfo> complete_mapping;
+    complete_mapping.resize(group->size());
+    auto get_mapping = [&](TableKey key) -> LinkInfo& {
+        size_t ndx = Group::key2ndx(key);
+        // Removed tables leave gaps, so the maximum index can be greater than
+        // the size. This is very unusual, though.
+        if (ndx >= complete_mapping.size()) {
+            complete_mapping.resize(ndx + 1);
+        }
+        return complete_mapping[ndx];
+    };
+
     auto all_table_keys = group->get_table_keys();
-    for (auto key : all_table_keys) {
-        auto cur_table = group->get_table(key);
+    for (auto table_key : all_table_keys) {
+        auto cur_table = group->get_table(table_key).unchecked_ptr();
         REALM_ASSERT(cur_table);
 
         LinkInfo* backlinks = nullptr;
         if (has_key_paths) {
-            backlinks = &complete_mapping[cur_table->get_key()];
+            backlinks = &get_mapping(table_key);
         }
         cur_table->for_each_backlink_column([&](ColKey backlink_col_key) {
             auto origin_table_key = cur_table->get_opposite_table_key(backlink_col_key);
             auto origin_link_col = cur_table->get_opposite_column(backlink_col_key);
-            auto& links = complete_mapping[origin_table_key];
+            auto& links = get_mapping(origin_table_key);
             links.forward_links.push_back(origin_link_col);
-            links.forward_tables.push_back(cur_table->get_key());
+            links.forward_tables.push_back(table_key);
 
             if (any_of(key_path_array.begin(), key_path_array.end(), [&](const KeyPath& key_path) {
                     return any_of(key_path.begin(), key_path.end(), [&](std::pair<TableKey, ColKey> pair) {
@@ -89,7 +99,7 @@ void DeepChangeChecker::find_related_tables(std::vector<RelatedTable>& related_t
     // Remove duplicates:
     // duplicates in link_columns can occur when a Mixed(TypedLink) contain links to different tables
     // duplicates in connected_tables can occur when there are different link paths to the same table
-    for (auto& [_, info] : complete_mapping) {
+    for (auto& info : complete_mapping) {
         sort_and_unique(info.forward_links);
         sort_and_unique(info.forward_tables);
     }
@@ -98,7 +108,7 @@ void DeepChangeChecker::find_related_tables(std::vector<RelatedTable>& related_t
     while (tables_to_check.size()) {
         auto table_key_to_check = tables_to_check.back();
         tables_to_check.pop_back();
-        auto& link_info = complete_mapping[table_key_to_check];
+        auto& link_info = get_mapping(table_key_to_check);
         if (link_info.processed_table) {
             continue;
         }
@@ -107,16 +117,13 @@ void DeepChangeChecker::find_related_tables(std::vector<RelatedTable>& related_t
         related_tables.push_back({table_key_to_check, std::move(link_info.forward_links)});
 
         // Add all tables reachable via a forward link to the vector of tables that need to be checked
-        for (auto linked_table_key : link_info.forward_tables) {
-            tables_to_check.push_back(linked_table_key);
-        }
+        tables_to_check.insert(tables_to_check.end(), link_info.forward_tables.begin(),
+                               link_info.forward_tables.end());
 
         // Backlinks can only come into consideration when added via key paths.
-        if (has_key_paths) {
-            for (auto linked_table_key : link_info.backlink_tables) {
-                tables_to_check.push_back(linked_table_key);
-            }
-        }
+        REALM_ASSERT(has_key_paths || link_info.backlink_tables.empty());
+        tables_to_check.insert(tables_to_check.end(), link_info.backlink_tables.begin(),
+                               link_info.backlink_tables.end());
     }
 }
 

test/object-store/benchmarks/object.cpp

@@ -767,7 +767,10 @@ TEST_CASE("Benchmark object notification delivery", "[benchmark][notifications]"
     _impl::RealmCoordinator::assert_no_open_realms();
 
     InMemoryTestFile config;
-    config.automatic_change_notifications = false;
+    // This test has meaningfully different performance with and without the
+    // background thread, since immediately waiting on the background thread
+    // is the worst-case scenario and makes it a pessimization
+    config.automatic_change_notifications = GENERATE(false, true);
     config.schema = Schema{{"object", {{"value", PropertyType::Int}}}};
     auto r = Realm::get_shared_realm(config);
 
@@ -789,4 +792,27 @@ TEST_CASE("Benchmark object notification delivery", "[benchmark][notifications]"
             r->refresh();
         }
     };
+
+    BENCHMARK("very large number of notifiers") {
+        std::vector<Object> objects(10'000, object);
+        std::vector<NotificationToken> tokens;
+        for (auto& object : objects) {
+            tokens.push_back(object.add_notification_callback([&](auto) {}));
+        }
+
+        auto r2 = Realm::get_shared_realm(config);
+        auto obj2 = *r2->read_group().get_table("class_object")->begin();
+
+        r2->begin_transaction();
+        obj2.set<int64_t>("value", 0);
+        r2->commit_transaction();
+        r->refresh();
+
+        tokens.clear();
+
+        r2->begin_transaction();
+        obj2.set<int64_t>("value", 0);
+        r2->commit_transaction();
+        r->refresh();
+    };
 }