-
Notifications
You must be signed in to change notification settings - Fork 89
significantly lower memory consumption for vscode-go users #192
Conversation
This means that users or devs running the language server via the vscode-go extension can easily capture pprof profiles. For example, to capture a heap profile and create an SVG from it: ```bash go tool pprof -svg $GOPATH/bin/go-langserver http://localhost:6060/debug/pprof/heap > heap.svg ``` Or a CPU profile: ```bash go tool pprof -svg $GOPATH/bin/go-langserver http://localhost:6060/debug/pprof/profile > cpu.svg ```
langserver/loader.go
Outdated
// TL;DR: 46% less memory consumption for users running with the vscode-go extension. | ||
// | ||
// [1] https://github.com/golang/go/issues/14735#issuecomment-194470114 | ||
func freeOSMemory() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This feels like something that should be done in main.go, not in the library code. Also, then you wouldn't need to pass the editor bool
all the way down, and you wouldn't need a sync.Once
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, that's true. The reason I did it this way is to keep it close to the problem (type-checking) and because I plan to use the editor
bool in this same location in another change soon.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome, great find!
langserver/loader.go
Outdated
// can give our servers plenty of RAM and allow Go to consume as much as it | ||
// wants. Go does reuse the memory not free'd to the OS, and as such enabling | ||
// this does _technically_ make our application perform less optimally -- but | ||
// in practice this has no observable affect in editor mode. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
typo: affect -> effect
langserver/loader.go
Outdated
// | ||
// | Real Before | Real After | Real Change | Go Before | Go After | Go Change | | ||
// |-------------|------------|-------------|-----------|----------|-----------| | ||
// | 7.61GB | 4.12GB | -45.86% | 3.92GB | 3.33GB | -15.05% | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After this change, does the real memory usage continue to grow, just at a slower rate? Or does it plateau at some amount (4.12 GB here)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great question.
I have observed that both before this change and after this change, the memory does plateau at a certain amount (8.3GB before this change, and 7.22GB after this change) and does not continue to grow. That amount can just be quite large for projects with many dependencies.
The amount appears to be 2x the currently in-use Go heap size (~3.3GB), which makes sense. It's not clear to me why Go can only release 1GB of the ~4 GB difference in unused memory, but I speculate it could be due to issues like golang/go#14045 or golang/go#14735 since our allocations all come in large spikes due to type-checking.
If we can make type-checking faster more fine-grained caching, then we can potentially spare ourselves keeping a ~3.3GB Go heap in-memory all the time and lower this number even further (in addition to performing less type-checking on-the-fly which is most of our allocations). This will be a much bigger win for us, so I'm working on this.
I don't like the concept of an opt-in "editor mode" for language servers or anything that makes the language server behave differently for specific clients. Wouldn't a specific flag do the job too (to enable/disable the aggressive memory freeing, preferably with the default being enabled)? |
@felixfbecker Understood. I have good reasons outside of this PR for why we might want such a mode, but let's punt on that discussion for now / wait until I have a provable need in a separate PR :) In the morning I'll update this PR as you suggest and replace |
… memory consumption) To try this change out: 1. `git checkout sg/mem` to the exact commit. 2. `go install github.com/sourcegraph/go-langserver` 3. Open some Go code and hover over a symbol, then make an edit, then hover, repeat... (each edit and hover triggers type-checking again). 4. You can also set `"go.languageServerFlags": ["-freeosmemory=false"]` in VS Code settings to try without this change (to compare memory usage). With `code github.com/docker/docker/cmd/dockerd/docker.go`, and making 10 edits/hovers, the change is: | Real Before | Real After | Real Change | Go Before | Go After | Go Change | |-------------|------------|-------------|-----------|----------|-----------| | 7.61GB | 4.12GB | -45.86% | 3.92GB | 3.33GB | -15.05% | Where `Real` means real memory reported by OS X Activity Monitor, and `Go` means memory reported by Go as being in use. TL;DR: 46% less memory consumption for users running with the vscode-go extension.
-freeosmemory=true
flag, which when set to false can be used to disable this behavior for testing purposes (we'll also probably disable on sourcegraph.com).Significantly helps #178 (probably fixes entirely, but I have more improvements coming soon that I believe will lower memory consumption even further :) ).