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

Disable semantic highlighting in large JS/TS files #94321

Closed
mjbvz opened this issue Apr 2, 2020 · 7 comments
Closed

Disable semantic highlighting in large JS/TS files #94321

mjbvz opened this issue Apr 2, 2020 · 7 comments
Assignees
Milestone

Comments

@mjbvz
Copy link
Collaborator

mjbvz commented Apr 2, 2020

Bug
Semantic highlighting can be slow for large JS/TS files (such as checker.ts in the TypeScript project). This can also slow down other operations, such as getting completions.

This has been impacting the TypeScript team's work in their codebase

Feature Request
Disable semantic highlighting for any JS/TS files that are larger than 200kb or so.

/cc @RyanCavanaugh @DanielRosenwasser

@mjbvz mjbvz added this to the March 2020 milestone Apr 2, 2020
@alexdima
Copy link
Member

alexdima commented Apr 2, 2020

We don't have file size as a selector, we have scheme and language, etc. I don't know how to have a provider that registers itself only for files of a certain file size. cc @jrieken if you have any ideas?

@aeschli One workaround would be to not make the call to tsserver if the document is above a certain size and return "no semantic tokens" from the provider.

@mjbvz
Copy link
Collaborator Author

mjbvz commented Apr 2, 2020

Yes my proposal would be to add a document size check to the JS/TS semantic highlighting provider at the very start of provideDocumentSemanticTokens:

if (document.getText().length > 20000) {
    return null;
}

@mattacosta
Copy link
Contributor

Why is TypeScript not doing that check itself and disabling semantic highlighting on the server side? That way other, better performing extensions aren't hit with arbitrary limits. If the client really needs to, wouldn't it also be better to disable highlighting if it detects that an extension is taking more than x amount of time to complete?

@DanielRosenwasser
Copy link
Member

Why is TypeScript not doing that check itself and disabling semantic highlighting on the server side?

Because maybe the client isn't an editor and it doesn't matter if the request takes 5 seconds.

@RyanCavanaugh
Copy link
Member

RyanCavanaugh commented Apr 2, 2020

@mattacosta the delay is impacting all VS Code users who edit large JS/TS files (in reality for any language that supports SH and returns a large set of highlights in large documents), but shipping an update to TS Server would only fix the delay for people on newer versions of TS. We'd prefer that all VS Code users not encounter responsiveness delays in large JS/TS files

@mattacosta
Copy link
Contributor

Because maybe the client isn't an editor and it doesn't matter if the request takes 5 seconds.

@DanielRosenwasser My workaround is to send a "client kind" when initializing, which (for me) is normally used to change some results in "cli" mode, but could also be used to allow long requests.

@RyanCavanaugh Fair enough.

@alexdima
Copy link
Member

alexdima commented Apr 3, 2020

From the discussion in #92789, I think there is no need to change the vscode API, so this can be customized as desired from the TS extension.

@alexdima alexdima removed their assignment Apr 3, 2020
@aeschli aeschli closed this as completed in 677b3cf Apr 3, 2020
@github-actions github-actions bot locked and limited conversation to collaborators May 18, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants