Gnus development mailing list
 help / color / mirror / Atom feed
* gnus-agent-expire-group-1 bug?
@ 2007-10-22  9:57 Katsumi Yamaoka
  2007-10-22 18:54 ` Reiner Steib
  0 siblings, 1 reply; 3+ messages in thread
From: Katsumi Yamaoka @ 2007-10-22  9:57 UTC (permalink / raw)
  To: ding

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

Hi,

The following section (prefixed with `-') in the function
`gnus-agent-expire-group-1' seems to do nothing, though I'm not
sure what it is for.  There's no `catch' for `sort-results', and
the `when' always gets nil.

(I found this while trying replacing `mapcar' with `mapc' or
`dolist'.
cf. http://article.gmane.org/gmane.emacs.gnus.general/65429)


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/x-patch, Size: 942 bytes --]

--- gnus-agent.el~	2007-10-04 21:53:35 +0000
+++ gnus-agent.el	2007-10-22 09:55:41 +0000
@@ -3329,27 +3329,6 @@
 
 		   (setcdr dlist (cddr dlist)))
 	       (setq dlist (cdr dlist)))))
-
-	 ;; Check the order of the entry positions.  They should be in
-	 ;; ascending order.  If they aren't, the positions must be
-	 ;; converted to markers.
-	 (when (let ((dlist dlist)
-		     (prev-pos -1)
-		     pos)
-		 (while dlist
-		   (if (setq pos (nth 3 (pop dlist)))
-		       (if (< pos prev-pos)
-			   (throw 'sort-results 'unsorted)
-			 (setq prev-pos pos)))))
-	   (gnus-message 7 "gnus-agent-expire: Unsorted overview; inserting markers to compensate.")
-	   (mapcar (lambda (entry)
-		     (let ((pos (nth 3 entry)))
-		       (if pos
-			   (setf (nth 3 entry)
-				 (set-marker (make-marker)
-					     pos)))))
-		   dlist))
-
 	 (gnus-message 7 "gnus-agent-expire: Merging entries... Done")
 
 	 (let* ((len (float (length dlist)))

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: gnus-agent-expire-group-1 bug?
  2007-10-22  9:57 gnus-agent-expire-group-1 bug? Katsumi Yamaoka
@ 2007-10-22 18:54 ` Reiner Steib
  2007-10-23  5:02   ` Kevin Greiner
  0 siblings, 1 reply; 3+ messages in thread
From: Reiner Steib @ 2007-10-22 18:54 UTC (permalink / raw)
  To: Kevin Greiner; +Cc: ding

On Mon, Oct 22 2007, Katsumi Yamaoka wrote:

> Hi,
>
> The following section (prefixed with `-') in the function
> `gnus-agent-expire-group-1' seems to do nothing, though I'm not
> sure what it is for.  There's no `catch' for `sort-results', and
> the `when' always gets nil.
>
> (I found this while trying replacing `mapcar' with `mapc' or
> `dolist'.
> cf. http://article.gmane.org/gmane.emacs.gnus.general/65429)
>
> --- gnus-agent.el~	2007-10-04 21:53:35 +0000
> +++ gnus-agent.el	2007-10-22 09:55:41 +0000
> @@ -3329,27 +3329,6 @@
>
>  		   (setcdr dlist (cddr dlist)))
>  	       (setq dlist (cdr dlist)))))
> -
> -	 ;; Check the order of the entry positions.  They should be in
> -	 ;; ascending order.  If they aren't, the positions must be
> -	 ;; converted to markers.
> -	 (when (let ((dlist dlist)
> -		     (prev-pos -1)
> -		     pos)
> -		 (while dlist
> -		   (if (setq pos (nth 3 (pop dlist)))
> -		       (if (< pos prev-pos)
> -			   (throw 'sort-results 'unsorted)
> -			 (setq prev-pos pos)))))
> -	   (gnus-message 7 "gnus-agent-expire: Unsorted overview; inserting markers to compensate.")
> -	   (mapcar (lambda (entry)
> -		     (let ((pos (nth 3 entry)))
> -		       (if pos
> -			   (setf (nth 3 entry)
> -				 (set-marker (make-marker)
> -					     pos)))))
> -		   dlist))
> -
>  	 (gnus-message 7 "gnus-agent-expire: Merging entries... Done")
>
>  	 (let* ((len (float (length dlist)))

Kevin, could you please comment on this?  The relevant log entry is

,----
| 2005-03-06  Kevin Greiner  <kevin.greiner@compsol.cc>
| [...]
| 	* gnus-agent.el [...]
| 	(gnus-agent-expire-group-1): Avoid using markers when the overview
| 	is in ascending order; greatly improves performance.
`----

