Emacs 22.2.90 pretest

Emacs 22.2.90 pretest

[Chong Yidong]

Emacs pretest version 22.2.90 is now available.  This is the first
pretest for Emacs 22.3, which will be a bugfix release.

The source tarball is available here:

http://alpha.gnu.org/gnu/emacs/pretest/emacs-22.2.90.tar.gz

The xdelta against Emacs 22.2 is here:

http://alpha.gnu.org/gnu/emacs/pretest/emacs-22.2-22.2.90.xdelta

The CVS tag is EMACS_PRETEST_22_2_90.


If you have problems building, please email emacs-devel@gnu.org.  For
all other bugs, please use M-x report-emacs-bug or send email to
emacs-pretest-bug@gnu.org.  For any other questions or problems, email
emacs-devel@gnu.org.

Thanks.

Re: Emacs 22.2.90 pretest

[Eli Zaretskii]

> From: Chong Yidong <cyd@stupidchicken.com>
> Date: Fri, 15 Aug 2008 13:08:55 -0400
> 
> Emacs pretest version 22.2.90 is now available.  This is the first
> pretest for Emacs 22.3, which will be a bugfix release.
> 
> The source tarball is available here:
> 
> http://alpha.gnu.org/gnu/emacs/pretest/emacs-22.2.90.tar.gz
> 
> The xdelta against Emacs 22.2 is here:
> 
> http://alpha.gnu.org/gnu/emacs/pretest/emacs-22.2-22.2.90.xdelta
> 
> The CVS tag is EMACS_PRETEST_22_2_90.

Thanks!

This pretest builds and seems to work fine on

   Linux fencepost 2.6.16.29-xen #1 SMP Wed Dec 6 07:32:36 EST 2006 x86_64 GNU/Linux

On MS-Windows, it fails to build because nt/emacsclient.rc is missing.
After fixing that, the rest of the build proceeds OK and the resulting
binary seems to work fine.

Also, the file info/.arch-inventory should not be in the tarball.

Re: Emacs 22.2.90 pretest

[Eli Zaretskii]

> Date: Sat, 16 Aug 2008 10:57:26 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: emacs-devel@gnu.org
> 
> This pretest builds and seems to work fine on
> 
>    Linux fencepost 2.6.16.29-xen #1 SMP Wed Dec 6 07:32:36 EST 2006 x86_64 GNU/Linux
> 
> On MS-Windows, it fails to build because nt/emacsclient.rc is missing.
> After fixing that, the rest of the build proceeds OK and the resulting
> binary seems to work fine.
> 
> Also, the file info/.arch-inventory should not be in the tarball.

The MS-DOS (a.k.a DJGPP) port also builds and seems to work fine.

Re: Emacs 22.2.90 pretest

[Eli Zaretskii]

> Date: Sat, 16 Aug 2008 10:57:26 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: emacs-devel@gnu.org
> 
> > From: Chong Yidong <cyd@stupidchicken.com>
> > Date: Fri, 15 Aug 2008 13:08:55 -0400
> > 
> > Emacs pretest version 22.2.90 is now available.  This is the first
> > pretest for Emacs 22.3, which will be a bugfix release.
> > 
> > The source tarball is available here:
> > 
> > http://alpha.gnu.org/gnu/emacs/pretest/emacs-22.2.90.tar.gz
> > 
> > The xdelta against Emacs 22.2 is here:
> > 
> > http://alpha.gnu.org/gnu/emacs/pretest/emacs-22.2-22.2.90.xdelta
> > 
> > The CVS tag is EMACS_PRETEST_22_2_90.
> 
> Thanks!

I notice that in this pretest, moving point with C-f and C-b or
inserting a single character is very sluggish: e.g., if I continuously
press C-f, Emacs cannot keep up(!), although this is a 3.2 GHz
machine.  It almost feels like working on a remote machine.

This recent change seems to be a likely suspect:

   2008-07-28  Chong Yidong  <cyd@stupidchicken.com>

	   * textmodes/flyspell.el (flyspell-word, flyspell-large-region)
	   (flyspell-region): Call ispell-maybe-find-aspell-dictionaries.

