[PATCH] emacs: call "notmuch tag" only once when archiving a thread

[PATCH] emacs: call "notmuch tag" only once when archiving a thread

[Jani Nikula]

Optimize thread archiving by combining all the -inbox tagging operations to
a single "notmuch tag" call. Also skip redisplay of tag changes in current
buffer, as it is immediately killed by the archiving functions.

For threads in the order of tens or a hundred inbox tagged messages, this
gives a noticeable speedup.

On the downside, IIRC Xapian does not perform very well if the query (in
this case a lot of message-ids OR'd together) is very big. It is unknown to
me at which point this approach would become slower than the original one
by one tagging approach, if ever.

Also, this introduces a limitation to the number of messages that can be
archived at the same time (through ARG_MAX limiting the command line). At
least on Linux this seems more like a theoretical limitation than a real
one.

Signed-off-by: Jani Nikula <jani@nikula.org>

---

On my Linux machines, 'getconf ARG_MAX' gives 2097152, leading me to
believe that notmuch-show would choke on the thread that would be limited
by this anyway...
---
 emacs/notmuch-show.el |   23 ++++++++++++++++++++---
 1 files changed, 20 insertions(+), 3 deletions(-)

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index 5502efd..b9ea839 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -1414,11 +1414,28 @@ argument, hide all of the messages."
   (interactive)
   (backward-button 1))
 
+(defun notmuch-show-thread-remove-tag (&rest toremove)
+  "Remove TOREMOVE tags from the current set of messages.
+
+Note: This function does not call `notmuch-show-set-tags' on the
+messages to redisplay the changed tags. This is meant to be
+called by `notmuch-show-archive-thread-internal' which kills the
+buffer afterwards."
+  (goto-char (point-min))
+  (let ((message-ids))
+    (loop do
+	  (let* ((current-tags (notmuch-show-get-tags))
+		 (new-tags (notmuch-show-del-tags-worker current-tags toremove)))
+	    (unless (equal current-tags new-tags)
+	      (add-to-list 'message-ids (notmuch-show-get-message-id))))
+	  until (not (notmuch-show-goto-message-next)))
+    (when message-ids
+      (apply 'notmuch-tag (mapconcat 'identity message-ids " OR ")
+	     (mapcar (lambda (s) (concat "-" s)) toremove)))))
+
 (defun notmuch-show-archive-thread-internal (show-next)
   ;; Remove the tag from the current set of messages.
-  (goto-char (point-min))
-  (loop do (notmuch-show-remove-tag "inbox")
-	until (not (notmuch-show-goto-message-next)))
+  (notmuch-show-thread-remove-tag "inbox")
   ;; Move to the next item in the search results, if any.
   (let ((parent-buffer notmuch-show-parent-buffer))
     (notmuch-kill-this-buffer)
-- 
1.7.5.4

Re: [PATCH] emacs: call "notmuch tag" only once when archiving a thread

[David Edmondson]

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

This seems like a good idea.

On Tue,  3 Jan 2012 20:29:06 +0200, Jani Nikula <jani@nikula.org> wrote:
> On the downside, IIRC Xapian does not perform very well if the query
> (in this case a lot of message-ids OR'd together) is very big. It is
> unknown to me at which point this approach would become slower than
> the original one by one tagging approach, if ever.

Unless this limit is quite small (<1000), I'd be inclined not to worry
about it.

> Also, this introduces a limitation to the number of messages that can
> be archived at the same time (through ARG_MAX limiting the command
> line). At least on Linux this seems more like a theoretical limitation
> than a real one.

What's the failure mode when this does happen?

> +  (let ((message-ids))

No need for both sets of brackets:

   (let (message-ids)

is sufficient.

> +    (loop do
> +	  (let* ((current-tags (notmuch-show-get-tags))
> +		 (new-tags (notmuch-show-del-tags-worker current-tags toremove)))
> +	    (unless (equal current-tags new-tags)
> +	      (add-to-list 'message-ids (notmuch-show-get-message-id))))
> +	  until (not (notmuch-show-goto-message-next)))

`loop' has the ability to accumulate results, which would probably be
cleaner than `add-to-list'. See 'Accumulation Clauses' in the emacs cl
info.

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

Re: [PATCH] emacs: call "notmuch tag" only once when archiving a thread

[Tomi Ollila]

On Tue,  3 Jan 2012 20:29:06 +0200, Jani Nikula <jani@nikula.org> wrote:
> Optimize thread archiving by combining all the -inbox tagging operations to
> a single "notmuch tag" call. Also skip redisplay of tag changes in current
> buffer, as it is immediately killed by the archiving functions.
> 
> For threads in the order of tens or a hundred inbox tagged messages, this
> gives a noticeable speedup.
> 
> On the downside, IIRC Xapian does not perform very well if the query (in
> this case a lot of message-ids OR'd together) is very big. It is unknown to
> me at which point this approach would become slower than the original one
> by one tagging approach, if ever.
> 
> Also, this introduces a limitation to the number of messages that can be
> archived at the same time (through ARG_MAX limiting the command line). At
> least on Linux this seems more like a theoretical limitation than a real
> one.

IIRC some systems have like 32768 byte command line limit.
If the change did tagging in like 100-message batches then this limit is
hardly exceeded (if message-id's max 80 characters then command line is
8000+ bytes)...
Hmm -- is there a length limit for message-id. If not maybe when appending 
message id's keep counting length and tag in batches based on that lenght
value.

Hundreds of messages per tagging operation instead of one is good
improvements.

> Signed-off-by: Jani Nikula <jani@nikula.org>

Tomi

Re: [PATCH] emacs: call "notmuch tag" only once when archiving a thread

[Aaron Ecay]

On Tue,  3 Jan 2012 20:29:06 +0200, Jani Nikula <jani@nikula.org> wrote:
> Optimize thread archiving by combining all the -inbox tagging operations to
> a single "notmuch tag" call.

Perhaps I’m missing something, but is there anything preventing emacs
from just doing the following?

notmuch tag -inbox thread:000whatever

-- 
Aaron Ecay

Re: [PATCH] emacs: call "notmuch tag" only once when archiving a thread

[Jani Nikula]

On Thu, 05 Jan 2012 15:10:33 -0500, Aaron Ecay <aaronecay@gmail.com> wrote:
> On Tue,  3 Jan 2012 20:29:06 +0200, Jani Nikula <jani@nikula.org> wrote:
> > Optimize thread archiving by combining all the -inbox tagging operations to
> > a single "notmuch tag" call.
> 
> Perhaps I’m missing something, but is there anything preventing emacs
> from just doing the following?
> 
> notmuch tag -inbox thread:000whatever

In the search view it does exactly this.

In the show view it only modifies the messages that are currently
visible. This is to make sure you don't accidentally archive things that
have arrived after refreshing the buffer. I think this is safest.

I suppose the point in the difference is that in search view you're
archiving the thread without seeing the messages anyway. In show view
you have had enough interest in the thread to open it, and you're making
the decision based on the messages you've seen.


BR,
Jani.

Re: [PATCH] emacs: call "notmuch tag" only once when archiving a thread

[Jameson Graef Rollins]

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

On Thu, 05 Jan 2012 22:32:16 +0200, Jani Nikula <jani@nikula.org> wrote:
> In the search view it does exactly this.

I worry about race conditions in this case, though.  I frequently
archive threads after I've read everything, but I still want to know if
new message to that thread come in.  If I attempt to archive a thread in
notmuch-search, but a new message has entered the thread without me
knowing it, I'll archive the new message before I had a chance to look
at it.

jamie.

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

Re: [PATCH] emacs: call "notmuch tag" only once when archiving a thread

[Jani Nikula]

On Thu, 05 Jan 2012 12:38:18 -0800, Jameson Graef Rollins <jrollins@finestructure.net> wrote:
> On Thu, 05 Jan 2012 22:32:16 +0200, Jani Nikula <jani@nikula.org> wrote:
> > In the search view it does exactly this.
> 
> I worry about race conditions in this case, though.  I frequently
> archive threads after I've read everything, but I still want to know if
> new message to that thread come in.  If I attempt to archive a thread in
> notmuch-search, but a new message has entered the thread without me
> knowing it, I'll archive the new message before I had a chance to look
> at it.

Just to be clear: The patch in question does not alter this
behaviour. The race you describe is there already.

And at a glance, it does not look like something that would be trivial
to fix. The buffer does not have the information to do that.


BR,
Jani.

Re: [PATCH] emacs: call "notmuch tag" only once when archiving a thread

[Tomi Ollila]

On Thu, 05 Jan 2012 22:58:30 +0200, Jani Nikula <jani@nikula.org> wrote:
> On Thu, 05 Jan 2012 12:38:18 -0800, Jameson Graef Rollins <jrollins@finestructure.net> wrote:
> > On Thu, 05 Jan 2012 22:32:16 +0200, Jani Nikula <jani@nikula.org> wrote:
> > > In the search view it does exactly this.
> > 
> > I worry about race conditions in this case, though.  I frequently
> > archive threads after I've read everything, but I still want to know if
> > new message to that thread come in.  If I attempt to archive a thread in
> > notmuch-search, but a new message has entered the thread without me
> > knowing it, I'll archive the new message before I had a chance to look
> > at it.
> 
> Just to be clear: The patch in question does not alter this
> behaviour. The race you describe is there already.
> 
> And at a glance, it does not look like something that would be trivial
> to fix. The buffer does not have the information to do that.

Hmm, I currently run 'notmuch new' from command line... I've been thinking
changing that but now knowing this issue I think I'm not going to do that...


> BR,
> Jani.

Tomi

Re: [PATCH] emacs: call "notmuch tag" only once when archiving a thread

[Aaron Ecay]

On Thu, 05 Jan 2012 22:32:16 +0200, Jani Nikula <jani@nikula.org> wrote:

[...]

> In the show view it only modifies the messages that are currently
> visible. This is to make sure you don't accidentally archive things that
> have arrived after refreshing the buffer. I think this is safest.

Hmm.  Perhaps it would make sense to add a check in the search view that
the thread being archived[1] has the same number of messages as it did
when the buffer was constructed.  (The information on how many messages
the thread has is in the buffer; we would then compare this to the result
of “notmuch count thread:000foo” when the user requests to archive.)  If
the counts don’t match, the interface should show a message in the echo
area and (probably) refuse to do the tagging.

We could also optionally use this strategy in the search view too.  The
error message is simple there: “New messages have arrived; refresh
thread view before archiving.”  (It doesn’t make as much sense to tell
people to refresh a search view – it could be an expensive operation
and/or may not be idempotent if some thread’s tags have been changed.
So it’s harder to say what the advice should be in that case.)

If other people think it would be useful, I can work on a patch to
implement this approach.

Footnotes:
[1] Or having its tags changed generally.

-- 
Aaron Ecay

Re: [PATCH] emacs: call "notmuch tag" only once when archiving a thread

[Austin Clements]

Quoth Aaron Ecay on Jan 08 at  7:56 pm:
> On Thu, 05 Jan 2012 22:32:16 +0200, Jani Nikula <jani@nikula.org> wrote:
> 
> [...]
> 
> > In the show view it only modifies the messages that are currently
> > visible. This is to make sure you don't accidentally archive things that
> > have arrived after refreshing the buffer. I think this is safest.
> 
> Hmm.  Perhaps it would make sense to add a check in the search view that
> the thread being archived[1] has the same number of messages as it did
> when the buffer was constructed.  (The information on how many messages
> the thread has is in the buffer; we would then compare this to the result
> of “notmuch count thread:000foo” when the user requests to archive.)  If
> the counts don’t match, the interface should show a message in the echo
> area and (probably) refuse to do the tagging.

That sounds like a clever workaround.

There's been quite a bit of discussion on fixing this properly.  See,
for example
id:"CAH-f9WsPj=1Eu=g3sOePJgCTBFs6HrLdLq18xMEnJ8aZ00yCEg@mail.gmail.com".
The gist is that we need to include message IDs (or document IDs) in
the search output and use these in tagging operations, rather than the
unstable thread:XXX queries.  Unfortunately, actually fixing this got
stalled since adding this information the text format is a fool's
errand (having been the fool, I can say this!), so we need to switch
Emacs over to using the JSON search format first.  However, once
that's done, it's a relatively simple change.

Re: [PATCH] emacs: call "notmuch tag" only once when archiving a thread

[David Edmondson]

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

On Sun, 8 Jan 2012 20:12:59 -0500, Austin Clements <amdragon@MIT.EDU> wrote:
> ... so we need to switch Emacs over to using the JSON search format
> first.

Is anyone working on this? I made an attempt ages ago[1], but have not kept
it working.

Footnotes: 
[1]  id:"1291114825-3513-1-git-send-email-dme@dme.org"

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

Re: [PATCH] emacs: call "notmuch tag" only once when archiving a thread

[Jani Nikula]

On Sun, 8 Jan 2012 20:12:59 -0500, Austin Clements <amdragon@MIT.EDU> wrote:
> Quoth Aaron Ecay on Jan 08 at  7:56 pm:
> > On Thu, 05 Jan 2012 22:32:16 +0200, Jani Nikula <jani@nikula.org> wrote:
> > 
> > [...]
> > 
> > > In the show view it only modifies the messages that are currently
> > > visible. This is to make sure you don't accidentally archive things that
> > > have arrived after refreshing the buffer. I think this is safest.
> > 
> > Hmm.  Perhaps it would make sense to add a check in the search view that
> > the thread being archived[1] has the same number of messages as it did
> > when the buffer was constructed.  (The information on how many messages
> > the thread has is in the buffer; we would then compare this to the result
> > of “notmuch count thread:000foo” when the user requests to archive.)  If
> > the counts don’t match, the interface should show a message in the echo
> > area and (probably) refuse to do the tagging.
> 
> That sounds like a clever workaround.

The downside is that there's still a race condition: you could get new
messages between checking the number of messages in the thread and
tagging. The window for error would be much smaller than now, but it's
still there. (You could check afterwards if this happened, and notify
the user, "oooops, I just tagged N messages more than you intended"...)

J.

Re: [PATCH] emacs: call "notmuch tag" only once when archiving a thread

[Tomi Ollila]

On Mon, 09 Jan 2012 08:41:15 +0000, Jani Nikula <jani@nikula.org> wrote:
> On Sun, 8 Jan 2012 20:12:59 -0500, Austin Clements <amdragon@MIT.EDU> wrote:
> > Quoth Aaron Ecay on Jan 08 at  7:56 pm:
> > > On Thu, 05 Jan 2012 22:32:16 +0200, Jani Nikula <jani@nikula.org> wrote:
> > > 
> > > [...]
> > > 
> > > > In the show view it only modifies the messages that are currently
> > > > visible. This is to make sure you don't accidentally archive things that
> > > > have arrived after refreshing the buffer. I think this is safest.
> > > 
> > > Hmm.  Perhaps it would make sense to add a check in the search view that
> > > the thread being archived[1] has the same number of messages as it did
> > > when the buffer was constructed.  (The information on how many messages
> > > the thread has is in the buffer; we would then compare this to the result
> > > of “notmuch count thread:000foo” when the user requests to archive.)  If
> > > the counts don’t match, the interface should show a message in the echo
> > > area and (probably) refuse to do the tagging.
> > 
> > That sounds like a clever workaround.
> 
> The downside is that there's still a race condition: you could get new
> messages between checking the number of messages in the thread and
> tagging. The window for error would be much smaller than now, but it's
> still there. (You could check afterwards if this happened, and notify
> the user, "oooops, I just tagged N messages more than you intended"...)

And this could also be "false alarm" if the the new messages arrived
after tagging but before checking...

> 
> J.

Tomi

Re: [PATCH] emacs: call "notmuch tag" only once when archiving a thread

[David Edmondson]

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

On Mon, 09 Jan 2012 12:38:58 +0200, Tomi Ollila <tomi.ollila@iki.fi> wrote:
> > The downside is that there's still a race condition: you could get new
> > messages between checking the number of messages in the thread and
> > tagging. The window for error would be much smaller than now, but it's
> > still there. (You could check afterwards if this happened, and notify
> > the user, "oooops, I just tagged N messages more than you intended"...)
> 
> And this could also be "false alarm" if the the new messages arrived
> after tagging but before checking...

Or another process could change the tags, meaning that the number stayed
the same but the set changed.

(Wheeee....)

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

Re: [PATCH] emacs: call "notmuch tag" only once when archiving a thread

[Mark Walters]

> There's been quite a bit of discussion on fixing this properly.  See,
> for example
> id:"CAH-f9WsPj=1Eu=g3sOePJgCTBFs6HrLdLq18xMEnJ8aZ00yCEg@mail.gmail.com".
> The gist is that we need to include message IDs (or document IDs) in
> the search output and use these in tagging operations, rather than the
> unstable thread:XXX queries.  Unfortunately, actually fixing this got
> stalled since adding this information the text format is a fool's
> errand (having been the fool, I can say this!), so we need to switch
> Emacs over to using the JSON search format first.  However, once
> that's done, it's a relatively simple change.

I will just mention a different possible solution to this problem. I
think it is probably more hassle than its worth but just in case someone
sees how to do it nicely. 

The idea is to try and put the race-free solution into the command-line
notmuch rather than the emacs part.

I will just consider one race to start with: the race in notmuch
search/notmuch tag. notmuch search gives us just the thread-ids and what
is in that thread can change. So we could ask for a "cookie" (some short
unique identifier) and notmuch itself would store the state of its
output (i.e., the thread-ids together with the message-ids and the
matched message-ids).

Then notmuch tag could be called with this cookie to say "apply this tag
to the threads as they were at cookie.

example use
notmuch search --cookie <query>
prints a cookie followed by the output of the search

notmuch tag --cookie=cookievalue +sometag matched-thread:id .. \
                                 all-thread:id .. <other search terms>

and notmuch would replace the matched-thread:id by the (matching)
message-ids in thread it stored at time cookievalue and all-thread by
all the messages at time cookievalue.

The nice feature is that this makes it easy for all clients to be race
free (not just emacs). The backend has the information to hand: there is
no encoding and decoding (via text or JSON) so storing message-ids etc
is simpler.  It also means that if notmuch wants to use document-ids
internally it can: it does not become a public interface.

The downside is trying to do the argument parsing in c and we have do
some reference counting or something to delete the cookies state at some
point.

Anyway it's just a thought.

Best wishes

Mark