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.

Sign up for GitHub

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

Jump to bottom

x/tools/gopls: determine how frequently to reload packages with missing dependencies #36918

Closed
stamblerre opened this issue Jan 31, 2020 · 2 comments
Closed

x/tools/gopls: determine how frequently to reload packages with missing dependencies #36918

stamblerre opened this issue Jan 31, 2020 · 2 comments
Labels
FrozenDueToAge gopls Issues related to the Go language server, gopls. Thinking Tools This label describes issues relating to any tools in the x/tools repository.

Comments

@stamblerre
Copy link
Contributor

Currently, when a user's package has a dependency that we can't resolve, we try to reload the package on every key stroke. We take care not to re-check it unless the metadata has changed, but it's still expensive to call packages.Load on every keystroke.

I wonder if we might be able to somehow make use of the goimports scanning logic in gopls to do this in a better way. It's possible that we can't do better in GOPATH mode (or in another build system), but I'd think we might be able to do something better in module mode.

@heschik: Is this a reasonable idea, or should we just stick with the current approach?
@stamblerre stamblerre added this to the gopls/v1.0.0 milestone Jan 31, 2020
@gopherbot gopherbot added Tools This label describes issues relating to any tools in the x/tools repository. gopls Issues related to the Go language server, gopls. labels Jan 31, 2020
@gopherbot
Copy link

Change https://golang.org/cl/217157 mentions this issue: internal/lsp/cache: refactor functions that return PackageHandles

gopherbot pushed a commit to golang/tools that referenced this issue Feb 7, 2020
@stamblerre
internal/lsp/cache: refactor functions that return PackageHandles
6dcdf1d 
Now that we build PackageHandles in load, we can simplify a lot of our
logic for rebuilding them and reloading metadata.

The only possibly-significant change is the missing imports logic. Now
that we always create package handles in load, we don't have to do the
extra "sameSet" logic. If the package handle hasn't been invalidated
since we last built it, we will keep the benefits of caching it. Otherwise,
it will be rebuilt in load.

Updates golang/go#36918

Change-Id: Ieb45898d248501e3cebdb95c8b518149ba7498e5
Reviewed-on: https://go-review.googlesource.com/c/tools/+/217157
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
@stamblerre stamblerre removed this from the gopls/v1.0.0 milestone Jun 24, 2020
@stamblerre stamblerre mentioned this issue Aug 27, 2020
Closed
@stamblerre stamblerre added this to the gopls/unplanned milestone Oct 21, 2020
@stamblerre stamblerre moved this from Critical to Needs Triage in vscode-go: gopls by default Nov 13, 2020
@stamblerre
Copy link
Contributor Author

At this point, the missing dependencies logic is only used in gopls to determine what types of show message warnings we should show the user. Packages with missing dependencies are only reloaded if we expect their metadata to have changed (package name or import change).

vscode-go: gopls by default automation moved this from Needs Triage to Done Nov 13, 2020
@stamblerre stamblerre removed this from the gopls/vscode-go milestone Nov 13, 2020
@golang golang locked and limited conversation to collaborators Nov 13, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge gopls Issues related to the Go language server, gopls. Thinking Tools This label describes issues relating to any tools in the x/tools repository.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@stamblerre @gopherbot