Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Markdown mode takes 1.7 seconds to load #264

Closed
jschaf opened this issue Sep 25, 2017 · 5 comments
Closed

Markdown mode takes 1.7 seconds to load #264

jschaf opened this issue Sep 25, 2017 · 5 comments

Comments

@jschaf
Copy link

jschaf commented Sep 25, 2017

Loading or requiring markdown-mode hangs Emacs for about 1.7 seconds.

Here's a command to verify (maybe update the load path -L).

emacs -Q -L ~/.emacs.d/elpa/markdown-mode-20170924.1720 \
  --eval='(setq start-time (float-time))' \
  --eval='(require (quote markdown-mode))' \
  --eval='(message "Markdown require: %f s" (- (float-time) start-time))'

I haven't looked into the source of the slow-down but it seems excessive.

System info:

  • OS: darwin
  • Emacs: 25.2.1
  • Graphic display: t
  • System configuration features: NOTIFY ACL LIBXML2 ZLIB TOOLKIT_SCROLL_BARS
jrblevin added a commit that referenced this issue Sep 25, 2017
Add documentation and notes related to issue GH-255 and the patch in
GH-264.
@jrblevin
Copy link
Owner

I just profiled the require statement and it appears that char-displayable-p is the culprit. This is used to determine the default values of a few defcustoms that involve Unicode glyphs; We don't want to use glyphs that aren't displayable.

The delay, I think, is because char-displayable-p needs to load the display font. There is a discussion in this mailing list thread. Anyone have thoughts on how to speed this up?

I used this invocation:

emacs -Q -L ~/work/markdown-mode --eval="(progn (profiler-start 'cpu) \
    (require 'markdown-mode) (profiler-report))"

Here is the profiler-report:

- normal-top-level                                               1222  98%
 - command-line                                                  1222  98%
  - command-line-1                                               1222  98%
   - eval                                                        1222  98%
    - progn                                                      1222  98%
     - require                                                   1218  98%
      - byte-code                                                1141  91%
       - custom-declare-variable                                 1084  87%
        - custom-initialize-reset                                1084  87%
         - eval                                                  1084  87%
          - funcall                                              1084  87%
           - #<compiled 0x40e73b47>                              1084  87%
              char-displayable-p                                 1084  87%
       + require                                                   52   4%
       + custom-declare-face                                        1   0%
      + redisplay_internal (C function)                            18   1%
      + defconst                                                    1   0%
     + profiler-report                                              2   0%
       profiler-start                                               1   0%
+ ...                                                              20   1%

@jschaf
Copy link
Author

jschaf commented Sep 27, 2017

Yep, I can verify that char-displayable-p is the issue. Setting the characters explicitly solves the issue, bringing the time down to 0.15 seconds.

λ emacs -Q -L ~/prog/markdown-mode   --eval='
  (progn
    (setq markdown-url-compose-char ?∞)
    (setq markdown-blockquote-display-char "▌")
    (setq markdown-hr-display-char ?─)
    (setq markdown-definition-display-char ?<2058>)
    (setq start-time (float-time))
    (require (quote markdown-mode))
    (message "Markdown require: %f s" (- (float-time) start-time)))'

Anyone have thoughts on how to speed this up?

The mailing list post was not especially inspiring. It's the standard rhetoric of only start emacs once.

The easy answer is to use ASCII. I'd say switching depends on how useful people find the nice characters.

Another avenue is to look at the base fonts for the major operating systems to see what percentage include a fallback font that covers the code points. I'd imagine by this point, most distros support all the characters. You could even hard code a list of supported fonts and use that instead of char-displayable-p. That's a little gross though.

A nice middle ground is to document the slowness and provide a work-around. Maybe reach out to the major Emacs config distros (spacmeacs, prelude, etc.) and ask if they want to customize how they use markdown-mode. Spacemacs has something like a use-unicode flag which they could use instead of char-displayable-p

Here's a workaround with use-package that I'm using since I know my font supports these characters.

(use-package markdown-mode
  :defer t
  :mode ("\\.md\\'" . gfm-mode)
  :init
  ;; Prevent ~1.7 second delay by avoiding `char-displayable-p'.  See
  ;; https://github.com/jrblevin/markdown-mode/issues/264
  (setq markdown-url-compose-char ?∞)
  (setq markdown-blockquote-display-char "")
  (setq markdown-hr-display-char ?─)
  (setq markdown-definition-display-char ?⁘))

@phst
Copy link
Contributor

phst commented Sep 28, 2017

Hmm, I'm wondering whether loading markdown-mode should even call char-displayable-p at all. The result is always terminal-specific (a graphical terminal might be able to display a character, a text terminal might not) and even buffer-specific (the code checks the buffer-local variable enable-multibyte-characters), so checking it during load will likely give the wrong result later on. One possible solution would be to turn markdown-url-compose-char and friends into sequences of characters and check them only when the characters are about to be displayed and we are in the right buffer.

@jrblevin
Copy link
Owner

Thanks for all of these suggestions. I see now that checking at load time isn't necessarily reliable, so I'm leaning towards making these variables sequences and checking at display time, as @phst suggested. I haven't checked, but allegedly (according to a mailing list thread) subsequent calls to char-displayable-p should be much faster.

jrblevin added a commit that referenced this issue Nov 10, 2017
With this commit, `markdown-hr-display-char` is permitted to be a
list.  The default value is now a sequence of possible characters
and the first one that is displayable will be used.

This is part of a series of changes to improve the load time by
deferring calls to `char-displayable-p`.  See GH-264.
jrblevin added a commit that referenced this issue Nov 10, 2017
With this commit, `markdown-url-compose-char` is permitted to be a
list.  The default value is now a sequence of possible characters and
the first one that is displayable will be used.

This is part of a series of changes to improve the load time by
deferring calls to `char-displayable-p`.  See GH-264.
jrblevin added a commit that referenced this issue Nov 10, 2017
With this commit, `markdown-blockquote-display-char` is permitted to
be a list.  The default value is now a sequence of possible strings
and the first one that is displayable will be used.

This is part of a series of changes to improve the load time by
deferring calls to `char-displayable-p`.  See GH-264.
jrblevin added a commit that referenced this issue Nov 10, 2017
With this commit, `markdown-definition-display-char` is permitted to
be a list.  The default value is now a sequence of possible characters
and the first one that is displayable will be used.

This is part of a series of changes to improve the load time by
deferring calls to `char-displayable-p`.  See GH-264.
@jrblevin
Copy link
Owner

Following the suggestion of @phst, I have updated the defcustoms in question to be lists and only check for displayable characters at fontification time. For backwards compatibility, atomic values for these variables are allowed as well. This reduces the load time down to around 0.1 s for me. Let me know if there are any issues and thanks for reporting @jschaf.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants