[PATCH 0/4] emacs: show: lazy handling of hidden parts

[PATCH 0/4] emacs: show: lazy handling of hidden parts

[Mark Walters]

This is a much better version of the WIP patch at
id:1367628568-11656-1-git-send-email-markwalters1009@gmail.com

There was some discussion on irc about the new invisibility handling
making large threads of messages with html parts slow to appear. This
is caused by the new code rendering all of these parts and then hiding
them. (I think this is exacerbated by the text/html having to fetch
the part as a separate request: it is not in the notmuch show output).

This code makes the rendering of all hidden parts lazy (ie it occurs
when the part is shown). This should make the common case of hidden
parts which are never viewed much faster.

The code is relatively simple: we store all the arguments to the part
insertion handler on the part button and then give them to the part
handler when needed. This is not very memory efficient (we already
store the whole message so we could extract it all again) but I think
it is unlikely to be a problem in practice.

The patch series looks very large but almost everything of interest
happens in the final patch: the rest is code rearrangement. In
particular patch 3/4 is entirely code rearrangement (and since it
changes some indentation looks like it changes much more than it
actually does).

Testing is always helpful but there are two particular things that
would be very useful: first, if anyone who has found it slow can see
if this fixes it and secondly if anyone with encryption setup could
test that the encryption buttons all work and look correct (I don't
have encryption set up).

In my small amount of testing it seems to work and all tests pass.

Best wishes

Mark


Mark Walters (4):
  emacs:show: separate out handling of application/octet-stream
  emacs: show: handle inline patch fake parts at top level
  emacs: show: move the insertion of the header button to the top level
  emacs: show: implement lazy hidden part handling

 emacs/notmuch-show.el |  136 ++++++++++++++++++++++++++++++-------------------
 emacs/notmuch-wash.el |    2 +-
 2 files changed, 84 insertions(+), 54 deletions(-)

-- 
1.7.9.1


*** BLURB HERE ***

Mark Walters (4):
  emacs:show: separate out handling of application/octet-stream
  emacs: show: handle inline patch fake parts at top level
  emacs: show: move the insertion of the header button to the top level
  emacs: show: implement lazy hidden part handling

 emacs/notmuch-show.el |  136 ++++++++++++++++++++++++++++++-------------------
 emacs/notmuch-wash.el |    2 +-
 2 files changed, 84 insertions(+), 54 deletions(-)

-- 
1.7.9.1

[PATCH 1/4] emacs:show: separate out handling of application/octet-stream

[Mark Walters]

Currently mime parts are basically handled based on their mime-type
with the exception of application/octet-stream parts. Deal with these
parts at the top level (notmuch-show-insert-bodypart).

This is needed later in the series as we need to put in a part button
for each part (which means knowing its mime type) while deferring the
actual insertion of the part.
---
 emacs/notmuch-show.el |   19 +++++++++++--------
 1 files changed, 11 insertions(+), 8 deletions(-)

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index face2a0..3b9a2ad 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -796,9 +796,9 @@ message at DEPTH in the current thread."
 (defun notmuch-show-insert-part-text/x-vcalendar (msg part content-type nth depth declared-type)
   (notmuch-show-insert-part-text/calendar msg part content-type nth depth declared-type))
 
-(defun notmuch-show-insert-part-application/octet-stream (msg part content-type nth depth declared-type)
+(defun notmuch-show-get-mime-type-of-application/octet-stream (part)
   ;; If we can deduce a MIME type from the filename of the attachment,
-  ;; do so and pass it on to the handler for that type.
+  ;; we return that.
   (if (plist-get part :filename)
       (let ((extension (file-name-extension (plist-get part :filename)))
 	    mime-type)
@@ -808,7 +808,7 @@ message at DEPTH in the current thread."
 	      (setq mime-type (mailcap-extension-to-mime extension))
 	      (if (and mime-type
 		       (not (string-equal mime-type "application/octet-stream")))
-		  (notmuch-show-insert-bodypart-internal msg part mime-type nth depth content-type)
+		  mime-type
 		nil))
 	  nil))))
 
