Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

LSP always return unrelated items and too many #42152

Closed
iamcco opened this issue Jun 2, 2020 · 14 comments
Closed

LSP always return unrelated items and too many #42152

iamcco opened this issue Jun 2, 2020 · 14 comments
Labels
analyzer-completion Issues with the analysis server's code completion feature analyzer-server area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion.

Comments

@iamcco
Copy link
Contributor

iamcco commented Jun 2, 2020

This tracker is for issues related to:

  • Analyzer

Info:

  • Flutter version: Flutter 1.17.2 • channel stable • https://github.com/flutter/flutter.git
  • Dart SDK Version (dart --version): Dart VM version: 2.8.3 (stable) (Tue May 26 18:39:38 2020 +0200) on "macos_x64"
  • Uusing MacOSX: 10.15.4

Step to repeoduce issue:

Create flutter project with flutter create xxxname

  • open lib/main.dart add print$ and $ is cursor, then press <ctrl>+<space> to trigger autocomplete
import 'package:flutter/material.dart';

void main() {
  runApp(MyApp());
  print$
}
....

Then checkout the Dart LSP output channel:

[Trace - 10:51:54 AM] Sending request 'textDocument/completion - (12)'.
Params: {
    "textDocument": {
        "uri": "file:///Users/aioiyuuko/development/followme/lib/main.dart"
    },
    "position": {
        "line": 4,
        "character": 7
    },
    "context": {
        "triggerKind": 1
    }
}


[Trace - 10:51:54 AM] Received response 'textDocument/completion - (12)' in 458ms.
Result: [
    {
        "label": "assert",
        "kind": 14,
        "sortText": "998945",
        "insertText": "assert${1:}",
        "insertTextFormat": 2,
        "textEdit": {
            "range": {
                "start": {
                    "line": 4,
                    "character": 2
                },
                "end": {
                    "line": 4,
                    "character": 7
                }
            },
            "newText": "assert${1:}"
        }
    },
    ......hide 26771 items here
    {
        "label": "Int32()",
        "kind": 4,
        "detail": "() → Int32",
        "sortText": "999997",
        "filterText": "Int32",
        "insertText": "Int32",
        "data": {
            "file": "/Users/aioiyuuko/development/followme/lib/main.dart",
            "offset": 81,
            "libId": 715,
            "displayUri": "dart:wasm",
            "rOffset": 76,
            "rLength": 5
        }
    }

]

The response items (26773 items) are too many, and most of them are unrelated items. This lag autocomplete when coding.

the full response complete items log is here
output.log 10 MB

@vsmenon vsmenon added the area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. label Jun 2, 2020
@vsmenon
Copy link
Member

vsmenon commented Jun 2, 2020

@devoncarew - should this be filed on an lsp component instead?

@bwilkerson
Copy link
Member

@DanTup

@DanTup
Copy link
Collaborator

DanTup commented Jun 2, 2020

There are two reasons for the large payload:

  1. The suggestFromUnimportedLibraries setting defaults to on, so completion includes symbols that aren't yet imported. Setting this to false (in iniaitializationOptions) will prevent that and drop the size of the payload significantly
  2. The LSP spec is not clear about the behaviour of IsIncomplete=false so it's not clear if we can filter based on the prefix. I've opened an issue at What's the expected behaviour of CompletionList.isIncomplete=false when the user presses backspace? microsoft/language-server-protocol#954 but haven't heard back yet. If the response is that we can filter on the prefix (and the client should re-request completions if the user backspaces), then we can apply additional filtering there.

@iamcco out of interest, which client and version are you using?

@iamcco
Copy link
Contributor Author

iamcco commented Jun 2, 2020

@DanTup Thanks for the info and I use coc.nvim. I will test option suggestFromUnimportedLibraries.

@iamcco
Copy link
Contributor Author

iamcco commented Jun 3, 2020

option suggestFromUnimportedLibraries: false does reduce response items.

@DanTup
Copy link
Collaborator

DanTup commented Jun 4, 2020

@iamcco what sort of times are you seeing with/without that option?

Depending on the response to microsoft/language-server-protocol#954 we might be able to reduce the payload for some cases (like this one where we could filter by the prefix).

When the payload is large, what's the impact in your editor? Is it just slow to show the completion list, or does it hang the editor (eg. if its single threaded and deserialising the JSON)?

The LSP spec has support for partial results which allows a server to deliver results in small batches. We could potentially send two batches (one for in-scope symbols and one for the auto-import symbols), however that would only speed up the initial completion list and not reduce the deserialisation costs (so if your editor is hanging during deserialisation that might not help). I haven't started any work on supporting that as I don't know of any clients that support it currently.

@iamcco
Copy link
Contributor Author

iamcco commented Jun 4, 2020

  • With option false the first time it take 1771 ms to return complete items, normal is 4xxx-5xx ms.
  • With option true the first time take 3030 ms to return compete items, normal is 4xxx-6xxx ms, few times are 17xx ms.

With all completeItems (option true) it slow down to show the autocomplete menu. Also a little slow down the client filter when forward typing.

My solution now is use the middleware to filter completeItems with the prefix character to reduce the result, so it will not slow down the client filter when typing.

@DanTup
Copy link
Collaborator

DanTup commented Jun 4, 2020

My solution now is use the middleware to filter completeItems with the prefix character to reduce the result, so it will not slow down the client filter when typing.

This is why I was hoping for clarification on microsoft/language-server-protocol#954 - we might be able to do that in the server.

I just did some testing in VS Code, and I found:

  • If you type a single character to trigger completion, then hit backspace, the completion widget closes
  • If you type several characters to trigger completion, then backspace - the completion widget does not go back to the server if the original completion request was made before you'd typed the character you deleted (eg. it can be assumed the server had already included all relevant completions)
  • If you invoke backspace with an existing prefix and then backspace (such that you now have a shorter prefix than was sent to the server), then it does re-call the server

So assuming the spec would be updated to use VS Code's behaviour as a reference, I believe it may be safe for the server to filter on the prefix (though we may lose some of the fuzzy matching that VS Code does).

@iamcco
Copy link
Contributor Author

iamcco commented Jun 4, 2020

Thanks for the testing.

@bwilkerson
Copy link
Member

@DanTup FYI: We have fuzzy matching filtering support in server for another client. I suspect that the matching it uses is close enough to VS Code's matching to allow us to re-use it.

@DanTup
Copy link
Collaborator

DanTup commented Jun 4, 2020

Great! In that case if I can get some confirmation from Code or LSP that the behaviour described above is deliberate and can be made explicit in the spec, we should be able to make some significant gains here (and when using LSP for VS Code, could also significantly improve LiveShare) without any loss in usability :-)

dart-bot pushed a commit that referenced this issue Jun 11, 2020
Bug: #42152.
Bug: #40026.
Change-Id: I4c921ddff3224d736c4236861d0ff156f58be595
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/150628
Commit-Queue: Danny Tuppeny <danny@tuppeny.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
@srawlins srawlins added analyzer-server analyzer-completion Issues with the analysis server's code completion feature labels Jun 16, 2020
@DanTup
Copy link
Collaborator

DanTup commented Jun 29, 2020

The latest dev versions of the Dart SDK and the Flutter master channel have the changes detailed above which should significantly reduce the number of completion items returned by filtering them based on the prefix to the left of the caret (see #40026 (comment)). If you're able to try this out, please let me know if you still see cases with huge payloads. Thanks!

@iamcco
Copy link
Contributor Author

iamcco commented Jun 30, 2020

I have test with version 2.9.0-19.0.dev and it wokrs great! Thank you!!

@iamcco iamcco closed this as completed Jun 30, 2020
@DanTup
Copy link
Collaborator

DanTup commented Jun 30, 2020

Great! Thanks for confirming 🙂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
analyzer-completion Issues with the analysis server's code completion feature analyzer-server area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion.
Projects
None yet
Development

No branches or pull requests

5 participants