Re: nnml/nnfolder marks faster

[Simon Josefsson <jas@extundo.com>]

Dan Christensen <jdc+news@uwo.ca> writes:

> Simon Josefsson <jas@extundo.com> writes:
>
>> On Fri, 21 Sep 2001, Dan Christensen wrote:
>>
>>> Simon Josefsson <jas@extundo.com> writes:
>>>
>>> > Possibly.  Did you try my patch in
>>> > <iluheu5adba.fsf@barbar.josefsson.org>?  I think it keeps `score' out
>>> > of .mrk.
>>>
>>> It doesn't work.  Score entries remain in the .mrk file.
>>
>> Oops, I just found a problem with the patch.  Please change
>>
>> +(defconst gnus-article-unpropagated-mark-lists '(cache download unsend)
>>
>> into
>>
>> +(defconst gnus-article-unpropagated-mark-lists '(cache download unsend score)
>
> Still doesn't work.  western.mrk included below.  (I made sure to
> change a mark in this group so the .mrk file got rewritten.)  The
> .mrk file also contains seen entries.  If it matters, I'm using
> nnfolder.
>
> More testing...  Ok, if I delete the western.mrk file, then no 
> score entries get put there, even if gnus-save-score is set to t.

Ok, good.

> On the other hand, if there are already score entries in the
> western.mrk file, then they remain there when the western.mrk
> file gets rewritten, even with the patch applied.

The patches below fixes this by making sure existing marks are removed
as well.

> In every case I've tried, the seen marks are written to the
> western.mrk file.

Yup, the patch didn't care about seen.

One could probably argue that seen marks should be specific to each
Gnus installation -- you could use the seen mark to see which articles
YOU have read in a shared mailbox in which some articles may have been
marked as read by other people -- so I added `seen' to
`gnus-article-unpropagated-mark-lists' as well.

>>> Even worse, they are changed to be incorrect.  For example, the score pair
>>> (579 . 2000) was changed to (579 . 1092)  (and lots of other 2000's
>>> were changed to 1092's as well).
>>
>> I suspect Gnus tries to sort the score marks as ranges somehow, which is a
>> bad idea.
>
> But this only happens in the .mrk file, not in the .newsrc.eld file.
> (Included file illustrates this.)

I think I found the problem, nnml/nnfolder calls some gnus-range
functions on those ranges, and since they aren't proper ranges they
are messed up.  Gnus takes care to not do that for score marks.  This
is fixed by not storing score in .mrk.

>>> - score entries aren't purged when the article they refer to is
>>>   deleted/expired
>>
>> Once we get the score mark out of the .mrk file, this and your main
>> problem should vanish, I hope.
>
> I think this is a separate problem.  The score entries aren't
> expired from .newsrc.eld either.  So this bug will still need to
> be fixed even if score entries don't propagate to the .mrk files.

Yup.  Let's take care of it once .mrk works.

Ok, here is the complete set of patches that fixes things, include
Paul's doc fix.  Please test it and I'll commit it.

Are there any outstanding issues with the .mrk/.marks after this?  How
is the performance for you?  Profiling results of
`nnfolder-request-update-info' and `nnfolder-request-set-mark' for
largish groups would be interesting.

2001-09-22  Simon Josefsson  <jas@extundo.com>

	* nnml.el (nnml-open-marks): Remove unpropagatable marks.

	* nnfolder.el (nnfolder-open-marks): Ditto.

	* gnus-sum.el (gnus-article-unpropagatable-p): New function.
	(gnus-update-marks): Use it.
	(gnus-update-marks): Use `gnus-article-mark-to-type' instead of
	hardcoded list.

	* gnus.el (gnus-article-special-mark-lists): Add killed.
	(gnus-article-unpropagated-mark-lists): New constant.

Index: gnus.el
===================================================================
RCS file: /usr/local/cvsroot/gnus/lisp/gnus.el,v
retrieving revision 6.54
diff -u -r6.54 gnus.el
--- gnus.el	2001/08/23 18:56:59	6.54
+++ gnus.el	2001/09/22 16:49:47
@@ -1800,8 +1800,22 @@
 
 (defconst gnus-article-special-mark-lists
   '((seen range)
+    (killed range)
     (bookmark tuple)
     (score tuple)))
+
+;; Propagate flags to server, with the following exceptions:
+;; `seen' is private to each gnus installation
+;; `cache' is a internal gnus flag for each gnus installation
+;; `download' is a agent flag private to each gnus installation
+;; `unsend' are for nndraft groups only
+;; `score' is not a proper mark
+(defconst gnus-article-unpropagated-mark-lists 
+  '(seen cache download unsend score)
+  "Marks that shouldn't be propagated to backends.
+Typical marks are those that make no sense in a standalone backend,
+such as a mark that says whether an article is stored in the cache
+(which doesn't make sense in a standalone backend).")
 
 (defvar gnus-headers-retrieved-by nil)
 (defvar gnus-article-reply nil)
Index: gnus-sum.el
===================================================================
RCS file: /usr/local/cvsroot/gnus/lisp/gnus-sum.el,v
retrieving revision 6.106
diff -u -r6.106 gnus-sum.el
--- gnus-sum.el	2001/09/10 09:29:50	6.106
+++ gnus-sum.el	2001/09/22 16:49:48
@@ -4811,6 +4811,10 @@
   (or (cadr (assq mark gnus-article-special-mark-lists))
       'list))
 
+(defun gnus-article-unpropagatable-p (mark)
+  "Return whether MARK should be propagated to backend."
+  (memq mark gnus-article-unpropagated-mark-lists))
+
 (defun gnus-adjust-marked-articles (info)
   "Set all article lists and remove all marks that are no longer valid."
   (let* ((marked-lists (gnus-info-marks info))
@@ -4870,7 +4874,6 @@
   "Enter the various lists of marked articles into the newsgroup info list."
   (let ((types gnus-article-mark-lists)
 	(info (gnus-get-info gnus-newsgroup-name))
-	(uncompressed '(score bookmark killed seen))
 	type list newmarked symbol delta-marks)
     (when info
       ;; Add all marks lists to the list of marks lists.
@@ -4902,27 +4905,20 @@
 		    (gnus-add-to-range list gnus-newsgroup-unseen)
 		  (gnus-compress-sequence gnus-newsgroup-articles))))
 
-	(unless (memq (cdr type) uncompressed)
+	(when (eq (gnus-article-mark-to-type (cdr type)) 'list)
 	  (setq list (gnus-compress-sequence (set symbol (sort list '<)) t)))
 
-	(when (gnus-check-backend-function
-	       'request-set-mark gnus-newsgroup-name)
-	  ;; propagate flags to server, with the following exceptions:
-	  ;; uncompressed:s are not proper flags (they are cons cells)
-	  ;; cache is a internal gnus flag
-	  ;; download are local to one gnus installation (well)
-	  ;; unsend are for nndraft groups only
-	  ;; xxx: generality of this?  this suits nnimap anyway
-	  (unless (memq (cdr type) (append '(cache download unsend)
-					   uncompressed))
-	    (let* ((old (cdr (assq (cdr type) (gnus-info-marks info))))
-		   (del (gnus-remove-from-range (gnus-copy-sequence old) list))
-		   (add (gnus-remove-from-range
-			 (gnus-copy-sequence list) old)))
-	      (when add
-		(push (list add 'add (list (cdr type))) delta-marks))
-	      (when del
-		(push (list del 'del (list (cdr type))) delta-marks)))))
+	(when (and (gnus-check-backend-function
+		    'request-set-mark gnus-newsgroup-name)
+		   (not (gnus-article-unpropagatable-p (cdr type))))
+	  (let* ((old (cdr (assq (cdr type) (gnus-info-marks info))))
+		 (del (gnus-remove-from-range (gnus-copy-sequence old) list))
+		 (add (gnus-remove-from-range
+		       (gnus-copy-sequence list) old)))
+	    (when add
+	      (push (list add 'add (list (cdr type))) delta-marks))
+	    (when del
+	      (push (list del 'del (list (cdr type))) delta-marks))))
 
 	(when list
 	  (push (cons (cdr type) list) newmarked)))
Index: nnml.el
===================================================================
RCS file: /usr/local/cvsroot/gnus/lisp/nnml.el,v
retrieving revision 6.26
diff -u -r6.26 nnml.el
--- nnml.el	2001/09/15 21:18:36	6.26
+++ nnml.el	2001/09/22 16:49:49
@@ -1000,16 +1000,17 @@
 	       nnml-marks-file-name 
 	       (nnmail-group-pathname group nnml-directory))))
     (if (file-exists-p file)
-	(setq nnml-marks (condition-case err
-			     (with-temp-buffer
-			       (gnus-sethash file
-					     (nth 5 (file-attributes file))
-					     nnml-marks-modtime)
-			       (nnheader-insert-file-contents file)
-			       (read (current-buffer)))
-			   (error (or (gnus-yes-or-no-p
-				       (format "Error reading nnml marks file %s (%s).  Continuing will use marks from .newsrc.eld.  Continue? " file err))
-				      (error "Cannot read nnml marks file %s (%s)" file err)))))
+	(condition-case err
+	    (with-temp-buffer
+	      (gnus-sethash file (nth 5 (file-attributes file))
+			    nnml-marks-modtime)
+	      (nnheader-insert-file-contents file)
+	      (setq nnml-marks (read (current-buffer)))
+	      (dolist (el gnus-article-unpropagated-mark-lists)
+		(setq nnml-marks (gnus-remassoc el nnml-marks))))
+	  (error (or (gnus-yes-or-no-p
+		      (format "Error reading nnml marks file %s (%s).  Continuing will use marks from .newsrc.eld.  Continue? " file err))
+		     (error "Cannot read nnml marks file %s (%s)" file err))))
       ;; User didn't have a .marks file.  Probably first time
       ;; user of the .marks stuff.  Bootstrap it from .newsrc.eld.
       (let ((info (gnus-get-info
@@ -1019,6 +1020,8 @@
 	(nnheader-message 7 "Bootstrapping marks for %s..." group)
 	(setq nnml-marks (gnus-info-marks info))
 	(push (cons 'read (gnus-info-read info)) nnml-marks)
+	(dolist (el gnus-article-unpropagated-mark-lists)
+	  (setq nnml-marks (gnus-remassoc el nnml-marks)))
 	(nnml-save-marks group server)))))
 
 (provide 'nnml)
Index: nnfolder.el
===================================================================
RCS file: /usr/local/cvsroot/gnus/lisp/nnfolder.el,v
retrieving revision 6.22
diff -u -r6.22 nnfolder.el
--- nnfolder.el	2001/09/08 20:55:16	6.22
+++ nnfolder.el	2001/09/22 16:49:49
@@ -1224,16 +1224,17 @@
 (defun nnfolder-open-marks (group server)
   (let ((file (nnfolder-group-marks-pathname group)))
     (if (file-exists-p file)
-	(setq nnfolder-marks (condition-case err
-				 (with-temp-buffer
-				   (gnus-sethash file
-						 (nth 5 (file-attributes file))
-						 nnfolder-marks-modtime)
-				   (nnheader-insert-file-contents file)
-				   (read (current-buffer)))
-			       (error (or (gnus-yes-or-no-p
-					   (format "Error reading nnfolder marks file %s (%s).  Continuing will use marks from .newsrc.eld.  Continue? " file err))
-					  (error "Cannot read nnfolder marks file %s (%s)" file err)))))
+	(condition-case err
+	    (with-temp-buffer
+	      (gnus-sethash file (nth 5 (file-attributes file)) 
+			    nnfolder-marks-modtime)
+	      (nnheader-insert-file-contents file)
+	      (setq nnfolder-marks (read (current-buffer)))
+	      (dolist (el gnus-article-unpropagated-mark-lists)
+		(setq nnfolder-marks (gnus-remassoc el nnfolder-marks))))
+	  (error (or (gnus-yes-or-no-p
+		      (format "Error reading nnfolder marks file %s (%s).  Continuing will use marks from .newsrc.eld.  Continue? " file err))
+		     (error "Cannot read nnfolder marks file %s (%s)" file err))))
       ;; User didn't have a .marks file.  Probably first time
       ;; user of the .marks stuff.  Bootstrap it from .newsrc.eld.
       (let ((info (gnus-get-info
@@ -1243,6 +1244,8 @@
 	(nnheader-message 7 "Bootstrapping marks for %s..." group)
 	(setq nnfolder-marks (gnus-info-marks info))
 	(push (cons 'read (gnus-info-read info)) nnfolder-marks)
+	(dolist (el gnus-article-unpropagated-mark-lists)
+	  (setq nnfolder-marks (gnus-remassoc el nnfolder-marks)))
 	(nnfolder-save-marks group server)))))
 
 (provide 'nnfolder)