-
-
Notifications
You must be signed in to change notification settings - Fork 338
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
use timer to improve the ivy-read performance #1218
Comments
This is probably a good idea performance-wise since it limits the number of collection updates when the user is typing. However, the main reason for this change is to work around an Emacs freeze on Windows when the dynamic collection is generated by an external command (see for example this bug in helm-ag which also affects counsel-ag and similar commands: emacsorphanage/helm-ag#188). Related to abo-abo#1218 (but only applies for dynamic collections, where the difference is really noticeable). Helps with abo-abo#1198 and abo-abo#786.
This is probably a good idea performance-wise since it limits the number of collection updates when the user is typing. However, the main reason for this change is to work around an Emacs freeze on Windows when the dynamic collection is generated by an external command (see for example this bug in helm-ag which also affects counsel-ag and similar commands: emacsorphanage/helm-ag#188). Related to #1218 (but only applies for dynamic collections, where the difference is really noticeable). Helps with #1198 and #786. Fixes #1237
I tried hacking the new delay code added by @jeberger used for dynamic collections by disabling the actual check for dynamic collection. It works well except for up/down movements which are also delayed. Probably something would need to be done to allow ivy--queue-exhibit to call ivy--exhibit with filtering disabled so the list can be updated quickly. However besides this issue, it is a great quality of life improvement for really large trees and counsel-find-file or similar. I ended up using 200ms delay which seems to suit my typing speed. With an AOSP tree using counsel-projectile-find-file, I was seeing >1s delay or more per character typed. Now I can actually type whole words before the filtering kicks in. |
Sorry for digging up an old topic here, but I'm in pretty much the same exact situation as @ambihelical described. Any tips on implementing this hack so I can get by until this issue gets some attention? |
|
Unfortunately that doesn't help me here because the collection provided by counsel-projectile-find-file is not dynamic. Setting that timeout has no effect in this case. |
@randymorris If I understood correctly, your problem with @redguardtoo Can this issue be closed? |
Startup time is noticeable but acceptable. My issue is that the filtering happens on each keypress. Often I type a second, third, or fourth key before the filtering finishes for the first keypress. This leads to me having to wait for the filtering to happen four times before I can see what keys I need to press to filter further. Debouncing as described in the original issue would make the filtering happen only after a timeout rather than after every keypress, just as appears to be implemented for dynamic collections. The way I see it I have two options:
|
That sounds really bad, I've never noticed such poor performance. What's the amount of candidates you have and what config are you using? |
The collection of candidates for me was just under 8,000 files. In coming up with a minimal config to give you, I found that this is the culprit:
I think I had followed this post when I migrated to ivy a long time ago, thus I've always had this issue and not realized that the fuzzy matching was the cause. I suppose this "fixes" it for me, although I'll have to get used to the less fuzzy matching if I want it to be fast. I still think that debouncing the keypresses even with a non-dynamic collection is probably not a bad idea. Thanks for taking the time to reply, and again, sorry for digging up an old topic. |
I was dealing with a tree with about 200-400K candidates (an AOSP tree), I don't remember the exact number of files it had to filter through, but it was in that range. I added the hack I mentioned to deal with it, with good results, although there were some issues. I don't use fuzzy matching though. You can see the hack I am using in my fork of swiper, branch try-async. |
@randymorris, try It's much faster. |
@randymorris I see, I suspected that |
I've set my config back to always use |
CCing @ericdanan, as a part of this discussion pertains to https://github.com/ericdanan/counsel-projectile. |
@randymorris You may also want to test whether calling |
I'm also using
Or maybe there's an easy way to check if the input string has been changed since the last invocation of |
Hmm, I might have a better fix for this:
We just need to implement one extra predicate that we can use to indicate whether another
then inside
|
I think this can be done with |
Does looks like a better fix than mine |
@randymorris @abo-abo I am experiencing the same issue. I use |
Is issue #2911 relevant to this? Thanks. |
Thanks @basil-conto By the way, @dminuoso has filed an issue in |
Does anyone have a workaround for the issue described by @sebastiansturm where moving through the results incurs a delay even though the input isn't changing? I would like to be able to move through results with Here's an example demonstrating the issue. Type something and then move through the results. Each (defun debounced-ivy-ready-example ()
(interactive)
(let ((ivy-dynamic-exhibit-delay-ms 1000))
(ivy-read "Test: " (lambda (q) (if (string= q "")
'()
(list q
(format "%s/%s" q q)
(format "%s/%s/%s" q q q))))
:dynamic-collection t
:action (lambda (result) (message "Got: %s" result))))) |
I have a small advice in my config that helps me solve the (defvar +ivy--queue-last-input nil)
(defun +ivy-queue-exhibit-a(f &rest args)
(if (equal +ivy--queue-last-input (ivy--input))
(ivy--exhibit)
(apply f args))
(setq +ivy--queue-last-input (ivy--input)))
(advice-add 'ivy--queue-exhibit :around #'+ivy-queue-exhibit-a) Ideally it would make sense to have such change upstream instead. |
@Gleek thank you for sharing, this is working great! |
@Gleek , Could you send a pull request to https://github.com/abo-abo/swiper ? Thank you for your great code, btw. |
Fixes abo-abo#1218 Adds an extra check to `ivy--queue-exhibit` to only debounce if the input has actually changed. It also maintains the previous input variable to match against.
Fixes abo-abo#1218 Adds an extra check to `ivy--queue-exhibit` to only debounce if the input has actually changed. It also maintains the previous input variable to match against.
Fixes abo-abo#1218 Adds an extra check to `ivy--queue-exhibit` to only debounce if the input has actually changed. It also maintains the previous input variable to match against.
Fixes abo-abo#1218 Adds an extra check to `ivy--queue-exhibit` to only debounce if the input has actually changed. It also maintains the previous input variable to match against.
Fixes abo-abo#1218 Adds an extra check to `ivy--queue-exhibit` to only debounce if the input has actually changed. It also maintains the previous input variable to match against.
Fixes abo-abo#1218 Adds an extra check to `ivy--queue-exhibit` to only debounce if the input has actually changed. It also maintains the previous input variable to match against.
Fixes abo-abo#1218 Adds an extra check to `ivy--queue-exhibit` to only debounce if the input has actually changed. It also maintains the previous input variable to match against.
Fixes abo-abo#1218 Adds an extra check to `ivy--queue-exhibit` to only debounce if the input has actually changed. It also maintains the previous input variable to match against.
Fixes abo-abo#1218 Adds an extra check to `ivy--queue-exhibit` to only debounce if the input has actually changed. It also maintains the previous input variable to match against.
Fixes abo-abo#1218 Adds an extra check to `ivy--queue-exhibit` to only debounce if the input has actually changed. It also maintains the previous input variable to match against.
Fixes abo-abo#1218 Adds an extra check to `ivy--queue-exhibit` to only debounce if the input has actually changed. It also maintains the previous input variable to match against.
Fixes abo-abo#1218 Adds an extra check to `ivy--queue-exhibit` to only debounce if the input has actually changed. It also maintains the previous input variable to match against.
Fixes abo-abo#1218 Adds an extra check to `ivy--queue-exhibit` to only debounce if the input has actually changed. It also maintains the previous input variable to match against.
Fixes abo-abo#1218 Adds an extra check to `ivy--queue-exhibit` to only debounce if the input has actually changed. It also maintains the previous input variable to match against.
Fixes abo-abo#1218 Adds an extra check to `ivy--queue-exhibit` to only debounce if the input has actually changed. It also maintains the previous input variable to match against.
Fixes abo-abo#1218 Adds an extra check to `ivy--queue-exhibit` to only debounce if the input has actually changed. It also maintains the previous input variable to match against.
Fixes abo-abo#1218 Adds an extra check to `ivy--queue-exhibit` to only debounce if the input has actually changed. It also maintains the previous input variable to match against.
This trick is used frequently in javascript search box.
When user input characters, don't update the collection display immediately. Cache the user input until user has not input any new characters say for 1 second.
After one second, we continue the UI update.
Could you at least make a option for
ivy-read
like:seconds-wait-for-user-input
. Its default value is zero which means updating UI immediately. But if it's more than zero, UI will wait to make sure the user finish the input.The text was updated successfully, but these errors were encountered: