[geary] Load local msgs and display new msgs more quickly: Closes bgo#713432
- From: Jim Nelson <jnelson src gnome org>
- To: commits-list gnome org
- Cc:
- Subject: [geary] Load local msgs and display new msgs more quickly: Closes bgo#713432
- Date: Sat, 18 Jan 2014 00:43:18 +0000 (UTC)
commit af10a76b1f870516e738470c5ab4e2fbebcd34b6
Author: Jim Nelson <jim yorba org>
Date: Fri Jan 17 16:34:19 2014 -0800
Load local msgs and display new msgs more quickly: Closes bgo#713432
This patch is a grab-bag of fixes to get mail onto the screen faster
and report new mail waiting on the server more quickly.
In a nutshell:
* Adds a NO_DELAY flag to Folder.open_async which indicates that
background remote connections should initiate ASAP rather than
wait for a local request that requires remote information.
* Reduce creation of ImapDB.Folders (which, previously, were
generated as though "cheap"), which means caching server
information. ImapDB now relies on ImapEngine to refresh that
information on its own.
* The background search table update is delayed to allow startup
database tasks priority.
* Rather than delay selection of a folder 100ms to prevent the user
from holding down a key or clicking madly, the initial selection
goes right through, but subsequent ones are delayed. This may
also help resolve bug #713468.
* And the big one: ImapEngine.Account doesn't load local and remote
folders in parallel at startup, but rather local first, reports
them to the user, and then loads the remote and pairs the two.
This gets the UI up and going much more quickly.
src/client/application/geary-controller.vala | 43 +++++++++++------
src/engine/api/geary-folder.vala | 14 +++++-
src/engine/imap-db/imap-db-account.vala | 11 ++++-
.../imap-engine/imap-engine-generic-account.vala | 20 ++++----
.../imap-engine/imap-engine-generic-folder.vala | 31 ++++++++----
src/engine/imap/api/imap-account.vala | 49 ++++++++++++++-----
src/engine/imap/api/imap-folder.vala | 4 +-
.../imap/response/imap-mailbox-information.vala | 9 ++++
.../imap/transport/imap-client-connection.vala | 2 +-
.../transport/imap-client-session-manager.vala | 2 +-
10 files changed, 130 insertions(+), 55 deletions(-)
---
diff --git a/src/client/application/geary-controller.vala b/src/client/application/geary-controller.vala
index 2953d5b..6218f56 100644
--- a/src/client/application/geary-controller.vala
+++ b/src/client/application/geary-controller.vala
@@ -66,7 +66,7 @@ public class GearyController : Geary.BaseObject {
private const string MOVE_MESSAGE_TOOLTIP_SINGLE = _("Move conversation");
private const string MOVE_MESSAGE_TOOLTIP_MULTIPLE = _("Move conversations");
- private const int SELECT_FOLDER_TIMEOUT_MSEC = 100;
+ private const int SELECT_FOLDER_TIMEOUT_USEC = 100 * 1000;
private const int SEARCH_TIMEOUT_MSEC = 100;
private const string PROP_ATTEMPT_OPEN_ACCOUNT = "attempt-open-account";
@@ -96,6 +96,7 @@ public class GearyController : Geary.BaseObject {
private UnityLauncher? unity_launcher = null;
private Libnotify? libnotify = null;
private uint select_folder_timeout_id = 0;
+ private int64 next_folder_select_allowed_usec = 0;
private Geary.Folder? folder_to_select = null;
private Geary.Nonblocking.Mutex select_folder_mutex = new Geary.Nonblocking.Mutex();
private Geary.Account? account_to_select = null;
@@ -945,24 +946,35 @@ public class GearyController : Geary.BaseObject {
return;
}
- // To prevent the user from selecting folders too quickly, we actually
- // schedule the action to happen after a timeout instead of acting
- // directly. If the user selects another folder during the timeout,
- // we nix the original timeout and start a new one.
- if (select_folder_timeout_id != 0)
- Source.remove(select_folder_timeout_id);
folder_to_select = folder;
- select_folder_timeout_id = Timeout.add(SELECT_FOLDER_TIMEOUT_MSEC, on_select_folder_timeout);
+
+ // To prevent the user from selecting folders too quickly, we prevent additional selection
+ // changes to occur until after a timeout has expired from the last one
+ int64 now = get_monotonic_time();
+ int64 diff = now - next_folder_select_allowed_usec;
+ if (diff < SELECT_FOLDER_TIMEOUT_USEC) {
+ // only start timeout if another timeout is not running ... this means the user can
+ // click madly and will see the last clicked-on folder 100ms after the first one was
+ // clicked on
+ if (select_folder_timeout_id == 0)
+ select_folder_timeout_id = Timeout.add((uint) (diff / 1000), on_select_folder_timeout);
+ } else {
+ do_select_folder.begin(folder_to_select, on_select_folder_completed);
+ folder_to_select = null;
+
+ next_folder_select_allowed_usec = now + SELECT_FOLDER_TIMEOUT_USEC;
+ }
}
private bool on_select_folder_timeout() {
- assert(folder_to_select != null);
-
select_folder_timeout_id = 0;
+ next_folder_select_allowed_usec = 0;
- do_select_folder.begin(folder_to_select, on_select_folder_completed);
+ if (folder_to_select != null)
+ do_select_folder.begin(folder_to_select, on_select_folder_completed);
folder_to_select = null;
+
return false;
}
@@ -970,6 +982,8 @@ public class GearyController : Geary.BaseObject {
if (folder == current_folder)
return;
+ debug("Switching to %s...", folder.to_string());
+
cancel_folder();
// This function is not reentrant. It should be, because it can be
@@ -992,9 +1006,6 @@ public class GearyController : Geary.BaseObject {
yield current_folder.close_async();
}
- if (folder != null)
- debug("switching to %s", folder.to_string());
-
current_folder = folder;
if (current_account != folder.account) {
current_account = folder.account;
@@ -1026,7 +1037,7 @@ public class GearyController : Geary.BaseObject {
update_ui();
- current_conversations = new Geary.App.ConversationMonitor(current_folder,
Geary.Folder.OpenFlags.NONE,
+ current_conversations = new Geary.App.ConversationMonitor(current_folder,
Geary.Folder.OpenFlags.NO_DELAY,
ConversationListStore.REQUIRED_FIELDS, MIN_CONVERSATION_COUNT);
if (inboxes.values.contains(current_folder)) {
@@ -1045,6 +1056,8 @@ public class GearyController : Geary.BaseObject {
yield current_conversations.start_monitoring_async(conversation_cancellable);
select_folder_mutex.release(ref mutex_token);
+
+ debug("Switched to %s", folder.to_string());
}
private void on_scan_error(Error err) {
diff --git a/src/engine/api/geary-folder.vala b/src/engine/api/geary-folder.vala
index b7ad434..a870896 100644
--- a/src/engine/api/geary-folder.vala
+++ b/src/engine/api/geary-folder.vala
@@ -67,7 +67,13 @@ public interface Geary.Folder : BaseObject {
* the messages (such as their flags or other metadata) may not be up-to-date
* when the folder opens. Not all folders will support this flag.
*/
- FAST_OPEN;
+ FAST_OPEN,
+ /**
+ * Do not delay opening a connection to the server.
+ *
+ * @see open_async
+ */
+ NO_DELAY;
public bool is_any_set(OpenFlags flags) {
return (this & flags) != 0;
@@ -321,6 +327,12 @@ public interface Geary.Folder : BaseObject {
* while it may take time (if ever) for the remote state to open. Thus, it's possible for
* the "opened" signal to fire some time *after* this method completes.
*
+ * { link OpenFlags.NO_DELAY} may be passed to force an immediate opening of the remote folder.
+ * This still will not occur in the context of the open_async call, but will initiate the
+ * connection immediately. Use this only when it's known that remote calls or remote
+ * notifications to the Folder are imminent or monitoring the Folder is vital (such as with the
+ * Inbox).
+ *
* However, even if the method returns before the Folder's OpenState is BOTH, this Folder is
* ready for operation if this method returns without error. The messages the folder returns
* may not reflect the full state of the Folder, however, and returned emails may subsequently
diff --git a/src/engine/imap-db/imap-db-account.vala b/src/engine/imap-db/imap-db-account.vala
index e9f8dcb..166d531 100644
--- a/src/engine/imap-db/imap-db-account.vala
+++ b/src/engine/imap-db/imap-db-account.vala
@@ -5,6 +5,8 @@
*/
private class Geary.ImapDB.Account : BaseObject {
+ private const int POPULATE_SEARCH_TABLE_DELAY_SEC = 30;
+
private class FolderReference : Geary.SmartReference {
public Geary.FolderPath path;
@@ -132,8 +134,13 @@ private class Geary.ImapDB.Account : BaseObject {
background_cancellable = new Cancellable();
- // Kick off a background update of the search table.
- populate_search_table_async.begin(background_cancellable);
+ // Kick off a background update of the search table, but since the database is getting
+ // hammered at startup, wait a bit before starting the update
+ Timeout.add_seconds(POPULATE_SEARCH_TABLE_DELAY_SEC, () => {
+ populate_search_table_async.begin(background_cancellable);
+
+ return false;
+ });
initialize_contacts(cancellable);
diff --git a/src/engine/imap-engine/imap-engine-generic-account.vala
b/src/engine/imap-engine/imap-engine-generic-account.vala
index 2fce9e5..18e99af 100644
--- a/src/engine/imap-engine/imap-engine-generic-account.vala
+++ b/src/engine/imap-engine/imap-engine-generic-account.vala
@@ -304,24 +304,26 @@ private abstract class Geary.ImapEngine.GenericAccount : Geary.AbstractAccount {
private async void enumerate_folders_async(Cancellable? cancellable) throws Error {
check_open();
- // get all local folders
- Gee.HashMap<FolderPath, ImapDB.Folder> local_children = yield enumerate_local_folders_async(null,
- cancellable);
+ // enumerate local folders first
+ Gee.HashMap<FolderPath, ImapDB.Folder> local_folders = yield enumerate_local_folders_async(
+ null, cancellable);
// convert to a list of Geary.Folder ... build_folder() also reports new folders, so this
- // gets the word out quickly
+ // gets the word out quickly (local_only folders have already been reported)
Gee.Collection<Geary.Folder> existing_list = new Gee.ArrayList<Geary.Folder>();
- existing_list.add_all(build_folders(local_children.values));
+ existing_list.add_all(build_folders(local_folders.values));
existing_list.add_all(local_only.values);
+ // build a map of all existing folders
Gee.HashMap<FolderPath, Geary.Folder> existing_folders
= Geary.traverse<Geary.Folder>(existing_list).to_hash_map<FolderPath>(f => f.path);
- // get all remote (server) folder paths
- Gee.HashMap<FolderPath, Imap.Folder> remote_folders = yield enumerate_remote_folders_async(null,
- cancellable);
+ // now that all local have been enumerated and reported (this is important to assist
+ // startup of the UI), enumerate the remote folders
+ Gee.HashMap<FolderPath, Imap.Folder>? remote_folders = yield enumerate_remote_folders_async(
+ null, cancellable);
- // combine the two and make sure everything is up-to-date
+ // pair the local and remote folders and make sure everything is up-to-date
yield update_folders_async(existing_folders, remote_folders, cancellable);
}
diff --git a/src/engine/imap-engine/imap-engine-generic-folder.vala
b/src/engine/imap-engine/imap-engine-generic-folder.vala
index e93c065..f0081a5 100644
--- a/src/engine/imap-engine/imap-engine-generic-folder.vala
+++ b/src/engine/imap-engine/imap-engine-generic-folder.vala
@@ -437,6 +437,12 @@ private class Geary.ImapEngine.GenericFolder : Geary.AbstractFolder, Geary.Folde
public override async bool open_async(Geary.Folder.OpenFlags open_flags, Cancellable? cancellable = null)
throws Error {
if (open_count++ > 0) {
+ // even if opened or opening, respect the NO_DELAY flag
+ if (open_flags.is_all_set(OpenFlags.NO_DELAY)) {
+ cancel_remote_open_timer();
+ wait_for_open_async.begin();
+ }
+
debug("Not opening %s: already open (open_count=%d)", to_string(), open_count);
return false;
@@ -444,29 +450,32 @@ private class Geary.ImapEngine.GenericFolder : Geary.AbstractFolder, Geary.Folde
this.open_flags = open_flags;
- open_internal(cancellable);
+ open_internal(open_flags, cancellable);
return true;
}
- private void open_internal(Cancellable? cancellable) {
+ private void open_internal(Folder.OpenFlags open_flags, Cancellable? cancellable) {
remote_semaphore = new Geary.Nonblocking.ReportingSemaphore<bool>(false);
// start the replay queue
replay_queue = new ReplayQueue(this);
- // do NOT open the remote side here; wait for the ReplayQueue to require a remote connection
- // or wait_for_open_async() to be called ... this allows for fast local-only operations
- // to occur, local-only either because (a) the folder has all the information required
- // (for a list or fetch operation), or (b) the operation was de facto local-only.
- // In particular, EmailStore will open and close lots of folders, causing a lot of
- // connection setup and teardown
-
+ // Unless NO_DELAY is set, do NOT open the remote side here; wait for the ReplayQueue to
+ // require a remote connection or wait_for_open_async() to be called ... this allows for
+ // fast local-only operations to occur, local-only either because (a) the folder has all
+ // the information required (for a list or fetch operation), or (b) the operation was de
+ // facto local-only. In particular, EmailStore will open and close lots of folders,
+ // causing a lot of connection setup and teardown
+ //
// However, want to eventually open, otherwise if there's no user interaction (i.e. a
// second account Inbox they don't manipulate), no remote connection will ever be made,
// meaning that folder normalization never happens and unsolicited notifications never
// arrive
- start_remote_open_timer();
+ if (open_flags.is_all_set(OpenFlags.NO_DELAY))
+ wait_for_open_async.begin();
+ else
+ start_remote_open_timer();
}
private void start_remote_open_timer() {
@@ -649,7 +658,7 @@ private class Geary.ImapEngine.GenericFolder : Geary.AbstractFolder, Geary.Folde
if (remote_reason.is_error()) {
debug("Reestablishing broken connect to %s", to_string());
- open_internal(null);
+ open_internal(OpenFlags.NO_DELAY, null);
return;
}
diff --git a/src/engine/imap/api/imap-account.vala b/src/engine/imap/api/imap-account.vala
index 8d71405..0cd39c4 100644
--- a/src/engine/imap/api/imap-account.vala
+++ b/src/engine/imap/api/imap-account.vala
@@ -12,9 +12,10 @@
* that a Geary.Account implementation would need (in particular, { link Geary.ImapEngine.Account}
* and makes them into simple async calls.
*
- * Geary.Imap.Account does __no__ management of the { link Imap.Folder} objects it returns. Thus,
- * calling a fetch or list operation several times in a row will return separate Folder objects
- * each time. It is up to the higher layers of the stack to manage these objects.
+ * Geary.Imap.Account manages the { link Imap.Folder} objects it returns, but only in the sense
+ * that it will not create new instances repeatedly. Otherwise, it does not refresh or update the
+ * Imap.Folders themselves (such as update their { link Imap.StatusData} periodically).
+ * That's the responsibility of the higher layers of the stack.
*/
private class Geary.Imap.Account : BaseObject {
@@ -28,6 +29,7 @@ private class Geary.Imap.Account : BaseObject {
private Nonblocking.Mutex cmd_mutex = new Nonblocking.Mutex();
private Gee.HashMap<FolderPath, MailboxInformation> path_to_mailbox = new Gee.HashMap<
FolderPath, MailboxInformation>();
+ private Gee.HashMap<FolderPath, Imap.Folder> folders = new Gee.HashMap<FolderPath, Imap.Folder>();
private Gee.List<MailboxInformation>? list_collector = null;
private Gee.List<StatusData>? status_collector = null;
@@ -143,28 +145,38 @@ private class Geary.Imap.Account : BaseObject {
}
public async bool folder_exists_async(FolderPath path, Cancellable? cancellable) throws Error {
- return path_to_mailbox.contains(path);
+ return path_to_mailbox.has_key(path);
}
public async Imap.Folder fetch_folder_async(FolderPath path, Cancellable? cancellable)
throws Error {
check_open();
+ if (folders.has_key(path))
+ return folders.get(path);
+
// if not in map, use list_children_async to add it (if it exists)
- if (!path_to_mailbox.contains(path))
+ if (!path_to_mailbox.has_key(path)) {
+ debug("Listing children to find %s", path.to_string());
yield list_children_async(path.get_parent(), cancellable);
+ }
MailboxInformation? mailbox_info = path_to_mailbox.get(path);
if (mailbox_info == null)
throw_not_found(path);
+ Imap.Folder folder;
if (!mailbox_info.attrs.contains(MailboxAttribute.NO_SELECT)) {
StatusData status = yield fetch_status_async(path, cancellable);
- return new Imap.Folder(session_mgr, status, mailbox_info);
+ folder = new Imap.Folder(session_mgr, status, mailbox_info);
} else {
- return new Imap.Folder.unselectable(session_mgr, mailbox_info);
+ folder = new Imap.Folder.unselectable(session_mgr, mailbox_info);
}
+
+ folders.set(path, folder);
+
+ return folder;
}
private async StatusData fetch_status_async(FolderPath path, Cancellable? cancellable)
@@ -211,8 +223,18 @@ private class Geary.Imap.Account : BaseObject {
Gee.Map<StatusCommand, MailboxSpecifier> cmd_map = new Gee.HashMap<
StatusCommand, MailboxSpecifier>();
foreach (MailboxInformation mailbox_info in child_info) {
+ // if already have an Imap.Folder for this mailbox, use that
+ if (folders.has_key(mailbox_info.path)) {
+ child_folders.add(folders.get(mailbox_info.path));
+
+ continue;
+ }
+
+ // if new mailbox is unselectable, don't bother doing a STATUS command
if (mailbox_info.attrs.contains(MailboxAttribute.NO_SELECT)) {
- child_folders.add(new Imap.Folder.unselectable(session_mgr, mailbox_info));
+ Imap.Folder folder = new Imap.Folder.unselectable(session_mgr, mailbox_info);
+ folders.set(folder.path, folder);
+ child_folders.add(folder);
continue;
}
@@ -255,7 +277,10 @@ private class Geary.Imap.Account : BaseObject {
}
status_results.remove(found_status);
- child_folders.add(new Imap.Folder(session_mgr, found_status, mailbox_info));
+
+ Imap.Folder folder = new Imap.Folder(session_mgr, found_status, mailbox_info);
+ folders.set(folder.path, folder);
+ child_folders.add(folder);
}
if (status_results.size > 0)
@@ -307,10 +332,8 @@ private class Geary.Imap.Account : BaseObject {
// stash all MailboxInformation by path
// TODO: remove any MailboxInformation for this parent that is not found (i.e. has been
// removed on the server)
- foreach (MailboxInformation mailbox_info in list_results) {
- FolderPath path = mailbox_info.mailbox.to_folder_path(mailbox_info.delim);
- path_to_mailbox.set(path, mailbox_info);
- }
+ foreach (MailboxInformation mailbox_info in list_results)
+ path_to_mailbox.set(mailbox_info.path, mailbox_info);
return (list_results.size > 0) ? list_results : null;
}
diff --git a/src/engine/imap/api/imap-folder.vala b/src/engine/imap/api/imap-folder.vala
index 87eae1d..a38dc75 100644
--- a/src/engine/imap/api/imap-folder.vala
+++ b/src/engine/imap/api/imap-folder.vala
@@ -65,7 +65,7 @@ private class Geary.Imap.Folder : BaseObject {
this.session_mgr = session_mgr;
this.info = info;
- path = info.mailbox.to_folder_path(info.delim);
+ path = info.path;
properties = new Imap.FolderProperties.status(status, info.attrs);
}
@@ -73,7 +73,7 @@ private class Geary.Imap.Folder : BaseObject {
internal Folder.unselectable(ClientSessionManager session_mgr, MailboxInformation info) {
this.session_mgr = session_mgr;
this.info = info;
- path = info.mailbox.to_folder_path(info.delim);
+ path = info.path;
properties = new Imap.FolderProperties(0, 0, 0, null, null, info.attrs);
}
diff --git a/src/engine/imap/response/imap-mailbox-information.vala
b/src/engine/imap/response/imap-mailbox-information.vala
index 1e6520b..f29294e 100644
--- a/src/engine/imap/response/imap-mailbox-information.vala
+++ b/src/engine/imap/response/imap-mailbox-information.vala
@@ -30,10 +30,19 @@ public class Geary.Imap.MailboxInformation : Object {
*/
public MailboxAttributes attrs { get; private set; }
+ /**
+ * The { link Geary.FolderPath} for the mailbox.
+ *
+ * This is constructed from the supplied { link mailbox} and { link delim} returned from the
+ * server.
+ */
+ public Geary.FolderPath path { get; private set; }
+
public MailboxInformation(MailboxSpecifier mailbox, string? delim, MailboxAttributes attrs) {
this.mailbox = mailbox;
this.delim = delim;
this.attrs = attrs;
+ path = mailbox.to_folder_path(delim);
}
/**
diff --git a/src/engine/imap/transport/imap-client-connection.vala
b/src/engine/imap/transport/imap-client-connection.vala
index 433406c..6b91369 100644
--- a/src/engine/imap/transport/imap-client-connection.vala
+++ b/src/engine/imap/transport/imap-client-connection.vala
@@ -30,7 +30,7 @@ public class Geary.Imap.ClientConnection : BaseObject {
*/
public const uint DEFAULT_COMMAND_TIMEOUT_SEC = 15;
- private const int FLUSH_TIMEOUT_MSEC = 100;
+ private const int FLUSH_TIMEOUT_MSEC = 10;
private enum State {
UNCONNECTED,
diff --git a/src/engine/imap/transport/imap-client-session-manager.vala
b/src/engine/imap/transport/imap-client-session-manager.vala
index 4acc6b7..abfd436 100644
--- a/src/engine/imap/transport/imap-client-session-manager.vala
+++ b/src/engine/imap/transport/imap-client-session-manager.vala
@@ -5,7 +5,7 @@
*/
public class Geary.Imap.ClientSessionManager : BaseObject {
- public const int DEFAULT_MIN_POOL_SIZE = 2;
+ public const int DEFAULT_MIN_POOL_SIZE = 1;
private const int AUTHORIZED_SESSION_ERROR_RETRY_TIMEOUT_MSEC = 1000;
public bool is_open { get; private set; default = false; }
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]
