Skip to content
This repository has been archived by the owner on Jul 15, 2023. It is now read-only.

buildOnSave concurrency should be limited #1890

Closed
mgood opened this issue Aug 29, 2018 · 4 comments
Closed

buildOnSave concurrency should be limited #1890

mgood opened this issue Aug 29, 2018 · 4 comments

Comments

@mgood
Copy link

mgood commented Aug 29, 2018

vscode-go version: 0.6.88

VSCode recently brought my system to a crawl after doing a find/replace that affected a number of files in a large Go project.

When I was able to get my terminal to respond I found a large number of process running like:

go build -i -o /var/folders/nf/XXXXX/T/go-code-check.164325259 my/packages/here

I had not changed go.buildOnSave, so it would have used the default of package. It appears that since I had saved files from a number of packages simultaneously, this triggered a large number of calls to build those packages which were all running at the same time.

Looking at the code there seem to be two code paths in goBuild that would be affected by this in slightly different ways. When buildWorkspace is true it appears to scan for all packages and then launch the go tool in parallel for all of them. Otherwise for the package case it still appears that goBuild can be called many times concurrently for each file that was saved, leading to a large number of running go processes.

On my system this caused the go processes to use all the available memory. Since they were constantly swapping and competing for resources it took a long time for the system to complete any of the builds.

The process of building the workspace also appears like it will be doing redundant work by invoking go build separately for each package. Typically Go could analyze the package tree to only compile each package once, but by calling it in separate commands this could lead to Go rebuilding dependencies that are shared amongst other packages. The go build command can take a list of packages, so it seems like this should pass all the packages to one invocation of go build.

@ramya-rao-a
Copy link
Contributor

The go build command can take a list of packages, so it seems like this should pass all the packages to one invocation of go build.

Thanks for bringing this up. Yes, we should pass the list of packages instead of calling go build on each package

it still appears that goBuild can be called many times concurrently for each file that was saved

We can skip building the package when it does not correspond to the file that is open in the current active editor.

PRs are very much welcome.
Would you like to give this a try, since you have already figured out the workings of goBuild.ts?

@ramya-rao-a
Copy link
Contributor

@mgood If you are working on this, wait a few days till October starts before submitting the PR to win a t-shirt! See https://open.microsoft.com/2018/09/18/hacktoberfest-2018-microsoft/?WT.mc_id=hacktoberfest-twitter-beverst for more details

@mgood
Copy link
Author

mgood commented Sep 26, 2018

Thanks, I haven't started on it, but I'll keep that in mind.

ramya-rao-a pushed a commit that referenced this issue Oct 20, 2018
Instead of running go build for each package in workspace,
add all packages directly to it.

Closes #1890
@ramya-rao-a
Copy link
Contributor

The fix to this bug is now available in the latest update to the Go extension (0.7.0)
Thanks to @ToothlessGear!

@vscodebot vscodebot bot locked and limited conversation to collaborators Dec 4, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants