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

x/tools/gopls: slow completions from github.com/aws/aws-sdk-go/service/ec2 #37450

Open
heschik opened this issue Feb 25, 2020 · 3 comments
Open

x/tools/gopls: slow completions from github.com/aws/aws-sdk-go/service/ec2 #37450

heschik opened this issue Feb 25, 2020 · 3 comments
Labels
Milestone

Comments

@heschik
Copy link
Contributor

@heschik heschik commented Feb 25, 2020

Completing out of the ec2 package is horribly slow, apparently because astutil.PathEnclosingInterval doesn't scale. We could add indexes, or stop returning results after the first few hundred.

profile001

@muirdm @stamblerre

@heschik heschik added this to the gopls/v1.0.0 milestone Feb 25, 2020
@bmurphy1976

This comment has been minimized.

Copy link

@bmurphy1976 bmurphy1976 commented Feb 25, 2020

Awesome! I was just coming here to report the same thing (although with much less detail). I thought my VSCode intellisense was broken, but after debugging it I came to find it was simply taking 12 seconds to load the data for the ec2.EC2 type:

[Trace - 17:04:53.416 PM] Received response 'textDocument/completion - (16)' in 12041ms.
@heschik heschik modified the milestones: gopls/v1.0.0, gopls/v0.4.0 Feb 26, 2020
@gopherbot

This comment has been minimized.

Copy link

@gopherbot gopherbot commented Feb 26, 2020

Change https://golang.org/cl/221021 mentions this issue: internal/lsp/source: speed up completion candidate formatting

@muirdm

This comment has been minimized.

Copy link

@muirdm muirdm commented Feb 26, 2020

The proof-of-concept CL above helps a lot, but it is still too slow if you have completion documentation enabled. We can probably speed that up too using the same cache-types.Object-to-ast.Node approach, but I want to hear what others think.

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

Successfully merging a pull request may close this issue.

None yet
4 participants
You can’t perform that action at this time.