minad / corfu

:desert_island: corfu.el - COmpletion in Region FUnction
GNU General Public License v3.0
1.15k stars 43 forks source link

Mac polling #289

Closed minad closed 1 year ago

minad commented 1 year ago

It seems that while-no-input doesn't work properly on Mac. It uses polling, see https://lists.gnu.org/archive/html/bug-gnu-emacs/2023-01/msg01180.html. On Emacs 29 floating point values are allowed. We could let bind polling-period to 0.05 around the while-no-input. @jdtsmith I think you observed this issue before. Can you please give changing polling-period a try? This affects Vertico, Corfu and Consult since all these packages use while-no-input.

minad commented 1 year ago

It seems the NS port is just badly implemented. We should not fix this here. See https://lists.gnu.org/archive/html/bug-gnu-emacs/2023-01/msg01186.html.

jdtsmith commented 1 year ago

Huh, very interesting. That actually explains why you have more faith in the power of while-no-input than me :). I don't use the NS port, but the rogue (and superior) emacs-mac port, which is based on it. Not sure if it does anything special for input polling.

I've found tweaking the input code with sit-for seems to allow more performant interruption, clearly below 1 s (150ms is typical):

(while-no-input
  (while ipy--hidden-output-in-progress
    (accept-process-output process 0.1 nil 1) ; disables timers
    (sit-for .001)))
minad commented 1 year ago

Yes, for me while-no-input works perfectly on X11. Both Vertico and Corfu are highly responsive, no hanging at all.

jdtsmith commented 1 year ago

while-no-input works perfectly

Interruption in less than 50ms? I get some hanging on consult-buffer file-preview with vertico, only on larger files with "complex" mode startup (org typically). It's not terrible. Not sure if that's protected by while-no-input. If I get some time I might look for that code and add some sit-for's to see if it makes a difference.

minad commented 1 year ago

No, consult-buffer preview is non-interruptible. We cannot make that interruptible since mode setup is too complex and brittle. If we interrupt it we break Emacs entirely. Corfu and Vertico only interrupt around dedicated and hopefully safe operations (pure computations). But as you are aware it already leads to problems with broken (non-interruptible) backends, e.g., lsp-mode.

minad commented 1 year ago

Note that you can delay consult preview already via consult-customize :preview :debounce. See the README.

jdtsmith commented 1 year ago

Makes sense.

I tried the bug poster's repro and it indeed makes mine crawl. polling-period is integer for me (let-binding a float leads to an error). Changing from 2 to 1 (let-binding) doesn't really help much. If you come up with a test harness of some kind, happy to test here.

minad commented 1 year ago

I think you have to use Emacs 29 which supports float polling-period. No effect on Emacs 28 and older.

jdtsmith commented 1 year ago

it already leads to problems with broken (non-interruptible) backends, e.g., lsp-mode.

On that topic, I guess you've already seen the issues with didChange sync corfu seems to elicit in eglot? Copied you on a discussion.

jdtsmith commented 1 year ago

No effect on Emacs 28 and older.

Well, one effect: it reports an error when set to float, so any code would need to have version checks.

Error in post-command-hook (sayhi): (wrong-type-argument integerp 0.05)
minad commented 1 year ago

Yes, I've seen it. I don't have anything to add. One should check that the Capf is interruptible. See where while-no-input is used in the Corfu code.

minad commented 1 year ago

Well, one effect: it reports an error when set to float, so any code would need to have version checks.

I've already decided to not add a workaround here. The workaround will only work on 29 and it would be better to improve interruptibility on Emacs 30 by using a separate thread. It is not the job of the frontend to fix broken Emacs ports.

jojojames commented 1 year ago

For users using emacs 29, is the recommendation to tweak the polling rate or that doesn't work either?

minad commented 1 year ago

Yes, probably. But ideally the mechanism gets improved with a separate thread as Eli wrote.

jdtsmith commented 1 year ago

Just an FYI. The emacs-mac port I use apparently does not suffer from this problem:

- Emulation of `select' without periodic polling
   It doesn't use CPU time while the Lisp interpreter is idle and
   waiting for some events to come, even with subprocesses or
   network connections.

So this is probably why I can interrupt process output in 100ms or so on slow hardware.