[geary/wip/713150-conversations] Optimization: get associations and load emails in one call
- From: Jim Nelson <jnelson src gnome org>
- To: commits-list gnome org
- Cc:
- Subject: [geary/wip/713150-conversations] Optimization: get associations and load emails in one call
- Date: Fri, 27 Mar 2015 00:38:46 +0000 (UTC)
commit 0e4423a44c6fdb2abc54fd7eadb7ee830ac2fe15
Author: Jim Nelson <jim yorba org>
Date: Thu Mar 26 17:38:22 2015 -0700
Optimization: get associations and load emails in one call
src/client/application/geary-controller.vala | 3 +
src/engine/api/geary-account.vala | 4 +-
src/engine/api/geary-associated-emails.vala | 34 +++++++++++--
.../api/geary-folder-supports-associations.vala | 3 +-
src/engine/app/app-conversation-monitor.vala | 51 +++++++-------------
src/engine/imap-db/imap-db-account.vala | 6 +-
src/engine/imap-db/imap-db-conversation.vala | 16 ++++--
src/engine/imap-db/imap-db-folder.vala | 6 +-
.../imap-engine/imap-engine-generic-account.vala | 8 ++--
.../imap-engine/imap-engine-minimal-folder.vala | 6 +-
10 files changed, 76 insertions(+), 61 deletions(-)
---
diff --git a/src/client/application/geary-controller.vala b/src/client/application/geary-controller.vala
index dfc9c2e..4c54249 100644
--- a/src/client/application/geary-controller.vala
+++ b/src/client/application/geary-controller.vala
@@ -1579,6 +1579,9 @@ public class GearyController : Geary.BaseObject {
int list_height = main_window.conversation_list_view.get_allocated_height();
int cells = (list_height / cell_dimensions.cell_height) + 1;
+ // add one LOAD_MORE_PERCENTAGE, as though the user had requested one load more already
+ cells += (int) Math.round((double) cells * LOAD_MORE_PERCENTAGE);
+
if (current_conversations.min_window_count < cells)
current_conversations.min_window_count = cells;
}
diff --git a/src/engine/api/geary-account.vala b/src/engine/api/geary-account.vala
index 204a5c3..e6f63d8 100644
--- a/src/engine/api/geary-account.vala
+++ b/src/engine/api/geary-account.vala
@@ -404,8 +404,8 @@ 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, EmailSearchPredicate? search_predicate,
- Cancellable? cancellable = null) throws Error;
+ Gee.Collection<Geary.EmailIdentifier> email_ids, Geary.Email.Field required_fields,
+ EmailSearchPredicate? search_predicate, Cancellable? cancellable = null) throws Error;
/**
* Return a listing of local { link Email} fulfilling the required fields.
diff --git a/src/engine/api/geary-associated-emails.vala b/src/engine/api/geary-associated-emails.vala
index d84ca3f..d64a254 100644
--- a/src/engine/api/geary-associated-emails.vala
+++ b/src/engine/api/geary-associated-emails.vala
@@ -19,7 +19,14 @@ public class Geary.AssociatedEmails : BaseObject {
/**
* All associated { link EmailIdentifier}s.
*/
- public Gee.Collection<Geary.EmailIdentifier> email_ids { get; private set; }
+ public Gee.Set<Geary.EmailIdentifier> email_ids { get; private set; }
+
+ /**
+ * All associated { link Email}s with { link required_fields} fulfilled, if possible.
+ *
+ * It's possible for the Email to have ''more'' than the required fields as well.
+ */
+ public Gee.Map<Geary.EmailIdentifier, Geary.Email> emails { get; private set; }
/**
* All known { link FolderPath}s for each { link EmailIdentifier}.
@@ -28,15 +35,30 @@ public class Geary.AssociatedEmails : BaseObject {
*/
public Gee.MultiMap<Geary.EmailIdentifier, Geary.FolderPath?> known_paths { get; private set; }
- public AssociatedEmails() {
- email_ids = new Gee.ArrayList<EmailIdentifier>();
+ /**
+ * The required { link Geary.Email.Field}s specified when the { link AssociatedEmails} was
+ * generated.
+ */
+ public Geary.Email.Field required_fields { get; private set; }
+
+ public AssociatedEmails(Geary.Email.Field required_fields) {
+ this.required_fields = required_fields;
+
+ email_ids = new Gee.HashSet<EmailIdentifier>();
+ emails = new Gee.HashMap<EmailIdentifier, Email>();
known_paths = new Gee.HashMultiMap<Email, FolderPath?>();
}
- public void add(Geary.EmailIdentifier email_id, Gee.Collection<Geary.FolderPath?> paths) {
- email_ids.add(email_id);
+ /**
+ * Add the { link Email} to the set of { link AssociatedEmails}.
+ *
+ * No checking is performed to ensure the Email fulfills { link required_fields}.
+ */
+ public void add(Geary.Email email, Gee.Collection<Geary.FolderPath?> paths) {
+ email_ids.add(email.id);
+ emails.set(email.id, email);
foreach (FolderPath path in paths)
- known_paths.set(email_id, path);
+ known_paths.set(email.id, path);
}
}
diff --git a/src/engine/api/geary-folder-supports-associations.vala
b/src/engine/api/geary-folder-supports-associations.vala
index 9886972..20a5685 100644
--- a/src/engine/api/geary-folder-supports-associations.vala
+++ b/src/engine/api/geary-folder-supports-associations.vala
@@ -52,7 +52,8 @@ public interface Geary.FolderSupport.Associations : Geary.Folder {
* @see Geary.Folder.find_boundaries_async
*/
public abstract async Gee.Collection<Geary.AssociatedEmails>? local_list_associated_emails_async(
- Geary.EmailIdentifier? initial_id, int count, Geary.Account.EmailSearchPredicate? predicate,
+ Geary.EmailIdentifier? initial_id, int count, Geary.Email.Field required_fields,
+ Geary.Account.EmailSearchPredicate? predicate,
Gee.Collection<Geary.EmailIdentifier>? primary_loaded_ids,
Gee.Collection<Geary.EmailIdentifier>? already_seen_ids, Cancellable? cancellable = null)
throws Error;
diff --git a/src/engine/app/app-conversation-monitor.vala b/src/engine/app/app-conversation-monitor.vala
index 448d315..43b8a03 100644
--- a/src/engine/app/app-conversation-monitor.vala
+++ b/src/engine/app/app-conversation-monitor.vala
@@ -19,8 +19,8 @@ public class Geary.App.ConversationMonitor : BaseObject {
* These are the fields Conversations require to thread emails together. These fields will
* be retrieved regardless of the Field parameter passed to the constructor.
*/
- public const Geary.Email.Field REQUIRED_FIELDS = Geary.Email.Field.REFERENCES |
- Geary.Email.Field.FLAGS | Geary.Email.Field.DATE;
+ public const Geary.Email.Field REQUIRED_FIELDS =
+ Geary.Email.Field.FLAGS | Geary.Email.Field.DATE | Geary.Email.Field.PROPERTIES;
// An approximate multipler of messages-in-folder-to-conversation ... because the Engine doesn't
// offer a way to directly load conversations in a folder as a vector unto itself, this class
@@ -501,7 +501,7 @@ public class Geary.App.ConversationMonitor : BaseObject {
Gee.Collection<EmailIdentifier> primary_email_ids = new Gee.HashSet<EmailIdentifier>();
try {
associations = yield supports_associations.local_list_associated_emails_async(
- low_id, count, predicate_instance.search_predicate, primary_email_ids,
+ low_id, count, required_fields, predicate_instance.search_predicate, primary_email_ids,
all_email_id_to_conversation.keys, cancellable);
} catch (Error err) {
debug("Unable to load associated emails from %s: %s", supports_associations.to_string(),
@@ -510,7 +510,7 @@ public class Geary.App.ConversationMonitor : BaseObject {
}
if (associations != null && associations.size > 0) {
- yield process_associations_async(supports_associations.path, associations, primary_email_ids,
+ process_associations(supports_associations.path, associations, primary_email_ids,
cancellable);
}
@@ -544,7 +544,7 @@ public class Geary.App.ConversationMonitor : BaseObject {
Gee.Collection<AssociatedEmails>? associations = null;
try {
associations = yield folder.account.local_search_associated_emails_async(
- email_ids, predicate_instance.search_predicate, cancellable);
+ email_ids, required_fields, predicate_instance.search_predicate, cancellable);
} catch (Error err) {
debug("Unable to search for associated emails: %s", err.message);
}
@@ -552,20 +552,20 @@ public class Geary.App.ConversationMonitor : BaseObject {
if (associations == null || associations.size == 0)
return;
- yield process_associations_async(path, associations, email_ids, cancellable);
+ process_associations(path, associations, email_ids, cancellable);
Logging.debug(Logging.Flag.CONVERSATIONS, "[%s] ConversationMonitor::process_email completed: %d
emails",
folder.to_string(), email_ids.size);
}
- private async void process_associations_async(FolderPath path, Gee.Collection<AssociatedEmails>
associations,
+ private void process_associations(FolderPath path, Gee.Collection<AssociatedEmails> associations,
Gee.Collection<Geary.EmailIdentifier> original_email_ids, Cancellable? cancellable) {
Gee.HashSet<Conversation> added = new Gee.HashSet<Conversation>();
Gee.HashMultiMap<Conversation, Email> appended = new Gee.HashMultiMap<Conversation, Email>();
Gee.HashMultiMap<Conversation, EmailIdentifier> paths_changed = new Gee.HashMultiMap<Conversation,
EmailIdentifier>();
foreach (AssociatedEmails association in associations) {
- yield process_association_async(association, path, original_email_ids, added, appended,
- paths_changed, cancellable);
+ process_association(association, path, original_email_ids, added, appended, paths_changed,
+ cancellable);
}
if (added.size > 0) {
@@ -591,7 +591,7 @@ public class Geary.App.ConversationMonitor : BaseObject {
notify_email_paths_changed(conversation, paths_changed.get(conversation));
}
- private async void process_association_async(AssociatedEmails association, FolderPath path,
+ private void process_association(AssociatedEmails association, FolderPath path,
Gee.Collection<EmailIdentifier> original_email_ids, Gee.Set<Conversation> added,
Gee.MultiMap<Conversation, Email> appended, Gee.MultiMap<Conversation, EmailIdentifier>
paths_changed,
Cancellable? cancellable) {
@@ -633,15 +633,17 @@ public class Geary.App.ConversationMonitor : BaseObject {
// for all ids, add known paths and associate id to conversation in the various tables,
// since the paths and conversation may have changed
- //
- // if email is not loaded, add to list for loading
- Gee.HashSet<EmailIdentifier> list_email_ids = new Gee.HashSet<EmailIdentifier>();
+ Gee.Collection<Email> emails_appended = new Gee.ArrayList<Email>();
foreach (EmailIdentifier associated_id in association.email_ids) {
if (conversation.has_email(associated_id)) {
if (conversation.add_paths(associated_id, association.known_paths[associated_id]))
paths_changed.set(conversation, associated_id);
} else {
- list_email_ids.add(associated_id);
+ // don't worry if add() reports paths_changed, because we've already checked if the
+ // email was known, don't want to report "email-paths-changed" for a new add
+ Geary.Email email = association.emails[associated_id];
+ conversation.add(email, association.known_paths[associated_id], null);
+ emails_appended.add(email);
}
all_email_id_to_conversation[associated_id] = conversation;
@@ -652,29 +654,12 @@ public class Geary.App.ConversationMonitor : BaseObject {
primary_email_id_to_conversation[associated_id] = conversation;
}
- // load remaining emails for the conversation objects
- Gee.Collection<Email>? emails = null;
- try {
- emails = yield folder.account.local_list_email_async(list_email_ids, required_fields,
- cancellable);
- } catch (Error err) {
- debug("Unable to list local account email: %s", err.message);
- }
-
- // add all emails and each known path(s) to the Conversation and EmailIdentifier mapping
- // ... don't worry if add() reports paths_changed, because we've already checked if the
- // email was known, don't want to report "email-paths-changed" for a new add
- if (emails != null) {
- foreach (Email email in emails)
- conversation.add(email, association.known_paths[email.id], null);
- }
-
// if new, added, otherwise appended (if not already added)
if (!conversations.contains(conversation)) {
conversations.add(conversation);
added.add(conversation);
- } else if (!added.contains(conversation) && emails != null) {
- foreach (Email email in emails)
+ } else if (!added.contains(conversation) && emails_appended.size > 0) {
+ foreach (Email email in emails_appended)
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 ed1273f..b81bc6e 100644
--- a/src/engine/imap-db/imap-db-account.vala
+++ b/src/engine/imap-db/imap-db-account.vala
@@ -680,8 +680,8 @@ private class Geary.ImapDB.Account : BaseObject {
}
public async Gee.Collection<Geary.AssociatedEmails>? search_associated_emails_async(
- Gee.Collection<ImapDB.EmailIdentifier> db_ids, Geary.Account.EmailSearchPredicate? search_predicate,
- Cancellable? cancellable) throws Error {
+ Gee.Collection<ImapDB.EmailIdentifier> db_ids, Email.Field required_fields,
+ Geary.Account.EmailSearchPredicate? search_predicate, Cancellable? cancellable) throws Error {
check_open();
Gee.Collection<AssociatedEmails> associations = new Gee.ArrayList<AssociatedEmails>();
@@ -701,7 +701,7 @@ private class Geary.ImapDB.Account : BaseObject {
found_ids.add_all(associated_ids);
AssociatedEmails? association = Conversation.do_generate_associations(cx, associated_ids,
- search_predicate, cancellable);
+ required_fields, search_predicate, cancellable);
if (association != null && association.email_ids.size > 0)
associations.add(association);
}
diff --git a/src/engine/imap-db/imap-db-conversation.vala b/src/engine/imap-db/imap-db-conversation.vala
index 1d6a8e3..905a86c 100644
--- a/src/engine/imap-db/imap-db-conversation.vala
+++ b/src/engine/imap-db/imap-db-conversation.vala
@@ -261,15 +261,19 @@ private Gee.HashSet<ImapDB.EmailIdentifier>? do_fetch_associated_email_ids(Db.Co
}
internal AssociatedEmails? do_generate_associations(Db.Connection cx,
- Gee.Collection<ImapDB.EmailIdentifier> associated_ids, Geary.Account.EmailSearchPredicate? predicate,
- Cancellable? cancellable) throws Error {
- AssociatedEmails association = new AssociatedEmails();
+ Gee.Collection<ImapDB.EmailIdentifier> associated_ids, Geary.Email.Field required_fields,
+ Geary.Account.EmailSearchPredicate? predicate, Cancellable? cancellable) throws Error {
+ AssociatedEmails association = new AssociatedEmails(required_fields);
+
+ // need to pull flags for the predicate function
+ if (predicate != null)
+ required_fields |= Geary.Email.Field.FLAGS;
+
foreach (ImapDB.EmailIdentifier associated_id in associated_ids) {
Email? email;
Gee.Collection<FolderPath?>? known_paths;
try {
- Account.do_fetch_message(cx, associated_id.message_id,
- predicate != null ? Geary.Email.Field.NONE : Geary.Email.Field.FLAGS,
+ Account.do_fetch_message(cx, associated_id.message_id, required_fields,
false, predicate, out email, out known_paths, cancellable);
} catch (Error err) {
if (err is EngineError.NOT_FOUND)
@@ -279,7 +283,7 @@ internal AssociatedEmails? do_generate_associations(Db.Connection cx,
}
if (email != null)
- association.add(email.id, known_paths);
+ association.add(email, known_paths);
}
return association.email_ids.size > 0 ? association : null;
diff --git a/src/engine/imap-db/imap-db-folder.vala b/src/engine/imap-db/imap-db-folder.vala
index c437f8f..6343dc1 100644
--- a/src/engine/imap-db/imap-db-folder.vala
+++ b/src/engine/imap-db/imap-db-folder.vala
@@ -598,8 +598,8 @@ private class Geary.ImapDB.Folder : BaseObject, Geary.ReferenceSemantics {
}
public async Gee.Collection<Geary.AssociatedEmails>? list_associated_emails_async(
- ImapDB.EmailIdentifier? start_id, int count, Geary.Account.EmailSearchPredicate? predicate,
- Gee.Collection<Geary.EmailIdentifier>? primary_email_ids,
+ ImapDB.EmailIdentifier? start_id, int count, Email.Field required_fields,
+ Geary.Account.EmailSearchPredicate? predicate, Gee.Collection<Geary.EmailIdentifier>?
primary_email_ids,
Gee.Collection<Geary.EmailIdentifier>? already_seen_ids, Cancellable? cancellable) throws Error {
if (count == 0)
return null;
@@ -661,7 +661,7 @@ private class Geary.ImapDB.Folder : BaseObject, Geary.ReferenceSemantics {
found_ids.add_all(associated_ids);
AssociatedEmails? association = Conversation.do_generate_associations(cx, associated_ids,
- predicate, cancellable);
+ required_fields, predicate, cancellable);
if (association != null && association.email_ids.size > 0) {
list.add(association);
if (list.size >= count)
diff --git a/src/engine/imap-engine/imap-engine-generic-account.vala
b/src/engine/imap-engine/imap-engine-generic-account.vala
index 6cfb356..e20b38e 100644
--- a/src/engine/imap-engine/imap-engine-generic-account.vala
+++ b/src/engine/imap-engine/imap-engine-generic-account.vala
@@ -868,10 +868,10 @@ 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, Account.EmailSearchPredicate? search_predicate,
- Cancellable? cancellable = null) throws Error {
- return yield local.search_associated_emails_async(check_ids(email_ids), search_predicate,
- cancellable);
+ Gee.Collection<Geary.EmailIdentifier> email_ids, Email.Field required_fields,
+ Account.EmailSearchPredicate? search_predicate, Cancellable? cancellable = null) throws Error {
+ return yield local.search_associated_emails_async(check_ids(email_ids), required_fields,
+ search_predicate, cancellable);
}
public override async Gee.Collection<Geary.Email>? local_list_email_async(
diff --git a/src/engine/imap-engine/imap-engine-minimal-folder.vala
b/src/engine/imap-engine/imap-engine-minimal-folder.vala
index f029181..feea4d1 100644
--- a/src/engine/imap-engine/imap-engine-minimal-folder.vala
+++ b/src/engine/imap-engine/imap-engine-minimal-folder.vala
@@ -1479,8 +1479,8 @@ private class Geary.ImapEngine.MinimalFolder : Geary.Folder, Geary.FolderSupport
}
public virtual async Gee.Collection<AssociatedEmails>? local_list_associated_emails_async(
- EmailIdentifier? initial_id, int count, Account.EmailSearchPredicate? predicate,
- Gee.Collection<EmailIdentifier>? primary_email_ids,
+ EmailIdentifier? initial_id, int count, Email.Field required_fields,
+ Account.EmailSearchPredicate? predicate, Gee.Collection<EmailIdentifier>? primary_email_ids,
Gee.Collection<Geary.EmailIdentifier>? already_seen_ids, Cancellable? cancellable = null)
throws Error {
check_open("local_list_associated_emails_async");
@@ -1493,7 +1493,7 @@ private class Geary.ImapEngine.MinimalFolder : Geary.Folder, Geary.FolderSupport
throw new EngineError.BAD_PARAMETERS("count may not be negative");
return yield local_folder.list_associated_emails_async((ImapDB.EmailIdentifier) initial_id,
- count, predicate, primary_email_ids, already_seen_ids, cancellable);
+ count, required_fields, predicate, primary_email_ids, already_seen_ids, cancellable);
}
public void schedule_op(ReplayOperation op) throws Error {
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]
