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

Performance regression in commit e98883f50f5794 #653

Closed
hpdeifel opened this issue Sep 6, 2016 · 10 comments
Closed

Performance regression in commit e98883f50f5794 #653

hpdeifel opened this issue Sep 6, 2016 · 10 comments

Comments

@hpdeifel
Copy link

hpdeifel commented Sep 6, 2016

Ivy is currently so slow for me that it is barely usable. Typing letters has a noticeable lag. git bisect revealed commit e98883f as the culprit.

This lag happened in all ivy commands that I tested (counsel-M-x, ivy-find-file and a few others).

I can't reproduce it with plain emacs -Q, but calling (package-initialize) with an otherwise empty config suffices to show the bug.

Replacing (require 'flx nil 'noerror) with nil in ivy--format-minibuffer-line also makes ivy fast again.

@abo-abo
Copy link
Owner

abo-abo commented Sep 7, 2016

Works fine for me. Here's the profiler result:

ivy--exhibit                              33          0.271637048   0.0082314256

Less than 0.01 second is spent on filtering and redisplay after each key press.

@hpdeifel
Copy link
Author

hpdeifel commented Sep 7, 2016

I get

 ivy--exhibit   59          8.5247626979  0.1444875033

that doesn't sound much, but feels incredibly laggy.

profiler.el says that 86% of the time is spend in the and in ivy--format-minibuffer-line where the (require 'flx nil 'noerror) is that I mentioned. Strangely it doesn't show any children of this and.

Keep in mind that this only happens when (package-initialize) is called. I have quite a number of packages installed, so maybe the require has to go through all these package include paths to determine that flx is not there. Profiling require itself gives me:

require        1276        8.8469242039  0.0069333261

But that must be wrong, since replacing that require with nil fixes the problem.

@abo-abo
Copy link
Owner

abo-abo commented Sep 7, 2016

You mean removing only the require or the whole highlighting statement? Removing the require is, of course, and easy fix.

@hpdeifel
Copy link
Author

hpdeifel commented Sep 7, 2016

I mean this diff:

diff --git a/ivy.el b/ivy.el
index e74c0c5..7468e2e 100644
--- a/ivy.el
+++ b/ivy.el
@@ -2695,7 +2695,7 @@ SEPARATOR is used to join the candidates."
                       str))
                    (cl-incf i)))))
             ((and
-              (require 'flx nil 'noerror)
+              nil
               (or (eq ivy--regex-function 'ivy--regex-fuzzy)
                   (and (eq ivy--regex-function 'swiper--re-builder)
                        (let ((caller (ivy-state-caller ivy-last)))

Since I don't have flx installed, this is equivalent in my case and much faster

@jferguson-gnubio
Copy link

I also got a huge performance regression, that I assume is the same thing, moving from counsel-20160826.2343 to counsel-20160906.750 from melpa. Sorry I haven't done a proper bisect, but it sounds the same. counsel-M-x and counsel-find-file are very slow to start up.

@jferguson-gnubio
Copy link

jferguson-gnubio commented Sep 8, 2016

Apologies I'm not particularly proficient at Elisp, profiling etc, but with some experimentation on a variation of the patch hpdeifel suggested, for counsel-M-x

(require 'flx nil 'noerror) => 5627 cpu samples
(featurep 'flx)             => 942 cpu samples

I think the culprit might be that I have the auto-compile package, with auto-compile-on-load, which seems to result in a lot of the require overhead.

abo-abo added a commit that referenced this issue Sep 9, 2016
@abo-abo
Copy link
Owner

abo-abo commented Sep 9, 2016

Please check if it's better now.

@hpdeifel
Copy link
Author

hpdeifel commented Sep 9, 2016 via email

@jferguson-gnubio
Copy link

Works nicely - excellent, thank you.

@abo-abo
Copy link
Owner

abo-abo commented Sep 9, 2016

Thanks for the confirmation.

@abo-abo abo-abo closed this as completed Sep 9, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants