Lsp-mode: Not relevant completion candidates and blocking input

[rrudakov]

Describe the bug
After recent updates when I'm typing I see not relevant completion candidates (looks like from previous completion) for a few seconds, then candidates list updates and I see proper values, but during this few seconds I cannot type anything.

To Reproduce
Noticeable in large java projects.

Which Language Server did you use
lsp-java

OS
Linux 5.4.0-47-generic #51-Ubuntu SMP Fri Sep 4 19:50:52 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux

Screenshot

Logs

lsp-log-io.txt

[yyoncho]

Follow this https://emacs-lsp.github.io/lsp-mode/page/performance/ and provide perf report.

[rrudakov]

lsp-log.txt
profiler-report.txt

[yyoncho]

@rrudakov I don't see anything pointing out that lsp-mode is blocking. Can you test without company-quickhelp? Also, can you do M-x toggle-debug-on-quit and then C-g when Emacs blocks? This will show what is blocking right now. Do that multiple times so we can be sure that we are at the right thing.

[rrudakov]

I've tried many times and debugger always stops on the following function:

Debugger entered--Lisp error: (quit)
  redisplay_internal\ \(C\ function\)()

Not sure if it will be helpful :(

[rrudakov]

lsp-log2.txt
profiler-report2.txt

Please, check this reports.

[yyoncho]

Is this with company-quickhelp being off? I am assuming that you are using the default company frontends, right? Can you test with company-posframe as a frontend?

Can you test with (setq lsp-completion-use-last-result nil) (this is what is causing the stale results). Are you able to reproduce with lsp-start-plain.el? If it is display issue the following settings might help:

(setq lsp-completion-show-detail nil
        lsp-completion-show-type nil
       company-tooltip-maximum-width 80
        company-tooltip-minimum-width 80)

Debugger entered--Lisp error: (quit)
redisplay_internal\ (C\ function)()

I don't know how to diagnose redisplay related issues. I am cc-ing @dgutov who might have a clue.

[rrudakov]

Is this with company-quickhelp being off?

yes

I am assuming that you are using the default company frontends, right? Can you test with company-posframe as a frontend?

Yes, I use lucid build and I found company-posframe much slower than default frontend for me, but I'll try.

Can you test with (setq lsp-completion-use-last-result nil)

It's much better, at least input is not blocked anymore, the completion is slow, but I think it's server's problem. Thanks!

[yyoncho]

(setq lsp-completion-use-last-result nil)

Hm, this is odd. IMO there is another general problem with your setup most likely related to the speed the company popup is rendered. The effect from lsp-completion-use-last-result being nil is that we will send fewer results to company to render.

[rrudakov]

The initial popup with stale results appears very quick, that's true. When I set lsp-completion-use-last-result to nil I see visible delay before popup will be shown (I think it appears when actual response from server is received), but in the second case typing is not blocked.

My company mode setting is following:

(use-package company
  :config
  (setq company-global-modes '(not org-mode markdown-mode shell-mode))
  (setq company-tooltip-align-annotations t
        company-idle-delay 0.1
        company-require-match nil
        company-abort-manual-when-too-short t)

  (global-company-mode))

[yyoncho]

@rrudakov did you see the same with lsp-start-plain.el?

[rrudakov]

@rrudakov did you see the same with lsp-start-plain.el?

No, no ideas what can be wrong with my configuration. Will try to figure it out. Thank you for help!

[ericdallo]

@rrudakov do you use doom-emacs?

I had the same issue:
This is the callstack when I use C-g while emacs is blocked, I tested multiple times and the result is always on lsp-completion--sort-completions.

This is my company-frontend variable: (company-pseudo-tooltip-frontend company-echo-metadata-frontend)
c/c @yyoncho

[kiennq]

I saw this in the call stack

  (if (and (company-manual-begin) (= company-candidates-length 1)) (progn (company-complete-common)))
  +company/complete()

The (company-manual-begin) will set company--manual-action to t, which in turn will make non-essential to nil, thus make the completion un-skippable by new key pressed. That explains why you got typing blocked.

Either doom-emacs or company-mode should fix that. @dgutov

[ericdallo]

I found this on doom-emacs, is that correct? should doom-emacs remove that?

[yyoncho]

In my mind, the completion request is always less important than the user input and lsp-mode should always drop the processing and ignore non-essential flag

[kiennq]

I found this on doom-emacs, is that correct? should doom-emacs remove that?

Yes, I think so.

In my mind, the completion request is always less important than the user input and lsp-mode should always drop the processing and ignore non-essential flag

I think this is kind of convention in Emacs to set non-essential when running code that should not disturb the user.

non-essential is a variable defined in ‘simple.el’.
Its value is nil
Documentation:
Whether the currently executing code is performing an essential task.
This variable should be non-nil only when running code that should not
disturb the user. E.g., it can be used to prevent Tramp from prompting
the user for a password when we are simply scanning a set of files in the
background or displaying possible completions before the user even asked
for it.

In this case, the code is using company-manual-begin, which is user manually trigger the completion (not auto), thus wouldn't it make sense to wait for result, instead of breaking with new input, since user is specifically ask for it?
Doom-emacs can always change to use company-auto-begin though.

[dgutov]

company--manual-action makes non-essential being set to nil only with non-nil company-require-match values. Users (or starter kits) who don't want this can set company-require-match to nil.

[dgutov]

thus wouldn't it make sense to wait for result, instead of breaking with new input, since user is specifically ask for it?

Indeed.

@yyoncho

the completion request is always less important than the user input and lsp-mode should always drop the processing and ignore non-essential flag

The intended effect is to have company disallow the user enter a string that has no completions (when we're really sure the user wants completion, at least).

IME that saves me time, because I'm punished less for my typos (not having to undo and initiate completion again).

[kiennq]

company--manual-action makes non-essential being set to nil only with non-nil company-require-match values. Users (or starter kits) who don't want this can set company-require-match to nil.

In company--fetch-candidates I saw (non-essential (not (company-explicit-action-p))), how is company-require-match-p is involved here?

[dgutov]

That's a good point.

But the logic there was exactly as you said ("wouldn't it make sense to wait for result, instead of breaking with new input, since user is specifically ask for it"). E.g. to have M-x company-complete wait until the result is available instead of reacting to any user keypress.

The company-require-match-p mechanics was one of the other goals, though. Possibly a more important one.

[rrudakov]

@ericdallo I use vanilla emacs built from source with lucid toolkit. I don't use doom-emacs.

[dgutov]

Commenting on the prior discussion:

I see not relevant completion candidates (looks like from previous completion)

This sounds like the lsp-completion-use-last-result=t behavior is missing some invalidation when the completion popup is dismissed.

Debugger entered--Lisp error: (quit) ... redisplay_internal\ (C\ function)()

Can't say for sure, but this might still mean Emacs is waiting for a network response. Which is what the profiler seems to indicate.

[yyoncho]

thus wouldn't it make sense to wait for result, instead of breaking with new input, since user is specifically ask for it?

If you want to see the result you wont start typing. The fact that you continue to type indicates that you want to further filter the results. Furthermore, if you explicitly call complete -> emacs blocks -> user types -> result complete -> the typed keys pop up the result from the blocking call will be disregarded either way and it is just a waste(at least in the lsp-mode case).

[yyoncho]

I see not relevant completion candidates (looks like from previous completion)

This sounds like the lsp-completion-use-last-result=t behavior is missing some invalidation when the completion popup is dismissed.

That is the whole point of use-last-result - it shows the previous result when we don't have the time to get the data from the server. My observation is that it makes emacs much slicker because of the constant show/hide of the completion popup does not happen.

[dgutov]

If you want to see the result you wont start typing. The fact that you continue to type indicates that you want to further filter the results.

Maybe you do. But you don't want to abort completion, for instance. Unless you do type C-g.

if you explicitly call complete -> emacs blocks -> user types -> result complete -> the typed keys pop up the result from the blocking call will be disregarded either way

The user can press keys that don't insert chars in the buffer. Such as Alt-Tab, for example.

it shows the previous result when we don't have the time to get the data from the server

The "previous result" should be invalidated when completion is aborted or finished.

[yyoncho]

@dgutov

Here it is my expectations how the completion should happen (and I think that this is what other editors are doing).

1. user requests completion either by triggering it explicitly or by pressing trigger char
2. company-mode(completion framework) ask the backend for results and it returns a promise like structure.
3. User performs an action and depending on whether the action invalidates the completion result(e. g. inserting char, switching buffer, etc) or not(e. g. go to next completion item in the list) the completion framework either waits for the completion to finish or it cancels the promise.
4. (Optional) if result retrieving is slow, completion framework shows some indicator like loading in the popup.

At no point the input is blocked due to completion.

The user can press keys that don't insert chars in the buffer. Such as Alt-Tab,
for example.

Yes, they can - AFAIK being in accept-process-output loop and checking input-pending-p in order to simulate async call there is no way to know what is the actual command that is going to be executed.

[yyoncho]

it shows the previous result when we don't have the time to get the data from the server

The "previous result" should be invalidated when completion is aborted or finished.

we do that - we reuse the previous result as apart of one completion session.

[kiennq]

we do that - we reuse the previous result as apart of one completion session.

Sorry, current we do not do that. Here is the PR to fix that https://github.com/emacs-lsp/lsp-mode/pull/2205

[dgutov]

At no point the input is blocked due to completion.

"Other editors" don't have C-g which every Emacs user should know about.

If the user invoked a completion command explicitly, Emacs might as well block until the result is known. It both makes debugging completion easier and allows implementing company-require-match (a feature I don't see in contemporary editors, but clearly, and fondly, remember from IDEs of the past). It also simply makes sense.

there is no way to know what is the actual command that is going to be executed

All the more reason to only break out of the loop on input in only select situations.

(completion framework) ask the backend for results and it returns a promise like structure

Right, and if you used company-mode's async support, you would do that. And you wouldn't have to worry about when to about due to input (the current completion framework would choose, be it company or otherwise).

Alas, completion-at-point-functions don't have any real support for promises, so it's currently an effort of fitting a round peg in a hexagonal hole.

[yyoncho]

@dgutov thank you. I agree with you.

@kiennq , based on @dgutov response I think that we should consider implementing a real async backend. IMO creating lsp-company backend after we spend so much time on integrating with company capf will be very unfortunate. I would propose 2 potential solutions.

1. company-capf to be extended to try calling :company-candidates and if it returns a function like the async backends do(?) to continue with it. I think that this will narrow down to one line fix company-capf.el.
2. lsp-mode to put a proxy on top of it and handle candidates request when it detects that it is in lsp-mode case.

[dgutov]

@yyoncho We seem to agree on an ideal state of affairs (in a way), but maybe not on the interim solution.

Ideally, c-a-p-f adds a documented async interface, and we use that.

But what is the goal you are currently trying to achieve? If you wanted to see completion behavior become closer to "other editors" (according to the 4-item list you wrote above), it won't happen. At least, not automatically.

Because what happens when a "company async" backend is used? When you invoke company-manual-begin, one way or another, the framework "synchronizes" the response inside company--force-sync. There is a minor difference regarding the conditions for that to happen, but with the default value of company-require-match the difference is negligible.

To put it another way, the backend interface is important, but unless we agree about the desired behavior (or the possible space of behaviors that can be configured), a change in the former won't immediately help.