[geary/wip/794174-conversation-monitor-max-cpu: 2/3] Don't use the database for internal ConversationMonitor bookkeeping.



commit f9fb400afe589843427ee2f7ce5fd8c5931bfb52
Author: Michael James Gratton <mike vee net>
Date:   Fri Mar 9 12:14:15 2018 +1100

    Don't use the database for internal ConversationMonitor bookkeeping.
    
    This replaces the use of Geary.Folder.find_boundaries_async in
    ConversationMonitor with a simple sorted set of known in-folder message
    ids. This is both easy and fast, reduces needless DB load when loading
    conversations, and also allows allows removing what is otherwise
    single-use implementation overhead in classes deriving from Folder.
    
    * src/engine/app/app-conversation-monitor.vala (ConversationMonitor):
      Replace get_lowest_email_id_async() with window_lowest property, update
      call sites. Implement the property by using a sorted set of known
      listed email ids from the base folder. Update the set as messages in
      the base folder are listed. Rename notify_emails_removed to just
      removed and remove ids from the set if any messages from the base
      folder are removed.
    
    * src/engine/api/geary-folder.vala (Folder): Remove
      get_lowest_email_id_async since it is now unused, do same for all
      subclasses.

 src/engine/api/geary-folder.vala                   |    9 --
 src/engine/app/app-conversation-monitor.vala       |  103 +++++++++++---------
 .../app-fill-window-operation.vala                 |    4 +-
 .../conversation-monitor/app-insert-operation.vala |    3 +-
 .../conversation-monitor/app-remove-operation.vala |    6 +-
 .../conversation-monitor/app-reseed-operation.vala |    3 +-
 src/engine/imap-db/outbox/smtp-outbox-folder.vala  |   28 ------
 .../imap-db/search/imap-db-search-folder.vala      |   20 +----
 .../imap-engine/imap-engine-minimal-folder.vala    |   25 -----
 test/engine/api/geary-folder-mock.vala             |   10 --
 10 files changed, 67 insertions(+), 144 deletions(-)