Bye, Reiner.
-- 
       ,,,
      (o o)
---ooO-(_)-Ooo---  |  PGP key available  |  http://rsteib.home.pages.de/



^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: gnus-agent-expire-group-1 bug?
  2007-10-22 18:54 ` Reiner Steib
@ 2007-10-23  5:02   ` Kevin Greiner
  0 siblings, 0 replies; 3+ messages in thread
From: Kevin Greiner @ 2007-10-23  5:02 UTC (permalink / raw)
  To: Reiner.Steib, ding

Reiner Steib wrote:
> On Mon, Oct 22 2007, Katsumi Yamaoka wrote:
>
>   
>> Hi,
>>
>> The following section (prefixed with `-') in the function
>> `gnus-agent-expire-group-1' seems to do nothing, though I'm not
>> sure what it is for.  There's no `catch' for `sort-results', and
>> the `when' always gets nil.
>>
>> (I found this while trying replacing `mapcar' with `mapc' or
>> `dolist'.
>> cf. http://article.gmane.org/gmane.emacs.gnus.general/65429)
>>
>> --- gnus-agent.el~	2007-10-04 21:53:35 +0000
>> +++ gnus-agent.el	2007-10-22 09:55:41 +0000
>> @@ -3329,27 +3329,6 @@
>>
>>  		   (setcdr dlist (cddr dlist)))
>>  	       (setq dlist (cdr dlist)))))
>> -
>> -	 ;; Check the order of the entry positions.  They should be in
>> -	 ;; ascending order.  If they aren't, the positions must be
>> -	 ;; converted to markers.
>> -	 (when (let ((dlist dlist)
>> -		     (prev-pos -1)
>> -		     pos)
>> -		 (while dlist
>> -		   (if (setq pos (nth 3 (pop dlist)))
>> -		       (if (< pos prev-pos)
>> -			   (throw 'sort-results 'unsorted)
>> -			 (setq prev-pos pos)))))
>> -	   (gnus-message 7 "gnus-agent-expire: Unsorted overview; inserting markers to compensate.")
>> -	   (mapcar (lambda (entry)
>> -		     (let ((pos (nth 3 entry)))
>> -		       (if pos
>> -			   (setf (nth 3 entry)
>> -				 (set-marker (make-marker)
>> -					     pos)))))
>> -		   dlist))
>> -
>>  	 (gnus-message 7 "gnus-agent-expire: Merging entries... Done")
>>
>>  	 (let* ((len (float (length dlist)))
>>     
>
> Kevin, could you please comment on this?  The relevant log entry is
>
> ,----
> | 2005-03-06  Kevin Greiner  <kevin.greiner@compsol.cc>
> | [...]
> | 	* gnus-agent.el [...]
> | 	(gnus-agent-expire-group-1): Avoid using markers when the overview
> | 	is in ascending order; greatly improves performance.
> `----
>
> Bye, Reiner.
>   
I've added the missing catch form to the when function.  The code now 
looks like:

     (when (catch 'sort-results
         (let ((dlist dlist)
               (prev-pos -1)
               pos)
           (while dlist
             (if (setq pos (nth 3 (pop dlist)))
             (if (< pos prev-pos)
                 (throw 'sort-results 'unsorted)
               (setq prev-pos pos))))))
        ...)

For those interested, if any, there was a time when updates to the 
agent's overview file resulted in it having entries that were out of 
order.  The errors were proving difficult to isolate so the original 
gnus-agent-expire-group-1 function used markers to record the position 
of each entry.  Then, as the function edited the buffer, emacs kept the 
markers updated.  It was all very simple and yet horribly inefficient.  
By the time of the 2005-03-06 check-in, I was fairly confident that the 
overview file was always ordered.  That implied that 
gnus-agent-expire-group-1 no longer needed to use markers which greatly 
improved performance.  Still, I wasn't absolutely sure that overview 
would always be sorted so I added an integrity check and fall-back code 
to convert positions into markers when the file proved to be unsorted.  
Apparently, the integrity check has always been broken which implies 
that people either never use gnus-agent-expire-group or that the 
overview buffer is finally sorted.

Kevin







^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2007-10-23  5:02 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2007-10-22  9:57 gnus-agent-expire-group-1 bug? Katsumi Yamaoka
2007-10-22 18:54 ` Reiner Steib
2007-10-23  5:02   ` Kevin Greiner

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).