Bad-ptr / persp-mode.el

named perspectives(set of buffers/window configs) for emacs
401 stars 44 forks source link

persp-frame-buffer-predicate performance issues #35

Open aaronjensen opened 8 years ago

aaronjensen commented 8 years ago

I do not understand what is actually causing the issues, but if persp-set-frame-buffer-predicate is set to restricted-buffer-list, the resulting predicate is called very often by persp-mode when it is doing file-symlink-p

For some more background and profiles see: https://github.com/bbatsov/projectile/issues/1003

The end result is a stutter when invoking helm projectile.

Setting persp-set-frame-buffer-prediate to nil alleviates this stutter.

Bad-ptr commented 8 years ago

Yes. I have already noticed it myself(it also slows down traveling through the company-mode's candidate list).

Probably this will fix it for most cases(replacing this function and regenerating buffer-predicate-s):

(defun persp-generate-frame-buffer-predicate (opt)
  (eval
   (if opt
       `(function
         (lambda (b)
           (if (string-prefix-p " " (buffer-name (current-buffer)))
               t
             ,(typecase opt
                (function
                 `(funcall ,opt b))
                (number
                 `(let ((*persp-restrict-buffers-to* ,opt))
                    (memq b (persp-buffer-list-restricted
                             (selected-frame) ,opt
                             persp-restrict-buffers-to-if-foreign-buffer t))))
                (symbol
                 (case opt
                   ('nil t)
                   ('restricted-buffer-list
                    '(memq b (persp-buffer-list-restricted
                              (selected-frame)
                              *persp-restrict-buffers-to*
                              persp-restrict-buffers-to-if-foreign-buffer
                              t)))
                   (t '(memq b (safe-persp-buffers (get-current-persp))))))
                (t t)))))
     nil)))

Can you test it?

aaronjensen commented 8 years ago

That is much better. This is a profile from several helm projectiles, which is the test I used before. As you can see, get-frame-persp is no longer getting top billing and I don't feel the stutter anymore. I was also sure to change `persp-set-frame-buffer-predicate back and restarted emacs completely.

+ read-from-minibuffer                                            434  21%
+ file-truename                                                   249  12%
  Automatic GC                                                    153   7%
+ helm-read-pattern-maybe                                         146   7%
- get-frame-persp                                                 112   5%
 - if                                                             111   5%
  - progn                                                         111   5%
   - unwind-protect                                               111   5%
    - let                                                         111   5%
     - get-current-persp                                          111   5%
      - let*                                                      111   5%
       - progn                                                    111   5%
        - if                                                      111   5%
         - catch                                                  111   5%
          - persp-kill-buffer-query-function                      111   5%
           + file-symlink-p                                        73   3%
           + file-exists-p                                         22   1%
           + kill-buffer                                            9   0%
           + insert-file-contents                                   3   0%
           + file-readable-p                                        2   0%
           + file-newer-than-file-p                                 1   0%
           + documentation                                          1   0%
 + safe-persp-name                                                  1   0%
+ apply                                                           103   5%
+ generate-new-buffer                                              90   4%
+ spaceline--eval-segment                                          52   2%
+ projectile-file-exists-p                                         47   2%
+ funcall                                                          47   2%
aaronjensen commented 8 years ago

By the way, @Bad-ptr could I trouble you to explain what was happening here? I do not understand what would have caused this predicate to get called as a result of invoking file-symlink-p (which could have been a red herring?) Any way, any explanation would be very appreciated. Thank you!

Bad-ptr commented 8 years ago

explain what was happening here

The extensively used with-temp-buffer macro expands to a code calling the kill-buffer function which triggers the buffer-predicate. kill-buffer calls replace-buffer-in-windows replace-buffer-in-windows calls switch-to-prev-buffer switch-to-prev-buffer uses buffer-predicate

aaronjensen commented 8 years ago

I see, thank you very much for the explanation and the fix!

Bad-ptr commented 8 years ago

Must be fixed by 6449c8511da1de2ddd1cb05bb4e1e7314c136535.

Bad-ptr commented 8 years ago

There is an alternative way -- we can redefine the with-temp-buffer macro to something like this:

(fset (intern "with-temp-buffer-old") (symbol-function 'with-temp-buffer))
(defvar temp-buffer-name-base " *temp*-9!8@7#6$5%4^3&2*1(0)-")
(defmacro with-temp-buffer (&rest body)
    "Create a temporary buffer, and evaluate BODY there like `progn'.
See also `with-temp-file' and `with-output-to-string'."
    (declare (indent 0) (debug t))
    (let ((temp-buffer-name (make-symbol "temp-buffer-name"))
          (temp-buffer (make-symbol "temp-buffer")))
      `(let* ((,temp-buffer-name temp-buffer-name-base)
              (,temp-buffer (get-buffer-create ,temp-buffer-name)))
         (with-current-buffer ,temp-buffer
           (unwind-protect
               (progn ,@body)
             (read-only-mode -1)
             (let ((inhibit-read-only t))
               (delete-region (point-min) (point-max))))))))

It is not calling the kill-buffer, reuses a single temp buffer and just erases the buffer content at the end. And as the most(if not all) emacs code is sequential it seems to work correctly.

It will allow to get rid of a 'dirty hack' in the buffer-predicate:

(if (string-prefix-p " " (buffer-name (current-buffer)))
    t

But it will require to recompile all *.elc's that uses this macro(maybe recompiling helm and company will be enough for most 'slowdown' cases).

aaronjensen commented 8 years ago

That seems like an interesting idea. Would you propose that as a change to emacs proper? It doesn't redefining it here would be good, yeah?