Notmuch Pick

Notmuch Pick

[Mark Walters]

Hi

Since I have had various requests for notmuch pick
(id:"1329096015-8078-2-git-send-email-markwalters1009@gmail.com") so I
have started a git repository at
git://github.com/markwalters1009/notmuch.git

The branch pick-6 is the current version. My intention is to start a new
branch each time I rebase on to current master but this may change. (I
suggest that people do not rely on a consistent history for this repository)

My intention is to try and get pick into mainline: there is a current
series id:"1339842107-10632-1-git-send-email-markwalters1009@gmail.com"
(allowing notmuch show --format=json to return non-entire threads). This
is the first chunk of the pick set. 

After that series there are some small emacs changes (primarily entry
points to pick), some small changes (25 lines of diff) to the core C code,
and the notmuch-pick.el file itself.

The plan is to put a stub notmuch-pick.el file into the main notmuch
repository and keep the real notmuch-pick.el in contrib until it becomes
good enough to be acceptable in mainline.

Patches, comments, reviews etc are all gratefully received!


USE OF PICK

This is just a quick introduction to the commands for pick: the two main
entry points are 'z' to start a notmuch pick "search", and 'Z' to
display the current search or "show" in pick form. Once in pick mode
RET displays the message in a split-pane window or M-RET opens it in a
new full-window buffer. Once in pick-mode '?' should document all the
available commands.

Best wishes

Mark

Re: Notmuch Pick

[Jameson Graef Rollins]

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

On Sat, Jun 16 2012, Mark Walters <markwalters1009@gmail.com> wrote:
> Since I have had various requests for notmuch pick
> (id:"1329096015-8078-2-git-send-email-markwalters1009@gmail.com") so I
> have started a git repository at
> git://github.com/markwalters1009/notmuch.git
>
> The branch pick-6 is the current version. My intention is to start a new
> branch each time I rebase on to current master but this may change. (I
> suggest that people do not rely on a consistent history for this repository)

Hey, Mark.  I took a look at this and I have a couple of comments:

726e11aff69e10499a9855e0ac2f15e518985c1f
    cli: notmuch-show with framing newlines between threads in JSON.

This patch introduces a change in the json output format, but there is
no subsequent update of the test suite, so it's causing a lot of test
failures.  Obviously this needs to be fixed, but it would probably be
nice to include a couple of other tests for the pick output itself.  At
the very least a sanity test to check that it's working at all would be
sufficient initially.

Would it also be useful to make this same change for the search out, for
consistency?  I notice the search output now uses newlines between all
fields, which should help for asynchronous processing, but it might be
nice to put newline separators between the initial and final brackets as
well.

df97df62b70b884a1cd367360ed6ff7eda0e8af6
    cli: add --headers_only option to notmuch-show.c

Your comment in this patch is very interesting:

    This is used by notmuch-pick.el (although not needed) because it gives a
    speed-up of at least a factor of a two; moreover it reduces the memory
    usage in emacs hugely.

The only difference between the regular show json output and the
--headers-only output, as far as I can tell, is the presence of the
content of text/plain parts if they exist in the message.  We previously
had a discussion about the show output not including any part bodies at
all, but we decided that the inclusion of text/plain bodies shouldn't
affect anything, so why not include them.  If they actually do, then I
argue we should just move to having show json not include any body parts
at all by default, and just have them be retrieved individually like we
do currently for non-text/plain parts.  This would make things cleaner,
and would get rid of the need to have this extra option, which really
doesn't produce a significantly different output.

jamie.

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

Re: Notmuch Pick

[Mark Walters]

On Mon, 18 Jun 2012, Jameson Graef Rollins <jrollins@finestructure.net> wrote:
> On Sat, Jun 16 2012, Mark Walters <markwalters1009@gmail.com> wrote:
>> Since I have had various requests for notmuch pick
>> (id:"1329096015-8078-2-git-send-email-markwalters1009@gmail.com") so I
>> have started a git repository at
>> git://github.com/markwalters1009/notmuch.git
>>
>> The branch pick-6 is the current version. My intention is to start a new
>> branch each time I rebase on to current master but this may change. (I
>> suggest that people do not rely on a consistent history for this repository)
>
> Hey, Mark.  I took a look at this and I have a couple of comments:
>
> 726e11aff69e10499a9855e0ac2f15e518985c1f
>     cli: notmuch-show with framing newlines between threads in JSON.
>
> This patch introduces a change in the json output format, but there is
> no subsequent update of the test suite, so it's causing a lot of test
> failures.  Obviously this needs to be fixed, but it would probably be
> nice to include a couple of other tests for the pick output itself.  At
> the very least a sanity test to check that it's working at all would be
> sufficient initially.

Hi 

As you say several tests do need fixing: I
thought I would leave that until people said they were basically happy
with the change. 

I am not sure about sanity tests for pick: how would that work while it
lives in contrib? Obviously it would need some tests before coming in to
proper mainline.

> Would it also be useful to make this same change for the search out, for
> consistency?  I notice the search output now uses newlines between all
> fields, which should help for asynchronous processing, but it might be
> nice to put newline separators between the initial and final brackets as
> well.

Right. 

> df97df62b70b884a1cd367360ed6ff7eda0e8af6
>     cli: add --headers_only option to notmuch-show.c
>
> Your comment in this patch is very interesting:
>
>     This is used by notmuch-pick.el (although not needed) because it gives a
>     speed-up of at least a factor of a two; moreover it reduces the memory
>     usage in emacs hugely.
>
> The only difference between the regular show json output and the
> --headers-only output, as far as I can tell, is the presence of the
> content of text/plain parts if they exist in the message.  We previously
> had a discussion about the show output not including any part bodies at
> all, but we decided that the inclusion of text/plain bodies shouldn't
> affect anything, so why not include them.  If they actually do, then I
> argue we should just move to having show json not include any body parts
> at all by default, and just have them be retrieved individually like we
> do currently for non-text/plain parts.  This would make things cleaner,
> and would get rid of the need to have this extra option, which really
> doesn't produce a significantly different output.

For one use of pick (displaying the structure of a single thread) this
is not important (and the asynchronous stuff is irrelevant too). For
another use of displaying the thread structure of a whole "folder" of
messages it is important. For example the output of 

notmuch show tag:notmuch

is about 70MB on my system, whereas with the --headers-only option this
drops to about 7MB. Note Emacs uses substantially  more than this much
memory to actually process the JSON, and on a low-powered laptop this
is enough to cause a swap-storm.

Your suggestion of just not including the text/plain is nice for
notmuch-pick, and is very simple (a single line change). However, it
does seem to slow the emacs show mode down noticeably on large threads
(something like 2s to 4s on a thread with 180 messages) so I worry that
this change might annoy people. What do you think?

Many thanks for the comments!

Mark

Re: Notmuch Pick

[Jameson Graef Rollins]

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

On Tue, Jun 19 2012, Mark Walters <markwalters1009@gmail.com> wrote:
> As you say several tests do need fixing: I
> thought I would leave that until people said they were basically happy
> with the change. 

Yeah, but failing tests are basically a non-starter for me.  You can
maybe get away with not adding new tests during development, but I'm
unlikely to even try out the patches if a bunch of the existing tests
are failing.

> I am not sure about sanity tests for pick: how would that work while it
> lives in contrib? Obviously it would need some tests before coming in to
> proper mainline.

Since you're not actually submitting these patches yet, does it really
need to live in contrib?  The goal is for it to ultimately *not* live in
contrib, so I would say just go ahead and make them apply in the main
source.

An example of a sanity test I'm talking about is simply display the pick
thread for something and test that it looks like expected.  Sort of a
zeroth order thing.  There are other emacs tests that do the same sort
of thing so it shouldn't be too hard to add.

>> Would it also be useful to make this same change for the search out, for
>> consistency?  I notice the search output now uses newlines between all
>> fields, which should help for asynchronous processing, but it might be
>> nice to put newline separators between the initial and final brackets as
>> well.
>
> Right. 

I would say go ahead and send to the list any patches that come up in
development that are generally useful.  Especially if they're small like
this they are likely to applied before this series, so it will be
smaller and easier to read when the times comes.

>> df97df62b70b884a1cd367360ed6ff7eda0e8af6
>>     cli: add --headers_only option to notmuch-show.c
>>
>> Your comment in this patch is very interesting:
>>
>>     This is used by notmuch-pick.el (although not needed) because it gives a
>>     speed-up of at least a factor of a two; moreover it reduces the memory
>>     usage in emacs hugely.
>>
>> The only difference between the regular show json output and the
>> --headers-only output, as far as I can tell, is the presence of the
>> content of text/plain parts if they exist in the message.  We previously
>> had a discussion about the show output not including any part bodies at
>> all, but we decided that the inclusion of text/plain bodies shouldn't
>> affect anything, so why not include them.  If they actually do, then I
>> argue we should just move to having show json not include any body parts
>> at all by default, and just have them be retrieved individually like we
>> do currently for non-text/plain parts.  This would make things cleaner,
>> and would get rid of the need to have this extra option, which really
>> doesn't produce a significantly different output.
>
> For one use of pick (displaying the structure of a single thread) this
> is not important (and the asynchronous stuff is irrelevant too). For
> another use of displaying the thread structure of a whole "folder" of
> messages it is important. For example the output of 
>
> notmuch show tag:notmuch
>
> is about 70MB on my system, whereas with the --headers-only option this
> drops to about 7MB. Note Emacs uses substantially  more than this much
> memory to actually process the JSON, and on a low-powered laptop this
> is enough to cause a swap-storm.
>
> Your suggestion of just not including the text/plain is nice for
> notmuch-pick, and is very simple (a single line change). However, it
> does seem to slow the emacs show mode down noticeably on large threads
> (something like 2s to 4s on a thread with 180 messages) so I worry that
> this change might annoy people. What do you think?

Threads that long are already a bit of a pain to deal with.  But I think
this reveals a weakness in the way we're displaying threads more than
anything.  It seems silly to me that we would retrieve all 180 messages
From such a thread, and format all of them, only to then hide all but
the one that the reader is seeing.  I think this "pick" series
illustrates this nicely.  Maybe it would be better to refactor the
current emacs show mode to only retrieve parts for messages that are
being displayed.  I'm not sure how difficult that would be, though.

I would also like to have access to more of the message headers in show
mode.  For instance, in emacs I want to access more headers with
notmuch-message-headers than are currently available.  Having json
always include all headers might be a bit of a performance hit for some
messages with lots of headers, but it would allow callers access to
headers they don't currently have access to at all.

So I think it would be nice and clean if the default json output
included all the message headers and the message structure, and then
part contents could be retrieved individually.  I imagine show mode
could be restructured such that it could use this efficiently.

jamie.

PS. where did the name "pick" come from?  It doesn't seem to fit with
the functionality to me.

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

Re: Notmuch Pick

[Peter Wang]

On Tue, 19 Jun 2012 11:52:01 -0700, Jameson Graef Rollins <jrollins@finestructure.net> wrote:
> 
> So I think it would be nice and clean if the default json output
> included all the message headers and the message structure, and then
> part contents could be retrieved individually.  I imagine show mode
> could be restructured such that it could use this efficiently.

Whatever is done, please keep the option to return all message bodies at
once, at least for short threads (the common case).  I want them anyway.
Don't force me to make N separate requests, especially over a high
latency link.

Peter