bug#5749: 24.0.50; Gomoku startup latency

bug#5749: 24.0.50; Gomoku startup latency

[Stephen Berman]

1. Let ~/.emacs consist of this sexp:
(setq initial-frame-alist '((fullscreen . fullheight)))
2. Start Emacs with the above init file.
3. M-x gomoku
=> There is a noticeable delay loading the game -- on my system 3-4
seconds.  This also occurs when the height frame parameter is assigned a
corresponding integer value.  The latency decreases as the height value
decreases.  (At the default height of 35, the startup time is
practically instantaneous.)

In GNU Emacs 24.0.50.1 (i686-pc-linux-gnu, GTK+ Version 2.18.6)
 of 2010-03-19 on escher
Windowing system distributor `The X.Org Foundation', version 11.0.10605000
Important settings:
  value of $LC_ALL: nil
  value of $LC_COLLATE: nil
  value of $LC_CTYPE: nil
  value of $LC_MESSAGES: nil
  value of $LC_MONETARY: nil
  value of $LC_NUMERIC: nil
  value of $LC_TIME: nil
  value of $LANG: en_US.UTF-8
  value of $XMODIFIERS: @im=local
  locale-coding-system: utf-8-unix
  default enable-multibyte-characters: t

bug#5749: 24.0.50; Gomoku startup latency

[Glenn Morris]

Stephen Berman wrote:

> 1. Let ~/.emacs consist of this sexp:
> (setq initial-frame-alist '((fullscreen . fullheight)))
> 2. Start Emacs with the above init file.
> 3. M-x gomoku
> => There is a noticeable delay loading the game -- on my system 3-4
> seconds.

This is interesting. Emacs 23.1 is almost instantaneous, but 23.1.94
is very slow, and also starts using a huge amount of memory (1GB+ for me).
Loading gomoku.el from 23.1 in 23.1.94 makes no difference.

In 23.1.94, the values of cons-cells-consed and intervals-consed go up
enormously (by ~ 70 million and 10 million) after running `gomoku'.

> In GNU Emacs 24.0.50.1 (i686-pc-linux-gnu, GTK+ Version 2.18.6)
>  of 2010-03-19 on escher

bug#5749: 24.0.50; Gomoku startup latency

[Stefan Monnier]

>> 1. Let ~/.emacs consist of this sexp:
>> (setq initial-frame-alist '((fullscreen . fullheight)))
>> 2. Start Emacs with the above init file.
>> 3. M-x gomoku
>> => There is a noticeable delay loading the game -- on my system 3-4
>> seconds.

> This is interesting. Emacs 23.1 is almost instantaneous, but 23.1.94
> is very slow, and also starts using a huge amount of memory (1GB+ for me).
> Loading gomoku.el from 23.1 in 23.1.94 makes no difference.

Indeed.  ELP says the time is all spent in insert-buffer-substring,
called via append-to-buffer (in my case, 13 calls of 0.5s each).


        Stefan

bug#5749: 24.0.50; Gomoku startup latency

[YAMAMOTO Mitsuharu]

>>>>> On Tue, 23 Mar 2010 21:51:24 -0400, Glenn Morris <rgm@gnu.org> said:

> Stephen Berman wrote:
>> 1. Let ~/.emacs consist of this sexp:
>> (setq initial-frame-alist '((fullscreen . fullheight)))
>> 2. Start Emacs with the above init file.
>> 3. M-x gomoku
>> => There is a noticeable delay loading the game -- on my system 3-4
>> seconds.

> This is interesting. Emacs 23.1 is almost instantaneous, but 23.1.94
> is very slow, and also starts using a huge amount of memory (1GB+ for me).
> Loading gomoku.el from 23.1 in 23.1.94 makes no difference.

This is due to a behavioral change of `append-to-buffer', which used
to preserve the point when the first argument coincides with the
current buffer.  The following patch would work for this case.

Maybe other occurrences of `(append-to-buffer (current-buffer) ...)'
should also be checked if they work as intended.

				     YAMAMOTO Mitsuharu
				mituharu@math.s.chiba-u.ac.jp

=== modified file 'lisp/play/gomoku.el'
*** lisp/play/gomoku.el	2010-01-13 08:35:10 +0000
--- lisp/play/gomoku.el	2010-03-24 03:04:35 +0000
***************
*** 1040,1046 ****
  		    (= i (- m 2))
  		    (progn
  		      (while (>= i 3)
! 			(append-to-buffer (current-buffer) opoint (point))
  			(setq i (- i 2)))
  		      (goto-char (point-max))))
  	       (setq point (point))
--- 1040,1046 ----
  		    (= i (- m 2))
  		    (progn
  		      (while (>= i 3)
! 			(prepend-to-buffer (current-buffer) opoint (point))
  			(setq i (- i 2)))
  		      (goto-char (point-max))))
  	       (setq point (point))

bug#5749: 24.0.50; Gomoku startup latency

[Glenn Morris]

YAMAMOTO Mitsuharu wrote:

> This is due to a behavioral change of `append-to-buffer', which used
> to preserve the point when the first argument coincides with the
> current buffer.

This change doesn't seem to be documented in NEWS, was it intentional?

bug#5749: 24.0.50; Gomoku startup latency

[Chong Yidong]

Glenn Morris <rgm@gnu.org> writes:

> YAMAMOTO Mitsuharu wrote:
>
>> This is due to a behavioral change of `append-to-buffer', which used
>> to preserve the point when the first argument coincides with the
>> current buffer.
>
> This change doesn't seem to be documented in NEWS, was it intentional?

I suspect this was done as part of Stefan's mass conversion of code to
with-current-buffer.

bug#5749: 24.0.50; Gomoku startup latency

[Stefan Monnier]

>>> This is due to a behavioral change of `append-to-buffer', which used
>>> to preserve the point when the first argument coincides with the
>>> current buffer.
>> This change doesn't seem to be documented in NEWS, was it intentional?
> I suspect this was done as part of Stefan's mass conversion of code to
> with-current-buffer.

Indeed.  Note that the new behavior happens to be more correct w.r.t its
docstring:

   It is inserted into that buffer before its point.

so it promises to move point in the specified buffer.  At first
I thought we should fix append-to-buffer to behave as it used to, but
now I'm wondering whether we should fix the uses instead.



        Stefan

bug#5749: 24.0.50; Gomoku startup latency

[Chong Yidong]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

> Indeed.  Note that the new behavior happens to be more correct w.r.t its
> docstring:
>
>    It is inserted into that buffer before its point.
>
> so it promises to move point in the specified buffer.  At first
> I thought we should fix append-to-buffer to behave as it used to, but
> now I'm wondering whether we should fix the uses instead.

I've changed it to behave as it used to in the branch, because who know
how many other packages rely on the same assumption gomoku did.

As for whether to switch to the new behavior in the trunk, if so it
should be documented.

bug#5749: 24.0.50; Gomoku startup latency

[YAMAMOTO Mitsuharu]

>>>>> On Wed, 24 Mar 2010 16:19:20 -0400, Chong Yidong <cyd@stupidchicken.com> said:

> Stefan Monnier <monnier@iro.umontreal.ca> writes:
>> Indeed.  Note that the new behavior happens to be more correct
>> w.r.t its docstring:
>> 
>>   It is inserted into that buffer before its point.
>> 
>> so it promises to move point in the specified buffer.  At first I
>> thought we should fix append-to-buffer to behave as it used to, but
>> now I'm wondering whether we should fix the uses instead.

That's exactly what I thought, and that's why I posted a patch that
changes (append-to-buffer (current-buffer) ...) to (prepend-to-buffer
(current-buffer) ...), rather than reverting append-to-buffer.

> I've changed it to behave as it used to in the branch, because who
> know how many other packages rely on the same assumption gomoku did.

> As for whether to switch to the new behavior in the trunk, if so it
> should be documented.

If it is changed in the trunk, then it might be good to add some
notices to documents in Emacs 23.2, telling the behavior of
append-to-buffer might change in future with respect to the current
buffer, and suggesting prepend-to-buffer for those cases to make the
behavior consistent.

				     YAMAMOTO Mitsuharu
				mituharu@math.s.chiba-u.ac.jp

bug#5749: 24.0.50; Gomoku startup latency

[Stephen Berman]

On Wed, 24 Mar 2010 12:27:03 +0900 YAMAMOTO Mitsuharu <mituharu@math.s.chiba-u.ac.jp> wrote:

>>>>>> On Tue, 23 Mar 2010 21:51:24 -0400, Glenn Morris <rgm@gnu.org> said:
>
>> Stephen Berman wrote:
>>> 1. Let ~/.emacs consist of this sexp:
>>> (setq initial-frame-alist '((fullscreen . fullheight)))
>>> 2. Start Emacs with the above init file.
>>> 3. M-x gomoku
>>> => There is a noticeable delay loading the game -- on my system 3-4
>>> seconds.
>
>> This is interesting. Emacs 23.1 is almost instantaneous, but 23.1.94
>> is very slow, and also starts using a huge amount of memory (1GB+ for me).
>> Loading gomoku.el from 23.1 in 23.1.94 makes no difference.
>
> This is due to a behavioral change of `append-to-buffer', which used
> to preserve the point when the first argument coincides with the
> current buffer.  The following patch would work for this case.

I confirm that this patch eliminates the startup latency for me.

Steve Berman

bug#5749: 24.0.50; Gomoku startup latency

[Chong Yidong]

YAMAMOTO Mitsuharu <mituharu@math.s.chiba-u.ac.jp> writes:

> This is due to a behavioral change of `append-to-buffer', which used
> to preserve the point when the first argument coincides with the
> current buffer.

I've changed append-to-buffer to conform to the previous behavior.

Could someone check whether gomoku works properly now?

bug#5749: 24.0.50; Gomoku startup latency

[Stephen Berman]

On Wed, 24 Mar 2010 12:18:59 -0400 Chong Yidong <cyd@stupidchicken.com> wrote:

> YAMAMOTO Mitsuharu <mituharu@math.s.chiba-u.ac.jp> writes:
>
>> This is due to a behavioral change of `append-to-buffer', which used
>> to preserve the point when the first argument coincides with the
>> current buffer.
>
> I've changed append-to-buffer to conform to the previous behavior.
>
> Could someone check whether gomoku works properly now?

I applied your change (after reverting Yamamoto-san's change I had
previously applied) and confirm it also eliminates the startup latency.

Steve Berman

bug#5749: 24.0.50; Gomoku startup latency

[YAMAMOTO Mitsuharu]

>>>>> On Wed, 24 Mar 2010 12:18:59 -0400, Chong Yidong <cyd@stupidchicken.com> said:

> YAMAMOTO Mitsuharu <mituharu@math.s.chiba-u.ac.jp> writes:
>> This is due to a behavioral change of `append-to-buffer', which
>> used to preserve the point when the first argument coincides with
>> the current buffer.

> I've changed append-to-buffer to conform to the previous behavior.

Not actually.  It has changed the behavior for the case that the
destination buffer is *not* the current one (especially, when the
destigation buffer does not have any windows).

				     YAMAMOTO Mitsuharu
				mituharu@math.s.chiba-u.ac.jp

bug#5749: 24.0.50; Gomoku startup latency

[Chong Yidong]

YAMAMOTO Mitsuharu <mituharu@math.s.chiba-u.ac.jp> writes:

>> I've changed append-to-buffer to conform to the previous behavior.
>
> Not actually.  It has changed the behavior for the case that the
> destination buffer is *not* the current one (especially, when the
> destigation buffer does not have any windows).

Sorry, should be fixed now.  Thanks for checking.