[geary] Cache contact list store per account. Bug 771903



commit 2c980036bd6b55efcad84917a96f9c02760f7670
Author: Kacper Bielecki <kazjote gmail com>
Date:   Sun Dec 11 14:30:59 2016 +0100

    Cache contact list store per account. Bug 771903
    
    This implements new cache for ContactListStore.
    
    ContactListStore is created only once per account when account is opened
    with GearyController::open_account. It is destroyed in
    GearyController::close_account.
    
    ContactListStoreCache class is introduced to manage ContactListStore
    instances.
    
    ComposerWidget receives ContactListStoreCache instance instead of
    ContactListStore directly in constructor.
    
    To increase performance, backwards compatibility breaking changes are
    introduced to Engine API Geary.ContactStore. Signals:
    
    * Gee.ContactStore::contact_added(Contact)
    * Gee.ContactStore::contact_updated(Contact)
    
    are replaced with batch equivalents:
    
    * Gee::ContactStore::contacts_added(Gee.Collection<Contact>)
    * Gee::ContactStore::contacts_updated(Gee.Collection<Contact>)
    
    Geary.ComposerWidget::load_entry_completions is no longer async as it does
    not involve time consuming ContactListStore creation.
    
    CONTACT_MARKUP_NAME column is removed from ContactListStore as it used to keep
    state about highlighted areas of text. This is not possible anymore as
    ContactListStore is shared between multiple ComposerWidgets.
    
    Highlight implementation has been moved to Geary.ContactEntryCompletion instead.
    
    Additionally contacts_loaded signal is emitted from Geary.ImapEngine.GenericAccount
    and propagated to Geary.Account
    
    Geary.ContactListStore sort function is set upon receiving contacts_loaded
    signal instead of after initial contacts are loaded. This speeds up Geary
    startup for users with long contact lists.

 po/POTFILES.in                                     |    1 +
 src/CMakeLists.txt                                 |    1 +
 src/client/application/geary-controller.vala       |   15 +++++-
 src/client/composer/composer-widget.vala           |   51 ++++++++++---------
 src/client/composer/contact-entry-completion.vala  |   26 ++++++++--
 src/client/composer/contact-list-store-cache.vala  |   29 +++++++++++
 src/client/composer/contact-list-store.vala        |   43 +++++------------
 src/engine/api/geary-account.vala                  |    2 +
 src/engine/api/geary-contact-store.vala            |   36 ++++++++------
 src/engine/imap-db/imap-db-account.vala            |    6 ++-
 .../imap-engine/imap-engine-generic-account.vala   |    1 +
 11 files changed, 134 insertions(+), 77 deletions(-)
---
diff --git a/po/POTFILES.in b/po/POTFILES.in
index dea321b..271ab79 100644
--- a/po/POTFILES.in
+++ b/po/POTFILES.in
@@ -44,6 +44,7 @@ src/client/composer/composer-widget.vala
 src/client/composer/composer-window.vala
 src/client/composer/contact-entry-completion.vala
 src/client/composer/contact-list-store.vala
+src/client/composer/contact-list-store-cache.vala
 src/client/composer/email-entry.vala
 src/client/composer/spell-check-popover.vala
 src/client/conversation-list/conversation-list-cell-renderer.vala
diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
index bb56e06..218f525 100644
--- a/src/CMakeLists.txt
+++ b/src/CMakeLists.txt
@@ -358,6 +358,7 @@ client/composer/composer-widget.vala
 client/composer/composer-window.vala
 client/composer/contact-entry-completion.vala
 client/composer/contact-list-store.vala
+client/composer/contact-list-store-cache.vala
 client/composer/email-entry.vala
 client/composer/spell-check-popover.vala
 
diff --git a/src/client/application/geary-controller.vala b/src/client/application/geary-controller.vala
index 040dede..8b541c1 100644
--- a/src/client/application/geary-controller.vala
+++ b/src/client/application/geary-controller.vala
@@ -82,6 +82,7 @@ public class GearyController : Geary.BaseObject {
     private Cancellable cancellable_context_dependent_buttons = new Cancellable();
     private Gee.HashMap<Geary.Account, Cancellable> inbox_cancellables
         = new Gee.HashMap<Geary.Account, Cancellable>();
+    private ContactListStoreCache contact_list_store_cache = new ContactListStoreCache();
     private Gee.Set<Geary.App.Conversation> selected_conversations = new 
Gee.HashSet<Geary.App.Conversation>();
     private Geary.App.Conversation? last_deleted_conversation = null;
     private Gee.LinkedList<ComposerWidget> composer_widgets = new Gee.LinkedList<ComposerWidget>();
@@ -471,9 +472,18 @@ public class GearyController : Geary.BaseObject {
         account.report_problem.connect(on_report_problem);
         account.email_removed.connect(on_account_email_removed);
         connect_account_async.begin(account, cancellable_open_account);
+
+        ContactListStore list_store = this.contact_list_store_cache.create(account.get_contact_store());
+        account.contacts_loaded.connect(list_store.set_sort_function);
     }
     
     private void close_account(Geary.Account account) {
+        Geary.ContactStore contact_store = account.get_contact_store();
+        ContactListStore list_store = this.contact_list_store_cache.get(contact_store);
+
+        account.contacts_loaded.disconnect(list_store.set_sort_function);
+        this.contact_list_store_cache.unset(account.get_contact_store());
+
         account.report_problem.disconnect(on_report_problem);
         account.email_removed.disconnect(on_account_email_removed);
         disconnect_account_async.begin(account);
@@ -2108,9 +2118,10 @@ public class GearyController : Geary.BaseObject {
 
         ComposerWidget widget;
         if (mailto != null) {
-            widget = new ComposerWidget.from_mailto(current_account, mailto, application.config);
+            widget = new ComposerWidget.from_mailto(current_account, contact_list_store_cache,
+                mailto, application.config);
         } else {
-            widget = new ComposerWidget(current_account, compose_type, application.config);
+            widget = new ComposerWidget(current_account, contact_list_store_cache, compose_type, 
application.config);
         }
         widget.destroy.connect(on_composer_widget_destroy);
         widget.link_activated.connect((uri) => { open_uri(uri); });
diff --git a/src/client/composer/composer-widget.vala b/src/client/composer/composer-widget.vala
index b12ff01..625e683 100644
--- a/src/client/composer/composer-widget.vala
+++ b/src/client/composer/composer-widget.vala
@@ -359,6 +359,8 @@ public class ComposerWidget : Gtk.EventBox {
     // Is the composer closing (e.g. saving a draft or sending)?
     private bool is_closing = false;
 
+    private ContactListStoreCache contact_list_store_cache;
+
     private ComposerContainer container {
         get { return (ComposerContainer) parent; }
     }
@@ -371,8 +373,12 @@ public class ComposerWidget : Gtk.EventBox {
     public signal void link_activated(string url);
 
 
-    public ComposerWidget(Geary.Account account, ComposeType compose_type, Configuration config) {
+    public ComposerWidget(Geary.Account account,
+                          ContactListStoreCache contact_list_store_cache,
+                          ComposeType compose_type,
+                          Configuration config) {
         this.account = account;
+        this.contact_list_store_cache = contact_list_store_cache;
         this.config = config;
         this.compose_type = compose_type;
         if (this.compose_type == ComposeType.NEW_MESSAGE)
@@ -513,6 +519,7 @@ public class ComposerWidget : Gtk.EventBox {
         this.composer_container.set_focus_chain(chain);
 
         update_composer_view();
+        load_entry_completions();
     }
 
     public override void destroy() {
@@ -522,8 +529,10 @@ public class ComposerWidget : Gtk.EventBox {
         base.destroy();
     }
 
-    public ComposerWidget.from_mailto(Geary.Account account, string mailto, Configuration config) {
-        this(account, ComposeType.NEW_MESSAGE, config);
+    public ComposerWidget.from_mailto(Geary.Account account,
+        ContactListStoreCache contact_list_store_cache, string mailto, Configuration config) {
+
+        this(account, contact_list_store_cache, ComposeType.NEW_MESSAGE, config);
         
         Gee.HashMultiMap<string, string> headers = new Gee.HashMultiMap<string, string>();
         if (mailto.length > Geary.ComposedEmail.MAILTO_SCHEME.length) {
@@ -614,34 +623,27 @@ public class ComposerWidget : Gtk.EventBox {
         } catch (Error e) {
             debug("Could not open draft manager: %s", e.message);
         }
-
-        // For accounts with large numbers of contacts, loading the
-        // entry completions can some time, so do it after the UI has
-        // been shown
-        yield load_entry_completions();
     }
 
     /**
      * Loads and sets contact auto-complete data for the current account.
      */
-    private async void load_entry_completions() {
-        // XXX Since ContactListStore hooks into ContactStore to
-        // listen for contacts being added and removed,
-        // GearyController or some composer-related controller should
-        // construct an instance per account and keep it around for
-        // the lifetime of the app, since there can be tens of
-        // thousands of contacts for large accounts.
+    private void load_entry_completions() {
         Geary.ContactStore contacts = this.account.get_contact_store();
         if (this.contact_list_store == null ||
             this.contact_list_store.contact_store != contacts) {
-            ContactListStore store = new ContactListStore(contacts);
-            this.contact_list_store = store;
-            yield store.load();
+            ContactListStore? store = this.contact_list_store_cache.get(contacts);
 
-            this.to_entry.completion = new ContactEntryCompletion(store);
-            this.cc_entry.completion = new ContactEntryCompletion(store);
-            this.bcc_entry.completion = new ContactEntryCompletion(store);
-            this.reply_to_entry.completion = new ContactEntryCompletion(store);
+            if (store == null) {
+                error("Error loading contact_list_store from cache");
+            } else {
+                this.contact_list_store = store;
+
+                this.to_entry.completion = new ContactEntryCompletion(store);
+                this.cc_entry.completion = new ContactEntryCompletion(store);
+                this.bcc_entry.completion = new ContactEntryCompletion(store);
+                this.reply_to_entry.completion = new ContactEntryCompletion(store);
+            }
         }
     }
 
@@ -2074,7 +2076,8 @@ public class ComposerWidget : Gtk.EventBox {
         this.load_signature.begin(null, (obj, res) => {
                 this.editor.update_signature(this.load_signature.end(res));
             });
-        load_entry_completions.begin();
+
+        load_entry_completions();
 
         return true;
     }
diff --git a/src/client/composer/contact-entry-completion.vala 
b/src/client/composer/contact-entry-completion.vala
index 2f3d24f..dafa0d0 100644
--- a/src/client/composer/contact-entry-completion.vala
+++ b/src/client/composer/contact-entry-completion.vala
@@ -23,8 +23,6 @@ public class ContactEntryCompletion : Gtk.EntryCompletion {
         if (!contacts.match_prefix_contact(current_address_key, contact, out highlighted_result))
             return false;
 
-        contacts.list_store.set_highlighted_result(iter, highlighted_result, current_address_key);
-
         return true;
     }
 
@@ -39,12 +37,32 @@ public class ContactEntryCompletion : Gtk.EntryCompletion {
 
         Gtk.CellRendererText text_renderer = new Gtk.CellRendererText();
         pack_start(text_renderer, true);
-        add_attribute(text_renderer, "markup", ContactListStore.Column.CONTACT_MARKUP_NAME);
-        
+        set_cell_data_func(text_renderer, cell_layout_data_func);
+
         set_inline_selection(true);
         match_selected.connect(on_match_selected);
         cursor_on_match.connect(on_cursor_on_match);
     }
+
+    private void cell_layout_data_func(Gtk.CellLayout cell_layout, Gtk.CellRenderer cell,
+        Gtk.TreeModel tree_model, Gtk.TreeIter iter) {
+        string highlighted_result = "";
+
+        GLib.Value contact_value;
+        tree_model.get_value(iter, ContactListStore.Column.CONTACT_OBJECT, out contact_value);
+
+        Geary.Contact? contact = (Geary.Contact) contact_value.get_object();
+
+        if (contact != null) {
+            string current_address_key;
+            this.get_addresses(this, null, out current_address_key);
+
+            this.match_prefix_contact(current_address_key, contact, out highlighted_result);
+        }
+
+        Gtk.CellRendererText text_renderer = (Gtk.CellRendererText) cell;
+        text_renderer.markup = highlighted_result;
+    }
     
     private bool on_match_selected(Gtk.EntryCompletion sender, Gtk.TreeModel model, Gtk.TreeIter iter) {
         string full_address = list_store.get_rfc822_string(iter);
diff --git a/src/client/composer/contact-list-store-cache.vala 
b/src/client/composer/contact-list-store-cache.vala
new file mode 100644
index 0000000..4f8aa6c
--- /dev/null
+++ b/src/client/composer/contact-list-store-cache.vala
@@ -0,0 +1,29 @@
+/* Copyright 2016 Software Freedom Conservancy Inc.
+ *
+ * This software is licensed under the GNU Lesser General Public License
+ * (version 2.1 or later).  See the COPYING file in this distribution.
+ */
+
+public class ContactListStoreCache {
+
+    private Gee.HashMap<Geary.ContactStore, ContactListStore> cache =
+        new Gee.HashMap<Geary.ContactStore, ContactListStore>();
+
+    public ContactListStore create(Geary.ContactStore contact_store) {
+        ContactListStore list_store = new ContactListStore(contact_store);
+
+        this.cache.set(contact_store, list_store);
+
+        list_store.load.begin();
+
+        return list_store;
+    }
+
+    public ContactListStore? get(Geary.ContactStore contact_store) {
+        return this.cache.get(contact_store);
+    }
+
+    public void unset(Geary.ContactStore contact_store) {
+        this.cache.unset(contact_store);
+    }
+}
diff --git a/src/client/composer/contact-list-store.vala b/src/client/composer/contact-list-store.vala
index 1e11dbc..54d945c 100644
--- a/src/client/composer/contact-list-store.vala
+++ b/src/client/composer/contact-list-store.vala
@@ -54,13 +54,11 @@ public class ContactListStore : Gtk.ListStore {
 
     public enum Column {
         CONTACT_OBJECT,
-        CONTACT_MARKUP_NAME,
         PRIOR_KEYS;
         
         public static Type[] get_types() {
             return {
                 typeof (Geary.Contact), // CONTACT_OBJECT
-                typeof (string),        // CONTACT_MARKUP_NAME
                 typeof (Gee.HashSet)    // PRIOR_KEYS
             };
         }
@@ -71,13 +69,13 @@ public class ContactListStore : Gtk.ListStore {
     public ContactListStore(Geary.ContactStore contact_store) {
         set_column_types(Column.get_types());
         this.contact_store = contact_store;
-        contact_store.contact_added.connect(on_contact_added);
-        contact_store.contact_updated.connect(on_contact_updated);
+        contact_store.contacts_added.connect(on_contacts_added);
+        contact_store.contacts_updated.connect(on_contacts_updated);
     }
 
     ~ContactListStore() {
-        this.contact_store.contact_added.disconnect(on_contact_added);
-        this.contact_store.contact_updated.disconnect(on_contact_updated);
+        this.contact_store.contacts_added.disconnect(on_contacts_added);
+        this.contact_store.contacts_updated.disconnect(on_contacts_updated);
     }
 
     /**
@@ -93,8 +91,9 @@ public class ContactListStore : Gtk.ListStore {
                 yield;
             }
         }
+    }
 
-        // set sort function *after* adding all the contacts
+    public void set_sort_function() {
         set_sort_func(Column.CONTACT_OBJECT, ContactListStore.sort_func);
         set_sort_column_id(Column.CONTACT_OBJECT, Gtk.SortType.ASCENDING);
     }
@@ -109,33 +108,13 @@ public class ContactListStore : Gtk.ListStore {
     public string get_rfc822_string(Gtk.TreeIter iter) {
         return get_contact(iter).get_rfc822_address().to_rfc822_string();
     }
-    
-    // Highlighted result should be Markup.escaped for presentation to the user
-    public void set_highlighted_result(Gtk.TreeIter iter, string highlighted_result,
-        string current_address_key) {
-        // get the previous keys for this row for comparison
-        GLib.Value prior_keys_value;
-        get_value(iter, Column.PRIOR_KEYS, out prior_keys_value);
-        Gee.HashSet<string> prior_keys = (Gee.HashSet<string>) prior_keys_value.get_object();
-        
-        // Changing a row in the list store causes Gtk.EntryCompletion to re-evaluate
-        // completion_match_func for that row. Thus we need to make sure the key has
-        // actually changed before settings the highlighting--otherwise we will cause
-        // an infinite loop.
-        if (!(current_address_key in prior_keys)) {
-            prior_keys.add(current_address_key);
-            set(iter, Column.CONTACT_MARKUP_NAME, highlighted_result, -1);
-        }
-    }
 
     private inline void add_contact(Geary.Contact contact) {
         if (contact.highest_importance >= CONTACT_VISIBILITY_THRESHOLD) {
-            string full_address = contact.get_rfc822_address().to_rfc822_string();
             Gtk.TreeIter iter;
             append(out iter);
             set(iter,
                 Column.CONTACT_OBJECT, contact,
-                Column.CONTACT_MARKUP_NAME, Markup.escape_text(full_address),
                 Column.PRIOR_KEYS, new Gee.HashSet<string>());
         }
     }
@@ -157,12 +136,14 @@ public class ContactListStore : Gtk.ListStore {
         } while (iter_next(ref iter));
     }
     
-    private void on_contact_added(Geary.Contact contact) {
-        add_contact(contact);
+    private void on_contacts_added(Gee.Collection<Geary.Contact> contacts) {
+        foreach (Geary.Contact contact in contacts)
+            add_contact(contact);
     }
     
-    private void on_contact_updated(Geary.Contact contact) {
-        update_contact(contact);
+    private void on_contacts_updated(Gee.Collection<Geary.Contact> contacts) {
+        foreach (Geary.Contact contact in contacts)
+            update_contact(contact);
     }
 
 }
diff --git a/src/engine/api/geary-account.vala b/src/engine/api/geary-account.vala
index b8eedcd..b6f9f41 100644
--- a/src/engine/api/geary-account.vala
+++ b/src/engine/api/geary-account.vala
@@ -47,6 +47,8 @@ public abstract class Geary.Account : BaseObject {
     public signal void email_sent(Geary.RFC822.Message rfc822);
     
     public signal void report_problem(Geary.Account.Problem problem, Error? err);
+
+    public signal void contacts_loaded();
     
     /**
      * Fired when folders become available or unavailable in the account.
diff --git a/src/engine/api/geary-contact-store.vala b/src/engine/api/geary-contact-store.vala
index fd70936..1daef39 100644
--- a/src/engine/api/geary-contact-store.vala
+++ b/src/engine/api/geary-contact-store.vala
@@ -11,17 +11,34 @@ public abstract class Geary.ContactStore : BaseObject {
     
     private Gee.Map<string, Contact> contact_map;
     
-    public signal void contact_added(Contact contact);
+    public signal void contacts_added(Gee.Collection<Contact> contacts);
     
-    public signal void contact_updated(Contact contact);
+    public signal void contacts_updated(Gee.Collection<Contact> contacts);
     
     internal ContactStore() {
         contact_map = new Gee.HashMap<string, Contact>();
     }
     
     public void update_contacts(Gee.Collection<Contact> new_contacts) {
-        foreach (Contact contact in new_contacts)
-            update_contact(contact);
+        Gee.LinkedList<Contact> added = new Gee.LinkedList<Contact>();
+        Gee.LinkedList<Contact> updated = new Gee.LinkedList<Contact>();
+
+        foreach (Contact contact in new_contacts) {
+            Contact? old_contact = contact_map[contact.normalized_email];
+            if (old_contact == null) {
+                contact_map[contact.normalized_email] = contact;
+                added.add(contact);
+            } else if (old_contact.highest_importance < contact.highest_importance) {
+                old_contact.highest_importance = contact.highest_importance;
+                updated.add(contact);
+            }
+        }
+
+        if (!added.is_empty)
+            contacts_added(added);
+
+        if (!updated.is_empty)
+            contacts_updated(updated);
     }
     
     public abstract async void mark_contacts_async(Gee.Collection<Contact> contacts, ContactFlags? to_add,
@@ -30,15 +47,4 @@ public abstract class Geary.ContactStore : BaseObject {
     public Contact? get_by_rfc822(Geary.RFC822.MailboxAddress address) {
         return contact_map[address.address.normalize().casefold()];
     }
-    
-    private void update_contact(Contact contact) {
-        Contact? old_contact = contact_map[contact.normalized_email];
-        if (old_contact == null) {
-            contact_map[contact.normalized_email] = contact;
-            contact_added(contact);
-        } else if (old_contact.highest_importance < contact.highest_importance) {
-            old_contact.highest_importance = contact.highest_importance;
-            contact_updated(old_contact);
-        }
-    }
 }
diff --git a/src/engine/imap-db/imap-db-account.vala b/src/engine/imap-db/imap-db-account.vala
index 86ce37c..870f63a 100644
--- a/src/engine/imap-db/imap-db-account.vala
+++ b/src/engine/imap-db/imap-db-account.vala
@@ -60,6 +60,8 @@ private class Geary.ImapDB.Account : BaseObject {
         new Gee.HashMap<string, string>();
 
     public signal void email_sent(Geary.RFC822.Message rfc822);
+
+    public signal void contacts_loaded();
     
     // Only available when the Account is opened
     public SmtpOutboxFolder? outbox { get; private set; default = null; }
@@ -607,8 +609,10 @@ private class Geary.ImapDB.Account : BaseObject {
             return Db.TransactionOutcome.DONE;
         }, cancellable);
         
-        if (outcome == Db.TransactionOutcome.DONE)
+        if (outcome == Db.TransactionOutcome.DONE) {
             contact_store.update_contacts(contacts);
+            contacts_loaded();
+        }
     }
     
     public async Gee.Collection<Geary.ImapDB.Folder> list_folders_async(Geary.FolderPath? parent,
diff --git a/src/engine/imap-engine/imap-engine-generic-account.vala 
b/src/engine/imap-engine/imap-engine-generic-account.vala
index 5625c26..fdadb40 100644
--- a/src/engine/imap-engine/imap-engine-generic-account.vala
+++ b/src/engine/imap-engine/imap-engine-generic-account.vala
@@ -45,6 +45,7 @@ private abstract class Geary.ImapEngine.GenericAccount : Geary.Account {
         
         this.remote.login_failed.connect(on_login_failed);
         this.local.email_sent.connect(on_email_sent);
+        this.local.contacts_loaded.connect(() => { contacts_loaded(); });
         
         search_upgrade_monitor = local.search_index_monitor;
         db_upgrade_monitor = local.upgrade_monitor;


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