Skip to content
This repository was archived by the owner on Oct 12, 2022. It is now read-only.

enable godef-based textDocument/definition implementation #195

Merged
merged 30 commits into from
Jun 22, 2017
Merged

Conversation

emidoots
Copy link
Member

  • The goal of this change is to help High memory & CPU usage after running for awhile #178 and Language Server stops working after a while microsoft/vscode-go#853 by reducing CPU usage consumed by our typechecking operations.
  • Godef internally uses the $GOPATH/pkg binary .a files to resolve the definition location of symbols, meaning it does not have to typecheck the entire program in order to perform a textDocument/definition request.
  • The approach used by this change is taking the godef tool as a base, converting it into a usable Go package that we can import, and enabling this feature under a flag. In practice this uses almost no CPU in comparison to our old approach.
  • Additionally included in this change is a flag which lowers the default parallelism used by workspace/symbol.

Note: This change on it's own still is not good enough to close either of those issues, as hover still uses a type-checking-based implementation and thus hovers still use far too much CPU resources. I am working currently on a new hover implementation that will also use godef / avoid type-checking.

Note: Both of these features will be disabled on Sourcegraph.com, as we cannot use $GOPATH/pkg there and want workspace/symbol to use as much CPU as possible there. In the future, we can consolidate these two code paths further.

Stephen Gutekanst added 29 commits June 14, 2017 18:28
…improves performance)

This change adds a `-usebinarypkgcache=true` flag which uses `$GOPATH/pkg` binary `.a`
files in order to improve the performance of `textDocument/definition` requests. In short,
we no longer require typechecking to perform these requests which saves us several hundreds
of MB of memory _and_, more importantly, a lot of CPU resources.

Because this uses .a files from disk, this option will be disabled on sourcegraph.com where
only the VFS is used. In the future, however, we can and should consider consolidating these
two code paths where possible.

Our existing test suite has proven very useful during development in ensuring this
implementation is at least on par with our current one, so there is no concern of this being
sub-par or broken in subtle ways as far as I see.
This change adds a `-maxparallelism` flag which defaults to 1/2 the NumCPU
on the machine. It is used to control the language server's use of
parallelism in editor environments where using all available resources to
answer a query is not most ideal.
Copy link
Member

@keegancsmith keegancsmith left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is great! I'd be very keen to make it the default way if we can work out a nice way for storing the .a files on the server.

@emidoots emidoots merged commit 0f37f10 into master Jun 22, 2017
@emidoots emidoots deleted the sg/perf2 branch June 22, 2017 19:20
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants