GNU bug report logs - #58860
29.0.50; [PATCH] semantic/fw.el: speed up the 'semantic-find-file-noselect'

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 58860 in the body.
You can then email your comments to 58860 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: lin Sun <sunlin7 <at> yahoo.com>
To: "bug-gnu-emacs <at> gnu.org" <bug-gnu-emacs <at> gnu.org>
Subject: 29.0.50;
 [PATCH] semantic/fw.el: speed up the 'semantic-find-file-noselect'
Date: Sat, 29 Oct 2022 05:01:42 +0000

[Message part 1 (text/plain, inline)]

Hi,

The emacs will be extremely slow after I first time enable the
semantic mode and open a simple C source file which includes the
stdio.h.
I find the semantic mode will try to analyse the chained header files,
by calling the function (semantic-find-file-noselect ...) to open the
file and analyse its content, while the `find-file-hook` may have many
heavy functions.
Disabling the `find-file-hook` do have great performance improvement
for semantic mode.
Please help review this patch. Thanks

Best regards
Lin

[0001-semantic-fw.el-speed-up-the-semantic-find-file-nosel.patch (text/x-patch, attachment)]

Message #8 received at 58860 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: lin Sun <sunlin7 <at> yahoo.com>, Lars Ingebrigtsen <larsi <at> gnus.org>,
 Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 58860 <at> debbugs.gnu.org
Subject: Re: bug#58860: 29.0.50;
 [PATCH] semantic/fw.el: speed up the 'semantic-find-file-noselect'
Date: Sat, 29 Oct 2022 09:31:01 +0300

> Date: Sat, 29 Oct 2022 05:01:42 +0000
> From:  lin Sun via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <bug-gnu-emacs <at> gnu.org>
> 
> The emacs will be extremely slow after I first time enable the
> semantic mode and open a simple C source file which includes the
> stdio.h.
> I find the semantic mode will try to analyse the chained header files,
> by calling the function (semantic-find-file-noselect ...) to open the
> file and analyse its content, while the `find-file-hook` may have many
> heavy functions.
> Disabling the `find-file-hook` do have great performance improvement
> for semantic mode.
> Please help review this patch. Thanks
> 
> 
> From 7ae72df1680f405d4b8d718fb13015e002e0076d Mon Sep 17 00:00:00 2001
> From: Lin Sun <sunlin7 <at> yahoo.com>
> Date: Sat, 29 Oct 2022 00:00:07 +0000
> Subject: [PATCH] * semantic/fw.el: speed up the 'semantic-find-file-noselect'
> 
> Disabling the find-file-hook for semantic-find-file-noselect to
> improve the performance on semantic chained source file analysis.
> ---
>  lisp/cedet/semantic/fw.el | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/lisp/cedet/semantic/fw.el b/lisp/cedet/semantic/fw.el
> index 9917c4c..cf5c60b 100644
> --- a/lisp/cedet/semantic/fw.el
> +++ b/lisp/cedet/semantic/fw.el
> @@ -361,6 +361,8 @@ semantic-find-file-noselect
>  	 (enable-local-variables :safe)
>  	 ;; ... or eval variables
>  	 (enable-local-eval nil)
> +	 ;; also disable the find-file-hook
> +	 (find-file-hook nil)
>  	 )
>      (save-match-data
>        (find-file-noselect file nowarn rawfile wildcards))))
> -- 
> 2.7.0

Thanks.  But I don't see how we can override customizations of users
and modes in such a brutal way.  find-file-hook is an important method
of getting Emacs to do something when a file is visited, it is used
for many different things in Emacs.  I find the change you propose too
drastic for my palate.  We certainly risk breaking some other use
cases.

Lars, Stefan, WDYT?

Message #11 received at 58860 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: lin Sun <sunlin7 <at> yahoo.com>
Cc: 58860 <at> debbugs.gnu.org
Subject: Re: bug#58860: 29.0.50; [PATCH] semantic/fw.el: speed up the
 'semantic-find-file-noselect'
Date: Sat, 29 Oct 2022 11:06:29 -0400

> I find the semantic mode will try to analyse the chained header files,
> by calling the function (semantic-find-file-noselect ...) to open the
> file and analyse its content, while the `find-file-hook` may have many
> heavy functions.
> Disabling the `find-file-hook` do have great performance improvement
> for semantic mode.

Putting expensive code in `find-file-hook` is usually a problem
in itself.  And like Eli, I find it risky to disable it wholesale.

But the rest of code seems to call this function only if the file is not
yet visited and it always tries to kill the buffer afterwards, so really
this could just as well use a temp buffer and
`insert-file-contents` instead.

So in the end I guess the patch is OK (except for the comment, which
just paraphrases the code and thus doesn't add any information).
The other disabled thingies in that function are already similarly
problematic :-)


        Stefan

Message #14 received at 58860 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: sunlin7 <at> yahoo.com, 58860 <at> debbugs.gnu.org
Subject: Re: bug#58860: 29.0.50;
 [PATCH] semantic/fw.el: speed up the 'semantic-find-file-noselect'
Date: Sun, 30 Oct 2022 08:56:40 +0200

> Cc: 58860 <at> debbugs.gnu.org
> Date: Sat, 29 Oct 2022 11:06:29 -0400
> From:  Stefan Monnier via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <bug-gnu-emacs <at> gnu.org>
> 
> > I find the semantic mode will try to analyse the chained header files,
> > by calling the function (semantic-find-file-noselect ...) to open the
> > file and analyse its content, while the `find-file-hook` may have many
> > heavy functions.
> > Disabling the `find-file-hook` do have great performance improvement
> > for semantic mode.
> 
> Putting expensive code in `find-file-hook` is usually a problem
> in itself.  And like Eli, I find it risky to disable it wholesale.
> 
> But the rest of code seems to call this function only if the file is not
> yet visited and it always tries to kill the buffer afterwards, so really
> this could just as well use a temp buffer and
> `insert-file-contents` instead.
> 
> So in the end I guess the patch is OK (except for the comment, which
> just paraphrases the code and thus doesn't add any information).
> The other disabled thingies in that function are already similarly
> problematic :-)

It all made sense to me, until the last paragraph: it seems to
contradict the other two?  You say that disabling find-file-hook is
dangerous, and that the code in question could have used
insert-file-contents into a temporary buffer, in which case
find-file-hook would not come into play.  So I assumed you'd suggest
to rewrite that function to use a temporary buffer.  Instead, you say
that the patch is fine?..  What did I miss?

Message #17 received at 58860 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: sunlin7 <at> yahoo.com, 58860 <at> debbugs.gnu.org
Subject: Re: bug#58860: 29.0.50; [PATCH] semantic/fw.el: speed up the
 'semantic-find-file-noselect'
Date: Sun, 30 Oct 2022 08:37:44 -0400

> It all made sense to me, until the last paragraph: it seems to
> contradict the other two?  You say that disabling find-file-hook is
> dangerous, and that the code in question could have used
> insert-file-contents into a temporary buffer, in which case
> find-file-hook would not come into play.  So I assumed you'd suggest
> to rewrite that function to use a temporary buffer.  Instead, you say
> that the patch is fine?..  What did I miss?

Yes, a better patch is probably to use `insert-file-contents`, but that
might involve further non trivial changes (e.g. I don't know if the
rest of the CEDET code relies on the fact that `find-buffer-visiting`
will find those buffers).  The patch is a minor tweak to the code which
brings it yet a bit closer to a version of the code that uses just
`insert-file-contents`, so it's not ideal but it doesn't make the
code any worse.


        Stefan

Message #20 received at 58860 <at> debbugs.gnu.org (full text, mbox):

From: lin Sun <sunlin7 <at> yahoo.com>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 58860 <at> debbugs.gnu.org
Subject: Re: bug#58860: 29.0.50;
 [PATCH] semantic/fw.el: speed up the 'semantic-find-file-noselect'
Date: Sun, 30 Oct 2022 17:13:18 +0000

Hi Stefan, Eli,

Thanks for your comments, and agree with you.
I'll check my local `find-file-hook` first, and remove the heavy functions.
For using with-temp-buffer and insert-file-contents together, maybe
just a little improvement, not the key point.
So, please close this ticket, and if I find other significant
enhancements, I'll send a new patch. Thank you.

Best Regard
Lin

Message #25 received at 58860-done <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: lin Sun <sunlin7 <at> yahoo.com>
Cc: 58860-done <at> debbugs.gnu.org, monnier <at> iro.umontreal.ca
Subject: Re: bug#58860: 29.0.50;
 [PATCH] semantic/fw.el: speed up the 'semantic-find-file-noselect'
Date: Mon, 31 Oct 2022 15:22:20 +0200

> From: lin Sun <sunlin7 <at> yahoo.com>
> Date: Sun, 30 Oct 2022 17:13:18 +0000
> Cc: Eli Zaretskii <eliz <at> gnu.org>, 58860 <at> debbugs.gnu.org
> 
> Hi Stefan, Eli,
> 
> Thanks for your comments, and agree with you.
> I'll check my local `find-file-hook` first, and remove the heavy functions.
> For using with-temp-buffer and insert-file-contents together, maybe
> just a little improvement, not the key point.
> So, please close this ticket, and if I find other significant
> enhancements, I'll send a new patch. Thank you.

Thanks, closing.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.