bmag / emacs-purpose

Manage Windows and Buffers According to Purposes
GNU General Public License v3.0
498 stars 23 forks source link

Kill Extension interrupts cursor flow slowing down all emacs text editing #76

Open emelin opened 8 years ago

emelin commented 8 years ago

When enabling the 'Kill Extension' by

 (require 'window-purpose-x)
 (purpose-x-kill-setup)

and having purpose-mode switched on e.g. in the message buffer, the cursor flow gets a kind of 'hick-up' and is interrupted every 2 seconds or so. As a result, editing of text in all purpose enabled buffers becomes slow and pretty inconvenient.

I wonder why this seems to happen only with the 'Kill Extension'. If I switch off the window-purpose mode or call

  (purpose-x-kill-unset)

the cursor flows again fluently, without any significant hick-ups.

I am on GNU Emacs 25.0.50.1 (x86_64-apple-darwin14.5.0, NS appkit-1348.17 Version 10.10.5 (Build 14F1509))

Any hints on what causes this rather strange and annoying behavior are very much welcome.

bmag commented 8 years ago

Thanks for reporting. This is strange, I haven't seen such hick-ups before, but it's been a while since last time I used 25.0. The of Kill extension is to make replace-buffer-in-windows aware of purposes, so the buffer is replaced by a buffer with the same before. Kill extension uses an advice to override replace-buffer-in-windows (I didn't find a better alternative), which is called mainly by the command kill-buffer.

Kill extension should not affect text editing and cursor flow in any way, but two things to look for are:

I don't have time right now to dig deeper, but I'll put it on my list.

emelin commented 8 years ago

Thank you for this quick answer full of valuable hints!

If time has come and you need any further testing for tracking this issue down, please let me know.

bmag commented 8 years ago

I'm confused. You're saying that disabling the kill extension (M-x purpose-x-kill-unset) eliminates the hick-up, but that replace-buffer-in-windows is never called when the hick-up happens?

Can you post here the profiler's report?

Anyway, if the problem is in replace-buffer-in-windows, you can try evaluating this code after loading the kill extension and see if it helps:

(defun purpose-first-buffer-with-purpose (purpose &optional exclude-buffers)
  (cl-loop for buff in (buffer-list)
           if (and (eql purpose (purpose-buffer-purpose buff))
                   (not (memq buff exclude-buffers)))
           return buff))

(defun purpose-x-replace-buffer-in-windows-1 (&optional buffer-or-name)
  (interactive "bBuffer to replace: ")
  (let* ((buffer (window-normalize-buffer buffer-or-name))
         (purpose (purpose-buffer-purpose buffer))
         (next-buffer (purpose-first-buffer-with-purpose purpose (list buffer))))
    (dolist (window (window-list-1 nil nil t))
      (if (eq (window-buffer window) buffer)
          (unless (window--delete window t t)
            (let ((dedicated (purpose-window-purpose-dedicated-p window))
                  (deletable (window-deletable-p window)))
              (cond
               ((and dedicated next-buffer)
                ;; dedicated, so replace with a buffer with the same purpose
                (set-window-buffer window next-buffer))
               ((and dedicated deletable (null next-buffer))
                ;; dedicated, but no other buffers with the same purpose, so
                ;; delete the window/frame
                (if (eq deletable 'frame)
                    (delete-frame (window-frame window))
                  (delete-window window)))
               (t
                ;; 1) not dedicated, or 2) dedicated and no other buffers with
                ;; the same purpose, but the window isn't deletable.
                ;; remove dedicated status and switch to previous buffer
                (purpose-set-window-purpose-dedicated-p window nil)
                (set-window-dedicated-p window nil)
                (switch-to-prev-buffer window 'kill)))))
        ;; Unrecord BUFFER in WINDOW.
        (unrecord-window-buffer window buffer)))))

The difference between this version of purpose-x-replace-buffer-in-windows-1 and the packaged one, is that this one doesn't look for all the buffers with the same purpose as the buffer being killed, but only looks for the first matching one. Theoretically it should be faster.

bmag commented 8 years ago

@emelin two weeks from now I will have time to look into this, but I need you to provide a short recipe to reproduce the problem, so that I can reproduce it and debug it. Can you write here what packages need to be installed, and how they need to be configured to experience the problem? (basically a minimal init.el) Thanks

emelin commented 8 years ago

Ok, great! Thank you.

I'll provide the information in time.

On 2016-01-31 Sun 22:36, Bar wrote:

@emelin two weeks from now I will have time to look into this, but I need you to provide a short recipe to reproduce the problem, so that I can reproduce it and debug it. Can you write here what packages need to be installed, and how they need to be configured to experience the problem? (basically a minimal init.el) Thanks


Reply to this email directly or view it on GitHub: https://github.com/bmag/emacs-purpose/issues/76#issuecomment-177603974

bmag commented 8 years ago

@emelin ping

emelin commented 8 years ago

Thanks for your kind 'ping'.

Well, I thought maybe v. 1.5 might fix the issue, but it didn't.

Below is the minimal init you ask me to provide. The hick-up effect is still noticeable, however less strong than with my full setup. (Possibly because there are more git projects around and open at the same time, but that is just a guess.)

I recommend to use really long lines and make the cursor move on them. Then you should be able to experience the hick-up.

(require 'smart-mode-line)
(sml/setup)
(sml/apply-theme 'automatic)

(require 'window-purpose)
(purpose-mode)
(require 'window-purpose-x)
(purpose-x-kill-setup)

(require 'projectile)
(projectile-global-mode 1)
(setq projectile-require-project-root t) ;;; the default
;;  (setq projectile-require-project-root nil)

As mentioned in previous posts above, the trouble is caused by some bad interplay of the smart-mode-line being updated through projectile with purpose-mode sitting somewhere in between, possibly hard to track down. This can be demonstrated by changing the var projectile-mode-line from a function (the default) to a string (e.g. as below). Then the hick-up is gone but cause of the trouble is still around.

(setq projectile-mode-line " Pj")

Thank you for taking the time looking into this.

bmag commented 8 years ago

@emelin I did not succeed in reproducing the bug on a Lubuntu 14.04 machine, using Emacs 25.0.91 (second release candidate for Emacs 25.1). I am sorry, but I don't think I can help you any further.

I did not notice any slowdown scrolling through files, with the config file you provided. I tested by using C-f, C-b, C-n,C-p,C-vandM-v` in various files, with several different frame sizes, with the frame split in 1, 2 or 6 windows. According to the profiler, most of the time is spent in the display engine, when formatting the mode-line string for projectile.

smart-mode-line uses rich-minority to show the mode-line lighters for each minor mode. The profiler shows that rich-minority spends most of its time formatting projectile's mode-line string. I have advised all Purpose's functions to output a message when they're invoked, and it seems that formatting projectile's mode-line (IOW calling projectile-project-name) doesn't invoke any of Purpose's functions.

I have used the latest versions of the packages: projectile-20160228.8, rich-minority-20151201.400, smart-mode-line-20160125.900, window-purpose-20160227.630.

Below is a profiler's report of running in a frame split into 6 windows, and scrolling in three of them: through window.el file from Emacs' source code, through lexer.py from Pygments python pacakge, and through a Dired buffer of Emacs' lisp directory.

At the end is the code I used to advice all Purpose functions.

- redisplay_internal (C function)                                9600  65%
 - eval                                                          9413  64%
  - sml/generate-minor-modes                                     9351  64%
   - rm--mode-list-as-string-list                                9079  62%
    - mapcar                                                     9059  62%
     - format-mode-line                                          8932  61%
      - eval                                                     8920  61%
       - if                                                      8811  60%
        - format                                                 8801  60%
         + projectile-project-name                               8801  60%
          file-remote-p                                             6   0%
       + purpose--modeline-string                                 109   0%
     + rm--propertize                                              20   0%
   + sml/fill-width-available                                     198   1%
     sml/count-occurrences-starting-at                              4   0%
  + sml/generate-modified-status                                   44   0%
 + jit-lock-function                                               94   0%
 + tool-bar-make-keymap                                            25   0%
 + #<compiled 0x21a9209>                                           13   0%
   file-remote-p                                                   10   0%
 + #<compiled 0x21a9209>                                            9   0%
 + #<compiled 0x21a9209>                                            8   0%
   kill-this-buffer-enabled-p                                       4   0%
- command-execute                                                3909  26%
 - call-interactively                                            3902  26%
  - funcall-interactively                                        2908  19%
   - scroll-up-command                                           1290   8%
    - scroll-up                                                  1205   8%
     - eval                                                      1186   8%
      - sml/generate-minor-modes                                 1177   8%
       - rm--mode-list-as-string-list                            1173   8%
        - mapcar                                                 1170   8%
         - format-mode-line                                      1162   7%
          - eval                                                 1162   7%
           - if                                                  1152   7%
            - format                                             1152   7%
             + projectile-project-name                           1152   7%
           + purpose--modeline-string                              10   0%
         + rm--propertize                                           4   0%
       + sml/fill-width-available                                   4   0%
      + sml/generate-modified-status                                9   0%
     + jit-lock-function                                           15   0%
   - scroll-down-command                                         1066   7%
    - scroll-down                                                 650   4%
     - eval                                                       650   4%
      - sml/generate-minor-modes                                  632   4%
       - rm--mode-list-as-string-list                             620   4%
        - mapcar                                                  620   4%
         - format-mode-line                                       620   4%
          - eval                                                  620   4%
           - if                                                   615   4%
            - format                                              615   4%
             + projectile-project-name                            615   4%
           + purpose--modeline-string                               5   0%
       + sml/fill-width-available                                  12   0%
      + sml/generate-modified-status                               10   0%
   + previous-line                                                134   0%
   + dired-next-line                                               99   0%
   + next-line                                                     94   0%
   + dired-previous-line                                           90   0%
   + execute-extended-command                                      72   0%
   + end-of-buffer                                                 29   0%
   + windmove-left                                                 22   0%
   + windmove-down                                                  4   0%
  + byte-code                                                     994   6%
+ ...                                                             961   6%
+ timer-event-handler                                              33   0%
+ eldoc-pre-command-refresh-echo-area                              32   0%
+ internal-echo-keystrokes-prefix                                  16   0%
+ internal-timer-start-idle                                         8   0%
(defun my-print-fn (oldfun &rest args)
  (message "Here: %s" oldfun)
  (apply oldfun args))

(setq my-atoms nil)
(mapatoms (lambda (sym) (push sym my-atoms)))
(setq my-atoms (nreverse my-atoms))
(setq my-good-atoms (--filter (string-match-p "purpose" (symbol-name it)) my-atoms))
(setq my-good-fns (-filter #'fboundp my-good-atoms))
(setq my-good-vars (-filter #'boundp my-good-atoms))

(defun my-add-advices ()
  (interactive)
  (dolist (fn my-good-fns)
    (advice-add fn :around #'my-print-fn)))

(defun my-remove-advices ()
  (interactive)
  (dolist (fn my-good-fns)
    (advice-remove fn #'my-print-fn)))

;; (global-set-key (kbd "<f5>") #'my-add-advices)
;; (global-set-key (kbd "<f6>") #'my-remove-advices)

(advice-add 'projectile-project-name :before #'my-add-advices)
(advice-add 'projectile-project-name :after #'my-remove-advices)
;; (advice-remove 'projectile-project-name #'my-add-advices)
;; (advice-remove 'projectile-project-name #'my-remove-advices)

;; (advice-add 'purpose-x-replace-buffer-in-windows :around #'my-print-fn)
;; (advice-remove 'purpose-x-replace-buffer-in-windows #'my-print-fn)
bmag commented 8 years ago

I recommend to use really long lines and make the cursor move on them. Then you should be able to experience the hick-up.

Emacs' display engine is known to be slow when long lines are involved, regardless of purpose-mode. Search for "emacs long lines performance" or "emacs long lines slow" in your favorite search engine and you'll see.

emelin commented 8 years ago

Thank you very much for your detailed research, indeed!

I will try to compare your profiler's report with mine when there is some time.

For now, I'll stay and live with the hacked projectile mode-line workaround.

Best regards, emelin

On 2016-03-03 Thu 12:18, Bar wrote:

@emelin I did not succeed in reproducing the bug on a Lubuntu 14.04 machine, using Emacs 25.0.91 (second release candidate for Emacs 25.1). I am sorry, but I don't think I can help you any further.

I did not notice any slowdown scrolling through files, with the config file you provided. I tested by using C-f, C-b, C-n,C-p,C-vandM-v` in various files, with several different frame sizes, with the frame split in 1, 2 or 6 windows. According to the profiler, most of the time is spent in the display engine, when formatting the mode-line string for projectile.

smart-mode-line uses rich-minority to show the mode-line lighters for each minor mode. The profiler shows that rich-minority spends most of its time formatting projectile's mode-line string. I have advised all Purpose's functions to output a message when they're invoked, and it seems that formatting projectile's mode-line (IOW calling projectile-project-name) doesn't invoke any of Purpose's functions.

I have used the latest versions of the packages: projectile-20160228.8, rich-minority-20151201.400, smart-mode-line-20160125.900, window-purpose-20160227.630.

Below is a profiler's report of running in a frame split into 6 windows, and scrolling in three of them: through window.el file from Emacs' source code, through lexer.py from Pygments python pacakge, and through a Dired buffer of Emacs' lisp directory.

At the end is the code I used to advice all Purpose functions.

- redisplay_internal (C function)                                9600  65%
 - eval                                                          9413  64%
  - sml/generate-minor-modes                                     9351  64%
   - rm--mode-list-as-string-list                                9079  62%
    - mapcar                                                     9059  62%
     - format-mode-line                                          8932  61%
      - eval                                                     8920  61%
       - if                                                      8811  60%
        - format                                                 8801  60%
         + projectile-project-name                               8801  60%
          file-remote-p                                             6   0%
       + purpose--modeline-string                                 109   0%
     + rm--propertize                                              20   0%
   + sml/fill-width-available                                     198   1%
     sml/count-occurrences-starting-at                              4   0%
  + sml/generate-modified-status                                   44   0%
 + jit-lock-function                                               94   0%
 + tool-bar-make-keymap                                            25   0%
 + #<compiled 0x21a9209>                                           13   0%
   file-remote-p                                                   10   0%
 + #<compiled 0x21a9209>                                            9   0%
 + #<compiled 0x21a9209>                                            8   0%
   kill-this-buffer-enabled-p                                       4   0%
- command-execute                                                3909  26%
 - call-interactively                                            3902  26%
  - funcall-interactively                                        2908  19%
   - scroll-up-command                                           1290   8%
    - scroll-up                                                  1205   8%
     - eval                                                      1186   8%
      - sml/generate-minor-modes                                 1177   8%
       - rm--mode-list-as-string-list                            1173   8%
        - mapcar                                                 1170   8%
         - format-mode-line                                      1162   7%
          - eval                                                 1162   7%
           - if                                                  1152   7%
            - format                                             1152   7%
             + projectile-project-name                           1152   7%
           + purpose--modeline-string                              10   0%
         + rm--propertize                                           4   0%
       + sml/fill-width-available                                   4   0%
      + sml/generate-modified-status                                9   0%
     + jit-lock-function                                           15   0%
   - scroll-down-command                                         1066   7%
    - scroll-down                                                 650   4%
     - eval                                                       650   4%
      - sml/generate-minor-modes                                  632   4%
       - rm--mode-list-as-string-list                             620   4%
        - mapcar                                                  620   4%
         - format-mode-line                                       620   4%
          - eval                                                  620   4%
           - if                                                   615   4%
            - format                                              615   4%
             + projectile-project-name                            615   4%
           + purpose--modeline-string                               5   0%
       + sml/fill-width-available                                  12   0%
      + sml/generate-modified-status                               10   0%
   + previous-line                                                134   0%
   + dired-next-line                                               99   0%
   + next-line                                                     94   0%
   + dired-previous-line                                           90   0%
   + execute-extended-command                                      72   0%
   + end-of-buffer                                                 29   0%
   + windmove-left                                                 22   0%
   + windmove-down                                                  4   0%
  + byte-code                                                     994   6%
+ ...                                                             961   6%
+ timer-event-handler                                              33   0%
+ eldoc-pre-command-refresh-echo-area                              32   0%
+ internal-echo-keystrokes-prefix                                  16   0%
+ internal-timer-start-idle                                         8   0%
(defun my-print-fn (oldfun &rest args)
  (message "Here: %s" oldfun)
  (apply oldfun args))

(setq my-atoms nil)
(mapatoms (lambda (sym) (push sym my-atoms)))
(setq my-atoms (nreverse my-atoms))
(setq my-good-atoms (--filter (string-match-p "purpose" (symbol-name it)) my-atoms))
(setq my-good-fns (-filter #'fboundp my-good-atoms))
(setq my-good-vars (-filter #'boundp my-good-atoms))

(defun my-add-advices ()
  (interactive)
  (dolist (fn my-good-fns)
    (advice-add fn :around #'my-print-fn)))

(defun my-remove-advices ()
  (interactive)
  (dolist (fn my-good-fns)
    (advice-remove fn #'my-print-fn)))

;; (global-set-key (kbd "<f5>") #'my-add-advices)
;; (global-set-key (kbd "<f6>") #'my-remove-advices)

(advice-add 'projectile-project-name :before #'my-add-advices)
(advice-add 'projectile-project-name :after #'my-remove-advices)
;; (advice-remove 'projectile-project-name #'my-add-advices)
;; (advice-remove 'projectile-project-name #'my-remove-advices)

;; (advice-add 'purpose-x-replace-buffer-in-windows :around #'my-print-fn)
;; (advice-remove 'purpose-x-replace-buffer-in-windows #'my-print-fn)

Reply to this email directly or view it on GitHub: https://github.com/bmag/emacs-purpose/issues/76#issuecomment-191694373

Miciah commented 6 years ago

I tried out purpose-x-kill-setup today and experienced extreme performance problems: around 15 seconds of latency for every command, even just moving the cursor up or down a line. The optimized purpose-x-replace-buffer-in-windows-1 definition in https://github.com/bmag/emacs-purpose/issues/76#issuecomment-171370770 eliminated any obvious performance problems.

I could not reproduce the problem using the minimal reproducer in https://github.com/bmag/emacs-purpose/issues/76#issuecomment-189637442.

In the profiling data, replace-buffer-in-windows shows up all over the place: directly underneath format-time-string (used by rcirc and spaceline), expand-file-name (used by projectile), file-readable-p (used by auto-revert-buffers), decode-coding-string (used by rcirc), and other places. It looks like replace-buffer-in-windows is such a prevalent and low-level function that any performance problems that it has can cause severe problems for many packages.