Subject: bug#18522: 24.4.50; mapcar is very slow

From: Lars Ingebrigtsen <[email-masked]>
Date: Fri, 26 Feb 2016 13:43:44 +1030

Eli Zaretskii <[email-masked]> writes:

>  . most of the buffers over which set-default loops were actually
>    killed, but they are still in all_buffers list which set-default
>    traverses, although they were supposed to be removed by GC
>  . killed buffers are not removed from all_buffers by GC because
>    they are referenced by gnus-buffers
>  . gnus-buffers references killed buffers because Peter kills buffers
>    behind Gnus back, instead of letting them be killed through
>    gnus-kill-buffer, which would have removed them from gnus-buffers

It's probably not just Peter, but...  code somewhere.  My gnus-buffers
is currently:

(#<buffer *unsent wide reply to Eli Zaretskii*> #<buffer *Article nnimap+hermes.netfonds.no:emacs-devel*> #<buffer  *Original Article nnimap+hermes.netfonds.no:emacs-devel*> #<buffer *Summary nnimap+hermes.netfonds.no:emacs-devel*> #<buffer *sent wide reply to Eli Zaretskii*> #<buffer *sent wide reply to Richard Stallman*> #<killed buffer> #<buffer *sent wide reply to Thierry Volpiatto*> #<buffer *sent wide reply to Lars Magne Ingebrigtsen*> #<buffer *sent wide reply to Matthew Styskal*> #<buffer *sent wide reply to Paul Eggert*<3>> #<buffer *sent wide reply to Zachary Kanfer*> #<buffer *sent wide reply to David Engster*> #<buffer *sent wide reply to Paul Eggert*<2>> #<buffer *sent wide reply to Paul Eggert*> #<killed buffer> #<killed buffer> #<killed buffer> #<killed buffer> #<killed buffer> #<killed buffer> #<killed buffer> #<killed buffer> #<killed buffer> #<killed buffer> #<killed buffer> #<killed buffer> #<killed buffer> #<killed buffer> #<killed buffer> #<buffer  *gnus article copy*> #<killed buffer> #<buffer  *nnmail message-id cache*> #<buffer  *nnimap hermes.netfonds.no nil  *nntpd**> #<buffer  *gnus work*> #<buffer .newsrc-dribble> #<buffer  *Gnus agent overview*> #<buffer *Group*>)

I'll fix the gnus-{add,kill}-buffer functions so that they remove all
the killed buffers.  That should help keep that list down to a
reasonable length, and let all the killed buffers be GC'd.

> If the above is an accurate account of what we've discovered, then we
> have several factors here that conspire to make Peter's Gnus slow:
>
>  . parse-time-string should try to avoid binding case-fold-search
>    globally, or at all

It's kinda weird.  This starts with:

(defun parse-time-string (string)

[...]

	(temp (parse-time-tokenize (downcase string))))

which calls

(defsubst parse-time-string-chars (char)
  (save-match-data
    (let (case-fold-search str)
      (cond ((eq char ?+) 1)
	    ((eq char ?-) -1)
	    ((eq char ?:) ?d)
	    ((string-match "[[:upper:]]" (setq str (string char))) ?A)
	    ((string-match "[[:lower:]]" str) ?a)
	    ((string-match "[[:digit:]]" str) ?0)))))

!?

Since we've already downcased the entire string, both the
`case-fold-search' and the match to [[:upper:]] seem rather
nonsensical?  So that should be fixed, but:

>  . set-default should skip killed buffers

Yes.  I think that would be a win in general.

> For the second issue, I propose to modify set-default to use
> FOR_EACH_LIVE_BUFFER instead of FOR_EACH_BUFFER.  Does anyone see a
> problem with that?

Hm...  If a buffer is killed, do the local variables still have an
effect?  I'm thinking of code like:

(with-temp-buffer
  (setq-local foo 'bar)
  (kill-buffer (current-buffer))
  (let ((buf (current-buffer)))
    (with-temp-buffer
      (let ((foo 'zot))
        (set-buffer buf)
        foo))))

Well, that answered itself.  :-) It returns zot.  (If we don't kill it
returns bar.)  So I don't see any reason not to use FOR_EACH_LIVE_BUFFER
here.

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