Skip to content
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

cmd/go: load packages in parallel #29758

Closed
jayconrod opened this issue Jan 15, 2019 · 4 comments
Closed

cmd/go: load packages in parallel #29758

jayconrod opened this issue Jan 15, 2019 · 4 comments
Labels
FrozenDueToAge GoCommand cmd/go NeedsFix The path to resolution is known, but the work has not been done. Performance ToolSpeed
Milestone

Comments

@jayconrod
Copy link
Contributor

Currently, when the Go command loads a set of packages and their dependencies, it does so on a single goroutine. Tracing shows that most cores are idle when loading packages for a large build, so there's significant performance opportunity here. On the one core that's busy, most of the time is spent in internal/load.LoadImport and internal/load.Package.load (which are mutually recursive).

One may expect the package loading process to be dominated by I/O, but the files loaded are usually in the kernel file cache, and we do a lot of work in userspace to parse files, extract imports, and apply build constraints. We actually spend very little time in the kernel, at least on Linux (system call overhead is very low).

Based on this information, it seems that we can significantly speed up the package loading process by parallelizing it. This will benefit go list in particular, which is heavily used by golang.org/x/tools/go/packages. go build and other commands may see some improvement, too.

@jayconrod jayconrod added this to the Go1.13 milestone Jan 15, 2019
@jayconrod jayconrod self-assigned this Jan 15, 2019
@jayconrod
Copy link
Contributor Author

cc @bcmills

@myitcv
Copy link
Member

myitcv commented Jan 16, 2019

Also just to note https://go-review.googlesource.com/c/tools/+/158097. Chatting with @heschik, ideally go list -find would be made sufficiently fast to not require dropping down to go list -m and re-building package discovery logic atop that.

So hopefully this change will go a good step towards that goal.

@bcmills bcmills added the NeedsFix The path to resolution is known, but the work has not been done. label Jan 17, 2019
@gopherbot
Copy link
Contributor

Change https://golang.org/cl/161397 mentions this issue: cmd/go: parallelize package loading

@gopherbot
Copy link
Contributor

Change https://golang.org/cl/167748 mentions this issue: cmd/go: refactor load.LoadPackage into other functions

gopherbot pushed a commit that referenced this issue Apr 1, 2019
LoadPackage was used to load a *load.Package for a command line
argument, after pattern expansion. It provided two special cases on
top of LoadImport. First, it ensured that "cmd/" packages in GOROOT
were installed in "$GOROOT/bin" or "$GOROOT/pkg/tool". Second, it
translated absolute paths to packages in GOROOT and GOPATH into
regular import paths.

With this change, LoadImport now ensures "cmd/" packages have the
right Target (without the need for a special case) and
search.ImportPaths translates absolute paths.

LoadPackage no longer handles these special cases and has been renamed
to LoadImportWithFlags, since it's still useful for loading implicit
dependencies.

Updates #29758

Change-Id: I9d54036f90c3ccd9b3a0fe0eaddaa7749593cc91
Reviewed-on: https://go-review.googlesource.com/c/go/+/167748
Run-TryBot: Jay Conrod <[email protected]>
TryBot-Result: Gobot Gobot <[email protected]>
Reviewed-by: Bryan C. Mills <[email protected]>
@golang golang locked and limited conversation to collaborators Apr 4, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge GoCommand cmd/go NeedsFix The path to resolution is known, but the work has not been done. Performance ToolSpeed
Projects
None yet
Development

No branches or pull requests

4 participants