[geary/wip/794174-conversation-monitor-max-cpu: 2/3] Don't use the database for internal ConversationMonitor bookkeeping.
- From: Michael Gratton <mjog src gnome org>
- To: commits-list gnome org
- Cc:
- Subject: [geary/wip/794174-conversation-monitor-max-cpu: 2/3] Don't use the database for internal ConversationMonitor bookkeeping.
- Date: Wed, 4 Apr 2018 06:02:52 +0000 (UTC)
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]
