oantolin / embark

Emacs Mini-Buffer Actions Rooted in Keymaps
GNU General Public License v3.0
917 stars 56 forks source link

embark-collect-live is slow if there are many candidates #101

Closed yoctocell closed 2 years ago

yoctocell commented 3 years ago

When doing C-h f or M-x in selectrum or icomplete, it only takes a split second to show the candidate list. When using the embark-collect-live feature, it can take up to 5 seconds to display the candidate list. Is there anyone else also experiencing the same performance issues?

(edit: forgot that occur was renamed)

oantolin commented 3 years ago

Yes, it is slow. It inserts all the candidates into a buffer and runs the annotator on all of them. Selectrum and Icomplete typically display and annotate only 10 candidates at a time (and even so aren't instantaneous)!

I wish I knew how to make it faster, but I don't. For now my advice is: don't start embark-collect-live right away, wait until you have typed a bit of the candidate. That way there are many fewer candidates and things are reasonably fast. Usually two letters is enough.

oantolin commented 3 years ago

I'm sorry I don't have better news for you. I'm very much open to suggestions from Emacs Lisp experts on how to speed things up.

One thing I have considered but have not tried yet, is to insert only the first 30 candidates right away, and then insert the rest on a cancelable timer.

yoctocell commented 3 years ago

On Sat, Jan 09 2021, Omar Antolín Camarena wrote:

Yes, it is slow. It insert all the candidates into a buffer and runs the annotator on all of them. Selectrum and Icomplete typically display and annotate only 10 candidates at a time!

I wish I knew how to make it faster, but I don't. For now my advice is: don't start embark-collect-live right away, wait until you have typed a bit of the candidate. That way there are many fewer candidates and things are reasonably fast. Usually two letters in enough.

Yeah, that definitely speeds things up. Thanks for the suggestion!

yoctocell commented 3 years ago

One thing I have considered but have not tried yet, is to insert only the first 30 candidates right away, and then insert the rest on a cancelable timer.

I would be interested in something like that, although my elisp-foo probably isn't up to the job.

minad commented 3 years ago

@oantolin Would that canceable timer abort as soon as the next key is pressed? I think that would work very well. This will be very similar to what I am doing with consult async process termination. There I also use some debouncing and refresh limiter.

protesilaos commented 3 years ago

I think that what Daniel does with the async process is a step in the right direction. The other element to it is to have an (optional?) minimum character input threshold. So, say, display all matches only after I have typed in 3+ characters, otherwise employ the async loading technique. I say "otherwise" because I assume people will want to see the buffer when they type M-x and just have embark-live-occur-after-delay set up.

minad commented 3 years ago

Async also checks for a minimum amount of characters btw. Async has a throttler, a debouncer and a minimum number of characters.

protesilaos commented 3 years ago

Async also checks for a minimum amount of characters btw. Async has a throttler, a debouncer and a minimum number of characters.

Yes, that is what I had in mind. Should have been more specific.

minad commented 3 years ago

See also https://github.com/oantolin/embark/pull/202#issuecomment-818452578 by @oantolin

Well, I wasn't suggesting abandoning tabulated-list-mode (although maybe I should have). We were talking about inserting the candidates in batches on a timer. I was thinking more of just replacing the function that inserts them, using everything else about tabulated-list-mode.

minad commented 3 years ago

I think we also discussed abandoning tabulated list mode, since it is an abuse in the case of grid view. And one avoids the intermediate step which could be faster for the live updates. At least I recall that I made the suggestion to not use tabulated list mode anymore. If this is a good idea or not - you know better. Maybe you don't want to end up like Marginalia and its beautiful formatting?

karthink commented 3 years ago

One thing I have considered but have not tried yet, is to insert only the first 30 candidates right away, and then insert the rest on a cancelable timer.

Have you given any more thought to this? I'm one of those stubborn users who don't employ any of the common selection frameworks (ivy/selectrum/vertico etc), preferring to rely on the full buffer functionality of the embark live-collect buffers. I believe even Protesilaos has moved on from using Embark for this by now.

So I can really feel the slow down on certain commands. This is only a few commands, though, such as M-x. If you can provide a blacklist for commands that shouldn't auto-open the live-export buffer, that would work well too and I can take embark-collect-completions-after-input off of minibuffer-setup-hook.

oantolin commented 3 years ago

I've been experimenting with replacing the tabulated-list-print function. I don't think I can make it much faster but I can make it more interruptible. Even this simple version seems to help:

(defun interruptible-tabulated-list-print (&rest _)
  (let ((inhibit-read-only t))
    (erase-buffer)
    (dolist (entry tabulated-list-entries)
      (while-no-input
        (apply tabulated-list-printer entry)))
    (goto-char (point-min))))

(advice-add 'tabulated-list-print
            :override #'interruptible-tabulated-list-print)

(setq embark-collect-live-initial-delay 0
      embark-collect-live-update-delay  0)

How does that feel on your computer, @karthink? On mine it feels pretty decent if I use grid view (which I do by default anyway), but with list view there are annotations and I like Marginalia's most expensive annotations. With those annotations it's still slow.

minad commented 3 years ago

Interruptible is great! Vertico and Iconplete do this too. See the wrapper around the call to vertico--recompute-candidates.

minad commented 3 years ago

What about only populating the annotations lazily? This would give a lovely flickering retro feeling when scrolling ;)

oantolin commented 3 years ago

I don't think that code above was really doing anything over than setting both delays to 0. 😆 Now that I more calmly review the situation, I realize that embark already wraps the updating of the auto-updating collect buffers in while-no-input. And I don't think I can really tell the difference between using the standard tabulated-list-print and the above substitute.

I have thought about populating the annotations lazily, but I'm not sure what the best way is. There is window-scroll-functions, maybe that's a good solution.

minad commented 3 years ago

The downside with scrolling is that you lose the buffer advantages. Isearch etc.

karthink commented 3 years ago

How does that feel on your computer, @karthink?

I couldn't really tell a difference. Is the sluggishness of the live-collect buffers mostly from marginalia? I feel a delay even in grid mode on commands like M-x. Auto-raising the live-collect buffers works well with commands that have < 100 completions, which is most of them. Thus my suggestion of a blacklist. Unfortunately the exceptions are my most used commands, like M-x and describe-symbol!

BTW, I had another idea that, like the blacklist, is also a bandaid:

I rarely need to look past the top thirty completions, especially since I use completion-all-sorted-completions. I've tried to use seq-take to show only the top completions, and this is much snappier. The problem with this approach has been that when I export the collection it only exports whatever's visible. If this can be tweaked, so that I can press a key in the minibuffer/live-collect-buffer to toggle between the top 30 completions and all completions, that might work too. Exporting can then show all of them by default.

oantolin commented 3 years ago

I couldn't really tell a difference.

I don't think there was a difference, I managed to delude myself that there was.

I've tried to use seq-take to show only the top completions, and this is much snappier. The problem with this approach has been that when I export the collection it only exports whatever's visible.

Well, a simple workaround for that export problem is to move back to the minibuffer and export from there. This could be automated, of course.

minad commented 3 years ago

@karthink Why do you prefer a full buffer? It seems you want to have both things at the same time, a buffer with all candidates where you can move and act freely. On the other hand you want it to be fast, which excludes displaying everything. If one starts now to hack embark or *Completions* to only show a small subset, the buffer benefits are lost.

Personally I am using both modes all the time. Vertico as a quick starting point and Embark collect when I want everything.

oantolin commented 3 years ago

For me the slowness really isn't a problem because by default I don't display the completions. I wait until I need them and then run embark-collect-completions once you typed two or three characters, embark-collect-completions feels instant.

minad commented 3 years ago

@oantolin I think it is okay to have Embark Collect Completions like this, there is no real need to make it faster. This would require introducing some kind of lazy buffer filling which will be brittle. For a fast immediate display we already have other options. I think it is okay to have the two distinct and with distinct performance characteristics.

karthink commented 3 years ago

On the other hand you want it to be fast, which excludes displaying everything.

@minad : My original question was if displaying everything can be faster than it is right now, because it's already fast enough for all but a few commands. Using Vertico (or icomplete) for quick access and embark-collect for comprehensive access makes sense, I was wondering if it's possible to do both using Embark. My suggestions above (not fully thought through) do not involve losing the full buffer functionality of collect buffers. I agree that lazy buffer populating might complicate things considerably.

For me the slowness really isn't a problem because by default I don't display the completions.

Me too. I have embark-collect-completions-after-input in minibuffer-setup-hook so the slowness is mitigated. BTW, is there an option to specify the minimum number of input characters to open the collect buffer instead of embark-collect-live-initial-delay?

minad commented 2 years ago

@oantolin This issue can probably be closed as wontfix since embark-collect-completions was removed in #466. The current performance is okay for what embark-collect and embark-live is designed for.

oantolin commented 2 years ago

Agreed.