Simplify sync::Transformer and eliminate the abstract interface

Transformer objects used to have some local state which had to be stored
between uses and had the implementation hidden in the cpp file. Neither of
those are true any more, so the design doesn't make sense either. It can now
just be a stack-allocated local created when needed rather than something which
has to be stored and reused.

CHANGELOG.md

@@ -21,6 +21,7 @@
 ### Internals
 * REALM_[ATMU]SAN cmake flags no longer override compilation options and can be combined with Debug|RelWithDebInfo|etc. build types. Rel[ATMU]SAN build type shortcuts are now all slightly optimized debug-based builds with sanitizers. REALM_ASAN now works with msvc (2019/2022) builds. ([PR #6911](https://github.com/realm/realm-core/pull/6911))
 * Remove ArrayWithFind's ability to use a templated callback parameter. The QueryStateBase consumers now use an index and the array leaf to get the actual value if needed. This allows certain queries such as count() to not do as many lookups to the actual values and results in a small performance gain. Also remove `find_action_pattern()` which was unused for a long time. This reduction in templating throughout the query system produces a small (~100k) binary size reduction. ([#7095](https://github.com/realm/realm-core/pull/7095))
+* Rework the internal interface for sync Transformers to simplify it and reduce the compiled size ([PR #7098](https://github.com/realm/realm-core/pull/7098)).
 
 ----------------------------------------------
 

src/realm/sync/noinst/client_history_impl.cpp

@@ -540,7 +540,6 @@ size_t ClientHistory::transform_and_apply_server_changesets(util::Span<Changeset
                 changeset.last_integrated_remote_version = m_sync_history_base_version;
         }
 
-        Transformer& transformer = get_transformer();          // Throws
         constexpr std::size_t commit_byte_size_limit = 102400; // 100 KB
 
         auto changeset_applier = [&](const Changeset* transformed_changeset) -> bool {
@@ -551,12 +550,12 @@ size_t ClientHistory::transform_and_apply_server_changesets(util::Span<Changeset
             }
             downloaded_bytes += transformed_changeset->original_changeset_size;
 
-            return !(m_db->other_writers_waiting_for_lock() &&
-                     transact->get_commit_size() >= commit_byte_size_limit && allow_lock_release);
+            return !(allow_lock_release && m_db->other_writers_waiting_for_lock() &&
+                     transact->get_commit_size() >= commit_byte_size_limit);
         };
-        auto changesets_transformed_count =
-            transformer.transform_remote_changesets(*this, sync_file_id, local_version, changesets_to_integrate,
-                                                    std::move(changeset_applier), logger); // Throws
+        sync::Transformer transformer;
+        auto changesets_transformed_count = transformer.transform_remote_changesets(
+            *this, sync_file_id, local_version, changesets_to_integrate, changeset_applier, logger); // Throws
         return changesets_transformed_count;
     }
     catch (const BadChangesetError& e) {

src/realm/sync/noinst/client_history_impl.hpp

@@ -272,10 +272,6 @@ class ClientHistory final : public _impl::History, public TransformHistory {
     ClientReplication& m_replication;
     DB* m_db = nullptr;
 
-    // FIXME: All history objects belonging to a particular client object
-    // (sync::Client) should use a single shared transformer object.
-    std::unique_ptr<Transformer> m_transformer;
-
     /// The version on which the first changeset in the continuous transactions
     /// history is based, or if that history is empty, the version associated
     /// with currently bound snapshot. In general, `m_ct_history_base_version +
@@ -412,7 +408,6 @@ class ClientHistory final : public _impl::History, public TransformHistory {
     void trim_sync_history();
     void do_trim_sync_history(std::size_t n);
     void clamp_sync_version_range(version_type& begin, version_type& end) const noexcept;
-    Transformer& get_transformer();
     void fix_up_client_file_ident_in_stored_changesets(Transaction&, file_ident_type);
     void record_current_schema_version();
     static void record_current_schema_version(Array& schema_versions, version_type snapshot_version);
@@ -535,13 +530,6 @@ inline void ClientHistory::clamp_sync_version_range(version_type& begin, version
     }
 }
 
-inline auto ClientHistory::get_transformer() -> Transformer&
-{
-    if (!m_transformer)
-        m_transformer = make_transformer(); // Throws
-    return *m_transformer;
-}
-
 
 /// \brief Create a "sync history" implementation of the realm::Replication
 /// interface.

src/realm/sync/noinst/server/server.cpp

@@ -714,14 +714,10 @@ class Worker : public ServerHistory::Context {
 
     // Overriding members of ServerHistory::Context
     std::mt19937_64& server_history_get_random() noexcept override final;
-    sync::Transformer& get_transformer() override final;
-    util::Buffer<char>& get_transform_buffer() override final;
 
 private:
     ServerImpl& m_server;
     std::mt19937_64 m_random;
-    const std::unique_ptr<Transformer> m_transformer;
-    util::Buffer<char> m_transform_buffer;
     ServerFileAccessCache m_file_access_cache;
 
     util::Mutex m_mutex;
@@ -976,8 +972,6 @@ class ServerImpl : public ServerImplBase, public ServerHistory::Context {
 
     // Overriding member functions in _impl::ServerHistory::Context
     std::mt19937_64& server_history_get_random() noexcept override final;
-    Transformer& get_transformer() noexcept override final;
-    util::Buffer<char>& get_transform_buffer() noexcept override final;
 
 private:
     Server::Config m_config;
@@ -1014,8 +1008,6 @@ class ServerImpl : public ServerImplBase, public ServerHistory::Context {
     ServerProtocol m_server_protocol;
     compression::CompressMemoryArena m_compress_memory_arena;
     MiscBuffers m_misc_buffers;
-    std::unique_ptr<Transformer> m_transformer;
-    util::Buffer<char> m_transform_buffer;
     int_fast64_t m_current_server_session_ident;
     Optional<network::DeadlineTimer> m_connection_reaper_timer;
     bool m_allow_load_balancing = false;
@@ -3756,7 +3748,6 @@ void ServerFile::finalize_work_stage_2()
 Worker::Worker(ServerImpl& server)
     : logger{"Worker: ", server.logger_ptr} // Throws
     , m_server{server}
-    , m_transformer{make_transformer()} // Throws
     , m_file_access_cache{server.get_config().max_open_files, logger, *this, server.get_config().encryption_key}
 {
     util::seed_prng_nondeterministically(m_random); // Throws
@@ -3777,17 +3768,6 @@ std::mt19937_64& Worker::server_history_get_random() noexcept
 }
 
 
-sync::Transformer& Worker::get_transformer()
-{
-    return *m_transformer;
-}
-
-
-util::Buffer<char>& Worker::get_transform_buffer()
-{
-    return m_transform_buffer;
-}
-
 void Worker::run()
 {
     for (;;) {
@@ -3913,8 +3893,6 @@ void ServerImpl::start()
     logger.info("Connection soft close timeout: %1 ms", m_config.soft_close_timeout);      // Throws
     logger.debug("Authorization header name: %1", m_config.authorization_header_name);     // Throws
 
-    m_transformer = make_transformer(); // Throws
-
     m_realm_names = _impl::find_realm_files(m_root_dir); // Throws
 
     initiate_connection_reaper_timer(m_config.connection_reaper_interval); // Throws
@@ -3985,18 +3963,6 @@ std::mt19937_64& ServerImpl::server_history_get_random() noexcept
 }
 
 
-Transformer& ServerImpl::get_transformer() noexcept
-{
-    return *m_transformer;
-}
-
-
-util::Buffer<char>& ServerImpl::get_transform_buffer() noexcept
-{
-    return m_transform_buffer;
-}
-
-
 void ServerImpl::listen()
 {
     network::Resolver resolver{get_service()};

src/realm/sync/noinst/server/server_history.cpp

@@ -1200,10 +1200,9 @@ bool ServerHistory::integrate_remote_changesets(file_ident_type remote_file_iden
         // Merge with causally unrelated changesets, and resolve the
         // conflicts if there are any.
         TransformHistoryImpl transform_hist{remote_file_ident, *this, recip_hist};
-        Transformer& transformer = m_context.get_transformer(); // Throws
+        Transformer transformer;
         transformer.transform_remote_changesets(transform_hist, m_local_file_ident, current_server_version,
-                                                parsed_transformed_changesets, std::move(apply),
-                                                logger); // Throws
+                                                parsed_transformed_changesets, apply, logger); // Throws
 
         for (std::size_t i = 0; i < num_changesets; ++i) {
             REALM_ASSERT(get_instruction_encoder().buffer().size() == 0);
@@ -2247,18 +2246,6 @@ void ServerHistory::record_current_schema_version(Array& schema_versions, versio
 }
 
 
-Transformer& ServerHistory::Context::get_transformer()
-{
-    throw util::runtime_error("Not supported");
-}
-
-
-util::Buffer<char>& ServerHistory::Context::get_transform_buffer()
-{
-    throw util::runtime_error("Not supported");
-}
-
-
 std::ostream& _impl::operator<<(std::ostream& out, const ServerHistory::HistoryContents& hc)
 {
     out << "client files:\n";

src/realm/sync/noinst/server/server_history.hpp

@@ -742,16 +742,6 @@ class ServerHistory::Context {
 public:
     virtual std::mt19937_64& server_history_get_random() noexcept = 0;
 
-    // @{
-    /// These are guaranteed to not be called until a remote changeset needs to
-    /// be integrated into the history.
-    ///
-    /// The default implementations throw std::runtime_error, with a message
-    /// saying "Not supported".
-    virtual sync::Transformer& get_transformer();
-    virtual util::Buffer<char>& get_transform_buffer();
-    // @}
-
 protected:
     Context() noexcept = default;
 };