---
diff --git a/src/engine/api/geary-folder.vala b/src/engine/api/geary-folder.vala
index b167d96..f6dbf7e 100644
--- a/src/engine/api/geary-folder.vala
+++ b/src/engine/api/geary-folder.vala
@@ -576,15 +576,6 @@ public abstract class Geary.Folder : BaseObject {
      * closed, even if it's not open.
      */
     public abstract async void wait_for_close_async(Cancellable? cancellable = null) throws Error;
-    
-    /**
-     * Find the lowest- and highest-ordered {@link EmailIdentifier}s in the
-     * folder, among the given set of EmailIdentifiers that may or may not be
-     * in the folder.  If none of the given set are in the folder, return null.
-     */
-    public abstract async void find_boundaries_async(Gee.Collection<Geary.EmailIdentifier> ids,
-        out Geary.EmailIdentifier? low, out Geary.EmailIdentifier? high,
-        Cancellable? cancellable = null) throws Error;
 
     /**
      * List a number of contiguous emails in the folder's vector.
diff --git a/src/engine/app/app-conversation-monitor.vala b/src/engine/app/app-conversation-monitor.vala
index 40817f5..4234825 100644
--- a/src/engine/app/app-conversation-monitor.vala
+++ b/src/engine/app/app-conversation-monitor.vala
@@ -105,11 +105,31 @@ public class Geary.App.ConversationMonitor : BaseObject {
         get; private set; default = new ConversationSet();
     }
 
+    /** The oldest message from the base folder in the loaded window. */
+    internal EmailIdentifier? window_lowest {
+        owned get {
+            return (this.window.is_empty) ? null : this.window.first();
+        }
+    }
+
     private Geary.Email.Field required_fields;
     private Geary.Folder.OpenFlags open_flags;
     private ConversationOperationQueue queue = null;
     private Cancellable? operation_cancellable = null;
 
+    // Set of known, in-folder emails, explicitly loaded for the
+    // monitor's window. This exists purely to support the
+    // window_lowest property above, but we need to maintain a sorted
+    // set of all known messages since if the last known email is
+    // removed, we won't know what the next lowest is. Only email
+    // listed by one of the load_by_*_id methods are added here. Other
+    // in-folder messages pulled in for a conversation aren't added,
+    // since they may not be within the load window.
+    private Gee.SortedSet<EmailIdentifier> window =
+        new Gee.TreeSet<EmailIdentifier>((a,b) => {
+            return a.natural_sort_comparator(b);
+        });
+
 
     /**
      * Fired when a message load has started.
@@ -370,31 +390,7 @@ public class Geary.App.ConversationMonitor : BaseObject {
         return flags;
     }
 
-    /** Returns the lowest id of any seen email in the base folder. */
-    internal async Geary.EmailIdentifier? get_lowest_email_id_async()
-        throws Error {
-        Geary.EmailIdentifier? earliest_id = null;
-        if (!this.conversations.is_empty) {
-            // XXX this is the only caller of
-            // Folder.find_boundaries_async in the whole code base,
-            // and the amount of DB work it's doing in the case of
-            // MinimalFolder's implementation is
-            // staggering. ConversationSet should be simply
-            // maintaining an ordered list of message ids that exist
-            // in the base folder and use that instead, which will be
-            // faster and remove a whole lot of pointless code
-            // overhead in Folder implementations.
-            yield this.base_folder.find_boundaries_async(
-                conversations.get_email_identifiers(),
-                out earliest_id,
-                null,
-                this.operation_cancellable
-            );
-        }
-        return earliest_id;
-    }
-
-    /** Loads messages from the base folder by identifier range. */
+    /** Loads messages from the base folder into the window. */
     internal async int load_by_id_async(EmailIdentifier? initial_id,
                                         int count,
                                         Folder.ListFlags flags = Folder.ListFlags.NONE)
@@ -407,17 +403,22 @@ public class Geary.App.ConversationMonitor : BaseObject {
 
         int load_count = 0;
         try {
-            Gee.Collection<Geary.Email>? email =
+            Gee.Collection<Geary.Email>? messages =
                 yield this.base_folder.list_email_by_id_async(
                     initial_id, count, required_fields, flags,
                     this.operation_cancellable
                 );
 
-            if (email != null) {
-                load_count = email.size;
+            if (messages != null && !messages.is_empty) {
+                load_count = messages.size;
+
+                foreach (Email email in messages) {
+                    this.window.add(email.id);
+                }
+
+                yield process_email_async(messages, new ProcessJobContext(true));
             }
 
-            yield process_email_async(email, new ProcessJobContext(true));
         } catch (Error err) {
             notify_scan_completed();
             throw err;
@@ -426,7 +427,7 @@ public class Geary.App.ConversationMonitor : BaseObject {
         return load_count;
     }
 
-    /** Loads messages from the base folder by identifier. */
+    /** Loads messages from the base folder into the window. */
     internal async void load_by_sparse_id(Gee.Collection<EmailIdentifier> ids,
                                           Folder.ListFlags flags = Folder.ListFlags.NONE)
         throws Error {
@@ -437,12 +438,18 @@ public class Geary.App.ConversationMonitor : BaseObject {
         }
 
         try {
-            yield process_email_async(
+            Gee.Collection<Geary.Email>? messages =
                 yield this.base_folder.list_email_by_sparse_id_async(
                     ids, required_fields, flags, this.operation_cancellable
-                ),
-                new ProcessJobContext(true)
-            );
+                );
+
+            if (messages != null && !messages.is_empty) {
+                foreach (Email email in messages) {
+                    this.window.add(email.id);
+                }
+
+                yield process_email_async(messages, new ProcessJobContext(true));
+            }
         } catch (Error err) {
             notify_scan_completed();
             throw err;
@@ -521,6 +528,23 @@ public class Geary.App.ConversationMonitor : BaseObject {
         }
     }
 
+   /** Notifies of removed conversations and removes emails from the window. */
+   internal void removed(Gee.Collection<Conversation> removed,
+                         Gee.MultiMap<Conversation, Email> trimmed,
+                         Gee.Collection<EmailIdentifier>? base_folder_removed) {
+        foreach (Conversation conversation in trimmed.get_keys()) {
+            notify_conversation_trimmed(conversation, trimmed.get(conversation));
+        }
+
+        if (removed.size > 0) {
+            notify_conversations_removed(removed);
+        }
+
+        if (base_folder_removed != null) {
+            this.window.remove_all(base_folder_removed);
+        }
+    }
+
     /**
      * Check conversations to see if they still exist in the base folder.
      *
@@ -548,17 +572,6 @@ public class Geary.App.ConversationMonitor : BaseObject {
         return evaporated;
     }
 
-    internal void notify_emails_removed(Gee.Collection<Conversation> removed,
-                                        Gee.MultiMap<Conversation, Email> trimmed) {
-        foreach (Conversation conversation in trimmed.get_keys()) {
-            notify_conversation_trimmed(conversation, trimmed.get(conversation));
-        }
-
-        if (removed.size > 0) {
-            notify_conversations_removed(removed);
-        }
-    }
-
     protected virtual void notify_scan_started() {
         scan_started();
     }
diff --git a/src/engine/app/conversation-monitor/app-fill-window-operation.vala 
b/src/engine/app/conversation-monitor/app-fill-window-operation.vala
index 6927c4e..456680c 100644
--- a/src/engine/app/conversation-monitor/app-fill-window-operation.vala
+++ b/src/engine/app/conversation-monitor/app-fill-window-operation.vala
@@ -42,10 +42,8 @@ private class Geary.App.FillWindowOperation : ConversationOperation {
             num_to_load, this.monitor.base_folder.to_string()
         );
 
-        EmailIdentifier? earliest_id =
-            yield this.monitor.get_lowest_email_id_async();
         int loaded = yield this.monitor.load_by_id_async(
-            earliest_id, num_to_load
+            this.monitor.window_lowest, num_to_load
         );
 
         // Check to see if we need any more, but only if we actually
diff --git a/src/engine/app/conversation-monitor/app-insert-operation.vala 
b/src/engine/app/conversation-monitor/app-insert-operation.vala
index 9531de5..76664ba 100644
--- a/src/engine/app/conversation-monitor/app-insert-operation.vala
+++ b/src/engine/app/conversation-monitor/app-insert-operation.vala
@@ -20,9 +20,8 @@ private class Geary.App.InsertOperation : ConversationOperation {
     }
 
     public override async void execute_async() throws Error {
-        Geary.EmailIdentifier? lowest = yield this.monitor.get_lowest_email_id_async();
+        Geary.EmailIdentifier? lowest = this.monitor.window_lowest;
         Gee.Collection<EmailIdentifier>? to_insert = null;
-
         if (lowest != null) {
             to_insert = new Gee.LinkedList<EmailIdentifier>();
             foreach (EmailIdentifier inserted in this.inserted_ids) {
diff --git a/src/engine/app/conversation-monitor/app-remove-operation.vala 
b/src/engine/app/conversation-monitor/app-remove-operation.vala
index eb8311b..8f95ecb 100644
--- a/src/engine/app/conversation-monitor/app-remove-operation.vala
+++ b/src/engine/app/conversation-monitor/app-remove-operation.vala
@@ -43,7 +43,11 @@ private class Geary.App.RemoveOperation : ConversationOperation {
         }
 
         // Fire signals, clean up
-        this.monitor.notify_emails_removed(removed, trimmed);
+        this.monitor.removed(
+            removed,
+            trimmed,
+            (this.source_folder == this.monitor.base_folder) ? this.removed_ids : null
+        );
 
         // Check we still have enough conversations if any were
         // removed
diff --git a/src/engine/app/conversation-monitor/app-reseed-operation.vala 
b/src/engine/app/conversation-monitor/app-reseed-operation.vala
index 9beaf1e..c7a4a66 100644
--- a/src/engine/app/conversation-monitor/app-reseed-operation.vala
+++ b/src/engine/app/conversation-monitor/app-reseed-operation.vala
@@ -21,8 +21,7 @@ private class Geary.App.ReseedOperation : ConversationOperation {
     }
 
     public override async void execute_async() throws Error {
-        EmailIdentifier? earliest_id =
-            yield this.monitor.get_lowest_email_id_async();
+        EmailIdentifier? earliest_id = this.monitor.window_lowest;
         if (earliest_id != null) {
             debug("Reseeding starting from Email ID %s on opened %s",
                   earliest_id.to_string(), this.monitor.base_folder.to_string());
diff --git a/src/engine/imap-db/outbox/smtp-outbox-folder.vala 
b/src/engine/imap-db/outbox/smtp-outbox-folder.vala
index 51464bc..19ecdb0 100644
--- a/src/engine/imap-db/outbox/smtp-outbox-folder.vala
+++ b/src/engine/imap-db/outbox/smtp-outbox-folder.vala
@@ -282,34 +282,6 @@ private class Geary.SmtpOutboxFolder :
         yield remove_email_async(list, cancellable);
     }
 
-    public override async void find_boundaries_async(Gee.Collection<Geary.EmailIdentifier> ids,
-        out Geary.EmailIdentifier? low, out Geary.EmailIdentifier? high,
-        Cancellable? cancellable = null) throws Error {
-        SmtpOutboxEmailIdentifier? outbox_low = null;
-        SmtpOutboxEmailIdentifier? outbox_high = null;
-        yield db.exec_transaction_async(Db.TransactionType.RO, (cx) => {
-            foreach (Geary.EmailIdentifier id in ids) {
-                SmtpOutboxEmailIdentifier? outbox_id = id as SmtpOutboxEmailIdentifier;
-                if (outbox_id == null)
-                    continue;
-
-                OutboxRow? row = do_fetch_row_by_ordering(cx, outbox_id.ordering, cancellable);
-                if (row == null)
-                    continue;
-
-                if (outbox_low == null || outbox_id.ordering < outbox_low.ordering)
-                    outbox_low = outbox_id;
-                if (outbox_high == null || outbox_id.ordering > outbox_high.ordering)
-                    outbox_high = outbox_id;
-            }
-
-            return Db.TransactionOutcome.DONE;
-        }, cancellable);
-
-        low = outbox_low;
-        high = outbox_high;
-    }
-
     public override Geary.Folder.OpenState get_open_state() {
         return is_open() ? Geary.Folder.OpenState.LOCAL : Geary.Folder.OpenState.CLOSED;
     }
diff --git a/src/engine/imap-db/search/imap-db-search-folder.vala 
b/src/engine/imap-db/search/imap-db-search-folder.vala
index b9d46f3..5eb9338 100644
--- a/src/engine/imap-db/search/imap-db-search-folder.vala
+++ b/src/engine/imap-db/search/imap-db-search-folder.vala
@@ -48,25 +48,7 @@ private class Geary.ImapDB.SearchFolder : Geary.SearchFolder, Geary.FolderSuppor
                 exclude_folder(folder);
         }
     }
-    
-    public override async void find_boundaries_async(Gee.Collection<Geary.EmailIdentifier> ids,
-        out Geary.EmailIdentifier? low, out Geary.EmailIdentifier? high,
-        Cancellable? cancellable = null) throws Error {
-        low = null;
-        high = null;
-        
-        // This shouldn't require a result_mutex lock since there's no yield.
-        Gee.TreeSet<ImapDB.SearchEmailIdentifier> in_folder = Geary.traverse<Geary.EmailIdentifier>(ids)
-            .cast_object<ImapDB.SearchEmailIdentifier>()
-            .filter(id => id in search_results)
-            .to_tree_set();
-        
-        if (in_folder.size > 0) {
-            low = in_folder.first();
-            high = in_folder.last();
-        }
-    }
-    
+
     private async void append_new_email_async(Geary.SearchQuery query, Geary.Folder folder,
         Gee.Collection<Geary.EmailIdentifier> ids, Cancellable? cancellable) throws Error {
         int result_mutex_token = yield result_mutex.claim_async();
diff --git a/src/engine/imap-engine/imap-engine-minimal-folder.vala 
b/src/engine/imap-engine/imap-engine-minimal-folder.vala
index 3808642..40c99ae 100644
--- a/src/engine/imap-engine/imap-engine-minimal-folder.vala
+++ b/src/engine/imap-engine/imap-engine-minimal-folder.vala
@@ -1053,31 +1053,6 @@ private class Geary.ImapEngine.MinimalFolder : Geary.Folder, Geary.FolderSupport
         this.update_flags_timer.start();
     }
 
-    public override async void find_boundaries_async(Gee.Collection<Geary.EmailIdentifier> ids,
-        out Geary.EmailIdentifier? low, out Geary.EmailIdentifier? high,
-        Cancellable? cancellable = null) throws Error {
-        low = null;
-        high = null;
-        
-        Gee.MultiMap<Geary.EmailIdentifier, Geary.FolderPath>? map
-            = yield account.get_containing_folders_async(ids, cancellable);
-        
-        if (map != null) {
-            Gee.ArrayList<Geary.EmailIdentifier> in_folder = new Gee.ArrayList<Geary.EmailIdentifier>();
-            foreach (Geary.EmailIdentifier id in map.get_keys()) {
-                if (path in map.get(id))
-                    in_folder.add(id);
-            }
-            
-            if (in_folder.size > 0) {
-                Gee.SortedSet<Geary.EmailIdentifier> sorted = Geary.EmailIdentifier.sort(in_folder);
-                
-                low = sorted.first();
-                high = sorted.last();
-            }
-        }
-    }
-    
     private void on_email_complete(Gee.Collection<Geary.EmailIdentifier> email_ids) {
         notify_email_locally_complete(email_ids);
     }
diff --git a/test/engine/api/geary-folder-mock.vala b/test/engine/api/geary-folder-mock.vala
index e208e42..55b8e5b 100644
--- a/test/engine/api/geary-folder-mock.vala
+++ b/test/engine/api/geary-folder-mock.vala
@@ -71,16 +71,6 @@ public class Geary.MockFolder : Folder {
         throw new EngineError.UNSUPPORTED("Mock method");
     }
 
-    public override async void
-        find_boundaries_async(Gee.Collection<Geary.EmailIdentifier> ids,
-                          out Geary.EmailIdentifier? low,
-                          out Geary.EmailIdentifier? high,
-                          Cancellable? cancellable = null)
-    throws Error {
-        throw new EngineError.UNSUPPORTED("Mock method");
-    }
-
-
     public override async Gee.List<Geary.Email>?
         list_email_by_id_async(Geary.EmailIdentifier? initial_id,
                                int count,


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]