minad / org-modern

:unicorn: Modern Org Style
GNU General Public License v3.0
1.54k stars 48 forks source link

Performance issue with larger buffers, (default-font-width) called #169

Closed bram85 closed 9 months ago

bram85 commented 9 months ago

In a ~1.5 MB org-mode file, performance is degraded a bit when typing text. Profiling revealed that the following code is a contributor to the slowness:

(defun org-modern--pre-redisplay (_)
  "Compute font parameters before redisplay."
  ;; [...]
  (setf org-modern--table-sp-width (default-font-width)
        (cadr org-modern--table-overline) (face-attribute 'org-table :foreground nil t)))

The default-font-width call turns out to be pretty expensive. Since I have org-modern-table set to nil, I don't really need this update. In my init file I advised the function to let default-font-width return 0 unconditionally, which made edits much smoother. My proposal is to put the setf call in a guard to check if org-modern-table is non-nil. Because I'm not sure if the return value can be properly cached without rendering issues.

(I could've made a PR for this but I don't have the FSF administration in place).

minad commented 9 months ago

Thanks for the report. I cannot reproduce this with shakespeare.org (5M) in my configuration (Emacs 29.1 on Linux with X11/Lucid). Do you have a profile? Could this be some other special problem in your setup? Please provide a minimal recipe based on emacs -Q and a profile. If calling default-font-width is too expensive to be called once per redisplay, I would consider this an Emacs bug. In my benchmark org-modern--pre-redisplay including default-font-width took less than 1% of the time. Can you try to figure out why default-font-width is that expensive in your setup? Do you use some special fonts or special Emacs setup?

bram85 commented 9 months ago

It's in conjunction with flyspell, which triggers some redisplays while typing, see profile below. The profile was created by adding a sentence to shakespeare.org. Indeed, the org-modern--pre-display and default-font-width only seem to consume a small amount of CPU cycles, which is in agreement with your own benchmark. Yet, when I eliminate the call to (default-font-width) in the redisplay hook, the experience is greatly improved when typing.

        1847  59% - flyspell-post-command-hook
        1831  59%  - flyspell-check-word-p
        1831  59%   - sit-for
        1064  34%    - redisplay
         181   5%     - redisplay_internal (C function)
         136   4%      + jit-lock-function
          45   1%      - redisplay--pre-redisplay-functions
          45   1%       - run-hook-with-args
          45   1%        - org-modern--pre-redisplay
          45   1%           default-font-width
          47   1%    - read-event
          47   1%     - redisplay_internal (C function)
          47   1%      - redisplay--pre-redisplay-functions
          47   1%       - run-hook-with-args
          47   1%        - org-modern--pre-redisplay
          47   1%           default-font-width
          16   0%    flyspell-word
         903  29% + command-execute
         348  11% + redisplay_internal (C function)
           0   0% + ...

(raw profiler output attached here).

I still need to investigate it from a clean Emacs config (emacs -Q).

minad commented 9 months ago

Bram Schoenmakers @.***> writes:

It's in conjunction with flyspell, which triggers some redisplays while typing, see profile below. The profile was created by adding a sentence to shakespeare.org. Indeed, the org-modern--pre-display and default-font-width only seem to consume a small amount of CPU cycles, which is in agreement with your own benchmark. Yet, when I eliminate the call to (default-font-width) in the redisplay hook, the experience is greatly improved when typing.

Thanks. From my experience Flyspell is extremely inefficient. I've created the alternative spell checker Jinx, see https://github.com/minad/jinx. Maybe you want to give that one a try?

bram85 commented 9 months ago

I already do on the platforms that have enchant 2 :)

Unfortunately I see the performance issue on Cygwin where enchant2 is not available, so I resort to flyspell.

minad commented 9 months ago

I think the best approach would be to ensure that Enchant/Jinx works properly on Cygwin :)

Any suggestions on what we have to improve in Jinx? See https://github.com/minad/jinx/blob/03efb689ca134127c186b8da5a2a91b04889ea2b/jinx.el#L570-L604 for the Jinx module compilation function.