searching by Message-ID after copy/move; speeding things up

searching by Message-ID after copy/move; speeding things up

[Michael Welsh Duggan]

When moving or adding an article to an IMAP group, Gnus takes note of
the UID of the message that has been added to the new group.  Using most
modern IMAP servers, getting this value is as simple as looking for a
COPYUID or APPENDUID response after the copy/append.

Some servers don't support COPYUID or APPENDUID, however, and on those
servers we have to do a little more work to figure out the new message
UID.  This is done using the nnimap-find-article-by-message-id
function.  This function executes a UID SEARCH HEADER Message-ID command
in order to get the article UID.  This works, but...

When working with an IMAP server that doesn't include Message-ID headers
in its index, finding the article ends up mapping to looking at every
single article in the group.  This can be slow, and gets slower as the
group gets larger.  Now, any modern IMAP server should not have this
problem.  Unfortunately there is at least on very popular server that
_does_ exhibit this problem: Microsoft Exchange.

Like many, I use Gnus to connect to a company run Exchange server
because that is my substitute for using the POS that is Microsoft
Outlook.  (That, and why would I _not_ be using Emacs?)  However, this
slowness on article moving problem has been a bit of a thorn in my
backside.  The first message I move to a particular group on any one day
can cause a delay of up to 1.5 minutes or longer.  And that is very,
very annoying.

So, I solved this problem in my own copy of Gnus.  I could easily post
the diffs here.  Unfortunately, my place of work refuses to give a
blanket disclaimer for a program (such as Gnus or Emacs) and will
instead only hand out case-by-case disclaimers (that are very irritating
to get).  And my patch goes a little over the 10 line "small change"
boundary.  As such, in the hopes that such a feature might make it into
Gnus mainline, I am instead going to explain a solution such that
someone else who is motivated could easily recreate something similar to
my implementation.

The idea is simple enough.  I added an extra optional argument to
nnimap-find-article-by-message-id which would be set when looking for
articles that have been _recently_ appended to a group.  Then if the
optional argument is set, I do a range-limited UID search limited to the
last N articles in the group.  If this fails, I revert to the
non-limited search.  I have a user option which represents the number of
articles to look through, with nil reverting back to the original
behavior.  I set the optional argument to t when called from the move or
append functions.

The change was quite simple, and most of my time was spent re-learning
how the nnimap library does things in general.  I hope that something
like this can make it into mainline Gnus.

-- 
Michael Welsh Duggan
(md5i@md5i.com)

Re: searching by Message-ID after copy/move; speeding things up

[Julien Danjou]

[-- Attachment #1.1: Type: text/plain, Size: 336 bytes --]

On Thu, Jun 14 2012, Michael Welsh Duggan wrote:

> The change was quite simple, and most of my time was spent re-learning
> how the nnimap library does things in general.  I hope that something
> like this can make it into mainline Gnus.

Attached is an experimental patch. Could you test it and tell me if this
resolve your problem?


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.2: 0001-Add-recent-argument-to-nnimap-find-article-by-messag.patch --]
[-- Type: text/x-diff, Size: 2472 bytes --]

From 743f0f50fcfb2779f1ba16382cc0fca9b1364585 Mon Sep 17 00:00:00 2001
From: Julien Danjou <julien@danjou.info>
Date: Thu, 14 Jun 2012 10:09:04 +0200
Subject: [PATCH] Add recent argument to nnimap-find-article-by-message-id

Signed-off-by: Julien Danjou <julien@danjou.info>
---
 lisp/ChangeLog |    7 +++++++
 lisp/nnimap.el |   11 ++++++++---
 2 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/lisp/ChangeLog b/lisp/ChangeLog
index 20a586e..9d0f184 100644
--- a/lisp/ChangeLog
+++ b/lisp/ChangeLog
@@ -1,3 +1,10 @@
+2012-06-14  Julien Danjou  <julien@danjou.info>
+
+	* nnimap.el (nnimap-find-article-by-message-id): Add docstring and
+	recent argument.
+	(nnimap-request-move-article): Use the recent option of
+	`nnimap-find-article-by-message-id'.
+
 2012-06-11  Lars Magne Ingebrigtsen  <larsi@gnus.org>
 
 	* gnus-art.el (gnus-article-read-summary-keys): Protect against the key
diff --git a/lisp/nnimap.el b/lisp/nnimap.el
index 5bdf226..0bd45de 100644
--- a/lisp/nnimap.el
+++ b/lisp/nnimap.el
@@ -867,7 +867,7 @@ textual parts.")
 		(cons internal-move-group
 		      (or (nnimap-find-uid-response "COPYUID" (cadr result))
 			  (nnimap-find-article-by-message-id
-			   internal-move-group message-id)))))
+			   internal-move-group message-id t)))))
 	  ;; Move the article to a different method.
 	  (let ((result (eval accept-form)))
 	    (when result
@@ -969,7 +969,10 @@ textual parts.")
 			       (cdr (assoc "SEARCH" (cdr result))))))))))
 
 
-(defun nnimap-find-article-by-message-id (group message-id)
+(defun nnimap-find-article-by-message-id (group message-id &optional recent)
+  "Search for message with MESSAGE-ID in GROUP.
+RECENT can be used to look only in message having the \Resent
+flag set, fastening the search.."
   (with-current-buffer (nnimap-buffer)
     (erase-buffer)
     (unless (equal group (nnimap-group nnimap-object))
@@ -977,7 +980,9 @@ textual parts.")
       (setf (nnimap-examined nnimap-object) group)
       (nnimap-send-command "EXAMINE %S" (utf7-encode group t)))
     (let ((sequence
-	   (nnimap-send-command "UID SEARCH HEADER Message-Id %S" message-id))
+	   (nnimap-send-command "UID SEARCH HEADER Message-Id %S%s"
+                                message-id
+                                (if recent " RECENT" "")))
 	  article result)
       (setq result (nnimap-wait-for-response sequence))
       (when (and result
-- 
1.7.10


[-- Attachment #1.3: Type: text/plain, Size: 26 bytes --]


-- 
           Julien

[-- Attachment #2: Type: application/pgp-signature, Size: 835 bytes --]

Re: searching by Message-ID after copy/move; speeding things up

[Michael Welsh Duggan]

Julien Danjou <julien@danjou.info> writes:

> On Thu, Jun 14 2012, Michael Welsh Duggan wrote:
>
>> The change was quite simple, and most of my time was spent re-learning
>> how the nnimap library does things in general.  I hope that something
>> like this can make it into mainline Gnus.
>
> Attached is an experimental patch. Could you test it and tell me if this
> resolve your problem?

I'm afraid not.  In some (but not all) instances the moved message
doesn't show up as RECENT.  (More specifically, UID SEARCH RECENT fails
to locate them.)  FWIW, RECENT doesn't seem to be very well specified in
the IMAP spec, either:

http://mail-archives.apache.org/mod_mbox/james-server-dev/200803.mbox/%3C88f6e29a0803210110x2784101crf66e32fd96fd1b7f@mail.gmail.com%3E

-- 
Michael Welsh Duggan
(mwd@cert.org)

Re: searching by Message-ID after copy/move; speeding things up

[Julien Danjou]

[-- Attachment #1: Type: text/plain, Size: 653 bytes --]

On Thu, Jun 14 2012, Michael Welsh Duggan wrote:

> I'm afraid not.  In some (but not all) instances the moved message
> doesn't show up as RECENT.  (More specifically, UID SEARCH RECENT fails
> to locate them.)  FWIW, RECENT doesn't seem to be very well specified in
> the IMAP spec, either:
>
> http://mail-archives.apache.org/mod_mbox/james-server-dev/200803.mbox/%3C88f6e29a0803210110x2784101crf66e32fd96fd1b7f@mail.gmail.com%3E

Too bad, I wanted to be smart. But you say sometimes it works? Maybe we
could do RECENT, then a limited search (didn't find yet how this can be
done) and then a total search. WDYT?

-- 
           Julien

[-- Attachment #2: Type: application/pgp-signature, Size: 835 bytes --]

Re: searching by Message-ID after copy/move; speeding things up

[Michael Welsh Duggan]

Julien Danjou <julien@danjou.info> writes:

> On Thu, Jun 14 2012, Michael Welsh Duggan wrote:
>
>> I'm afraid not.  In some (but not all) instances the moved message
>> doesn't show up as RECENT.  (More specifically, UID SEARCH RECENT fails
>> to locate them.)  FWIW, RECENT doesn't seem to be very well specified in
>> the IMAP spec, either:
>>
>> http://mail-archives.apache.org/mod_mbox/james-server-dev/200803.mbox/%3C88f6e29a0803210110x2784101crf66e32fd96fd1b7f@mail.gmail.com%3E
>
> Too bad, I wanted to be smart. But you say sometimes it works? Maybe we
> could do RECENT, then a limited search (didn't find yet how this can be
> done) and then a total search. WDYT?

Honestly, I'd just do a limited search.  You're already doing an
EXAMINE, so you just need to parse the results to get the EXISTS count.
Then a "UID SEARCH N:* HEADER Message-ID ..."  where N is the exist
count minus a small constant.

If the limited search fails, just re-call the function with recent set
to nil.

-- 
Michael Welsh Duggan
(mwd@cert.org)

Re: searching by Message-ID after copy/move; speeding things up

[Julien Danjou]

[-- Attachment #1.1: Type: text/plain, Size: 393 bytes --]

On Thu, Jun 14 2012, Michael Welsh Duggan wrote:

> Honestly, I'd just do a limited search.  You're already doing an
> EXAMINE, so you just need to parse the results to get the EXISTS count.
> Then a "UID SEARCH N:* HEADER Message-ID ..."  where N is the exist
> count minus a small constant.
>
> If the limited search fails, just re-call the function with recent set
> to nil.

Another try.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.2: 0001-Add-recent-argument-to-nnimap-find-article-by-messag.patch --]
[-- Type: text/x-diff, Size: 3325 bytes --]

From f14188ce0cdfba2bfe27bcf8e88b3945841a3a4b Mon Sep 17 00:00:00 2001
From: Julien Danjou <julien@danjou.info>
Date: Thu, 14 Jun 2012 10:09:04 +0200
Subject: [PATCH] Add recent argument to nnimap-find-article-by-message-id

Signed-off-by: Julien Danjou <julien@danjou.info>
---
 lisp/nnimap.el |   40 ++++++++++++++++++++++++++--------------
 1 file changed, 26 insertions(+), 14 deletions(-)

diff --git a/lisp/nnimap.el b/lisp/nnimap.el
index 1db8640..e50fc0e 100644
--- a/lisp/nnimap.el
+++ b/lisp/nnimap.el
@@ -123,6 +123,10 @@ will fetch all parts that have types that match that string.  A
 likely value would be \"text/\" to automatically fetch all
 textual parts.")
 
+(defcustom nnimap-request-move-articles-find-limit nil
+  "Limit the number of articles to look for after moving an article."
+  :type 'integer)
+
 (defvar nnimap-process nil)
 
 (defvar nnimap-status-string "")
@@ -867,7 +871,8 @@ textual parts.")
 		(cons internal-move-group
 		      (or (nnimap-find-uid-response "COPYUID" (cadr result))
 			  (nnimap-find-article-by-message-id
-			   internal-move-group server message-id)))))
+			   internal-move-group server message-id
+                           nnimap-request-move-articles-find-limit)))))
 	  ;; Move the article to a different method.
 	  (let ((result (eval accept-form)))
 	    (when result
@@ -969,21 +974,28 @@ textual parts.")
 			       (cdr (assoc "SEARCH" (cdr result))))))))))
 
 
-(defun nnimap-find-article-by-message-id (group server message-id)
-  "Search for message with MESSAGE-ID in GROUP from SERVER."
+(defun nnimap-find-article-by-message-id (group server message-id &optional limit)
+  "Search for message with MESSAGE-ID in GROUP from SERVER.
+If LIMIT, first try to limit the search to the N last articles."
   (with-current-buffer (nnimap-buffer)
     (erase-buffer)
-    (nnimap-change-group group server nil t)
-    (let ((sequence
-	   (nnimap-send-command "UID SEARCH HEADER Message-Id %S" message-id))
-	  article result)
-      (setq result (nnimap-wait-for-response sequence))
-      (when (and result
-		 (car (setq result (nnimap-parse-response))))
-	;; Select the last instance of the message in the group.
-	(and (setq article
-		   (car (last (cdr (assoc "SEARCH" (cdr result))))))
-	     (string-to-number article))))))
+    (let* ((number-of-article
+           (catch 'found
+             (dolist (result (cdr (nnimap-change-group group server nil t)))
+               (when (equal "EXISTS" (cadr result))
+                 (throw 'found (car result))))))
+           (sequence
+            (nnimap-send-command "UID SEARCH%s HEADER Message-Id %S"
+                                 (if limit
+                                     (format " %s:*" (- number-of-article limit))
+                                   "")
+                                 message-id)))
+      (when (nnimap-wait-for-response sequence)
+        (let ((article (car (last (cdr (assoc "SEARCH" (nnimap-parse-response)))))))
+          (if article
+              (string-to-number article)
+            (when limit
+              (nnimap-find-article-by-message-id group server message-id))))))))
 
 (defun nnimap-delete-article (articles)
   (with-current-buffer (nnimap-buffer)
-- 
1.7.10


[-- Attachment #1.3: Type: text/plain, Size: 243 bytes --]


While I was at it I modified a bunch of code in nnimap that I pushed
into master. I hope I didn't break anything, but I think it's a bit
cleaner that way. If it break something we can beat me and revert
anyway.

-- 
           Julien

[-- Attachment #2: Type: application/pgp-signature, Size: 835 bytes --]

Re: searching by Message-ID after copy/move; speeding things up

[Michael Welsh Duggan]

Julien Danjou <julien@danjou.info> writes:

> On Thu, Jun 14 2012, Michael Welsh Duggan wrote:
>
>> Honestly, I'd just do a limited search.  You're already doing an
>> EXAMINE, so you just need to parse the results to get the EXISTS count.
>> Then a "UID SEARCH N:* HEADER Message-ID ..."  where N is the exist
>> count minus a small constant.
>>
>> If the limited search fails, just re-call the function with recent set
>> to nil.
>
> Another try.

Thanks for all your work!  This works with a couple of modifications.

> -(defun nnimap-find-article-by-message-id (group server message-id)
> -  "Search for message with MESSAGE-ID in GROUP from SERVER."
> +(defun nnimap-find-article-by-message-id (group server message-id &optional limit)
> +  "Search for message with MESSAGE-ID in GROUP from SERVER.
> +If LIMIT, first try to limit the search to the N last articles."
>    (with-current-buffer (nnimap-buffer)
>      (erase-buffer)
> -    (nnimap-change-group group server nil t)
> -    (let ((sequence
> -	   (nnimap-send-command "UID SEARCH HEADER Message-Id %S" message-id))
> -	  article result)
> -      (setq result (nnimap-wait-for-response sequence))
> -      (when (and result
> -		 (car (setq result (nnimap-parse-response))))
> -	;; Select the last instance of the message in the group.
> -	(and (setq article
> -		   (car (last (cdr (assoc "SEARCH" (cdr result))))))
> -	     (string-to-number article))))))
> +    (let* ((number-of-article
> +           (catch 'found
> +             (dolist (result (cdr (nnimap-change-group group server nil t)))
> +               (when (equal "EXISTS" (cadr result))
> +                 (throw 'found (car result))))))
> +           (sequence
> +            (nnimap-send-command "UID SEARCH%s HEADER Message-Id %S"
> +                                 (if limit

Needs to be (if (and limit number-of-article) ...)

> +                                     (format " %s:*" (- number-of-article limit))

Needs to be (- (string-to-number number-of-article) limit -1)

The need for string-to-number should be obvious.  The -1 is because IMAP
message numbers are one-based rather than zero-based.  

> +                                   "")
> +                                 message-id)))
> +      (when (nnimap-wait-for-response sequence)
> +        (let ((article (car (last (cdr (assoc "SEARCH" (nnimap-parse-response)))))))
> +          (if article
> +              (string-to-number article)
> +            (when limit

Probably should be (and limit number-of-article) just in case.

> +              (nnimap-find-article-by-message-id group server message-id))))))))
>  
>  (defun nnimap-delete-article (articles)
>    (with-current-buffer (nnimap-buffer)
> -- 
> 1.7.10


> While I was at it I modified a bunch of code in nnimap that I pushed
> into master. I hope I didn't break anything, but I think it's a bit
> cleaner that way. If it break something we can beat me and revert
> anyway.

Of course I failed to read this the first time and got confused.  "What
is nnimap-change-group?"  It always helps to read the whole thing.  :)

-- 
Michael Welsh Duggan
(mwd@cert.org)

Re: searching by Message-ID after copy/move; speeding things up

[Julien Danjou]

[-- Attachment #1: Type: text/plain, Size: 91 bytes --]

Michael,

I've pushed the patch, tell me if anything's wrong.

-- 
           Julien

[-- Attachment #2: Type: application/pgp-signature, Size: 835 bytes --]

Re: searching by Message-ID after copy/move; speeding things up

[Michael Welsh Duggan]

Julien Danjou <julien@danjou.info> writes:

> Michael,
>
> I've pushed the patch, tell me if anything's wrong.

Looks good.  I would ask you to add the optional limit argument to call
to the nnimap-find-article-by-message-id call in
nnimap-request-accept-article as well.  This will get called when
gnus-message-archive-group is set and mail is sent, for example.

Also, you may want to add line breaks to
nnimap-find-article-by-message-id to keep it within the 79-column
limit.

-- 
Michael Welsh Duggan
(mwd@cert.org)

Re: searching by Message-ID after copy/move; speeding things up

[Julien Danjou]

[-- Attachment #1: Type: text/plain, Size: 538 bytes --]

On Tue, Jun 19 2012, Michael Welsh Duggan wrote:

> Looks good.  I would ask you to add the optional limit argument to call
> to the nnimap-find-article-by-message-id call in
> nnimap-request-accept-article as well.  This will get called when
> gnus-message-archive-group is set and mail is sent, for example.

I've added something like that but I had to rename the defcustom for
more consistency. You're probably the one to use it for now, so that
should not be a problem, but I prefer to inform you.

-- 
           Julien

[-- Attachment #2: Type: application/pgp-signature, Size: 835 bytes --]

Re: searching by Message-ID after copy/move; speeding things up

[Michael Welsh Duggan]

Julien Danjou <julien@danjou.info> writes:

> On Tue, Jun 19 2012, Michael Welsh Duggan wrote:
>
>> Looks good.  I would ask you to add the optional limit argument to call
>> to the nnimap-find-article-by-message-id call in
>> nnimap-request-accept-article as well.  This will get called when
>> gnus-message-archive-group is set and mail is sent, for example.
>
> I've added something like that but I had to rename the defcustom for
> more consistency. You're probably the one to use it for now, so that
> should not be a problem, but I prefer to inform you.

Thanks!  This works perfectly.

-- 
Michael Welsh Duggan
(mwd@cert.org)