sublimelsp / LSP-typescript

TypeScript, JavaScript support for Sublime LSP plugin
MIT License
135 stars 11 forks source link

Editor ui unresponsive for seconds when writing types #128

Open Narretz opened 2 years ago

Narretz commented 2 years ago

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.

rchl commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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

Narretz commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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

michaelknoch commented 2 years ago

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

Narretz commented 2 years ago

@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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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

rchl commented 2 years ago

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 commented 1 year ago

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.