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

Editor ui unresponsive for seconds when writing types #128

Open
Narretz opened this issue Jan 25, 2022 · 18 comments
Open

Editor ui unresponsive for seconds when writing types #128

Narretz opened this issue Jan 25, 2022 · 18 comments

Comments

@Narretz
Copy link

Narretz commented Jan 25, 2022

I've experienced unresponsiveness when writing types. It happened after switching to ST4, but that might just be coincidence. The other option would be that the project has grown or introduced weird types.

Anyway, whenever I write something like:
const thing = Record<string, string> = {'abc': 'def'}

As soon as I get to the type arg part, the editor ui freezes for a few seconds (it still records input, but it's delayed). Looking at the lsp-typescript logs, I assume it's because the server sends a metric ton of suggested types:

::  -> LSP-typescript textDocument/didChange: {'textDocument': {'uri': 'file:///<path>', 'version': 216}, 'contentChanges': [{'rangeLength': 0, 'range': {'start': {'line': 337, 'character': 31}, 'end': {'line': 337, 'character': 31}}, 'text': 's'}, {'rangeLength': 0, 'range': {'start': {'line': 337, 'character': 32}, 'end': {'line': 337, 'character': 32}}, 'text': 't'}, {'rangeLength': 0, 'range': {'start': {'line': 337, 'character': 33}, 'end': {'line': 337, 'character': 33}}, 'text': 'r'}, {'rangeLength': 0, 'range': {'start': {'line': 337, 'character': 34}, 'end': {'line': 337, 'character': 34}}, 'text': 'o'}, {'rangeLength': 0, 'range': {'start': {'line': 337, 'character': 35}, 'end': {'line': 337, 'character': 35}}, 'text': 'i'}, {'rangeLength': 0, 'range': {'start': {'line': 337, 'character': 36}, 'end': {'line': 337, 'character': 36}}, 'text': 'n'}, {'rangeLength': 0, 'range': {'start': {'line': 337, 'character': 37}, 'end': {'line': 337, 'character': 37}}, 'text': 'g'}]}
:: --> LSP-typescript typescript/inlayHints(55): {'textDocument': {'uri': 'file:///<path>'}}
:: <<< LSP-typescript 53: <params with 33598344 characters>
:: <<< LSP-typescript 49: {'signatures': [{'label': 'Record<K extends string | number | symbol, T>', 'parameters': [{'label': 'K extends string | number | symbol'}, {'label': 'T'}], 'documentation': {'kind': 'markdown', 'value': 'Construct a type with a set of properties K of type T'}}], 'activeParameter': 1, 'activeSignature': 0}
:: <<< LSP-typescript 51: {'signatures': [{'label': 'Record<K extends string | number | symbol, T>', 'parameters': [{'label': 'K extends string | number | symbol'}, {'label': 'T'}], 'documentation': {'kind': 'markdown', 'value': 'Construct a type with a set of properties K of type T'}}], 'activeParameter': 1, 'activeSignature': 0}
:: <<< LSP-typescript 48: {'inlayHints': []}
:: <-  LSP-typescript textDocument/publishDiagnostics:

Suspicious is LSP-typescript 53: <params with 33598344 characters>

If my hunch is correct, is there something that can be done about this?

Can I limit the number of suggestions the server sends in the first place? Or is it a problem with LSP's base handling of huge messages? Note that the log output itself is delayed, i.e. the ui becomes responsive again at around the same time the huge message from the language server is logged. That would suggest that it's a problem with LSP rather than Sublime Text itself.

@Narretz Narretz changed the title Editor ui unresponsive for seconds when creating types Editor ui unresponsive for seconds when writing types Jan 25, 2022
@rchl
Copy link
Member

rchl commented Jan 25, 2022

Yes, I've noticed the same in some files (type definition files) in some projects. The 33MB of data is what causes the freeze due to ST parsing the results and then passing to the ST's completions API.

The results are controlled by the underlying language server https://github.com/typescript-language-server/typescript-language-server/ or, if we go even deeper, by the typescript server API.

I don't see an immediate solution for that as if we'd limit the results it will be an arbitrary choice that can cause issues (missing completions).

@Narretz
Copy link
Author

Narretz commented Jan 25, 2022

Hm, that's unfortunate.

You wrote: The 33MB of data is what causes the freeze due to ST parsing the result

so it's not LSP parsing the results in the plugin process?

If I were to edit the typescript-language-server, where would I restrict the number of complete suggestions sent? Here:
https://github.com/typescript-language-server/typescript-language-server/blob/bd2a889a23f19aae1ac7fb902ed161f26009624d/src/lsp-server.ts#L497?

@rchl
Copy link
Member

rchl commented Jan 25, 2022

so it's not LSP parsing the results in the plugin process?

Plugin process is a separate process so it wouldn't result in freezes in the editor. It's either parsing of the completions in
https://github.com/sublimelsp/LSP/blob/ff2074a90c5635dfa93284bc7e4587f5547c3a9c/plugin/documents.py#L691-L731 or just passing those parsed completions to the ST api. I think the latter since the former is happening on the async thread (which could in some cases still cause blocking but less likely).

If I were to edit the typescript-language-server, where would I restrict the number of complete suggestions sent? Here:
typescript-language-server/typescript-language-server@bd2a889/src/lsp-server.ts#L497?

Yes.

@Narretz
Copy link
Author

Narretz commented Jan 25, 2022

Thanks a lot, I will make some tests and update here with my findings.

@Narretz
Copy link
Author

Narretz commented Jan 25, 2022

Here's what I found:

First, I uncommented this line https://github.com/sublimelsp/LSP/blob/ff2074a90c5635dfa93284bc7e4587f5547c3a9c/plugin/documents.py#L730 to stop completions from being sent to Sublime Text core.

The behavior stayed basically the same.

Then I commented out the lines where the data is transformed:
https://github.com/sublimelsp/LSP/blob/ff2074a90c5635dfa93284bc7e4587f5547c3a9c/plugin/documents.py#L719-L724

After that, it felt like I could type a bit more before the freezes started, but they didn't go away.

I then limited the completion size here: https://github.com/typescript-language-server/typescript-language-server/blob/bd2a889a23f19aae1ac7fb902ed161f26009624d/src/lsp-server.ts#L516

I realized that there are a ton of completions being sent - up to 60000. Limit 10000 didn't change a lot, 5000 was okay, and 2500 very usable.

So it looks like the bottleneck might actually be the json parsing?

And I need to find out why I get 60000 completions ...

edit: parsing the json definitely gets slow with huge responses (in seconds):

LSP: Content-Length 3252017
LSP: decode time 0.002498
LSP: parse time 0.106073

LSP: Content-Length 3257913
LSP: decode time 0.003009
LSP: parse time 0.031351

LSP: Content-Length 5157922
LSP: decode time 0.003830
LSP: parse time 0.119995

LSP: Content-Length 32692943
LSP: decode time 0.017769
LSP: parse time 0.408967

Half a second for 30MB is not great, but on its own it doesn't explain why the ui freezes for multiple seconds. Or why the UI freezes at all.

@rwols
Copy link
Member

rwols commented Jan 25, 2022

2500 is still way too much completions IMO. The server should do something more clever and sort by "best" 200, and set isIncomplete to true. I have a feeling the UI freezes because ST renders 2500 completion items upfront in its completion widget.

@rchl
Copy link
Member

rchl commented Jan 25, 2022

There is a ton of built-in types in global scope of JS context so depending on where you are editing, all might be provided.

Also TS exposes completions for all exports that exist in all NPM packages used by a project. It all adds up.

That said, in my project here I seem to be getting around 3K completions so not as much but I know I had some cases where there was a lot more.

@Narretz
Copy link
Author

Narretz commented Jan 26, 2022

In my case it's aws-sdk which contains 1000s of types.

The server should do something more clever and sort by "best" 200, and set isIncomplete to true.

That sounds good. A default limit of 1000 would probably also work, overridable of course.

@rchl
Copy link
Member

rchl commented Jan 26, 2022

The server should do something more clever and sort by "best" 200, and set isIncomplete to true.

That sounds good. A default limit of 1000 would probably also work, overridable of course.

That's heuristics and it would be very hard to know what's the "best".

The typescript itself is in the best position to know what's best so you could argue that it should be discussed in https://github.com/microsoft/TypeScript/issues

I'm not sure if isIncomplete=true would work though because if something is not provided in the initial set of "best" completions, I think that typing something that is not included would just close completions popup rather than fetching more.

@Narretz
Copy link
Author

Narretz commented Aug 18, 2022

fwiw I've solved the problem with the many types by using the new on_server_response_async api:

    def on_server_response_async(self, method: str, response: Response) -> None:
        if method == "textDocument/completion" and isinstance(response.result, dict) and response.result.get("items") :
            response.result["items"] = response.result["items"][0:1000]

@rchl
Copy link
Member

rchl commented Aug 18, 2022

I wouldn't call it "fixed" but if it works for you... :)

@michaelknoch
Copy link

can you elaborate on how to workaround the lags? I don't know where to apply the snippet. @Narretz

@Narretz
Copy link
Author

Narretz commented Aug 23, 2022

@michaelknoch

I used PackageResourceViewer to extract the LSP-typescript package and modify its plugin.py file by adding this to the LspTypescriptPlugin class:

    def on_server_response_async(self, method: str, response: Response) -> None:
        if method == "textDocument/completion" and isinstance(response.result, dict) and response.result.get("items") :
            response.result["items"] = response.result["items"][0:1000]

edit: and import Response at the top

from LSP.plugin.core.protocol import Point, Response

It limits the number of completions sent to the Sublime Completion API to 1000 items.
You could also limit the number of items in the typescript-language-server, but it will be overriden everytime you update LSP-Typescript.

With an extracted package, the override stays around (but you have to update it when the plugin.py changes). You can use OverrideReport to get notified when this happens. https://packagecontrol.io/packages/OverrideAudit

I wouldn't call it "fixed" but if it works for you... :)

I didn't call it "fixed" I called it "solved ;)

@rchl
Copy link
Member

rchl commented Aug 23, 2022

One has to be aware when using this workaround that this can filter out useful suggestions and confuse people.

Saying that user types the first character and typescript returns 3000 completions that we limit to 1000 then those 2000 extra completions will not show up as suggestions, even when typing following characters.

Only when canceling completions popup and triggering it manually again, it will then return new (hopefully smaller than 1000 completions) list.

@michaelknoch
Copy link

It didn't work at first because the type Response was missing. Adding Response to the existing import fixed it.

from LSP.plugin.core.protocol import Point, Response

I think this change dramatically improved performance and writing UX for me. So far I haven't missed any type suggestions. Are you guys sure this shouldn't land in master? @rchl

@rchl
Copy link
Member

rchl commented Aug 23, 2022

Unfortunately sacrificing correctness is not the correct way to do it in general.

@rchl
Copy link
Member

rchl commented Aug 23, 2022

The biggest bottleneck is in ST handling the completions that it was handed through its API. This is what is causing the editor to freeze. We should at least try to report it to ST and see if something can be done about it.

@TerminalFi
Copy link

The biggest bottleneck is in ST handling the completions that it was handed through its API. This is what is causing the editor to freeze. We should at least try to report it to ST and see if something can be done about it.

I wonder if VSCode freezes

I also
Wonder if ST could do progressive population of the completion list as it is queried or scrolled.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants