Simplify notifier registration and collection replication

CHANGELOG.md

@@ -2,7 +2,7 @@
 
 ### Enhancements
 * <New feature description> (PR [#????](https://github.com/realm/realm-core/pull/????))
-* None.
+* Improve performance of rolling back write transactions after making changes. If no KVO observers are used this is now constant time rather than taking time proportional to the number of changes to be rolled back. Rollbacks with KVO observers are 10-20% faster. ([PR #6513](https://github.com/realm/realm-core/pull/6513))
 
 ### Fixed
 * <How do the end-user experience this issue? what was the impact?> ([#????](https://github.com/realm/realm-core/issues/????), since v?.?.?)
@@ -18,6 +18,8 @@
 
 ### Internals
 * Clear out SubscriptionStore and cancel pending notifications upon rollback to PBS after client migration to FLX. ([#6389](https://github.com/realm/realm-core/issues/6389))
+* Simplify the non-sync replication log by emitting the same instruction type for all three types of collections rather than different instructions per collection type. This has no functional effect but eliminates some duplicated code. ([PR #6513](https://github.com/realm/realm-core/pull/6513))
+* Remove TransactionChangeInfo::track_all, which was only ever used by the global notifier. ([PR #6513](https://github.com/realm/realm-core/pull/6513))
 
 ----------------------------------------------
 

src/realm/exec/realm_trawler.cpp

@@ -972,33 +972,15 @@ class HistoryLogger {
         return true;
     }
 
-    bool list_set(size_t ndx)
+    bool collection_set(size_t ndx)
     {
-        std::cout << "List set at " << ndx << std::endl;
+        std::cout << "Collection set at " << ndx << std::endl;
         return true;
     }
 
-    bool list_insert(size_t ndx)
+    bool collection_insert(size_t ndx)
     {
-        std::cout << "List insert at " << ndx << std::endl;
-        return true;
-    }
-
-    bool dictionary_insert(size_t, realm::Mixed key)
-    {
-        std::cout << "Dictionary insert at " << key << std::endl;
-        return true;
-    }
-
-    bool dictionary_set(size_t, realm::Mixed key)
-    {
-        std::cout << "Dictionary set at " << key << std::endl;
-        return true;
-    }
-
-    bool dictionary_erase(size_t, realm::Mixed key)
-    {
-        std::cout << "Dictionary erase at " << key << std::endl;
+        std::cout << "Collection insert at " << ndx << std::endl;
         return true;
     }
 
@@ -1029,39 +1011,21 @@ class HistoryLogger {
         return true;
     }
 
-    bool list_move(size_t from_link_ndx, size_t to_link_ndx)
+    bool collection_move(size_t from_link_ndx, size_t to_link_ndx)
     {
         std::cout << "List move from " << from_link_ndx << " to " << to_link_ndx << std::endl;
         return true;
     }
 
-    bool list_erase(size_t ndx)
-    {
-        std::cout << "List erase at " << ndx << std::endl;
-        return true;
-    }
-
-    bool list_clear(size_t old_list_size)
-    {
-        std::cout << "List clear. Old size: " << old_list_size << std::endl;
-        return true;
-    }
-
-    bool set_insert(size_t ndx)
-    {
-        std::cout << "Set insert at " << ndx << std::endl;
-        return true;
-    }
-
-    bool set_erase(size_t ndx)
+    bool collection_erase(size_t ndx)
     {
-        std::cout << "Set erase at " << ndx << std::endl;
+        std::cout << "Collection erase at " << ndx << std::endl;
         return true;
     }
 
-    bool set_clear(size_t old_set_size)
+    bool collection_clear(size_t old_list_size)
     {
-        std::cout << "Set clear. Old size: " << old_set_size << std::endl;
+        std::cout << "Collection clear. Old size: " << old_list_size << std::endl;
         return true;
     }
 

src/realm/group.cpp

@@ -833,29 +833,23 @@ void Group::remove_table(size_t table_ndx, TableKey key)
     // columns of other tables, however, to do that, we would have to
     // automatically remove the "offending" link columns from those other
     // tables. Such a behaviour is deemed too obscure, and we shall therefore
-    // require that a removed table does not contain foreigh origin backlink
+    // require that a removed table does not contain foreign origin backlink
     // columns.
     if (table->is_cross_table_link_target())
         throw CrossTableLinkTarget(table->get_name());
 
-    // There is no easy way for Group::TransactAdvancer to handle removal of
-    // tables that contain foreign target table link columns, because that
-    // involves removal of the corresponding backlink columns. For that reason,
-    // we start by removing all columns, which will generate individual
-    // replication instructions for each column removal with sufficient
-    // information for Group::TransactAdvancer to handle them.
-    size_t n = table->get_column_count();
-    Replication* repl = *get_repl();
-    if (repl) {
-        // This will prevent sync instructions for column removals to be generated
-        repl->prepare_erase_class(key);
-    }
-    for (size_t i = n; i > 0; --i) {
-        ColKey col_key = table->spec_ndx2colkey(i - 1);
-        table->remove_column(col_key);
+    {
+        // We don't want to replicate the individual column removals along the
+        // way as they're covered by the table removal
+        Table::DisableReplication dr(*table);
+        for (size_t i = table->get_column_count(); i > 0; --i) {
+            ColKey col_key = table->spec_ndx2colkey(i - 1);
+            table->remove_column(col_key);
+        }
     }
 
     size_t prior_num_tables = m_tables.size();
+    Replication* repl = *get_repl();
     if (repl)
         repl->erase_class(key, prior_num_tables); // Throws
 
@@ -1323,12 +1317,12 @@ class Group::TransactAdvancer {
         return true; // No-op
     }
 
-    bool list_set(size_t)
+    bool collection_set(size_t)
     {
         return true;
     }
 
-    bool list_insert(size_t)
+    bool collection_insert(size_t)
     {
         return true;
     }
@@ -1366,46 +1360,21 @@ class Group::TransactAdvancer {
         return true; // No-op
     }
 
-    bool list_move(size_t, size_t) noexcept
+    bool collection_move(size_t, size_t) noexcept
     {
         return true; // No-op
     }
 
-    bool list_erase(size_t) noexcept
+    bool collection_erase(size_t) noexcept
     {
         return true; // No-op
     }
 
-    bool list_clear(size_t) noexcept
-    {
-        return true; // No-op
-    }
-
-    bool dictionary_insert(size_t, Mixed)
-    {
-        return true; // No-op
-    }
-    bool dictionary_set(size_t, Mixed)
-    {
-        return true; // No-op
-    }
-    bool dictionary_erase(size_t, Mixed)
+    bool collection_clear(size_t) noexcept
     {
         return true; // No-op
     }
 
-    bool set_insert(size_t)
-    {
-        return true; // No-op
-    }
-    bool set_erase(size_t)
-    {
-        return true; // No-op
-    }
-    bool set_clear(size_t)
-    {
-        return true; // No-op
-    }
     bool typed_link_change(ColKey, TableKey)
     {
         return true; // No-op
@@ -1476,7 +1445,7 @@ void Group::refresh_dirty_accessors()
 }
 
 
-void Group::advance_transact(ref_type new_top_ref, util::NoCopyInputStream& in, bool writable)
+void Group::advance_transact(ref_type new_top_ref, util::NoCopyInputStream* in, bool writable)
 {
     REALM_ASSERT(is_attached());
     // Exception safety: If this function throws, the group accessor and all of
@@ -1513,10 +1482,10 @@ void Group::advance_transact(ref_type new_top_ref, util::NoCopyInputStream& in,
     // This is no longer needed in Core, but we need to compute "schema_changed",
     // for the benefit of ObjectStore.
     bool schema_changed = false;
-    if (has_schema_change_notification_handler()) {
-        _impl::TransactLogParser parser; // Throws
+    if (in && has_schema_change_notification_handler()) {
         TransactAdvancer advancer(*this, schema_changed);
-        parser.parse(in, advancer); // Throws
+        _impl::TransactLogParser parser; // Throws
+        parser.parse(*in, advancer);     // Throws
     }
 
     m_top.detach();                                           // Soft detach

src/realm/group.hpp

@@ -696,7 +696,7 @@ class Group : public ArrayParent {
     class TransactAdvancer;
     /// Memory mappings must have been updated to reflect any growth in filesize before
     /// calling advance_transact()
-    void advance_transact(ref_type new_top_ref, util::NoCopyInputStream&, bool writable);
+    void advance_transact(ref_type new_top_ref, util::NoCopyInputStream*, bool writable);
     void refresh_dirty_accessors();
     void flush_accessors_for_commit();
 

src/realm/impl/transact_log.cpp

@@ -16,12 +16,9 @@
  *
  **************************************************************************/
 
-#include <realm/global_key.hpp>
 #include <realm/impl/transact_log.hpp>
-#include <realm/list.hpp>
 
-namespace realm {
-namespace _impl {
+namespace realm::_impl {
 
 
 bool TransactLogEncoder::select_table(TableKey key)
@@ -33,33 +30,7 @@ bool TransactLogEncoder::select_table(TableKey key)
 
 bool TransactLogEncoder::select_collection(ColKey col_key, ObjKey key)
 {
-    append_simple_instr(instr_SelectList, col_key, key.value); // Throws
-    return true;
-}
-
-/******************************** Dictionary *********************************/
-
-bool TransactLogEncoder::dictionary_insert(size_t dict_ndx, Mixed key)
-{
-    REALM_ASSERT(key.get_type() == type_String);
-    append_string_instr(instr_DictionaryInsert, key.get_string()); // Throws
-    append_simple_instr(dict_ndx);
-    return true;
-}
-
-bool TransactLogEncoder::dictionary_set(size_t dict_ndx, Mixed key)
-{
-    REALM_ASSERT(key.get_type() == type_String);
-    append_string_instr(instr_DictionarySet, key.get_string()); // Throws
-    append_simple_instr(dict_ndx);
-    return true;
-}
-
-bool TransactLogEncoder::dictionary_erase(size_t ndx, Mixed key)
-{
-    REALM_ASSERT(key.get_type() == type_String);
-    append_string_instr(instr_DictionaryErase, key.get_string()); // Throws
-    append_simple_instr(ndx);
+    append_simple_instr(instr_SelectCollection, col_key, key.value); // Throws
     return true;
 }
 
@@ -69,5 +40,4 @@ void TransactLogParser::parser_error() const
     throw Exception(ErrorCodes::BadChangeset, "Bad transaction log");
 }
 
-} // namespace _impl
-} // namespace realm
+} // namespace realm::_impl