GNU bug report logs - #33939
26.1; Avoid loading libs eagerly or unnecessarily in mhtml-mode/sgml-mode

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 33939 in the body.
You can then email your comments to 33939 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: Leo Liu <sdl.web <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: 26.1;
 Avoid loading libs eagerly or unnecessarily in mhtml-mode/sgml-mode
Date: Tue, 01 Jan 2019 10:12:05 +0800

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

Switching from html-mode I have noticed mhtml-mode can take a second or
two (unpleasant noticeable delay) to load on my 2013 macbook air. The
following is a small patch that alleviate the problem. Is it safe for
emacs-26?

[mhtml.diff (text/x-patch, inline)]

diff --git a/lisp/textmodes/mhtml-mode.el b/lisp/textmodes/mhtml-mode.el
index b99f7881..2ad3c8eb 100644
--- a/lisp/textmodes/mhtml-mode.el
+++ b/lisp/textmodes/mhtml-mode.el
@@ -21,13 +21,9 @@
 
 ;;; Code:
 
-(eval-and-compile
-  (require 'flyspell)
-  (require 'sgml-mode))
+(eval-and-compile (require 'sgml-mode))
 (require 'js)
 (require 'css-mode)
-(require 'prog-mode)
-(require 'font-lock)
 
 (defcustom mhtml-tag-relative-indent t
   "How <script> and <style> bodies are indented relative to the tag.
@@ -349,6 +345,8 @@ This is used by `mhtml--pre-command'.")
       ;; HTML.
       (sgml-indent-line))))
 
+(declare-function flyspell-generic-progmode-verify "flyspell")
+
 (defun mhtml--flyspell-check-word ()
   (let ((submode (get-text-property (point) 'mhtml-submode)))
     (if submode
diff --git a/lisp/textmodes/sgml-mode.el b/lisp/textmodes/sgml-mode.el
index eb6ebf52..ad3357e0 100644
--- a/lisp/textmodes/sgml-mode.el
+++ b/lisp/textmodes/sgml-mode.el
@@ -33,10 +33,9 @@
 ;;; Code:
 
 (require 'dom)
-(require 'seq)
-(eval-when-compile (require 'subr-x))
 (eval-when-compile
   (require 'skeleton)
+  (require 'subr-x)
   (require 'cl-lib))
 
 (defgroup sgml nil
@@ -2240,7 +2239,7 @@ The result is cached in `html--buffer-classes-cache'."
         (cdr html--buffer-classes-cache)
       (let* ((dom (libxml-parse-html-region (point-min) (point-max)))
              (classes
-              (seq-mapcat
+              (mapcan
                (lambda (el)
                  (when-let* ((class-list
                               (cdr (assq 'class (dom-attributes el)))))
@@ -2258,7 +2257,7 @@ The result is cached in `html--buffer-ids-cache'."
       (let* ((dom
               (libxml-parse-html-region (point-min) (point-max)))
              (ids
-              (seq-mapcat
+              (mapcan
                (lambda (el)
                  (when-let* ((id-list
                               (cdr (assq 'id (dom-attributes el)))))

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Leo Liu <sdl.web <at> gmail.com>
Cc: 33939 <at> debbugs.gnu.org
Subject: Re: bug#33939: 26.1;
 Avoid loading libs eagerly or unnecessarily in mhtml-mode/sgml-mode
Date: Sat, 05 Jan 2019 11:47:38 +0200

> From: Leo Liu <sdl.web <at> gmail.com>
> Date: Tue, 01 Jan 2019 10:12:05 +0800
> 
> Switching from html-mode I have noticed mhtml-mode can take a second or
> two (unpleasant noticeable delay) to load on my 2013 macbook air. The
> following is a small patch that alleviate the problem. Is it safe for
> emacs-26?

I cannot establish whether these changes are safe for Emacs 26.2,
since they are not really trivial.  Perhaps if you told more about
each change, I could make up my mind.

Some of the questions I would like to be able to answer in order to
make the decision:

  . which of the 'require's you want to remove take the lion's share
    of the load time?
  . why was seq-mapcat used originally instead of mapcan, and what is
    the semantics of replacing the former by the latter?
  . why did you switch the order of eval-when-compile in sgml-mode.el?

Thanks.

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

From: Leo Liu <sdl.web <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 33939 <at> debbugs.gnu.org
Subject: Re: bug#33939: 26.1;
 Avoid loading libs eagerly or unnecessarily in mhtml-mode/sgml-mode
Date: Sun, 06 Jan 2019 12:34:26 +0800

On 2019-01-05 11:47 +0200, Eli Zaretskii wrote:
> I cannot establish whether these changes are safe for Emacs 26.2,
> since they are not really trivial.  Perhaps if you told more about
> each change, I could make up my mind.

Fair.

> Some of the questions I would like to be able to answer in order to
> make the decision:
>
>   . which of the 'require's you want to remove take the lion's share
>     of the load time?

flyspell which saves ~7000 line of code.

>   . why was seq-mapcat used originally instead of mapcan, and what is
>     the semantics of replacing the former by the latter?

I am not entirely sure why it was used in the first place. Maybe people
now learns about seq.el first.

seq-mapcat doesn't mutate its args but in both cases fresh lists are
created using split-string so mapcan is safe as well i.e. the semantics
of html-current-buffer-ids and html-current-buffer-classes are intact.

I have also tested them before and after in a large HTML buffer.

>   . why did you switch the order of eval-when-compile in sgml-mode.el?

Totally cosmetic. We could keep the original order.

Thanks,
Leo

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Leo Liu <sdl.web <at> gmail.com>
Cc: 33939 <at> debbugs.gnu.org
Subject: Re: bug#33939: 26.1;
 Avoid loading libs eagerly or unnecessarily in mhtml-mode/sgml-mode
Date: Sun, 06 Jan 2019 17:25:41 +0200

> From:  Leo Liu <sdl.web <at> gmail.com>
> Cc: 33939 <at> debbugs.gnu.org
> Date: Sun, 06 Jan 2019 12:34:26 +0800
> 
> >   . which of the 'require's you want to remove take the lion's share
> >     of the load time?
> 
> flyspell which saves ~7000 line of code.

Using declare-function instead of that is a no-brainer, which is good.

> >   . why was seq-mapcat used originally instead of mapcan, and what is
> >     the semantics of replacing the former by the latter?
> 
> I am not entirely sure why it was used in the first place. Maybe people
> now learns about seq.el first.
> 
> seq-mapcat doesn't mutate its args but in both cases fresh lists are
> created using split-string so mapcan is safe as well i.e. the semantics
> of html-current-buffer-ids and html-current-buffer-classes are intact.
> 
> I have also tested them before and after in a large HTML buffer.
> 
> >   . why did you switch the order of eval-when-compile in sgml-mode.el?
> 
> Totally cosmetic. We could keep the original order.

OK.  Would you be okay with doing only the first of these,
i.e. avoiding to load flyspell, on the emacs-26 branch?  Or is the use
of seq.el still slowing down the load significantly?

Thanks.

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

From: Leo Liu <sdl.web <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 33939-done <at> debbugs.gnu.org
Subject: Re: bug#33939: 26.1;
 Avoid loading libs eagerly or unnecessarily in mhtml-mode/sgml-mode
Date: Mon, 07 Jan 2019 07:39:11 +0800

Fixed in 26.2.

On 2019-01-06 17:25 +0200, Eli Zaretskii wrote:
> OK.  Would you be okay with doing only the first of these,
> i.e. avoiding to load flyspell, on the emacs-26 branch?  Or is the use
> of seq.el still slowing down the load significantly?

Sounds good. I just did that which shaves off ~100ms.

(require 'mhtml-mode) takes ~1.5 seconds on my machine. I think the bulk
of the time is spent loading js.el which loads cc-mode but delay loading
js is trickier so I'll leave it off for now.

Thanks for reviewing the bug.

Leo

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

From: Leo Liu <sdl.web <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 33939 <at> debbugs.gnu.org
Subject: Re: bug#33939: 26.1;
 Avoid loading libs eagerly or unnecessarily in mhtml-mode/sgml-mode
Date: Mon, 07 Jan 2019 08:33:43 +0800

On 2019-01-06 17:25 +0200, Eli Zaretskii wrote:
> OK.  Would you be okay with doing only the first of these,
> i.e. avoiding to load flyspell, on the emacs-26 branch?  Or is the use
> of seq.el still slowing down the load significantly?

I just discovered that css-mode.el also unnecessarily loads eww which in
turn loads gazillion of other things (including gnus).

Do you mind if I also remove (require 'eww) from css-mode.el? This
shaves off another 800ms making (require 'mhtml-mode) at a more
reasonable 650ms. Sound like a win ;)

Leo

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Leo Liu <sdl.web <at> gmail.com>
Cc: 33939 <at> debbugs.gnu.org
Subject: Re: bug#33939: 26.1;
 Avoid loading libs eagerly or unnecessarily in mhtml-mode/sgml-mode
Date: Mon, 07 Jan 2019 05:37:45 +0200

> From:  Leo Liu <sdl.web <at> gmail.com>
> Cc: 33939 <at> debbugs.gnu.org
> Date: Mon, 07 Jan 2019 08:33:43 +0800
> 
> I just discovered that css-mode.el also unnecessarily loads eww which in
> turn loads gazillion of other things (including gnus).
> 
> Do you mind if I also remove (require 'eww) from css-mode.el? This
> shaves off another 800ms making (require 'mhtml-mode) at a more
> reasonable 650ms. Sound like a win ;)

I don't mind, of course, but if you want this to go to the release
branch, please show the proposed patch.

Thanks.

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

From: Leo Liu <sdl.web <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 33939 <at> debbugs.gnu.org
Subject: Re: bug#33939: 26.1;
 Avoid loading libs eagerly or unnecessarily in mhtml-mode/sgml-mode
Date: Mon, 07 Jan 2019 12:16:39 +0800

On 2019-01-07 05:37 +0200, Eli Zaretskii wrote:
> I don't mind, of course, but if you want this to go to the release
> branch, please show the proposed patch.
>
> Thanks.

Should have attached it in the first place. This single line change
delivers the most speed up.

diff --git a/lisp/textmodes/css-mode.el b/lisp/textmodes/css-mode.el
index 016f0e8f..67a0c9f7 100644
--- a/lisp/textmodes/css-mode.el
+++ b/lisp/textmodes/css-mode.el
@@ -32,7 +32,6 @@
 
 ;;; Code:
 
-(require 'eww)
 (require 'cl-lib)
 (require 'color)
 (require 'seq)

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Leo Liu <sdl.web <at> gmail.com>
Cc: 33939 <at> debbugs.gnu.org
Subject: Re: bug#33939: 26.1;
 Avoid loading libs eagerly or unnecessarily in mhtml-mode/sgml-mode
Date: Mon, 07 Jan 2019 21:56:59 +0200

> From:  Leo Liu <sdl.web <at> gmail.com>
> Cc: 33939 <at> debbugs.gnu.org
> Date: Mon, 07 Jan 2019 12:16:39 +0800
> 
> On 2019-01-07 05:37 +0200, Eli Zaretskii wrote:
> > I don't mind, of course, but if you want this to go to the release
> > branch, please show the proposed patch.
> >
> > Thanks.
> 
> Should have attached it in the first place. This single line change
> delivers the most speed up.
> 
> diff --git a/lisp/textmodes/css-mode.el b/lisp/textmodes/css-mode.el
> index 016f0e8f..67a0c9f7 100644
> --- a/lisp/textmodes/css-mode.el
> +++ b/lisp/textmodes/css-mode.el
> @@ -32,7 +32,6 @@
>  
>  ;;; Code:
>  
> -(require 'eww)
>  (require 'cl-lib)
>  (require 'color)
>  (require 'seq)
> 

This is OK for the emacs-26 branch, thanks.

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

From: Leo Liu <sdl.web <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 33939 <at> debbugs.gnu.org
Subject: Re: bug#33939: 26.1;
 Avoid loading libs eagerly or unnecessarily in mhtml-mode/sgml-mode
Date: Tue, 08 Jan 2019 05:20:10 +0800

On 2019-01-07 21:56 +0200, Eli Zaretskii wrote:
> This is OK for the emacs-26 branch, thanks.

Done.

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

From: Leo Liu <sdl.web <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 33939 <at> debbugs.gnu.org
Subject: Re: bug#33939: 26.1;
 Avoid loading libs eagerly or unnecessarily in mhtml-mode/sgml-mode
Date: Tue, 08 Jan 2019 06:43:20 +0800

On 2019-01-07 21:56 +0200, Eli Zaretskii wrote:
> This is OK for the emacs-26 branch, thanks.

I was bored enough to check if js.el had any low-hanging fruits and
found another one. js.el uses nothing from thingatpt since
forward-symbol now lives in subr.el. Should I go ahead and put the
following patch on emacs-26?

Sorry to discover these issues one after another. The situation is worse
than I imagined.

diff --git a/lisp/progmodes/js.el b/lisp/progmodes/js.el
index 65ffb0e0..1e2c3411 100644
--- a/lisp/progmodes/js.el
+++ b/lisp/progmodes/js.el
@@ -48,10 +48,9 @@
 
 (require 'cc-mode)
 (require 'newcomment)
-(require 'thingatpt)                    ; forward-symbol etc
 (require 'imenu)
 (require 'moz nil t)
-(require 'json nil t)
+(require 'json)
 (require 'sgml-mode)
 (require 'prog-mode)
 

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Leo Liu <sdl.web <at> gmail.com>
Cc: 33939 <at> debbugs.gnu.org
Subject: Re: bug#33939: 26.1;
 Avoid loading libs eagerly or unnecessarily in mhtml-mode/sgml-mode
Date: Tue, 08 Jan 2019 05:31:53 +0200

> From:  Leo Liu <sdl.web <at> gmail.com>
> Cc: 33939 <at> debbugs.gnu.org
> Date: Tue, 08 Jan 2019 06:43:20 +0800
> 
> On 2019-01-07 21:56 +0200, Eli Zaretskii wrote:
> > This is OK for the emacs-26 branch, thanks.
> 
> I was bored enough to check if js.el had any low-hanging fruits and
> found another one. js.el uses nothing from thingatpt since
> forward-symbol now lives in subr.el. Should I go ahead and put the
> following patch on emacs-26?

If this causes a significant slowdown of loading, yes, please.  If the
slowdown is not significant, please do this on master.

Thanks.

> Sorry to discover these issues one after another.

No need to be sorry, thanks for looking at these issues.

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

From: Leo Liu <sdl.web <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 33939 <at> debbugs.gnu.org
Subject: Re: bug#33939: 26.1;
 Avoid loading libs eagerly or unnecessarily in mhtml-mode/sgml-mode
Date: Tue, 08 Jan 2019 12:40:42 +0800

On 2019-01-08 05:31 +0200, Eli Zaretskii wrote:
> If this causes a significant slowdown of loading, yes, please.  If the
> slowdown is not significant, please do this on master.

Installed on the master branch. Thanks.

Previous Next

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