-
Notifications
You must be signed in to change notification settings - Fork 17.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
x/tools/gopls: long initial workspace load durations for workspace with many packages #69523
Comments
Thanks for filing this thoughtful issue. I've wanted the ability to bypass the initial workspace load for a while. This is in line with the work in https://go.dev/issue/68002, which I'm working my way up to now. I expect this to be doable, though there will almost certainly be many edge cases to take care of (for example: producing reasonable errors for workspace-wide misconfiguration). |
So, I "got this working" today, in the sense that I removed the guardrails preventing gopls from working prior to the initial workspace load. In principle, I think the guardrails are misguided anyway: we don't enforce the same restrictions when reloading a workspace (for example, after making a go.mod change), so there's really no reason we can't allow gopls to function prior to the initial workspace load. However, the change actually poses a significant challenge for testing, since we seek the completion of the initial workspace load in many places as a signal that gopls is ready to be used. Now there is no such signal. @rma-stripe I'm curious if patching the following CL and installing gopls works well for you. That would at least be a signal that we're on the right track.
That's surprising to me. Can you say more? I'd expect the post-processing of packages should be a minor cost. Certainly gopls type checking those packages can take a while, but that is not considered to be part of the initial workspace load. |
In general the patch works well. I was able to visually time when the initial workspace load completes by looking for the Prior to the initial workspace load's completion, features like jump-to-definition and definition-on-highlight work. Some other features, like "Find all references" are blocked on the initial workspace load completion, which makes sense, as we can't find all references to a symbol without introspecting the whole workspace. An issue I was only able to repro once: By repeatedly opening not-yet-loaded packages by hand (maybe one every 5 seconds) prior to the initial workspace load's completion, I was able to get gopls in a stalled/jammed state where it would not create Were this patch upstreamed, we would consider it a significant improvement over the current behavior of gopls in our custom-patched version.
I don't fully remember what this was about, sorry. We did some work prior to filing this issue trying to figure out what parts of IWL are slow, but the details of those experiments have since been deleted by our Slack retention policy. |
What could have happened there is this: if you ran a find-references request, it would have blocked other requests, and would itself have awaited initialization. Does that sound plausible? If we were to land this CL, we'd also want to allow navigation to be handled asynchronously to workspace-wide requests. The good news is that we recently added a mechanism for such concurrency. If we're going to land this, we need to do approximately the following:
This is a non-trivial amount of work -- at least several days work to do it properly. However, I do think it's worth doing as it can really improve the experience of using gopls on large codebases. |
Investigating code paths that await initialization, there are a number of perhaps surprising conditions to sort out. Notably, if you open a file outside a workspace folder, and ask for file-oriented information about that file, gopls currently awaits initialization of each workspace folder, because it has to see which build that file most logically "belongs" to. I suspect this is what occurred to get @rma-stripe's session into a blocked state. This mechanism is particularly gnarly, because we really don't want to allow other messages to proceed until we've resolved the correct build for that file request, because we need to acquire the Snapshot (a logical point in time) before we allow other state-affecting notifications to be processed. |
I think it was likely some other edge case. If this were true, you could repro by:
However, the very last part of Step 3 ("observe that the symbol definition doesn't appear") doesn't hold true, because opening a new file in VSCode causes the find-references context to cancel:
So long as the context cancelation for the workspace-wide req reliably occurs, the IDE shouldn't get into such a jammed state. Maybe that allows for landing the changeset without blocking it on "allow[ing] navigation to be handled asynchronously to workspace-wide requests", maybe not? |
Hello there!
Mostly the huge CPU usage comes after gopls will be triggered with a new file save (and fixing the indentations) or something else. "go clean -modcache" didn't help.
|
(this is not going to make v0.17. as the fix is too risky, but I plan to work on it soon after the release) |
gopls version
go env
What did you do?
My organization within the company is responsible for maintaining a large Go monorepo (~20k packages). In this monorepo, all packages fall under 1 Go module.
For the most part, we run a fairly off-the-shelf gopls setup without any customization of note. This is what our stats look like (from
bea7373d8a8268c2e3a260c1b8d41f96c4f7489e
):What did you see happen?
The experience of using gopls is good (i.e. rpcs like jump-to-definition, refactor symbol, etc. are fast) once initial workspace load has occurred, but initial workspace loads are a poor experience for our users. This poor experience is owed to two behaviors:
Firstly, initial workspace loads (IWLs) are slow. This is partially owed to the performance of the expensive upfront
go list
call itself:…but the majority of the time is spent on calling
packages.Load
on all of the packages thatgo list
returns.Secondly, because IWLs are blocking and VSCode waits on code actions before the IDE is able to save files, users are unable to save files/exit their IDEs/etc:
…while IWL is incomplete. This is partially a corollary of the first issue. While it is true that making code actions non-blocking for saves would fix this UX issue, if IWL were fast, this UX issue would not be perceived by users in the first place.
We have dipped our toes in trying to patch this behavior. For example, an early attempt at hacking around this behavior (on a
v1.16.1
base) no-ops the initialization routine altogether, relying solely on gopls loading in package data as files are manually opened by the user. As expected, this hack has some large tradeoffs associated with it, because initialization is a load bearing part of gopls: it breaks features like "Rename symbol" and "Find all references", and it occasionally causes the wrong imports to be pulled in. However, buggy as it is, this hack in essence describes what we think the desirable IWL behavior would be:go list
and lazy loads package data into the currently "active" snapshotWhat did you expect to see?
This issue is a feature request.
We would appreciate any tips/guidance in writing an upstream-able patch, or a better hack.
Editor and settings
No response
Logs
No response
The text was updated successfully, but these errors were encountered: