Hi Albrecht, On 03/12/2019 04:16:10 PM Tue, Albrecht Dreß wrote:
Hi Peter:
If RTTs are still an issue, we would need a more careful fix. From the look of the code, it appears that the intent was to find the nearest common ancestor of the old path to the folder and the new one, and re-scan just from that node. If the heuristic for finding it ever worked, it clearly no longer does, but a more careful search could probably be constructed.This is of course correct… However, as the folder has already been renamed in the first step, there is no need to establish a new connection for the LIST and LSUB queries - they just re-use the same connection (you can see that if you enable libnetclient debug messages). This completely avoids the costly start of encryption, login, etc. process for the scan. The full LSUB and LIST processing for my (though not very complex; ~10 folders in 3 nesting levels) ISP IMAP connection at ~1.5MBit/s needs ~200ms (remember that RFC 3501, sect. 6.3.8 requires “The LIST command SHOULD return its data quickly, without undue delay.”, and I don't see a reason why a server should be slower for LSUB, to be honest). Looking at the debug output, in my use case I estimate that the payload size for starting the encryption, logging in and renaming makes up > 50% of the total data volume. If the connection is /so/ slow that this might be a show-stopper, I wonder how long it would take to load a real-life message from the server… Just my € 0.01, though – if someone uses IMAP over, say, packet radio at 9.6kBaud every byte counts, of course!
Well, in case Balsa *has* such a bandwidth-challenged user: the attached patch finds the nearest common ancestor, and scans from there. It also has some NULL guards that seem to be needed. I've tested it with a toy folder tree on a gmail server. What do you think?
Actually, this patch has been a byproduct of a larger change I'm preparing, which addresses subscription management and choosing the folder for renaming. As to simplify life, I also re-scan the server structure instead of relying on the cached data. I will re-think this approach…
Looking forward to that!
Cheers, Albrecht.
Best, Peter
diff --git a/libbalsa/imap/imap-handle.c b/libbalsa/imap/imap-handle.c
index 4d62c27e1..536126e45 100644
--- a/libbalsa/imap/imap-handle.c
+++ b/libbalsa/imap/imap-handle.c
@@ -548,6 +548,8 @@ imap_handle_idle_disable(ImapMboxHandle *h)
gboolean
imap_handle_op_cancelled(ImapMboxHandle *h)
{
+ g_return_val_if_fail(h != NULL, FALSE);
+
return h->op_cancelled;
}
diff --git a/src/folder-conf.c b/src/folder-conf.c
index fe20d7458..845476b94 100644
--- a/src/folder-conf.c
+++ b/src/folder-conf.c
@@ -544,6 +544,8 @@ folder, parent);
}
if (button == GTK_RESPONSE_OK) {
GError* err = NULL;
+ const gchar *p, *q, *r;
+
/* Close the mailbox before renaming,
* otherwise the rescan will try to close it
* under its old name.
@@ -564,34 +566,44 @@ folder, parent);
sdd->mbnode->dir = g_strdup(parent);
/* Rescan as little of the tree as possible. */
- if (sdd->old_parent
- && !strncmp(parent, sdd->old_parent, strlen(parent))) {
- /* moved it up the tree */
- BalsaMailboxNode *mbnode =
- balsa_find_dir(sdd->parent->server, parent);
- if (mbnode) {
- balsa_mailbox_node_rescan(mbnode);
- g_object_unref(mbnode);
- } else
- printf("Parent not found!?\n");
- } else if (sdd->old_parent
- && !strncmp(parent, sdd->old_parent,
- strlen(sdd->old_parent))) {
- /* moved it down the tree */
- BalsaMailboxNode *mbnode =
- balsa_find_dir(sdd->parent->server, sdd->old_parent);
- if (mbnode) {
+ /* Find the longest common prefix of the new parent and
+ * the old parent, and rescan there. */
+ p = parent;
+ q = sdd->old_parent;
+ r = NULL;
+
+ while (*p != '\0' && *q != '\0' && *p == *q) {
+ if (*p == sdd->mbnode->delim)
+ r = p;
+ p++;
+ q++;
+ }
+
+ if (*p == '\0' && *q == '\0') {
+ /* No change of parent, just rename in place; we
+ * could possibly modify the tree without going to
+ * the server, but if we rescan the parent we can be
+ * sure that we see it the same way as the server. */
+ balsa_mailbox_node_rescan(sdd->mbnode->parent);
+ } else if (r == NULL) {
+ /* No common ancestor, must rescan the whole tree */
+ BalsaMailboxNode *mbnode = sdd->mbnode->parent;
+
+ while ((mbnode->mailbox != NULL) && (mbnode->parent != NULL))
+ mbnode = mbnode->parent;
+ balsa_mailbox_node_rescan(mbnode);
+ } else {
+ BalsaMailboxNode *mbnode;
+
+ /* r points to the last delimiter in parent, in the
+ * common part of the path; we replace it with '\0'
+ * to truncate the string. */
+ parent[r - parent] = '\0';
+ mbnode = balsa_find_dir(sdd->parent->server, parent);
+ if (mbnode != NULL) {
balsa_mailbox_node_rescan(mbnode);
g_object_unref(mbnode);
}
- } else {
- /* moved it sideways: a chain of folders might
- * go away, so we'd better rescan the complete IMAP server
- */
- BalsaMailboxNode *mb = sdd->mbnode->parent;
- while ((mb->mailbox != NULL) && (mb->parent != NULL))
- mb = mb->parent;
- balsa_mailbox_node_rescan(mb);
}
}
}
diff --git a/src/main-window.c b/src/main-window.c
index a1d72726f..0a4be4381 100644
--- a/src/main-window.c
+++ b/src/main-window.c
@@ -1853,20 +1853,12 @@ threading_change_state(GSimpleAction * action,
BalsaWindow *window = BALSA_WINDOW(user_data);
GtkWidget *index;
gboolean thread_messages;
- BalsaMailboxNode *mbnode;
- LibBalsaMailbox *mailbox;
thread_messages = g_variant_get_boolean(state);
index = balsa_window_find_current_index(window);
- balsa_index_set_thread_messages(BALSA_INDEX(index), thread_messages);
-
- /* bw->current_index may have been destroyed and cleared during
- * set-threading: */
- index = balsa_window_find_current_index(window);
- if (index && (mbnode = BALSA_INDEX(index)->mailbox_node)
- && (mailbox = mbnode->mailbox))
- bw_enable_expand_collapse(window, mailbox);
+ if (index != NULL)
+ balsa_index_set_thread_messages(BALSA_INDEX(index), thread_messages);
g_simple_action_set_state(action, state);
}
pgpise6aMMFjN.pgp
Description: PGP signature
_______________________________________________ balsa-list mailing list [email protected] https://mail.gnome.org/mailman/listinfo/balsa-list