It seems that its effect is to call ispell-maybe-find-aspell-dictionaries
on every editing command, which is silly, IMO.  Even if we do need to do
that on every command (and I'd like to hear a reason why), the call
should only be made if Ispell is actually Aspell, which in my case it
isn't.  On top of that, ispell-maybe-find-aspell-dictionaries
obviously was not designed to be called frequently: it calls
ispell-check-version, which is expensive and is supposed to be run
once in an Ispell session (it makes unnecessary destructive changes to
the Ispell buffer, invokes another Ispell process, etc.).

Why was this change made?

Re: Emacs 22.2.90 pretest

[Chong Yidong]

Eli Zaretskii <eliz@gnu.org> writes:

> I notice that in this pretest, moving point with C-f and C-b or
> inserting a single character is very sluggish: e.g., if I continuously
> press C-f, Emacs cannot keep up(!), although this is a 3.2 GHz
> machine.  It almost feels like working on a remote machine.
>
> This recent change seems to be a likely suspect:
>
>    2008-07-28  Chong Yidong  <cyd@stupidchicken.com>
>
> 	   * textmodes/flyspell.el (flyspell-word, flyspell-large-region)
> 	   (flyspell-region): Call ispell-maybe-find-aspell-dictionaries.
>
> It seems that its effect is to call ispell-maybe-find-aspell-dictionaries
> on every editing command, which is silly, IMO.  Even if we do need to do
> that on every command (and I'd like to hear a reason why), the call
> should only be made if Ispell is actually Aspell, which in my case it
> isn't.  On top of that, ispell-maybe-find-aspell-dictionaries
> obviously was not designed to be called frequently: it calls
> ispell-check-version, which is expensive and is supposed to be run
> once in an Ispell session (it makes unnecessary destructive changes to
> the Ispell buffer, invokes another Ispell process, etc.).
>
> Why was this change made?

This was bug#232:

http://emacsbugs.donarmstrong.com/cgi-bin/bugreport.cgi?bug=232

Does undoing this change make the problem go away?  If so, it's probably
better to revert it.  (I attempted to backport a change from the trunk
to the branch, but there were other changes to the trunk that apparently
makes the dictionary initialization less expensive.)

Re: Emacs 22.2.90 pretest

[Agustin Martin]

On Sat, Aug 16, 2008 at 09:43:19AM -0400, Chong Yidong wrote:
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > I notice that in this pretest, moving point with C-f and C-b or
> > inserting a single character is very sluggish: e.g., if I continuously
> > press C-f, Emacs cannot keep up(!), although this is a 3.2 GHz
> > machine.  It almost feels like working on a remote machine.
> >
> > This recent change seems to be a likely suspect:
> >
> >    2008-07-28  Chong Yidong  <cyd@stupidchicken.com>
> >
> > 	   * textmodes/flyspell.el (flyspell-word, flyspell-large-region)
> > 	   (flyspell-region): Call ispell-maybe-find-aspell-dictionaries.
> >
> > It seems that its effect is to call ispell-maybe-find-aspell-dictionaries
> > on every editing command, which is silly, IMO.  Even if we do need to do
> > that on every command (and I'd like to hear a reason why), the call
> > should only be made if Ispell is actually Aspell, which in my case it
> > isn't.  On top of that, ispell-maybe-find-aspell-dictionaries
> > obviously was not designed to be called frequently: it calls
> > ispell-check-version, which is expensive and is supposed to be run
> > once in an Ispell session (it makes unnecessary destructive changes to
> > the Ispell buffer, invokes another Ispell process, etc.).
> >
> > Why was this change made?
> 
> This was bug#232:
> 
> http://emacsbugs.donarmstrong.com/cgi-bin/bugreport.cgi?bug=232
> 
> Does undoing this change make the problem go away?  If so, it's probably
> better to revert it.  (I attempted to backport a change from the trunk
> to the branch, but there were other changes to the trunk that apparently
> makes the dictionary initialization less expensive.)

I think only the change in (flyspell-word) is causing the problem. There was
no call there before new function (ispell-set-spellchecker-params) call was
added to HEAD. This makes sure expensive (ispell-check-version) call is done
only when there is an spellchecker change (or is the first time is used).
However (ispell-maybe-find-aspell-dictionaries) does not do that check, so
is very expensive when put there, since (flyspell-word) is continuosly
called.

The two other calls should be cheap, since they are run only once each time
functions containing it are called interactively.

-- 
Agustin

Re: Emacs 22.2.90 pretest

[Chong Yidong]

Agustin Martin <agustin.martin@hispalinux.es> writes:

> I think only the change in (flyspell-word) is causing the
> problem. There was no call there before new function
> (ispell-set-spellchecker-params) call was added to HEAD. This makes
> sure expensive (ispell-check-version) call is done only when there is
> an spellchecker change (or is the first time is used).  However
> (ispell-maybe-find-aspell-dictionaries) does not do that check, so is
> very expensive when put there, since (flyspell-word) is continuosly
> called.

Indeed; I've reverted the change in flyspell-word.  Thanks.

Re: Emacs 22.2.90 pretest

[Eli Zaretskii]

> From: Chong Yidong <cyd@stupidchicken.com>
> Cc: emacs-devel@gnu.org
> Date: Sat, 16 Aug 2008 09:43:19 -0400
> 
> > Why was this change made?
> 
> This was bug#232:
> 
> http://emacsbugs.donarmstrong.com/cgi-bin/bugreport.cgi?bug=232
> 
> Does undoing this change make the problem go away?

I've run out of free time this weekend.  Can someone who sees this
problem please check that and answer Yidong's question?  Thanks.

Re: Emacs 22.2.90 pretest

[Chong Yidong]

Eli Zaretskii <eliz@gnu.org> writes:

> This pretest builds and seems to work fine on
>
>    Linux fencepost 2.6.16.29-xen #1 SMP Wed Dec 6 07:32:36 EST 2006 x86_64 GNU/Linux
>
> On MS-Windows, it fails to build because nt/emacsclient.rc is missing.
> After fixing that, the rest of the build proceeds OK and the resulting
> binary seems to work fine.

Thanks.  I see that Jason checked in a fix.

> Also, the file info/.arch-inventory should not be in the tarball.

I've changed make-dist to omit it.  Thanks!

Re: Emacs 22.2.90 pretest

[Jason Rumney]

Eli Zaretskii wrote:
> On MS-Windows, it fails to build because nt/emacsclient.rc is missing.
>   

I fixed that in make-dist and admin/admin.el yesterday, so future builds 
should include it.

Re: Emacs 22.2.90 pretest

[Eli Zaretskii]

> Date: Sun, 17 Aug 2008 13:09:56 +0800
> From: Jason Rumney <jasonr@gnu.org>
> CC: Chong Yidong <cyd@stupidchicken.com>, emacs-devel@gnu.org
> 
> Eli Zaretskii wrote:
> > On MS-Windows, it fails to build because nt/emacsclient.rc is missing.
> >   
> 
> I fixed that in make-dist and admin/admin.el yesterday, so future builds 
> should include it.

Thanks.

Re: Emacs 22.2.90 pretest

[Lennart Borgman (gmail)]

Chong Yidong wrote:
> Emacs pretest version 22.2.90 is now available.  This is the first
> pretest for Emacs 22.3, which will be a bugfix release.
> 
> The source tarball is available here:
> 
> http://alpha.gnu.org/gnu/emacs/pretest/emacs-22.2.90.tar.gz
> 
> The xdelta against Emacs 22.2 is here:
> 
> http://alpha.gnu.org/gnu/emacs/pretest/emacs-22.2-22.2.90.xdelta
> 
> The CVS tag is EMACS_PRETEST_22_2_90.
> 
> 
> If you have problems building, please email emacs-devel@gnu.org.  For
> all other bugs, please use M-x report-emacs-bug or send email to
> emacs-pretest-bug@gnu.org.  For any other questions or problems, email
> emacs-devel@gnu.org.


Thanks, but wouldn't it be good to mention the pretest also on Emacs 
home page?

Re: Emacs 22.2.90 pretest

[YAMAMOTO Mitsuharu]

2008-08-02  Eli Zaretskii  <eliz@gnu.org>

	* fileio.c (Fexpand_file_name): Convert the value of $HOME to a
	multibyte string.

The above change contains the same problem I pointed out in
http://lists.gnu.org/archive/html/bug-gnu-emacs/2008-03/msg00026.html.
I.e., DECODE_FILE may GC, and pointers to Lisp String contents are not
valid after that because of relocations by compaction.

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

Re: Emacs 22.2.90 pretest

[Chong Yidong]

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

> 2008-08-02  Eli Zaretskii  <eliz@gnu.org>
>
> 	* fileio.c (Fexpand_file_name): Convert the value of $HOME to a
> 	multibyte string.
>
> The above change contains the same problem I pointed out in
> http://lists.gnu.org/archive/html/bug-gnu-emacs/2008-03/msg00026.html.
> I.e., DECODE_FILE may GC, and pointers to Lisp String contents are not
> valid after that because of relocations by compaction.

How bout turning off GC in that chunk of code?


*** emacs/src/fileio.c.~1.580.2.16.~	2008-08-16 09:37:53.000000000 -0400
--- emacs/src/fileio.c	2008-08-20 10:41:49.000000000 -0400
***************
*** 1383,1390 ****
--- 1383,1392 ----
  	  tem = build_string (newdir);
  	  if (!STRING_MULTIBYTE (tem))
  	    {
+ 	      int count = inhibit_garbage_collection ();
  	      hdir = DECODE_FILE (tem);
  	      newdir = SDATA (hdir);
+ 	      unbind_to (count, Qnil);
  	    }
  #ifdef DOS_NT
  	  collapse_newdir = 0;

Re: Emacs 22.2.90 pretest

[Eli Zaretskii]

> From: Chong Yidong <cyd@stupidchicken.com>
> Date: Wed, 20 Aug 2008 10:44:18 -0400
> Cc: emacs-devel@gnu.org
> 
> YAMAMOTO Mitsuharu <mituharu@math.s.chiba-u.ac.jp> writes:
> 
> > 2008-08-02  Eli Zaretskii  <eliz@gnu.org>
> >
> > 	* fileio.c (Fexpand_file_name): Convert the value of $HOME to a
> > 	multibyte string.
> >
> > The above change contains the same problem I pointed out in
> > http://lists.gnu.org/archive/html/bug-gnu-emacs/2008-03/msg00026.html.
> > I.e., DECODE_FILE may GC, and pointers to Lisp String contents are not
> > valid after that because of relocations by compaction.
> 
> How bout turning off GC in that chunk of code?

That means Emacs could accidentally run out of memory when GC would
salvage some.  So I don't like this idea.

I think copying `nm's contents into a local buffer up front, like
DOS_NT does, is a much better solution, and it costs nothing.

Re: Emacs 22.2.90 pretest

[Eli Zaretskii]

> Date: Wed, 20 Aug 2008 17:10:07 +0900
> From: YAMAMOTO Mitsuharu <mituharu@math.s.chiba-u.ac.jp>
> 
> 2008-08-02  Eli Zaretskii  <eliz@gnu.org>
> 
> 	* fileio.c (Fexpand_file_name): Convert the value of $HOME to a
> 	multibyte string.
> 
> The above change contains the same problem I pointed out in
> http://lists.gnu.org/archive/html/bug-gnu-emacs/2008-03/msg00026.html.
> I.e., DECODE_FILE may GC, and pointers to Lisp String contents are not
> valid after that because of relocations by compaction.

Please suggest which variables to GCPRO.

The code is convoluted, but I did try to walk through it and see if
any variables need to be protected from GC.  Most of the code that
uses nm[] (the only variable you mentioned back in March) is on
DOS_NT, where the original string is copied to a stack-allocated
buffer right at the beginning, and manipulated there; the original
pointer to a Lisp_Object is forgotten, so GC cannot possibly cause
harm on those platforms.

Perhaps I missed something, but in that case please make specific
suggestions which variables to protect and why, because the general
principle that GC can move strings is obviously not news to me.

Re: Emacs 22.2.90 pretest

[YAMAMOTO Mitsuharu]

>>>>> On Wed, 20 Aug 2008 19:07:02 +0300, Eli Zaretskii <eliz@gnu.org> said:

>> 2008-08-02 Eli Zaretskii <eliz@gnu.org>
>> 
>> * fileio.c (Fexpand_file_name): Convert the value of $HOME to a
>> multibyte string.
>> 
>> The above change contains the same problem I pointed out in
>> http://lists.gnu.org/archive/html/bug-gnu-emacs/2008-03/msg00026.html.
>> I.e., DECODE_FILE may GC, and pointers to Lisp String contents are
>> not valid after that because of relocations by compaction.

> Please suggest which variables to GCPRO.

GCPRO doesn't help here.  It just protects Lisp Objects from being
collected, but not for Lisp String contents from being relocated.
Anyway GCPRO is nowadays NOOP on most platforms because of
conservative GC.

> The code is convoluted, but I did try to walk through it and see if
> any variables need to be protected from GC.  Most of the code that
> uses nm[] (the only variable you mentioned back in March) is on
> DOS_NT, where the original string is copied to a stack-allocated
> buffer right at the beginning, and manipulated there; the original
> pointer to a Lisp_Object is forgotten, so GC cannot possibly cause
> harm on those platforms.

Yes, `nm' is not corrupted if DOS_NT because of copying.  But
otherwise, it may be corrupted by GC and it is actually used
afterwards.

  1455	  if (1
  1456	#ifndef DOS_NT
  1457	      /* /... alone is not absolute on DOS and Windows. */
  1458	      && !IS_DIRECTORY_SEP (nm[0])
  1459	#endif

> Perhaps I missed something, but in that case please make specific
> suggestions which variables to protect and why, because the general
> principle that GC can move strings is obviously not news to me.

You can find some follow-up changes by others in the trunk to address
this issue, but with some `FIXME' comment.  I'm not sure if it is good
to apply such changes to the EMACS_22_BASE branch as that comment may
imply they are somewhat experimental.

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

Re: Emacs 22.2.90 pretest

[Eli Zaretskii]

> Date: Thu, 21 Aug 2008 09:51:33 +0900
> From: YAMAMOTO Mitsuharu <mituharu@math.s.chiba-u.ac.jp>
> Cc: emacs-devel@gnu.org
> 
> > Please suggest which variables to GCPRO.
> 
> GCPRO doesn't help here.  It just protects Lisp Objects from being
> collected, but not for Lisp String contents from being relocated.

Really? that's news to me.  So what means do we have for protecting
pointers to Lisp strings from GC?

> Yes, `nm' is not corrupted if DOS_NT because of copying.  But
> otherwise, it may be corrupted by GC and it is actually used
> afterwards.
> 
>   1455	  if (1
>   1456	#ifndef DOS_NT
>   1457	      /* /... alone is not absolute on DOS and Windows. */
>   1458	      && !IS_DIRECTORY_SEP (nm[0])
>   1459	#endif

Is nm the only variable in danger?  If so, how about if we simply copy
it on all platforms?

Re: Emacs 22.2.90 pretest

[YAMAMOTO Mitsuharu]

>>>>> On Thu, 21 Aug 2008 08:35:11 +0300, Eli Zaretskii <eliz@gnu.org> said:

>> > Please suggest which variables to GCPRO.
>> 
>> GCPRO doesn't help here.  It just protects Lisp Objects from being
>> collected, but not for Lisp String contents from being relocated.

> Really? that's news to me.

Yes.  It seems to be a common misunderstanding.

http://lists.gnu.org/archive/html/bug-gnu-emacs/2008-03/msg00034.html

This can cause and has actually been caused nasty bugs that are
difficult to reproduce or debug.  So every developer should be aware
of this.

  GCPRO1 (s);
  p = SDATA (s);
  SOME_OPERATION_INVOLVING_GC; /* e.g., DECODE_FILE, ENCODE_UTF_8 */
  /* p no longer points to valid data if GC happened.  */
  /* One should do p = SDATA (s) again before using p.  */

> So what means do we have for protecting pointers to Lisp strings
> from GC?

Nothing can prevent Lisp String contents from being relocated by GC.

>> Yes, `nm' is not corrupted if DOS_NT because of copying.  But
>> otherwise, it may be corrupted by GC and it is actually used
>> afterwards.
>> 
>> 1455 if (1 1456 #ifndef DOS_NT 1457 /* /... alone is not absolute
>> on DOS and Windows. */ 1458 && !IS_DIRECTORY_SEP (nm[0]) 1459
>> #endif

> Is nm the only variable in danger?  If so, how about if we simply
> copy it on all platforms?

I think it is one possible solution if properly commented.

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

Re: Emacs 22.2.90 pretest

[Kenichi Handa]

In article <wlpro228rj.wl%mituharu@math.s.chiba-u.ac.jp>, YAMAMOTO Mitsuharu <mituharu@math.s.chiba-u.ac.jp> writes:

> > Is nm the only variable in danger?  If so, how about if we simply
> > copy it on all platforms?

> I think it is one possible solution if properly commented.

Another is to do something like this (excerpted from send_process ()).

		      int offset = 0;
[...]
		      /* Running filters might relocate buffers or strings.
			 Arrange to relocate BUF.  */
		      if (BUFFERP (object))
			offset = BUF_PTR_BYTE_POS (XBUFFER (object), buf);
		      else if (STRINGP (object))
			offset = buf - SDATA (object);

#ifdef EMACS_HAS_USECS
		      wait_reading_process_output (0, 20000, 0, 0, Qnil, NULL, 0);
#else
		      wait_reading_process_output (1, 0, 0, 0, Qnil, NULL, 0);
#endif

		      if (BUFFERP (object))
			buf = BUF_BYTE_ADDRESS (XBUFFER (object), offset);
		      else if (STRINGP (object))
			buf = offset + SDATA (object);

---
Kenichi Handa
handa@ni.aist.go.jp

Re: Emacs 22.2.90 pretest

[YAMAMOTO Mitsuharu]

2008-07-21  Ami Fischman  <ami@fischman.org>  (tiny change)

	* print.c (print_object): Check print_depth before searching for
	circularities.

For the above entry, the installed change is a bit different from what
the patch author proposed originally.  Actually, a follow-up change
was made only in the trunk and the result became the same with the
original one.

2008-07-26  Andreas Schwab  <schwab@suse.de>

	* print.c (print_object): Fix off-by-one in last change.

If the original patch is correct, the same change should also be
applied to the EMACS_22_BASE branch.

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