Commit af10a76b authored by Jim Nelson's avatar Jim Nelson

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.
parent cf8679e9
......@@ -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) {
......
......@@ -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
......
......@@ -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);
......
......@@ -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);
}
......
......@@ -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;
}
......
......@@ -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;
}
......
......@@ -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);
}
......
......@@ -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);
}
/**
......
......@@ -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,
......
......@@ -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; }
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment