-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Move inlayHints LSP request to "resolve" or range pattern for perf #3394
Comments
Yeah, I feel pull-based range model will make the most sense. |
Why don't we make |
I imagine |
It's an open question what is the best way to synchronize such things: microsoft/vscode-languageserver-node#576 (comment) The obvious drawback of push-based model is that it doesn't work with viewport optimization |
Configuration has also moved to a pull based model (see microsoft/language-server-protocol#676). In general the LSP is evolving in that direction. |
Configuration is communicated in the opposite direction though, so it's not really a good example in this case :) |
True! |
Viewport optimization is quite questionable (if you mean sending requests only for hints withing the visible file range?), because the user scrolls the file and moves around quite frequently, it would be a hell to cache all this stuff and at the end of the day this will result into making more small requests instead of issuing one big. |
To not repeat myself, see
microsoft/vscode#86415 (comment) for
why I think range optimization is essential for good performane
…On Mon, 2 Mar 2020 at 14:13, Veetaha ***@***.***> wrote:
Viewport optimization is quite questionable (if you mean sending requests
only for hints withing the visible file range?), because the user scrolls
the file and moves around quite frequently, it would be a hell to cache all
this stuff and at the end of the day this will result into making more
small requests instead of issuing one big.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#3394>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AANB3M4KGTO5CYHTT3LIBNLRFOWIZANCNFSM4K7TG6QQ>
.
|
well, it's not essential, in a sense that modern CPUs are crazily fast and
can plow though almost any amount of work in no time. It is important
optimization, because it cuts O(N) work to O(1) work.
On Mon, 2 Mar 2020 at 14:21, Aleksey Kladov <aleksey.kladov@gmail.com>
wrote:
… To not repeat myself, see
microsoft/vscode#86415 (comment)
for why I think range optimization is essential for good performane
On Mon, 2 Mar 2020 at 14:13, Veetaha ***@***.***> wrote:
> Viewport optimization is quite questionable (if you mean sending requests
> only for hints withing the visible file range?), because the user scrolls
> the file and moves around quite frequently, it would be a hell to cache all
> this stuff and at the end of the day this will result into making more
> small requests instead of issuing one big.
>
> —
> You are receiving this because you commented.
> Reply to this email directly, view it on GitHub
> <#3394>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AANB3M4KGTO5CYHTT3LIBNLRFOWIZANCNFSM4K7TG6QQ>
> .
>
|
@kjeremy , I've looked a bit into codelens and I don't see how we can apply the |
For the range approach (instead of resolve) maybe we could use this as inspiration: https://github.com/microsoft/vscode/blob/f74e473238aca7b79c08be761d99a0232838ca4c/src/vs/editor/contrib/viewportSemanticTokens/viewportSemanticTokens.ts |
It may make sense to split the inlay hints LSP request into two:
inlayHints
which would compute where hints would go.inlayHints/resolve
which would compute the actual hints.I imagine this would work similar to
codeLens/resolve
which can dramatically speed up scrolling. We could also follow a viewport model like semantic token ranges.The text was updated successfully, but these errors were encountered: