Bug 444883

I was not able to provide a test project. For small projects the parser is fast enough to give correct results right away. 

It would be nice if you could give me some hints with which configuration I can play around.

I can confirm this, it's very annoying. I'm pretty sure this is a recent regression from changes to KTextEditor's code completion widget. I will move the report there.

The reason for the slowness is likely because very recently, a change was made in ktexteditor that allows requerying the underlying completion source for completions. The reason for requerying is to get more relevant completions based on the newly typed characters, so for e.g you type

mov

You get a list of completions as a result. You type 'movIt' and you get new and more relevant completions. It's also important to note that the requerying doesn't happen for every typed character although I very much wanted to do that but it had two issues
- the completion pop up blinked every time you typed a char (very annoying) 
- it made kdevelop's completion too slow to the point that it was unbearable

So, the model is requeried but not for every character. 

The requerying itself is fine and necessary to be able to allow the completion source to help us more. In Kate there is no difference in performance. The reason for this slowness is in kdevelop and thus it should be optimized to handle this case.

Hi Waqar,

thanks for having a look at this. The problem is not (only) that it is slow in KDevelop, the problem is also that it now misbehaves. Try it out with a somewhat large project; the re-querying will somehow forget the search keyword on updating the list, leaving the first entry to be completed always, unless another character is typed again.

Additionally, cancellation doesn't work properly; e.g. typing "foo();<Enter>" will very easily leave you with some nonsense completion (e.g. "foo();__func__") added at the end of the line, I guess because the updated results only become available after the completion isn't relevant any more.

I think I already mentioned in one of the related merge requests that any changes to the completion list needs to be very carefully tested in KDevelop, because it is basically the only consumer for a large part of its functionality. Checking that things still work in Kate simply isn't enough.

As it is now, the changes done in KTextEditor leave the completion widget's most important downstream (KDevelop) in a pretty broken state, with (to me) no clear path to fixing it.

Best,
Sven

You are right with that, but if I don't misremember, we did CC KDevelop people on all of these requests, with in most cases not that much responses.

If there is no feedback about this, what shall we do? It would halt any development if we then just need to wait infinitely.

Naturally I can understand that all people are busy, but that will lead to such issues.

Hi, sorry, I didn't want to play a blame game, I am thankful for Waqar's work on this topic and I do feel somewhat responsible for the issue at hand because I know I should have taken the time to test those changes. I didn't, so I don't get to complain.

Still, this issue needs to be resolved somehow, and for this we first need to figure out what's wrong. My impression is that the completion widget now misbehaves in several ways when the model has a delay in updating its contents. This potential delay is part of the design, and the widget should not misbehave (open when it shouldn't open, or forget its filter string) when consumers have a delay in providing their items. Is it possible that this concern was overlooked in the changes because Kate's models are all extremely fast?

This cannot be fixed by making KDevelop's models faster, that's many thousand, probably over ten thousand lines of code written providing the completion items. kdev-python alone has ~3300 SLOC in its completion module. They are also somewhat optimized, performance was always a concern here, and if it now takes 300 msecs, I very much doubt you can just go there and make it return the results in 3 msecs.

@Sven

> Additionally, cancellation doesn't work properly; e.g. typing "foo();<Enter>"... 

That might be a bug that was there all the time but only became an issue due to the slowness. 

Anyways, the relevant change which is currently in effect is I think a couple of lines just. I purposely tried to keep it very minimal to not break anything. 

However, I don't think reverting back to the old state is a good idea though because this really improves the completions with LSP. 

Lack of testing/communication makes things a lot harder for us foss devs....

Interesting enough, the completion works well for me for the clangd LSP stuff.
And that seemed always comparable slow.
It would be really good to know how we screw up like that for KDevelop :/

I have also noticed the problem (or related) in Kate. I have chosen to not auto-select the first suggestion and quite often, when I have pressed arrow down to select a completion, the selection is reset to nothing... Just as if a model-reset had been done....

@sars

What completion backends do you have enabled? I am trying to reproduce this (lsp-clangd, tags, document) but haven't been able to.

@waqar

Now that I want to reproduce it I have a hard time doing so... The last time I remember it happening I have a vague memory that the LSP plugin was re-highlighting at the same time as I was typing and selecting the completions...

At least I can reproduce a quirk that if I type the beginning of a completion word, press arrow down to select it, and continue typing that same word, it will clear the selection (LSP + document backends)...

You can try in KDevelop, it's really easy to reproduce there. Just start typing something that will open a completion (e.g. member completion on anything), and keep adding characters. At some point it will flicker and forget what you typed, jumping to the first entry (and clearing the filter). With the next character typed, all is good again, until this pattern repeats.

Will try. Don't have a kdevelop build close by atm so might take a while.

The problematic change is this I think https://invent.kde.org/frameworks/ktexteditor/-/merge_requests/183

Rethinking about this, I think maybe we can make this "reinvoke" option something that the completion source can choose. This would allow sources which get *all* possible matches on first invocation to remain fast while the lazy sources will still be able to provide more completions as the user types in more characters.

That might make sense, yes. I don't think any of the KDevelop sources profit from getting a larger part of the current word; they are designed to just provide all possible items and then filter.

I don't quite understand why there are _more_ results after typing more? if at all, it should reduce the result set. Is clangd different in that regard?

I'm also going to look into this issue a bit, as it has such a drastic effect on my workflow. And again, just like Sven, I feel bad for not testing this earlier and spotting it right then. So no harm done by anyone, we can figure out a solution together, like we always did :)

@milian: There are more results because the filter gets cleared, it just shows all, not just matching after the bug triggers.

I was more talking about this part from waqar's earlier comment:

{quote}
The reason for the slowness is likely because very recently, a change was made in ktexteditor that allows requerying the underlying completion source for completions. The reason for requerying is to get more relevant completions based on the newly typed characters, so for e.g you type

mov

You get a list of completions as a result. You type 'movIt' and you get new and more relevant completions. It's also important to note that the requerying doesn't happen for every typed character although I very much wanted to do that but it had two issues
- the completion pop up blinked every time you typed a char (very annoying) 
- it made kdevelop's completion too slow to the point that it was unbearable

So, the model is requeried but not for every character. 
{/quote}

I don't understand why that should be done on the model level, instead of on the filter level afterwards - typing `mov` once should give you all results already. then typing `movIt` would only reduce the result set?

Ah. I didn't understand that either, but trusted the people who had used it with the LSP stuff to have a valid reason for this behaviour.

I have been thinking about this for a while and might have a solution.

We are limited by LSP(s), and also because how we are doing things(in Kate). One major problem on Kate's side is that we start completing after one character is typed. The LSP server, once it gets the 'm', is smart enough (or dumb) to not send everything that matches. Hence we run out of possible matches quickly once filtering happens on typing more chars.

The quick solution is to honor the "Minimum length to complete (default 3)" setting by default. Starting completions on the first char is imo not a good idea in itself. Plus we need to revert the hack that I put in earlier.

Why is this happening, though? Do the LSP servers cut off the completion list after N items because it gets too long (performance-wise) otherwise?

But yes, this seems to be the obvious difference to the KDevelop case then: here, we get all possible matches right from the start and then only ever filter that list on typing, and there's nothing better that could possibly be done.

 > Do the LSP servers cut off the completion list after N items because it gets too long (performance-wise) otherwise

Every lsp is different, but I think that's what clangd does... It's up to the server to implement however it wants as long as the protocol is followed.

But I disagree with there is nothing better that could be done.

I mean, in the KDevelop case, there's nothing better to be done. It won't profit from re-computing the list after typing a few characters, it has no useful additional information over what it had in the beginning -- except for the filter string.

Anyways, The query on every character has broken UI so no point in making it work via hacks either.

Will make a MR to revert the hack + introduce the change to honor "minimum word length for completion". The minimum word length change can still be ignored by a model if it wants to.

That sounds pragmatic. Thank you very much!

A possibly relevant merge request was started @ https://invent.kde.org/frameworks/ktexteditor/-/merge_requests/224

*** Bug 446423 has been marked as a duplicate of this bug. ***

A possibly relevant merge request was started @ https://invent.kde.org/frameworks/ktexteditor/-/merge_requests/241

Git commit b357a5db49d78323a37cf14d21208b6500f06ad1 by Christoph Cullmann, on behalf of Milian Wolff.
Committed on 29/12/2021 at 13:54.
Pushed by cullmann into branch 'master'.

Apply word filter on async completion models

When an async model resets itself in a delayed fashion, we showed
all its items and ignored the word at the current cursor position.
By calling `cursorPositionChanged()` from the `modelReset()` slot
in `KateCompletionWidget` we can ensure that the newly arrived
results are properly filtered too.

This is important for KDevelop where the clang provided completion
items arrive with a noticeable delay. Something changed in handling
those items in the last few months, breaking the filtering. Now the
async results are correctly filtered again and the new unit test
coverage should hopefully prevent further breakage in the future.

M  +30   -0    autotests/src/codecompletiontestmodel.cpp
M  +15   -0    autotests/src/codecompletiontestmodel.h
M  +14   -0    autotests/src/completion_test.cpp
M  +1    -0    autotests/src/completion_test.h
M  +2    -0    src/completion/katecompletionwidget.cpp

https://invent.kde.org/frameworks/ktexteditor/commit/b357a5db49d78323a37cf14d21208b6500f06ad1

Git commit d20f4ca529a32c31cb606232756a3ca1c3fecf93 by Christoph Cullmann.
Committed on 31/12/2021 at 19:13.
Pushed by cullmann into branch 'master'.

try to fix behavior for vimode on completion

M  +0    -7    src/completion/katecompletionwidget.cpp

https://invent.kde.org/frameworks/ktexteditor/commit/d20f4ca529a32c31cb606232756a3ca1c3fecf93