bug#36712: 27.0.50; nnimap unconditionally setting marks in a loop

bug#36712: 27.0.50; nnimap unconditionally setting marks in a loop

[Eric Abrahamsen]

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


I started doing some very basic profiling of Gnus, and noticed that
getting new mail was resulting in a couple thousand calls to
`gnus-info-set-entry', as well as some of the range manipulation
functions.

It looks like updating IMAP group marks using qresync would loop over
all possible flag/mark pairs and unconditionally set everything, even
when there was nothing to set. With fewer than 100 IMAP groups in total,
that was a couple thousand extra function calls. Most of them with nil
arguments, but still.

I'm not super familiar with the code in question, but the attached diff
looks like it will pull the qresync behavior in line with the
non-qresync behavior, so I'm hoping it's correct. It cuts way down on
function calls, anyway!




In GNU Emacs 27.0.50 (build 4, x86_64-pc-linux-gnu, GTK+ Version 3.24.10)
 of 2019-07-15 built on slip
Repository revision: c687f25ee24bd29d623a23819eef930937252b25
Windowing system distributor 'The X.Org Foundation', version 11.0.12005000
System Description:	Arch Linux

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: nnimap-dont-add-marks.diff --]
[-- Type: text/x-patch, Size: 1367 bytes --]

diff --git a/lisp/gnus/nnimap.el b/lisp/gnus/nnimap.el
index 67c5db1e04..5f8e9d5ec0 100644
--- a/lisp/gnus/nnimap.el
+++ b/lisp/gnus/nnimap.el
@@ -1702,18 +1702,19 @@ nnimap-update-qresync-info
 	     (cdr (or (assoc (caddr type) flags) ; %Flagged
 		      (assoc (intern (cadr type) obarray) flags)
 		      (assoc (cadr type) flags))))) ; "\Flagged"
-	(setq marks (delq ticks marks))
-	(pop ticks)
-	;; Add the new marks we got.
-	(setq ticks (gnus-add-to-range ticks new-marks))
-	;; Remove the marks from messages that don't have them.
-	(setq ticks (gnus-remove-from-range
-		     ticks
-		     (gnus-compress-sequence
-		      (gnus-sorted-complement existing new-marks))))
-	(when ticks
-	  (push (cons (car type) ticks) marks)))
-      (gnus-info-set-marks info marks t))
+	(when new-marks
+	  (setq marks (delq ticks marks))
+	  (pop ticks)
+	  ;; Add the new marks we got.
+	  (setq ticks (gnus-add-to-range ticks new-marks))
+	  ;; Remove the marks from messages that don't have them.
+	  (setq ticks (gnus-remove-from-range
+		       ticks
+		       (gnus-compress-sequence
+			(gnus-sorted-complement existing new-marks))))
+	  (when ticks
+	    (push (cons (car type) ticks) marks))
+	  (gnus-info-set-marks info marks t))))
     ;; Add vanished to the list of unexisting articles.
     (when vanished
       (let* ((old-unexists (assq 'unexist marks))

bug#36712: Acknowledgement (27.0.50; nnimap unconditionally setting marks in a loop)

[Eric Abrahamsen]

Well, I'm fairly sure now this is the right thing to do. I'll leave this
open for a few more days, then push a fix.

bug#36712: Acknowledgement (27.0.50; nnimap unconditionally setting marks in a loop)

[Lars Ingebrigtsen]

Eric Abrahamsen <eric@ericabrahamsen.net> writes:

> Well, I'm fairly sure now this is the right thing to do. I'll leave this
> open for a few more days, then push a fix.

I think your patch looks correct...

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#36712: Acknowledgement (27.0.50; nnimap unconditionally setting marks in a loop)

[Eric Abrahamsen]

On 07/25/19 19:03 PM, Lars Ingebrigtsen wrote:
> Eric Abrahamsen <eric@ericabrahamsen.net> writes:
>
>> Well, I'm fairly sure now this is the right thing to do. I'll leave this
>> open for a few more days, then push a fix.
>
> I think your patch looks correct...

Okay, I went ahead and pushed it.

Thanks!