x/tools/gopls: performance regression on monorepos with gopls@v0.15.0+ #66647
Comments
hitzhangjie
changed the title
|
I encountered the same problem recently, the response was extremely slow. |
|
Hi, is this issue describing a recent regression, or has this always been a problem? |
findleyr
added
gopls
|
To be more specific: do you experience this problem with gopls@v0.14.2? If so, then I think we can fix the regression. If not, there may still be things we can do to get gopls working better for you on your monorepo (albeit perhaps with some features being limited). |
The issue I encountered was caused by a problem with GOROOT in my environment, and it is now working normally. Sorry to cause any disturbance. |
findleyr
added
the
WaitingForInfo
|
Another thing you can do is share details about the gopls process when you perform one of these operations. Is gopls using a lot of CPU for the entire time you're waiting? How much memory does gopls use? This will help us confirm that the problem is indeed with gopls (most likely), and not something in the extension host. |
|
I don't want to hog someone else's (apparently resolved) issue but I have extreme monorepo performance issues with gopls, switching to gopls 0.14.2 resolved the issues with any later version, completion is very slow (multiple seconds to bring up imports after using Add Import, multiple seconds to bring up symbols after typing there doesn't seem to by any memory/cpu use difference between 0.14.2 and other versions, the only difference seems to be that there are multiple |
|
@zhangfannie, do you also observe that the problem goes away when switching back to v0.14? |
|
It sounds like @gordallott's issues are related to goimports changes in gopls@v0.15.0+. I think there's definitely something more going on here, so thank you @gordallott for chiming in. Can you share more about your workspace? Do you have multiple modules? Do you use go.work files? Also, can you share an example of the |
|
sure, single module for the entire repo, some git submodules but not to other go repos. no go.work files these if i run that go list cmd with time, (and without the overlay because that part errors with file not found), i get |
Sorry for the late reply, gopls@v0.14 can work in my environment as well. Thank you. |
|
I reinstall gopls@v0.14.2 instead of the latest v0.15.2, the problem gone. |
|
Hi @gordallott, @zhangfannie, and @hitzhangjie. I have been traveling, but am back now and this issue is my top priority: the changes in gopls@v0.15.0+ were not intended to affect performance for most users. There is an intentional performance hit if you are, for example, on linux and open a foo_windows.go file, because previously gopls would not be able to load the windows build, and now it can. But that should only happen if you have multiple logical builds open in your editor. It sounds like what you are encountering is an across-the-board regression in your repositories, which is almost certainly a bug. I am glad that downgrading allows you to continue working. However, since you are able to reproduce the regression, it would be enormously helpful if you are able to occasionally help me investigate over the next week, by trying out different versions of gopls. Quick question, before I dig in more: are you all on Windows? I see that @hitzhangjie is, but what about @gordallott and @zhangfannie? I have seen some similar sounding feedback in our recent survey results, almost all from windows users. |
|
hey no sorry, I'm on linux, I'm running my vscode (on linux) through a vscode remote to a lxc container where gopls runs host/container info the only other special case is that I'm running zfs as my filesystem - nothing special enabled in zfs configurations, not even encryption. No other performance problems in any other applications |
|
Thanks @gordallott. And just to check: is your zfs configured to be case sensitive? I think that's the default, but since it's very plausible that this bug could be related to case insensitive file systems, I wanted to confirm.
No, if gopls@v0.14.2 works well, then this is certainly a gopls bug. |
|
my zfs is indeed case sensitive |
findleyr
changed the title
|
Thanks. Since this is clearly a gopls issue, transferring to the Go issue tracker so that it can be included in the gopls@v0.15.3 milestone. I will get to the bottom of this. |
|
If someone experiencing these problems is able to run gopls@v0.15.2 with the following settings, and report whether they still experience performance issues, it would be helpful: settings.json |
|
Is I just updated |
|
@jacobmischka yes it should be a valid setting (with "zeroConfig" as casing) if you are using gopls@v0.15.0+. It is a hidden setting to disable the dynamic workspace heuristics added in v0.15.0, specifically so that we can do this type of investigation. The setting will be removed in future versions. |
|
It doesn't like it :/ I also tried |
|
We're trying to get the codegen split into more smaller files to see if that helps, will report back. |
|
Thanks very much @jacobmischka for following up until we understood your problem. I think I understand it now, and have filed it as #66699. I think it's probably unrelated to the regressions reported by others in this issue, so let's continue discussion on #66699. For now, I think if you move the codegen into a different package that you don't frequently work on, it should improve your editing experience significantly. Of course, it's not great for your tools to have to force you into a different package layout. |
performance of gopls @ v0.15.2 seems the same with or without this config, unaffected by it. Both the slowness on save and the symbol autocomplete are both as slow as without the config |
|
Hi folks, I've looked into potential performance regressions quite a bit and have some theories, though no obvious root cause (https://go.dev/cl/577695 may help). It would be very helpful if someone experiencing problems with gopls@v0.15.2 (but not gopls@v0.14.2) could do the following:
With this information, I'll be able to see which operations are slow and which are fast, and that will help me narrow down a culprit. Thanks. |
|
Change https://go.dev/cl/577695 mentions this issue: |
|
gopls v0.15.2 with trace info - copied after a slow symbol lookup, followed by a slow save |
|
@gordallott thanks, that's extremely useful. Unfortunately, it confirms that there's something I really don't understand here, and which won't be fixed by the CL above: didChange, didOpen, and didSave should all be extremely fast operations, yet from your trace I can see that they are not. Could you perhaps click on one and share the last and longest trace for lsp.Server.didSave, from the bottom half of that view? You may need to remove file paths from the trace (though I would like to know whether they are I may also ask you to test with a new gopls CL that includes additional information. |
|
@gordallott can you also visit http://localhost:8080/session/1? How many Views do you have, and what are their IDs? |
With gopls@v0.15.0, zero config gopls made it much more likely that sessions would have multiple Views. Additionally, with improved build tag support, it made it more likely that these Views would share files. As a result, we encountered (and fixed) this latent bug: 1. User changes file x.go, invalidating view A and B. A and B are scheduled for diagnosis. 2. User changes file y.go, invalidating only view B. Step (1) is cancelled and view B is scheduled for diagnosis. 3. View A never gets rediagnosed. The fix was naive: just mark view A and B as dirty, and schedule a goroutine to diagnose all dirty views after each step. As before, step (2) would cancel the context from step (1). But there's a problem: diagnoses were happening on the *Snapshot* context, not the operation context. Therefore, if the goroutines of step (1) and (2) both find the same snapshots, the diagnostics of step (1) would *not* be cancelled, and would be performed in addition to the diagnostics of (2). In other words, following a sequence of invalidations, we could theoretically be collecting diagnostics N times rather than 1 time. In practice, this is not so much of a problem for smaller repositories. Most of the time, changes are arriving at the speed of keystrokes, and diagnostics are being scheduled faster than we can type. However, on slower machines, or when there is more overall work being scheduled, or when changes arrive simultaneously (such as with a 'save all' command or branch switch), it is quite possible in practice to cause gopls to do more work than necessary, including redundant loads. I'm not sure if this is what conspires to cause the regressions described in golang/go#66647, but it certainly is a real regression. Fix this by threading the correct context into diagnoseSnapshot. Additionally, add earlier context cancellation in a few cases where redundant work was being performed despite a context cancellation. For golang/go#66647 Change-Id: I67da1c186848286ca7b6221330a655d23820fd5d Reviewed-on: https://go-review.googlesource.com/c/tools/+/577695 LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Alan Donovan <adonovan@google.com>
the file references just happen to be the file I had open at the time, same setup, tried to complete a symbol, tried to save, both slow. same deal if I am editing any file, the contents of the file doesn't seem to make any difference. (though using a different git repo, everything will be fast again)
is this go installable? the hash isn't as there's a replace directive that needs to be removed from the go.mod - i guess I could just build from source?
only one view, two overlays from the two files i have open |
Interesting that the file doesn't matter here. That's surprising, as for most files didSave should be extremely fast. It should be slow only if gopls thinks that build information may have changed (for example, because the file has a
If/when we need to test a fix or or debugging CL, I'll provide instructions for how to install it (indeed, because of the replace, you'll need to check out the commit and build from source).
I see the view ID is 5, which means it has recreated the view 5 times. If you do some slow operations, or try to use gopls for a minute, does the ID increment? That would explain why you keep seeing I'm digging into the control flow, and pending your response to the question about the View ID I'll put together a CL with much more instrumentation to help us understand why state changes are taking so long. Greatly appreciate your willingness to go back-and-forth here. |
|
Change https://go.dev/cl/578036 mentions this issue: |
|
@gordallott here's a one-liner to install the debugging CL above in #66647 (comment). Run it from a temp directory as it creates a "tools" subdirectory in which to shallow clone the CL and build gopls. Once you've done that, can you please redo the exercise above, where you perform some slow operations and then share the trace of the slowest lsp.Server.didSave operation listed at http://localhost:8080/trace/lsp.Server.didSave. (also, still curious to hear whether the ID of the view at http://localhost:8080/session/1 keeps incrementing) |
|
okey doke, grabbed the above CL branch, latest info repeating the same thing (in a different file, different package, same repo) view stuffafter restarting gopls, completing a symbol and saving after a few more completions and saving twice trace infolsp.Server.didSave spans |
|
@gordallott it looks like the slowness is caused by reinitializing your build over and over. That would perfectly explain the symptoms you observe. I noticed that gopls had received at least two didChangeConfiguration notifications from VS Code, because of the line below (last != longest). This stands out to me. In my sessions, I get no didChangeConfiguration notifications, and would expect at most one. In gopls@v0.15.0+, any change to configuration causes a reinitialization--this was a vast simplification, and was assumed not to be a significant performance hit since configuration changes should be infrequent. If you start gopls with If you do, then we are very close to a root cause: there is some process that causes VS Code to send repeated didChangeConfiguration notifications, and gopls@v0.14.2 detects these as "minor" changes, whereas gopls@v0.15.0 assumes that any configuration change could affect state and reinitializes the build. The fix is probably for gopls to be smarter and detect when configuration has not changed, and/or get VS Code to stop sending didChangeConfiguration notifications! If you don't see a lot of didChangeConfiguration notifications, then it's back to the drawing board (I have a few other ideas that we can narrow down). |
|
Change https://go.dev/cl/578037 mentions this issue: |
|
looks like a fair amount, just going to dump the lines (w/timestamps) that it's found in my output also want to note that prior to this issue I already tried the standard unload all my extensions other than go techniques. so it's unlikely that any other extensions might be causing this - just having the go extension enabled by itself is enough |
|
Thanks! This is very helpful. @findleyr tells me the CL above should be a fix for this issue. We should also report a VS Code bug that it is sending spurious config change notifications. |
|
Hi @gordallott, @zhangfannie, and @hitzhangjie, I'd be very curious to know whether the CL referenced below fixes the performance regression for you. This CL prevents gopls from reinitializing if configuration did not actually change following a didChangeConfiguration notification. This one liner should be run from a temp directory, and installs gopls with the proposed fix. (It shallow clones gopls at the correct commit and installs from source). @gordallott the change notifications in #66647 (comment) look like a VS Code or VS Code Go bug (CC @hyangah). VS Code should not be sending repeated didChangeConfiguration notifications. I wonder if this is an artifact of your setup somehow. |
|
Gentle ping: I believe the CL in #66647 (comment) should fix the regression in v0.15.0+. If it does, I'd like to cherry-pick it into a prerelease today, to be released next week. If it doesn't fix it, then perhaps configuration is actually changing in a meaningful way, which gopls@v0.14.2 would tolerate. That would be another source of curiosity. |
|
Can confirm that the above CL fixes the issue (or at least, is as perceptually fast as 1.14 is), excellent stuff |
In CL 538796, options were made immutable on the View, meaning any change to options required a new View. As a result, the minorOptionsChange heuristic, which avoided recreating views on certain options changes, was removed. Unfortunately, in golang/go#66647, it appears that certain clients may send frequent didChangeConfiguration notifications. Presumably the configuration is unchanged, and yet gopls still reinitializes the view. This may be a cause of significant performance regression for these clients. Fix this by making didChangeConfiguration a no op if nothing changed, using reflect.DeepEqual to compare Options. Since Hooks are not comparable due to the GofumptFormat func value, they are excluded from comparison. A subsequent CL will remove hooks altogether. For golang/go#66647 Change-Id: I280059953d6b128461bef1001da3034f89ba3226 Reviewed-on: https://go-review.googlesource.com/c/tools/+/578037 LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Auto-Submit: Robert Findley <rfindley@google.com> Reviewed-by: Alan Donovan <adonovan@google.com>
|
Change https://go.dev/cl/578040 mentions this issue: |
…if configuration did not change In CL 538796, options were made immutable on the View, meaning any change to options required a new View. As a result, the minorOptionsChange heuristic, which avoided recreating views on certain options changes, was removed. Unfortunately, in golang/go#66647, it appears that certain clients may send frequent didChangeConfiguration notifications. Presumably the configuration is unchanged, and yet gopls still reinitializes the view. This may be a cause of significant performance regression for these clients. Fix this by making didChangeConfiguration a no op if nothing changed, using reflect.DeepEqual to compare Options. Since Hooks are not comparable due to the GofumptFormat func value, they are excluded from comparison. A subsequent CL will remove hooks altogether. For golang/go#66647 Updates golang/go#66730 Change-Id: I280059953d6b128461bef1001da3034f89ba3226 Reviewed-on: https://go-review.googlesource.com/c/tools/+/578037 LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Auto-Submit: Robert Findley <rfindley@google.com> Reviewed-by: Alan Donovan <adonovan@google.com> (cherry picked from commit e388fff) Reviewed-on: https://go-review.googlesource.com/c/tools/+/578040
|
That's fantastic to hear. I suspect this was affecting a number of users. Really appreciate you going the extra mile to follow up, @gordallott. I've cut a prerelease with the fix (among others). Would be great if you confirm that it still fixes the problem for you: @zhangfannie @hitzhangjie I'd also be very interested if this fixes your problems as well. |
yup, works wonderfully, no issues |
Excellent. Then I'm going to mark this as fixed. I hope this is the same regression affecting @zhangfannie and @hitzhangjie. Thanks again for your help. |
|
(This was released as gopls@v0.15.3). |
What version of Go, VS Code & VS Code Go extension are you using?
Version Information
go versionto get version of Go from the VS Code integrated terminal.gopls -v versionto get version of Gopls from the VS Code integrated terminal.golang.org/x/tools/gopls v0.15.2
golang.org/x/tools/gopls@v0.15.2 h1:4JKt4inO8JaFW3l/Fh9X1k/5JQn+iUOpdc4/Lpi0mOs=
github.com/BurntSushi/toml@v1.2.1 h1:9F2/+DoOYIOksmaJFPw1tGFy1eDnIJXg+UHjuD8lTak=
github.com/google/go-cmp@v0.6.0 h1:ofyhxvXcZhMsU5ulbFiLKl/XBFqE1GSq7atu8tAmTRI=
golang.org/x/exp/typeparams@v0.0.0-20221212164502-fae10dda9338 h1:2O2DON6y3XMJiQRAS1UWU+54aec2uopH3x7MAiqGW6Y=
golang.org/x/mod@v0.15.0 h1:SernR4v+D55NyBH2QiEQrlBAnj1ECL6AGrA5+dPaMY8=
golang.org/x/sync@v0.6.0 h1:5BMeUDZ7vkXGfEr1x9B4bRcTH4lpkTkpdh0T/J+qjbQ=
golang.org/x/telemetry@v0.0.0-20240209200032-7b892fcb8a78 h1:vcVnuftN4J4UKLRcgetjzfU9FjjgXUUYUc3JhFplgV4=
golang.org/x/text@v0.14.0 h1:ScX5w1eTa3QqT8oi6+ziP7dTV1S2+ALU0bI+0zXKWiQ=
golang.org/x/tools@v0.18.1-0.20240311201521-78fbdeb61842 h1:No0LMXYFkp3j4oEsPdtY8LUQz33gu79Rm9DE+izMeGQ=
golang.org/x/vuln@v1.0.1 h1:KUas02EjQK5LTuIx1OylBQdKKZ9jeugs+HiqO5HormU=
honnef.co/go/tools@v0.4.6 h1:oFEHCKeID7to/3autwsWfnuv69j3NsfcXbvJKuIcep8=
mvdan.cc/gofumpt@v0.6.0 h1:G3QvahNDmpD+Aek/bNOLrFR2XC6ZAdo62dZu65gmwGo=
mvdan.cc/xurls/v2@v2.5.0 h1:lyBNOm8Wo71UknhUs4QTFUNNMyxy2JEIaKKo0RWOh+8=
go: go1.21.6
code -vorcode-insiders -vto get version of VS Code or VS Code Insiders.863d2581ecda6849923a2118d93a088b0745d9d6
x64
Run Ctrl+Shift+P (Cmd+Shift+P on Mac OS) >
Go: Locate Configured Go Toolscommand.Share the Go related settings you have added/edited
Run
Preferences: Open Settings (JSON)command to open your settings.json file.Share all the settings with the
go.or["go"]orgoplsprefixes."go.editorContextMenuCommands": {
"addImport": false,
"addTags": false,
"benchmarkAtCursor": true,
"debugTestAtCursor": false,
"fillStruct": true,
"playground": false,
"testAtCursor": false,
"testCoverage": false,
"testFile": true,
"testPackage": true,
"toggleTestFile": false
},
"go.formatTool": "default",
"go.goroot": "/usr/local/go",
"go.lintOnSave": "off",
"go.lintTool": "golangci-lint",
"go.testEnvFile": "",
"go.testEnvVars": {},
"go.testFlags": [
"-v",
"-count=1"
],
"go.toolsEnvVars": {},
"go.toolsManagement.autoUpdate": true,
"gopls": {
"formatting.local": "my repo module"
},
Describe the bug
My project is organized as monorepo including nearly 70 sub projects, its structure is as following:
myproject/
go.mod
go.sum
subproject1/ (no separate go.mod)
subproject2/ (no separate go.mod)
...
subprojectN/ (no separate go.mod)
vendor/
my development environment is:
When I do some refactor, or jump to the definition, documentation, auto completion, it responds very very very slowly, nearly more than 30 seconds.
Steps to reproduce the behavior:
I cannot share the big monorepo to you, but I can investigate by myself if you could give me some suggestions about how to do.
Thank you.
The text was updated successfully, but these errors were encountered: