[geary/wip/713150-conversations] Further fixes, speed-up improvements
- From: Jim Nelson <jnelson src gnome org>
- To: commits-list gnome org
- Cc:
- Subject: [geary/wip/713150-conversations] Further fixes, speed-up improvements
- Date: Fri, 27 Feb 2015 02:25:14 +0000 (UTC)
commit 0d4161a69551ad8ae9e1a04e05fb2d261a75de7e
Author: Jim Nelson <jim yorba org>
Date: Thu Feb 26 17:13:14 2015 -0800
Further fixes, speed-up improvements
Searching for associated emails now merely returns EmailIdentifiers,
making it easy to strip out emails already loaded and only loading
unseen emails via a new Account local_list operation.
src/client/application/geary-controller.vala | 2 +-
src/client/composer/composer-widget.vala | 3 +-
src/engine/api/geary-account.vala | 16 ++++-
src/engine/api/geary-associated-emails.vala | 25 ++++----
src/engine/app/app-conversation-monitor.vala | 45 +++++++++-----
src/engine/imap-db/imap-db-account.vala | 63 ++++++++++++++------
src/engine/imap-db/imap-db-folder.vala | 27 ++++----
.../imap-engine/imap-engine-generic-account.vala | 12 +++-
8 files changed, 123 insertions(+), 70 deletions(-)
---
diff --git a/src/client/application/geary-controller.vala b/src/client/application/geary-controller.vala
index 59def5e..c60574c 100644
--- a/src/client/application/geary-controller.vala
+++ b/src/client/application/geary-controller.vala
@@ -55,7 +55,7 @@ public class GearyController : Geary.BaseObject {
public const string PROP_CURRENT_CONVERSATION ="current-conversations";
- public const int MIN_CONVERSATION_COUNT = 25;
+ public const int MIN_CONVERSATION_COUNT = 50;
private const string DELETE_MESSAGE_TOOLTIP_SINGLE = _("Delete conversation (Shift+Delete)");
private const string DELETE_MESSAGE_TOOLTIP_MULTIPLE = _("Delete conversations (Shift+Delete)");
diff --git a/src/client/composer/composer-widget.vala b/src/client/composer/composer-widget.vala
index f8a43c5..88c398f 100644
--- a/src/client/composer/composer-widget.vala
+++ b/src/client/composer/composer-widget.vala
@@ -635,7 +635,8 @@ public class ComposerWidget : Gtk.EventBox {
}
// TODO: Folder blacklist
- private bool local_search_predicate(Geary.EmailIdentifier email_id, bool only_partial,
+ // NOTE: This is called from a background thread.
+ private bool local_search_predicate(Geary.EmailIdentifier email_id, Geary.Email.Field fields,
Gee.Collection<Geary.FolderPath?> known_paths, Geary.EmailFlags flags) {
return !flags.contains(Geary.EmailFlags.DRAFT);
}
diff --git a/src/engine/api/geary-account.vala b/src/engine/api/geary-account.vala
index dcaa93c..b5f950d 100644
--- a/src/engine/api/geary-account.vala
+++ b/src/engine/api/geary-account.vala
@@ -42,7 +42,7 @@ public abstract class Geary.Account : BaseObject {
* It's possible (and likely) this will be called from the context of a background thread,
* so use appropriate locking.
*/
- public delegate bool EmailSearchPredicate(Geary.EmailIdentifier email_id, bool only_partial,
+ public delegate bool EmailSearchPredicate(Geary.EmailIdentifier email_id, Geary.Email.Field fields,
Gee.Collection<Geary.FolderPath?> known_paths, Geary.EmailFlags flags);
public Geary.AccountInformation information { get; protected set; }
@@ -363,8 +363,18 @@ public abstract class Geary.Account : BaseObject {
* locally.
*/
public abstract async Gee.Collection<Geary.AssociatedEmails>? local_search_associated_emails_async(
- Gee.Collection<Geary.EmailIdentifier> email_ids, Geary.Email.Field requested_fields,
- EmailSearchPredicate? search_predicate, Cancellable? cancellable = null) throws Error;
+ Gee.Collection<Geary.EmailIdentifier> email_ids, EmailSearchPredicate? search_predicate,
+ Cancellable? cancellable = null) throws Error;
+
+ /**
+ * Return a listing of local { link Email} fulfilling the required fields.
+ *
+ * This is akin to { link Folder.list_email_by_id_async} in that it doesn't throw an Error for
+ * unknown { link EmailIdentifiers}.
+ */
+ public abstract async Gee.Collection<Geary.Email>? local_list_email_async(
+ Gee.Collection<Geary.EmailIdentifier> email_ids, Geary.Email.Field required_fields,
+ Cancellable? cancellable = null) throws Error;
/**
* Return a single email fulfilling the required fields. The email to pull
diff --git a/src/engine/api/geary-associated-emails.vala b/src/engine/api/geary-associated-emails.vala
index 5581f2f..d84ca3f 100644
--- a/src/engine/api/geary-associated-emails.vala
+++ b/src/engine/api/geary-associated-emails.vala
@@ -5,39 +5,38 @@
*/
/**
- * An immutable representation of all known local { link Email}s which are associated with one
- * another due to their Message-ID, In-Reply-To, and References headers.
+ * An immutable representation of all known local { link EmailIdentifier}s which are associated with
+ * one another due to their Message-ID, In-Reply-To, and References headers.
*
* This object is free-form and does not impose any ordering or threading on the emails. It is
- * also not updated as new email arrives and email is removed.
+ * also not updated as new email arrives and email is removed. Treat it as a snapshot of the
+ * existing state of the local mail store.
*
* @see Account.local_search_associated_emails_async
- *
- * TODO: Necessary?
*/
public class Geary.AssociatedEmails : BaseObject {
/**
- * All associated { link Email}s.
+ * All associated { link EmailIdentifier}s.
*/
- public Gee.Collection<Geary.Email> emails { get; private set; }
+ public Gee.Collection<Geary.EmailIdentifier> email_ids { get; private set; }
/**
- * All known { link FolderPath}s for each { link Email}.
+ * All known { link FolderPath}s for each { link EmailIdentifier}.
*
* null if the Email is currently associated with no { link Folder}s.
*/
- public Gee.MultiMap<Geary.Email, Geary.FolderPath?> known_paths { get; private set; }
+ public Gee.MultiMap<Geary.EmailIdentifier, Geary.FolderPath?> known_paths { get; private set; }
public AssociatedEmails() {
- emails = new Gee.ArrayList<Email>();
+ email_ids = new Gee.ArrayList<EmailIdentifier>();
known_paths = new Gee.HashMultiMap<Email, FolderPath?>();
}
- public void add(Geary.Email email, Gee.Collection<Geary.FolderPath?> paths) {
- emails.add(email);
+ public void add(Geary.EmailIdentifier email_id, Gee.Collection<Geary.FolderPath?> paths) {
+ email_ids.add(email_id);
foreach (FolderPath path in paths)
- known_paths.set(email, path);
+ known_paths.set(email_id, path);
}
}
diff --git a/src/engine/app/app-conversation-monitor.vala b/src/engine/app/app-conversation-monitor.vala
index b186e9b..9b238a7 100644
--- a/src/engine/app/app-conversation-monitor.vala
+++ b/src/engine/app/app-conversation-monitor.vala
@@ -411,10 +411,10 @@ public class Geary.App.ConversationMonitor : BaseObject {
}
// NOTE: This is called from a background thread.
- private bool search_associated_predicate(EmailIdentifier email_id, bool only_partial,
+ private bool search_associated_predicate(EmailIdentifier email_id, Email.Field fields,
Gee.Collection<FolderPath?> known_paths, EmailFlags flags) {
// don't want partial emails
- if (only_partial)
+ if (!fields.fulfills(required_fields))
return false;
// if email is in this path, it's not blacklisted (i.e. if viewing the Spam folder, don't
@@ -456,7 +456,7 @@ public class Geary.App.ConversationMonitor : BaseObject {
Gee.Collection<AssociatedEmails>? associations = null;
try {
associations = yield folder.account.local_search_associated_emails_async(
- email_ids, required_fields, search_associated_predicate, null);
+ email_ids, search_associated_predicate, null);
} catch (Error err) {
debug("Unable to search for associated emails: %s", err.message);
}
@@ -470,18 +470,18 @@ public class Geary.App.ConversationMonitor : BaseObject {
Gee.HashMultiMap<Conversation, Email> appended = new Gee.HashMultiMap<Conversation, Email>();
foreach (AssociatedEmails association in associations) {
- // Get all EmailIdentifiers in association
- Gee.HashSet<EmailIdentifier> associated_ids = traverse<Email>(association.emails)
- .map<EmailIdentifier>(email => email.id)
- .to_hash_set();
-
// get all conversations for these emails (possible for multiple conversations to be
// started and then coalesce as new emails come in)
Gee.HashSet<Conversation> existing = new Gee.HashSet<Conversation>();
- foreach (EmailIdentifier associated_id in associated_ids) {
+ foreach (EmailIdentifier associated_id in association.email_ids) {
Conversation? conversation = all_email_id_to_conversation[associated_id];
if (conversation != null)
existing.add(conversation);
+
+ // only add to primary map if identifier is part of the original set of arguments
+ // and they're from this folder and not another one
+ if (email_ids.contains(associated_id) && path.equal_to(folder.path))
+ primary_email_id_to_conversation[associated_id] = conversation;
}
// Create or pick conversation for these emails
@@ -501,15 +501,26 @@ public class Geary.App.ConversationMonitor : BaseObject {
break;
}
+ // trim down the email identifiers we already have emails for
+ Gee.HashSet<EmailIdentifier> trimmed_ids = traverse<EmailIdentifier>(association.email_ids)
+ .filter(id => !all_email_id_to_conversation.has_key(id))
+ .to_hash_set();
+
+ // load remaining emails for the conversation objects
+ Gee.Collection<Email>? emails = null;
+ try {
+ emails = yield folder.account.local_list_email_async(trimmed_ids, required_fields, null);
+ } catch (Error err) {
+ debug("Unable to list local account email: %s", err.message);
+ }
+
+ if (emails == null || emails.size == 0)
+ continue;
+
// add all emails and each known path(s) to the Conversation and EmailIdentifier mapping
- foreach (Email email in association.emails) {
- conversation.add(email, association.known_paths[email]);
+ foreach (Email email in emails) {
+ conversation.add(email, association.known_paths[email.id]);
all_email_id_to_conversation[email.id] = conversation;
-
- // only add to primary map if identifier is part of the original set of arguments
- // and they're from this folder and not another one
- if (email_ids.contains(email.id) && path.equal_to(folder.path))
- primary_email_id_to_conversation[email.id] = conversation;
}
// if new, added, otherwise appended (if not already added)
@@ -517,7 +528,7 @@ public class Geary.App.ConversationMonitor : BaseObject {
conversations.add(conversation);
added.add(conversation);
} else if (!added.contains(conversation)) {
- foreach (Email email in association.emails)
+ foreach (Email email in emails)
appended.set(conversation, email);
}
}
diff --git a/src/engine/imap-db/imap-db-account.vala b/src/engine/imap-db/imap-db-account.vala
index 738510e..05ab9aa 100644
--- a/src/engine/imap-db/imap-db-account.vala
+++ b/src/engine/imap-db/imap-db-account.vala
@@ -680,26 +680,17 @@ private class Geary.ImapDB.Account : BaseObject {
}
public async Gee.Collection<Geary.AssociatedEmails>? search_associated_emails_async(
- Gee.Collection<Geary.EmailIdentifier> email_ids, Email.Field requested_fields,
- Geary.Account.EmailSearchPredicate? search_predicate, Cancellable? cancellable) throws Error {
+ Gee.Collection<ImapDB.EmailIdentifier> db_ids, Geary.Account.EmailSearchPredicate? search_predicate,
+ Cancellable? cancellable) throws Error {
check_open();
- // Cast all at once and report error if invalid found
- Gee.HashSet<ImapDB.EmailIdentifier> db_ids = new Gee.HashSet<ImapDB.EmailIdentifier>();
- foreach (Geary.EmailIdentifier email_id in email_ids) {
- ImapDB.EmailIdentifier? db_id = email_id as ImapDB.EmailIdentifier;
- if (db_id == null)
- throw new EngineError.BAD_PARAMETERS("Invalid identifier supplied to list conversation");
-
- db_ids.add(db_id);
- }
-
Gee.Collection<AssociatedEmails> associations = new Gee.ArrayList<AssociatedEmails>();
Gee.HashSet<ImapDB.EmailIdentifier> found_ids = new Gee.HashSet<ImapDB.EmailIdentifier>();
// Need flags for search predicate
+ Email.Field required_fields = Email.Field.NONE;
if (search_predicate != null)
- requested_fields = requested_fields | Geary.Email.Field.FLAGS;
+ required_fields = required_fields | Geary.Email.Field.FLAGS;
yield db.exec_transaction_async(Db.TransactionType.RO, (cx) => {
foreach (ImapDB.EmailIdentifier db_id in db_ids) {
@@ -718,15 +709,15 @@ private class Geary.ImapDB.Account : BaseObject {
foreach (ImapDB.EmailIdentifier associated_id in associated_ids) {
Email? email;
Gee.Collection<FolderPath?>? known_paths;
- do_fetch_message(cx, associated_id.message_id, requested_fields, search_predicate,
+ do_fetch_message(cx, associated_id.message_id, required_fields, search_predicate,
out email, out known_paths, cancellable);
if (email != null) {
- association.add(email, known_paths);
+ association.add(email.id, known_paths);
found_ids.add((ImapDB.EmailIdentifier) email.id);
}
}
- if (association.emails.size > 0)
+ if (association.email_ids.size > 0)
associations.add(association);
}
@@ -755,8 +746,7 @@ private class Geary.ImapDB.Account : BaseObject {
known_paths.add_all(folders);
// Allow caller to filter results in callback
- if (search_predicate != null
- && !search_predicate(email.id, !row.fields.fulfills(required_fields), known_paths,
email.email_flags)) {
+ if (search_predicate != null && !search_predicate(email.id, actual_fields, known_paths,
email.email_flags)) {
email = null;
known_paths = null;
}
@@ -1299,6 +1289,43 @@ private class Geary.ImapDB.Account : BaseObject {
return search_matches;
}
+ public async Gee.Collection<Email>? list_email_async(Gee.Collection<ImapDB.EmailIdentifier> email_ids,
+ Email.Field required_fields, Cancellable? cancellable) throws Error {
+ check_open();
+
+ if (email_ids.size == 0)
+ return null;
+
+ Gee.Collection<Email> emails = new Gee.ArrayList<Email>();
+ yield db.exec_transaction_async(Db.TransactionType.RO, (cx) => {
+ foreach (ImapDB.EmailIdentifier email_id in email_ids) {
+ try {
+ Email.Field fields;
+ MessageRow row = ImapDB.Folder.do_fetch_message_row(cx, email_id.message_id,
+ required_fields, out fields, cancellable);
+
+ if (!row.fields.fulfills(required_fields))
+ continue;
+
+ Email email = row.to_email(email_id);
+ ImapDB.Folder.do_add_attachments(cx, email, email_id.message_id, cancellable);
+
+ emails.add(email);
+ } catch (Error err) {
+ // ignore for list operation
+ if (err is EngineError.NOT_FOUND)
+ continue;
+
+ throw err;
+ }
+ }
+
+ return Db.TransactionOutcome.DONE;
+ }, cancellable);
+
+ return emails.size > 0 ? emails : null;
+ }
+
public async Geary.Email fetch_email_async(ImapDB.EmailIdentifier email_id,
Geary.Email.Field required_fields, Cancellable? cancellable = null) throws Error {
check_open();
diff --git a/src/engine/imap-db/imap-db-folder.vala b/src/engine/imap-db/imap-db-folder.vala
index 6fef755..38499ae 100644
--- a/src/engine/imap-db/imap-db-folder.vala
+++ b/src/engine/imap-db/imap-db-folder.vala
@@ -335,10 +335,7 @@ private class Geary.ImapDB.Folder : BaseObject, Geary.ReferenceSemantics {
stmt.bind_rowid(0, folder_id);
stmt.bind_int64(1, start_uid.value);
- locations = do_results_to_locations(stmt.exec(cancellable), flags, cancellable);
-
- if (locations.size > count)
- locations = locations.slice(0, count);
+ locations = do_results_to_locations(stmt.exec(cancellable), count, flags, cancellable);
return Db.TransactionOutcome.SUCCESS;
}, cancellable);
@@ -396,7 +393,7 @@ private class Geary.ImapDB.Folder : BaseObject, Geary.ReferenceSemantics {
stmt.bind_int64(1, start_uid.value);
stmt.bind_int64(2, end_uid.value);
- locations = do_results_to_locations(stmt.exec(cancellable), flags, cancellable);
+ locations = do_results_to_locations(stmt.exec(cancellable), int.MAX, flags, cancellable);
return Db.TransactionOutcome.SUCCESS;
}, cancellable);
@@ -440,7 +437,7 @@ private class Geary.ImapDB.Folder : BaseObject, Geary.ReferenceSemantics {
stmt.bind_int64(1, start_uid.value);
stmt.bind_int64(2, end_uid.value);
- locations = do_results_to_locations(stmt.exec(cancellable), flags, cancellable);
+ locations = do_results_to_locations(stmt.exec(cancellable), int.MAX, flags, cancellable);
return Db.TransactionOutcome.SUCCESS;
}, cancellable);
@@ -495,7 +492,7 @@ private class Geary.ImapDB.Folder : BaseObject, Geary.ReferenceSemantics {
Db.Statement stmt = cx.prepare(sql.str);
stmt.bind_rowid(0, folder_id);
- locations = do_results_to_locations(stmt.exec(cancellable), flags, cancellable);
+ locations = do_results_to_locations(stmt.exec(cancellable), int.MAX, flags, cancellable);
return Db.TransactionOutcome.SUCCESS;
}, cancellable);
@@ -2150,7 +2147,7 @@ private class Geary.ImapDB.Folder : BaseObject, Geary.ReferenceSemantics {
// Db.Result must include columns for "message_id", "ordering", and "remove_marker" from the
// MessageLocationTable
- private Gee.List<LocationIdentifier> do_results_to_locations(Db.Result results,
+ private Gee.List<LocationIdentifier> do_results_to_locations(Db.Result results, int count,
ListFlags flags, Cancellable? cancellable) throws Error {
Gee.List<LocationIdentifier> locations = new Gee.ArrayList<LocationIdentifier>();
@@ -2164,6 +2161,8 @@ private class Geary.ImapDB.Folder : BaseObject, Geary.ReferenceSemantics {
continue;
locations.add(location);
+ if (locations.size >= count)
+ break;
} while (results.next(cancellable));
return locations;
@@ -2258,8 +2257,8 @@ private class Geary.ImapDB.Folder : BaseObject, Geary.ReferenceSemantics {
Db.Statement stmt = cx.prepare(sql.str);
stmt.bind_rowid(0, folder_id);
- Gee.List<LocationIdentifier> locs = do_results_to_locations(stmt.exec(cancellable), flags,
- cancellable);
+ Gee.List<LocationIdentifier> locs = do_results_to_locations(stmt.exec(cancellable), int.MAX,
+ flags, cancellable);
return (locs.size > 0) ? locs : null;
}
@@ -2307,8 +2306,8 @@ private class Geary.ImapDB.Folder : BaseObject, Geary.ReferenceSemantics {
Db.Statement stmt = cx.prepare(sql.str);
stmt.bind_rowid(0, folder_id);
- Gee.List<LocationIdentifier> locs = do_results_to_locations(stmt.exec(cancellable), flags,
- cancellable);
+ Gee.List<LocationIdentifier> locs = do_results_to_locations(stmt.exec(cancellable), int.MAX,
+ flags, cancellable);
return (locs.size > 0) ? locs : null;
}
@@ -2322,8 +2321,8 @@ private class Geary.ImapDB.Folder : BaseObject, Geary.ReferenceSemantics {
""");
stmt.bind_rowid(0, folder_id);
- Gee.List<LocationIdentifier> locs = do_results_to_locations(stmt.exec(cancellable), flags,
- cancellable);
+ Gee.List<LocationIdentifier> locs = do_results_to_locations(stmt.exec(cancellable), int.MAX,
+ flags, cancellable);
return (locs.size > 0) ? locs : null;
}
diff --git a/src/engine/imap-engine/imap-engine-generic-account.vala
b/src/engine/imap-engine/imap-engine-generic-account.vala
index 8d1c9c6..8372b0c 100644
--- a/src/engine/imap-engine/imap-engine-generic-account.vala
+++ b/src/engine/imap-engine/imap-engine-generic-account.vala
@@ -859,12 +859,18 @@ private abstract class Geary.ImapEngine.GenericAccount : Geary.Account {
}
public override async Gee.Collection<Geary.AssociatedEmails>? local_search_associated_emails_async(
- Gee.Collection<Geary.EmailIdentifier> email_ids, Geary.Email.Field requested_fields,
- Account.EmailSearchPredicate? search_predicate, Cancellable? cancellable = null) throws Error {
- return yield local.search_associated_emails_async(email_ids, requested_fields, search_predicate,
+ Gee.Collection<Geary.EmailIdentifier> email_ids, Account.EmailSearchPredicate? search_predicate,
+ Cancellable? cancellable = null) throws Error {
+ return yield local.search_associated_emails_async(check_ids(email_ids), search_predicate,
cancellable);
}
+ public override async Gee.Collection<Geary.Email>? local_list_email_async(
+ Gee.Collection<Geary.EmailIdentifier> email_ids, Geary.Email.Field required_fields,
+ Cancellable? cancellable = null) throws Error {
+ return yield local.list_email_async(check_ids(email_ids), required_fields, cancellable);
+ }
+
public override async Geary.Email local_fetch_email_async(Geary.EmailIdentifier email_id,
Geary.Email.Field required_fields, Cancellable? cancellable = null) throws Error {
return yield local.fetch_email_async(check_id(email_id), required_fields, cancellable);
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]
