diff --git a/CHANGELOG.md b/CHANGELOG.md index e0bb11f5752..97bfc9be0bf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,8 @@ ### Enhancements * (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 * ([#????](https://github.com/realm/realm-core/issues/????), since v?.?.?) diff --git a/src/realm/group.hpp b/src/realm/group.hpp index 4e4a026f126..9659f550254 100644 --- a/src/realm/group.hpp +++ b/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 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; diff --git a/src/realm/object-store/impl/collection_notifier.cpp b/src/realm/object-store/impl/collection_notifier.cpp index e20f6f9cd89..a4c9c21ae31 100644 --- a/src/realm/object-store/impl/collection_notifier.cpp +++ b/src/realm/object-store/impl/collection_notifier.cpp @@ -259,14 +259,16 @@ std::vector::iterator CollectionNotifier::find_callback(ui void CollectionNotifier::unregister() noexcept { - std::lock_guard 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 lock(m_realm_mutex); - return m_realm != nullptr; + return m_is_alive.load(std::memory_order_acquire); } std::unique_lock CollectionNotifier::lock_target() diff --git a/src/realm/object-store/impl/collection_notifier.hpp b/src/realm/object-store/impl/collection_notifier.hpp index a37d122b84a..88e8b6f9cf3 100644 --- a/src/realm/object-store/impl/collection_notifier.hpp +++ b/src/realm/object-store/impl/collection_notifier.hpp @@ -252,6 +252,7 @@ class CollectionNotifier { mutable std::mutex m_realm_mutex; std::shared_ptr m_realm; + std::atomic m_is_alive = true; std::shared_ptr m_transaction; diff --git a/src/realm/object-store/impl/deep_change_checker.cpp b/src/realm/object-store/impl/deep_change_checker.cpp index efd1e68bdf7..7fd6d6c3251 100644 --- a/src/realm/object-store/impl/deep_change_checker.cpp +++ b/src/realm/object-store/impl/deep_change_checker.cpp @@ -40,7 +40,6 @@ void DeepChangeChecker::find_related_tables(std::vector& related_t struct LinkInfo { std::vector forward_links; std::vector forward_tables; - std::vector backlink_tables; bool processed_table = false; }; @@ -56,24 +55,35 @@ void DeepChangeChecker::find_related_tables(std::vector& 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 complete_mapping; Group* group = table.get_parent_group(); REALM_ASSERT(group); + std::vector 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 pair) { @@ -89,7 +99,7 @@ void DeepChangeChecker::find_related_tables(std::vector& 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& 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& 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()); } } diff --git a/test/object-store/benchmarks/object.cpp b/test/object-store/benchmarks/object.cpp index 34e762812c9..24bd3d2356f 100644 --- a/test/object-store/benchmarks/object.cpp +++ b/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 objects(10'000, object); + std::vector 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("value", 0); + r2->commit_transaction(); + r->refresh(); + + tokens.clear(); + + r2->begin_transaction(); + obj2.set("value", 0); + r2->commit_transaction(); + r->refresh(); + }; }