-
Notifications
You must be signed in to change notification settings - Fork 17.8k
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
x/tools/gopls: initial semantic token request slow #47465
Comments
The semantic token code will handle identifiers without any type information, so presumably it could be made to work if it were called before type checking gets done. I think of semantic tokens as being supplementary to the IDE's syntax marking, and much of the supplementary information is from types. When you ask for semantic tokens, has your editor already done syntax marking, or do the semantic tokens supply all the syntax marking? |
The intent is to have gopls as the (sole) source of truth for syntax marking. From what I've seen so far it provides most of the old syntax highlighting, as well as things that the old one can't provide. Even though LSP writes about "additional coloring" it would be really useful if gopls could provide "fast-enough" full syntax marking. With plugins like tree-sitter becoming increasingly popular there is a demand for something fast and lightweight. |
@leitzler: Does registering the tokens in initialization (instead of dynamically) improve the situation at all? You can patch in this CL to try it out: https://golang.org/cl/340478. |
Thanks for looking in to this @stamblerre. I tried that CL on top of d529aec and compared to d529aec now (this is on a slower machine, MBPr 2016, than what I used when I wrote the issue). Just opened up w/o CL 340478 just grep'ing the semTok request:
with CL 340478:
Doesn't seem to do any difference. |
Thanks for trying it out! I guess then that's just getting type information that's blocking it then. @pjweinb has https://golang.org/cl/339772 out right now, but I don't think it would fix the problem because we will still block on loading the file. I'm not exactly sure how we want to move forward here yet, since the type information enables us to provide better syntax highlighting in general, and most other clients already have good syntax highlighting so |
I think the original idea, that the time is going to type checking, is
correct. In gopls the semantic token request comes after several other
requests, and at least in my client, i don't see the slow response. for
tsprotocol.go (150KB, which would be too big, but I turned off that
checking), the RPC takes 93ms of which 12.6 ms are in the semantic token
processing. But it doesn't happen until about 2 seconds after
initialization, and after initial workspace load.
…On Sun, Aug 8, 2021 at 3:27 PM Pontus Leitzler ***@***.***> wrote:
Thanks for looking in to this @stamblerre <https://github.com/stamblerre>.
I tried that CL on top of d529aec and compared to d529aec now (this is on
a slower machine, MBPr 2016, than what I used when I wrote the issue). Just
opened up internal/lsp/semantic.go and restarted a few times:
w/o CL 340478 just grep'ing the semTok request:
[Trace - 21:19:31.886 PM] Sending request 'textDocument/semanticTokens/range - (2)'.
[Trace - 21:19:35.631 PM] Received response 'textDocument/semanticTokens/range - (2)' in 3744ms.
[Trace - 21:19:42.580 PM] Sending request 'textDocument/semanticTokens/range - (2)'.
[Trace - 21:19:46.372 PM] Received response 'textDocument/semanticTokens/range - (2)' in 3792ms.
[Trace - 21:19:51.799 PM] Sending request 'textDocument/semanticTokens/range - (2)'.
[Trace - 21:19:55.582 PM] Received response 'textDocument/semanticTokens/range - (2)' in 3782ms.
[Trace - 21:20:00.557 PM] Sending request 'textDocument/semanticTokens/range - (2)'.
[Trace - 21:20:04.502 PM] Received response 'textDocument/semanticTokens/range - (2)' in 3944ms.
[Trace - 21:20:09.771 PM] Sending request 'textDocument/semanticTokens/range - (2)'.
[Trace - 21:20:13.732 PM] Received response 'textDocument/semanticTokens/range - (2)' in 3960ms.
[Trace - 21:20:19.162 PM] Sending request 'textDocument/semanticTokens/range - (2)'.
[Trace - 21:20:23.137 PM] Received response 'textDocument/semanticTokens/range - (2)' in 3975ms.
with CL 340478:
[Trace - 21:21:05.430 PM] Sending request 'textDocument/semanticTokens/range - (2)'.
[Trace - 21:21:09.325 PM] Received response 'textDocument/semanticTokens/range - (2)' in 3894ms.
[Trace - 21:21:14.927 PM] Sending request 'textDocument/semanticTokens/range - (2)'.
[Trace - 21:21:18.816 PM] Received response 'textDocument/semanticTokens/range - (2)' in 3889ms.
[Trace - 21:21:24.381 PM] Sending request 'textDocument/semanticTokens/range - (2)'.
[Trace - 21:21:28.207 PM] Received response 'textDocument/semanticTokens/range - (2)' in 3825ms.
[Trace - 21:21:34.599 PM] Sending request 'textDocument/semanticTokens/range - (2)'.
[Trace - 21:21:38.467 PM] Received response 'textDocument/semanticTokens/range - (2)' in 3868ms.
[Trace - 21:21:47.691 PM] Sending request 'textDocument/semanticTokens/range - (2)'.
[Trace - 21:21:51.500 PM] Received response 'textDocument/semanticTokens/range - (2)' in 3809ms.
Doesn't seem to do any difference.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#47465 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABJIAI6A2BFER4EI5YWPFADT33LD7ANCNFSM5BHKA4DQ>
.
Triage notifications on the go with GitHub Mobile for iOS
<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
or Android
<https://play.google.com/store/apps/details?id=com.github.android&utm_campaign=notification-email>
.
|
I think we need to validate that semantic tokens doesn't block on the initial workspace load, because that should be asynchronous, and semantic tokens should be doing a |
Yeah, it seems like it is the package loading / getting type information that is blocking. I don't know in detail how it works so bare with me, but this is how I understand it. The client doesn't really do anything that require type information before the initial semantic token request, so I.e.:
|
While I agree that it might be nice to support semantic tokens without type information, it would add more complexity (type information is almost always available, except on startup and when an import changes), and that might make it look like semantic tokens are flickering. Since other clients already have a baseline implementation for semantic tokens, this doesn't seem like something we will be able to prioritize. |
What version of Go are you using (
go version
)?Does this issue reproduce with the latest release?
Yes
What did you do?
Start up a new instance of a LSP client that requests semantic tokens right away (opening
internal/lsp/semantic.go
inx/tools
@ 6e9046b).The request is a ranged request for the first 33 lines.
What did you expect to see?
A swifter response on the semantic token request.
What did you see instead?
The semantic token request takes ~1.7s:
(full log: https://gist.github.com/leitzler/d50361de1e196bebf074822062f98990)
If I understand it correctly
gopls
currently require full type check before responding to semantic token requests, and that is why it is so much slower on the initial request.Would it be possible to instead take on an iterative approach and first parse AST and deliver results basted on that, and then when type checking is done provide the rest? For example via
SemanticTokensPartialResult
or usingworkspace/semanticTokens/refresh
when type checking is done.The text was updated successfully, but these errors were encountered: