swiper icon indicating copy to clipboard operation
swiper copied to clipboard

use timer to improve the ivy-read performance

Open redguardtoo opened this issue 7 years ago • 25 comments

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 while wait to make sure the user finish the input.

redguardtoo avatar Sep 28 '17 12:09 redguardtoo

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.

ambihelical avatar Nov 29 '17 21:11 ambihelical

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?

randymorris avatar Jul 25 '18 13:07 randymorris

(setq ivy-dynamic-exhibit-delay-ms 250)

redguardtoo avatar Jul 25 '18 13:07 redguardtoo

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 avatar Jul 25 '18 13:07 randymorris

@randymorris If I understood correctly, your problem with counsel-projectile-find-file is the start-up cost. There isn't much that can be done here, since we need to call projectile-current-project-files which creates the list of files. After the list is created, the filtering should be very fast.

@redguardtoo Can this issue be closed?

abo-abo avatar Jul 25 '18 14:07 abo-abo

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:

  1. Somehow make non-dynamic collections have the same behavior that ivy-dynamic-exhibit-delay-ms provides for dynamic collections.
  2. Make counsel-projectile-find-file provide a dynamic collection and set ivy-dynamic-exhibit-delay-ms appropriately.

randymorris avatar Jul 25 '18 14:07 randymorris

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.

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?

abo-abo avatar Jul 25 '18 15:07 abo-abo

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:

(setq ivy-re-builders-alist '((t . ivy--regex-fuzzy)))

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.

randymorris avatar Jul 25 '18 16:07 randymorris

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.

ambihelical avatar Jul 25 '18 16:07 ambihelical

@randymorris, try find-file-in-project-by-selected from https://github.com/technomancy/find-file-in-project

It's much faster.

redguardtoo avatar Jul 25 '18 23:07 redguardtoo

@randymorris I see, I suspected that ivy--regex-fuzzy was the issue. Could you test it with and without flx installed? I wonder if most of the time is being spent on filtering the candidates or sorting them by frequency.

abo-abo avatar Jul 26 '18 15:07 abo-abo

I've set my config back to always use ivy--regex-fuzzy and removed flx. My initial reaction is that it feels faster, but I really need to try it for a bit to be sure. I'll be away for a long weekend but I'll check back in next week with a conclusion after I get some time to actually work with this config for a while.

randymorris avatar Jul 26 '18 17:07 randymorris

CCing @ericdanan, as a part of this discussion pertains to https://github.com/ericdanan/counsel-projectile.

basil-conto avatar Jul 26 '18 17:07 basil-conto

@randymorris You may also want to test whether calling projectile-find-file instead counsel-projectile-find-file is faster. If so, I guess the issue would be with either the matcher or the sort function used by counsel-projectile-find-file (both are customizable, see variables counsel-projectile-find-file-matcher and counsel-projectile-sort-files).

ericdanan avatar Jul 26 '18 21:07 ericdanan

I'm also using ivy-dynamic-exhibit-delay-ms but it seems to me that debouncing is currently so aggressive that resting your finger on the up/down arrow will suspend all updates to the minibuffer if your key repeat rate is faster than the delay interval (right?) While I guess the nicest option (but probably more difficult to implement?) would be to debounce only those commands that change the list of available candidates, a compromise might be to call ivy--exhibit immediately if several successive calls to ivy--queue-exhibit together exceed the delay interval? For example, the following snippet allows me to browse candidates interactively even with a large delay-ms value, though of course the update rate is pretty low:

(defvar ivy--exhibit-waiting-since nil)

(defun ivy--queue-exhibit ()
  "Insert Ivy completions display, possibly after a timeout for
dynamic collections.
Should be run via minibuffer `post-command-hook'."
  (if (and (> ivy-dynamic-exhibit-delay-ms 0)
           (ivy-state-dynamic-collection ivy-last))
    (let* ((now (float-time))
           (float-delay (/ ivy-dynamic-exhibit-delay-ms 1000.0))
           (t0 (or ivy--exhibit-waiting-since now))
           (out-of-patience (> (- now t0) float-delay)))
      (when ivy--exhibit-timer (cancel-timer ivy--exhibit-timer))
      (if out-of-patience
          (progn
            (setq ivy--exhibit-waiting-since nil)
            (ivy--exhibit))
        (setq
         ivy--exhibit-waiting-since t0
         ivy--exhibit-timer (run-with-timer float-delay nil 'ivy--exhibit))))
    (ivy--exhibit)))

Or maybe there's an easy way to check if the input string has been changed since the last invocation of ivy--queue-exhibit, and use that to decide how aggressively the display update should be debounced?

sebastiansturm avatar Jan 27 '20 22:01 sebastiansturm

Hmm, I might have a better fix for this:

but it seems to me that debouncing is currently so aggressive that resting your finger on the up/down arrow will suspend all updates to the minibuffer if your key repeat rate is faster than the delay interval

We just need to implement one extra predicate that we can use to indicate whether another ivy--exhibit should be queued, which is when the user has entered commands that modified the prompt, otherwise ivy--exhibit can be executed immediately. Hence, the following:

(let (last-pos)
  (defun ivy-input-changed? ()
    (let* ((pos (line-end-position))
           (changed (not (eq pos last-pos))))
      (setq last-pos pos)
      changed)))

then inside ivy--queue-exhibit we call the newly implemented predicate.

(defun ivy--queue-exhibit (args ...)
  (if (and (> ivy-dynamic-exhibit-delay-ms 0)
           (ivy-state-dynamic-collection ivy-last)
           (ivy-input-changed?))
      then
    else ...))

Inc0n avatar Sep 18 '20 22:09 Inc0n

I think this can be done with while-no-input? That will interrupt updating collection when there's new key and thus provide the same effect? https://github.com/abo-abo/swiper/pull/2567

kiennq avatar Sep 23 '20 01:09 kiennq

Does looks like a better fix than mine

Inc0n avatar Sep 23 '20 11:09 Inc0n

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.

@randymorris @abo-abo I am experiencing the same issue. I use counsel-projectile to open open file, when I type the file name, the issue occurs. But if I just paste the file name without typing, I get the result quickly.

huangfeiyu avatar Sep 27 '21 07:09 huangfeiyu

I am experiencing the same issue. I use counsel-projectile to open open file, when I type the file name, the issue occurs. But if I just paste the file name without typing, I get the result quickly.

Is issue #2911 relevant to this? Thanks.

basil-conto avatar Sep 27 '21 10:09 basil-conto

Thanks @basil-conto By the way, @dminuoso has filed an issue in counsel-projectil https://github.com/abo-abo/swiper/issues/2911

huangfeiyu avatar Sep 28 '21 01:09 huangfeiyu

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 C-n/C-p without any delay. I don't see any reason to add the delay unless the input changes.

Here's an example demonstrating the issue. Type something and then move through the results. Each ivy-next-line (C-n) or ivy-previous-line (C-p) also gets the 1000ms delay, even though the result set isn't changing.

(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)))))

mgalgs avatar Oct 29 '21 20:10 mgalgs

I have a small advice in my config that helps me solve the C-n/ C-p problem while having dynamic exhibit delay set to some number. I'm not sure if this will actually create any other issue as I'm not aware of the internals of ivy but it seems to have solved the primary issue for me.

(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 avatar Nov 06 '21 22:11 Gleek

@Gleek thank you for sharing, this is working great!

mgalgs avatar Nov 08 '21 18:11 mgalgs

@Gleek , Could you send a pull request to https://github.com/abo-abo/swiper ? Thank you for your great code, btw.

redguardtoo avatar Nov 08 '21 23:11 redguardtoo