@@ -885,11 +885,14 @@ message at DEPTH in the current thread."
   "Insert the body part PART at depth DEPTH in the current thread.
 
 If HIDE is non-nil then initially hide this part."
-  (let ((content-type (downcase (plist-get part :content-type)))
-	(nth (plist-get part :id))
-	(beg (point)))
-
-    (notmuch-show-insert-bodypart-internal msg part content-type nth depth content-type)
+  (let* ((content-type (downcase (plist-get part :content-type)))
+	 (mime-type (or (and (string= content-type "application/octet-stream")
+			     (notmuch-show-get-mime-type-of-application/octet-stream part))
+			content-type))
+	 (nth (plist-get part :id))
+	 (beg (point)))
+
+    (notmuch-show-insert-bodypart-internal msg part mime-type nth depth content-type)
     ;; Some of the body part handlers leave point somewhere up in the
     ;; part, so we make sure that we're down at the end.
     (goto-char (point-max))
-- 
1.7.9.1

[PATCH 2/4] emacs: show: handle inline patch fake parts at top level

[Mark Walters]

The inline patch fake part handler also modifies the content-type so
handle this in notmuch-show-insert-bodypart too.
---
 emacs/notmuch-show.el |    4 +++-
 emacs/notmuch-wash.el |    2 +-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index 3b9a2ad..f984143 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -814,7 +814,7 @@ message at DEPTH in the current thread."
 
 ;; Handler for wash generated inline patch fake parts.
 (defun notmuch-show-insert-part-inline-patch-fake-part (msg part content-type nth depth declared-type)
-  (notmuch-show-insert-part-*/* msg part "text/x-diff" nth depth "inline patch"))
+  (notmuch-show-insert-part-*/* msg part content-type nth depth declared-type))
 
 (defun notmuch-show-insert-part-text/html (msg part content-type nth depth declared-type)
   ;; text/html handler to work around bugs in renderers and our
@@ -888,6 +888,8 @@ If HIDE is non-nil then initially hide this part."
   (let* ((content-type (downcase (plist-get part :content-type)))
 	 (mime-type (or (and (string= content-type "application/octet-stream")
 			     (notmuch-show-get-mime-type-of-application/octet-stream part))
+			(and (string= content-type "inline patch")
+			     "text/x-diff")
 			content-type))
 	 (nth (plist-get part :id))
 	 (beg (point)))
diff --git a/emacs/notmuch-wash.el b/emacs/notmuch-wash.el
index 80c475c..8a68819 100644
--- a/emacs/notmuch-wash.el
+++ b/emacs/notmuch-wash.el
@@ -365,7 +365,7 @@ for error."
 	  (setq patch-end (match-beginning 0)))
       (save-restriction
 	(narrow-to-region patch-start patch-end)
-	(setq part (plist-put part :content-type "inline-patch-fake-part"))
+	(setq part (plist-put part :content-type "inline patch"))
 	(setq part (plist-put part :content (buffer-string)))
 	(setq part (plist-put part :id -1))
 	(setq part (plist-put part :filename
-- 
1.7.9.1

[PATCH 3/4] emacs: show: move the insertion of the header button to the top level

[Mark Walters]

Previously each of the part insertion handlers inserted the part
button themselves. Move this up into
notmuch-show-insert-bodypart. Since a small number of the handlers
modify the button (the encryption/signature ones) we need to pass the
header button as an argument into the individual part insertion
handlers.

The patch is large but mostly simple. The only things of note are that
we let the text/plain handler insert part buttons itself (as it does
not always insert one and it applies motmuch-wash to the whole part
including the button. Also this is by far the most important case so
this reduces the risk of annoying side effects.

The handler for inline-patch-fake-part is called from notmuch wash not
from notmuch-show-insert-bodypart so that needs to insert the header
button itself (rather than relying on notmuch-show-insert-part-*/*).
---
 emacs/notmuch-show.el |   91 +++++++++++++++++++++++-------------------------
 1 files changed, 44 insertions(+), 47 deletions(-)

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index f984143..2c48b24 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -575,8 +575,7 @@ message at DEPTH in the current thread."
   (mapcar (lambda (inner-part) (plist-get inner-part :content-type))
 	  (plist-get part :content)))
 
-(defun notmuch-show-insert-part-multipart/alternative (msg part content-type nth depth declared-type)
-  (notmuch-show-insert-part-header nth declared-type content-type nil)
+(defun notmuch-show-insert-part-multipart/alternative (msg part content-type nth depth declared-type button)
   (let ((chosen-type (car (notmuch-multipart/alternative-choose (notmuch-show-multipart/*-to-list part))))
 	(inner-parts (plist-get part :content))
 	(start (point)))
@@ -653,8 +652,7 @@ message at DEPTH in the current thread."
 	  content-type)
       nil)))
 
-(defun notmuch-show-insert-part-multipart/related (msg part content-type nth depth declared-type)
-  (notmuch-show-insert-part-header nth declared-type content-type nil)
+(defun notmuch-show-insert-part-multipart/related (msg part content-type nth depth declared-type button)
   (let ((inner-parts (plist-get part :content))
 	(start (point)))
 
@@ -673,16 +671,15 @@ message at DEPTH in the current thread."
       (indent-rigidly start (point) 1)))
   t)
 
-(defun notmuch-show-insert-part-multipart/signed (msg part content-type nth depth declared-type)
-  (let ((button (notmuch-show-insert-part-header nth declared-type content-type nil)))
-    (button-put button 'face 'notmuch-crypto-part-header)
-    ;; add signature status button if sigstatus provided
-    (if (plist-member part :sigstatus)
-	(let* ((from (notmuch-show-get-header :From msg))
-	       (sigstatus (car (plist-get part :sigstatus))))
-	  (notmuch-crypto-insert-sigstatus-button sigstatus from))
-      ;; if we're not adding sigstatus, tell the user how they can get it
-      (button-put button 'help-echo "Set notmuch-crypto-process-mime to process cryptographic MIME parts.")))
+(defun notmuch-show-insert-part-multipart/signed (msg part content-type nth depth declared-type button)
+  (button-put button 'face 'notmuch-crypto-part-header)
+  ;; add signature status button if sigstatus provided
+  (if (plist-member part :sigstatus)
+      (let* ((from (notmuch-show-get-header :From msg))
+	     (sigstatus (car (plist-get part :sigstatus))))
+	(notmuch-crypto-insert-sigstatus-button sigstatus from))
+    ;; if we're not adding sigstatus, tell the user how they can get it
+    (button-put button 'help-echo "Set notmuch-crypto-process-mime to process cryptographic MIME parts."))
 
   (let ((inner-parts (plist-get part :content))
 	(start (point)))
@@ -695,20 +692,19 @@ message at DEPTH in the current thread."
       (indent-rigidly start (point) 1)))
   t)
 
-(defun notmuch-show-insert-part-multipart/encrypted (msg part content-type nth depth declared-type)
-  (let ((button (notmuch-show-insert-part-header nth declared-type content-type nil)))
-    (button-put button 'face 'notmuch-crypto-part-header)
-    ;; add encryption status button if encstatus specified
-    (if (plist-member part :encstatus)
-	(let ((encstatus (car (plist-get part :encstatus))))
-	  (notmuch-crypto-insert-encstatus-button encstatus)
-	  ;; add signature status button if sigstatus specified
-	  (if (plist-member part :sigstatus)
-	      (let* ((from (notmuch-show-get-header :From msg))
-		     (sigstatus (car (plist-get part :sigstatus))))
-		(notmuch-crypto-insert-sigstatus-button sigstatus from))))
-      ;; if we're not adding encstatus, tell the user how they can get it
-      (button-put button 'help-echo "Set notmuch-crypto-process-mime to process cryptographic MIME parts.")))
+(defun notmuch-show-insert-part-multipart/encrypted (msg part content-type nth depth declared-type button)
+  (button-put button 'face 'notmuch-crypto-part-header)
+  ;; add encryption status button if encstatus specified
+  (if (plist-member part :encstatus)
+      (let ((encstatus (car (plist-get part :encstatus))))
+	(notmuch-crypto-insert-encstatus-button encstatus)
+	;; add signature status button if sigstatus specified
+	(if (plist-member part :sigstatus)
+	    (let* ((from (notmuch-show-get-header :From msg))
+		   (sigstatus (car (plist-get part :sigstatus))))
+	      (notmuch-crypto-insert-sigstatus-button sigstatus from))))
+    ;; if we're not adding encstatus, tell the user how they can get it
+    (button-put button 'help-echo "Set notmuch-crypto-process-mime to process cryptographic MIME parts."))
 
   (let ((inner-parts (plist-get part :content))
 	(start (point)))
@@ -721,8 +717,7 @@ message at DEPTH in the current thread."
       (indent-rigidly start (point) 1)))
   t)
 
-(defun notmuch-show-insert-part-multipart/* (msg part content-type nth depth declared-type)
-  (notmuch-show-insert-part-header nth declared-type content-type nil)
+(defun notmuch-show-insert-part-multipart/* (msg part content-type nth depth declared-type button)
   (let ((inner-parts (plist-get part :content))
 	(start (point)))
     ;; Show all of the parts.
@@ -734,8 +729,7 @@ message at DEPTH in the current thread."
       (indent-rigidly start (point) 1)))
   t)
 
-(defun notmuch-show-insert-part-message/rfc822 (msg part content-type nth depth declared-type)
-  (notmuch-show-insert-part-header nth declared-type content-type nil)
+(defun notmuch-show-insert-part-message/rfc822 (msg part content-type nth depth declared-type button)
   (let* ((message (car (plist-get part :content)))
 	 (body (car (plist-get message :body)))
 	 (start (point)))
@@ -756,7 +750,7 @@ message at DEPTH in the current thread."
       (indent-rigidly start (point) 1)))
   t)
 
-(defun notmuch-show-insert-part-text/plain (msg part content-type nth depth declared-type)
+(defun notmuch-show-insert-part-text/plain (msg part content-type nth depth declared-type button)
   (let ((start (point)))
     ;; If this text/plain part is not the first part in the message,
     ;; insert a header to make this clear.
@@ -769,8 +763,7 @@ message at DEPTH in the current thread."
 	(run-hook-with-args 'notmuch-show-insert-text/plain-hook msg depth))))
   t)
 
-(defun notmuch-show-insert-part-text/calendar (msg part content-type nth depth declared-type)
-  (notmuch-show-insert-part-header nth declared-type content-type (plist-get part :filename))
+(defun notmuch-show-insert-part-text/calendar (msg part content-type nth depth declared-type button)
   (insert (with-temp-buffer
 	    (insert (notmuch-get-bodypart-content msg part nth notmuch-show-process-crypto))
 	    ;; notmuch-get-bodypart-content provides "raw", non-converted
@@ -793,8 +786,8 @@ message at DEPTH in the current thread."
   t)
 
 ;; For backwards compatibility.
-(defun notmuch-show-insert-part-text/x-vcalendar (msg part content-type nth depth declared-type)
-  (notmuch-show-insert-part-text/calendar msg part content-type nth depth declared-type))
+(defun notmuch-show-insert-part-text/x-vcalendar (msg part content-type nth depth declared-type button)
+  (notmuch-show-insert-part-text/calendar msg part content-type nth depth declared-type button))
 
 (defun notmuch-show-get-mime-type-of-application/octet-stream (part)
   ;; If we can deduce a MIME type from the filename of the attachment,
@@ -813,10 +806,10 @@ message at DEPTH in the current thread."
 	  nil))))
 
 ;; Handler for wash generated inline patch fake parts.
-(defun notmuch-show-insert-part-inline-patch-fake-part (msg part content-type nth depth declared-type)
-  (notmuch-show-insert-part-*/* msg part content-type nth depth declared-type))
+(defun notmuch-show-insert-part-inline-patch-fake-part (msg part content-type nth depth declared-type button)
+  (notmuch-show-insert-part-*/* msg part content-type nth depth declared-type button))
 
-(defun notmuch-show-insert-part-text/html (msg part content-type nth depth declared-type)
+(defun notmuch-show-insert-part-text/html (msg part content-type nth depth declared-type button)
   ;; text/html handler to work around bugs in renderers and our
   ;; invisibile parts code. In particular w3m sets up a keymap which
   ;; "leaks" outside the invisible region and causes strange effects
@@ -824,11 +817,10 @@ message at DEPTH in the current thread."
   ;; tell w3m not to set a keymap (so the normal notmuch-show-mode-map
   ;; remains).
   (let ((mm-inline-text-html-with-w3m-keymap nil))
-    (notmuch-show-insert-part-*/* msg part content-type nth depth declared-type)))
+    (notmuch-show-insert-part-*/* msg part content-type nth depth declared-type button)))
 
-(defun notmuch-show-insert-part-*/* (msg part content-type nth depth declared-type)
+(defun notmuch-show-insert-part-*/* (msg part content-type nth depth declared-type button)
   ;; This handler _must_ succeed - it is the handler of last resort.
-  (notmuch-show-insert-part-header nth content-type declared-type (plist-get part :filename))
   (notmuch-mm-display-part-inline msg part nth content-type notmuch-show-process-crypto)
   t)
 
@@ -851,13 +843,13 @@ message at DEPTH in the current thread."
 
 ;; \f
 
-(defun notmuch-show-insert-bodypart-internal (msg part content-type nth depth declared-type)
+(defun notmuch-show-insert-bodypart-internal (msg part content-type nth depth declared-type button)
   (let ((handlers (notmuch-show-handlers-for content-type)))
     ;; Run the content handlers until one of them returns a non-nil
     ;; value.
     (while (and handlers
 		(not (condition-case err
-			 (funcall (car handlers) msg part content-type nth depth declared-type)
+			 (funcall (car handlers) msg part content-type nth depth declared-type button)
 		       (error (progn
 				(insert "!!! Bodypart insert error: ")
 				(insert (error-message-string err))
@@ -885,6 +877,9 @@ message at DEPTH in the current thread."
   "Insert the body part PART at depth DEPTH in the current thread.
 
 If HIDE is non-nil then initially hide this part."
+
+  ;; We handle text/plain specially as its code does things before
+  ;; inserting a part button (and does not always insert a part button).
   (let* ((content-type (downcase (plist-get part :content-type)))
 	 (mime-type (or (and (string= content-type "application/octet-stream")
 			     (notmuch-show-get-mime-type-of-application/octet-stream part))
@@ -892,9 +887,11 @@ If HIDE is non-nil then initially hide this part."
 			     "text/x-diff")
 			content-type))
 	 (nth (plist-get part :id))
-	 (beg (point)))
+	 (beg (point))
+	 (button (unless (string= mime-type "text/plain")
+		   (notmuch-show-insert-part-header nth mime-type content-type (plist-get part :filename)))))
 
-    (notmuch-show-insert-bodypart-internal msg part mime-type nth depth content-type)
+    (notmuch-show-insert-bodypart-internal msg part mime-type nth depth content-type button)
     ;; Some of the body part handlers leave point somewhere up in the
     ;; part, so we make sure that we're down at the end.
     (goto-char (point-max))
-- 
1.7.9.1

[PATCH 4/4] emacs: show: implement lazy hidden part handling

[Mark Walters]

This adds the actual code to do the lazy insertion of hidden parts.

We use a memory inefficient but simple method: when we come to insert
the part if it is hidden we just store all of the arguments to the
part insertion function as a button property. This means when we want
to show the part we can just resume where we left off.

The only slight subtlety/hack is that to simplify the handling of the
invisibility overlay (for the hiding unhiding later) we do insert some
dummy text which we remove when we show the part.
---
 emacs/notmuch-show.el |   32 ++++++++++++++++++++++++++++++--
 1 files changed, 30 insertions(+), 2 deletions(-)

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index 2c48b24..a14a135 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -559,6 +559,7 @@ message at DEPTH in the current thread."
 	 (overlay (button-get button 'overlay)))
     (when overlay
       (let* ((show (overlay-get overlay 'invisible))
+	     (lazy-part (button-get button :notmuch-lazy-part))
 	     (new-start (button-start button))
 	     (button-label (button-get button :base-label))
 	     (old-point (point))
@@ -569,7 +570,11 @@ message at DEPTH in the current thread."
 	(let ((old-end (button-end button)))
 	  (move-overlay button new-start (point))
 	  (delete-region (point) old-end))
-	(goto-char (min old-point (1- (button-end button))))))))
+	(goto-char (min old-point (1- (button-end button))))
+	(when (and show lazy-part)
+	  (save-excursion
+	    (button-put button :notmuch-lazy-part nil)
+	    (notmuch-show-lazy-part lazy-part button)))))))
 
 (defun notmuch-show-multipart/*-to-list (part)
   (mapcar (lambda (inner-part) (plist-get inner-part :content-type))
@@ -857,6 +862,24 @@ message at DEPTH in the current thread."
       (setq handlers (cdr handlers))))
   t)
 
+(defun notmuch-show-lazy-part (part-args button)
+  (interactive)
+  ;; We have to save the depth as we can't find the depth when narrowed
+  (let ((inhibit-read-only t)
+	(overlay (button-get button 'overlay))
+	(depth (notmuch-show-get-depth)))
+    (save-restriction
+      (narrow-to-region (overlay-start overlay) (1- (overlay-end overlay)))
+      (delete-region (overlay-start overlay) (1- (overlay-end overlay)))
+      (goto-char (overlay-start overlay))
+      (apply #'notmuch-show-insert-bodypart-internal (nconc part-args (list button)))
+      (indent-rigidly (overlay-start overlay)
+		      (1- (overlay-end overlay))
+		      depth))
+    ;; We deferred deleting this character to simplify handling of the
+    ;; overlay: all of the above takes place inside the overlay.
+    (delete-region (1- (overlay-end overlay)) (overlay-end overlay))))
+
 (defun notmuch-show-create-part-overlays (msg beg end hide)
   "Add an overlay to the part between BEG and END"
   (let* ((button (button-at beg))
@@ -891,7 +914,12 @@ If HIDE is non-nil then initially hide this part."
 	 (button (unless (string= mime-type "text/plain")
 		   (notmuch-show-insert-part-header nth mime-type content-type (plist-get part :filename)))))
 
-    (notmuch-show-insert-bodypart-internal msg part mime-type nth depth content-type button)
+    (if (not hide)
+        (notmuch-show-insert-bodypart-internal msg part mime-type nth depth content-type button)
+      (insert "lazy part")
+      (button-put button :notmuch-lazy-part
+                  (list msg part mime-type nth depth content-type)))
+
     ;; Some of the body part handlers leave point somewhere up in the
     ;; part, so we make sure that we're down at the end.
     (goto-char (point-max))
-- 
1.7.9.1

Re: [PATCH 2/4] emacs: show: handle inline patch fake parts at top level

[David Bremner]

Mark Walters <markwalters1009@gmail.com> writes:

> The inline patch fake part handler also modifies the content-type so
> handle this in notmuch-show-insert-bodypart too.

Pushed the first two in the series.

d