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: support multiple main packages with -pgo=auto #58099

Closed
prattmic opened this issue Jan 26, 2023 · 7 comments
Closed

cmd/go: support multiple main packages with -pgo=auto #58099

prattmic opened this issue Jan 26, 2023 · 7 comments
Assignees
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. FrozenDueToAge GoCommand cmd/go NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@prattmic
Copy link
Member

In Go 1.20, go build -pgo=auto can only be used with a single main package. Specifying multiple main packages (e.g., go build -pgo=auto cmd/compile cmd/link) is an error.

To support this, cmd/go needs to learn to build the transitive dependencies N times, once for each different profile used by each main package.

Follow up to #55022.

cc @cherrymui @aclements

@prattmic prattmic added the NeedsFix The path to resolution is known, but the work has not been done. label Jan 26, 2023
@prattmic prattmic added this to the Go1.21 milestone Jan 26, 2023
@bcmills bcmills added GoCommand cmd/go compiler/runtime Issues related to the Go compiler and/or runtime. labels Jan 26, 2023
@nemo-cpt
Copy link

nemo-cpt commented Feb 9, 2023

I hope everone here understands that this N-fold complete rebuild doesn't scale.

Having 100s of services and 1000s of packages you end up in extremely large caches (100x than normal) or basically dong every build without cache, or mix of both.

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/472357 mentions this issue: cmd/go: make PGO profile path per package

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/472358 mentions this issue: cmd/go: support multiple main packages with -pgo=auto

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/473275 mentions this issue: cmd/go: distinguish packages built for different main packages in printing

gopherbot pushed a commit that referenced this issue Mar 6, 2023
Currently, the PGO profile path is global for a single go command
invocation, as it applies to all packages being built (or none).
With -pgo=auto mode with multiple main packages, packages from a
single go command invocation could have different profiles. So it
is necessary that the PGO profile path is per package, which is
this CL does.

For #58099.

Change-Id: I148a15970ec907272db85b4b27ad6b08c41d6c0c
Reviewed-on: https://go-review.googlesource.com/c/go/+/472357
Run-TryBot: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Michael Pratt <mpratt@google.com>
Reviewed-by: Bryan Mills <bcmills@google.com>
gopherbot pushed a commit that referenced this issue Mar 6, 2023
…nting

In -pgo=auto mode, a package may be built multiple times. E.g. for

go build -pgo=auto cmd/a cmd/b

and both cmd/a and cmd/b imports package p, p may be built twice,
one using a's profile, one using b's. If we need to print p, e.g.
in "go list -deps" or when there is a build failure, p will be
printed twice, and currently we don't distinguish them.

We have a precedence for a similar case: for testing, there is the
original package, and the (internal) test version of the package
(which includes _test.go files). Packages that import the package
under testing may also have two versions (one imports the original,
one imports the testing version). In printing, the go command
distinguishes them by adding a "[p.test]" suffix for the latter,
as they are specifically built for the p.test binary.

We do the similar. When a package needs to be compiled multiple
times for different main packages, we attach the main package's
import path, like "p [cmd/a]" for package p built specifically
for cmd/a.

For #58099.

Change-Id: I4a040cf17e1dceb5ca1810c217f16e734c858ab6
Reviewed-on: https://go-review.googlesource.com/c/go/+/473275
Reviewed-by: Michael Pratt <mpratt@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Cherry Mui <cherryyz@google.com>
Reviewed-by: Bryan Mills <bcmills@google.com>
@aclements
Copy link
Member

Just tracking: this doesn't actually make -pgo=auto the default, but it does remove the blocker to doing so.

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/474236 mentions this issue: cmd/go: enable -pgo=auto by default

gopherbot pushed a commit that referenced this issue Mar 22, 2023
Updates #58099.
Updates #55022.

Change-Id: I32eacdf9f008d16566e0b30230ecc25d110a9811
Reviewed-on: https://go-review.googlesource.com/c/go/+/474236
Run-TryBot: Cherry Mui <cherryyz@google.com>
Reviewed-by: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Michael Pratt <mpratt@google.com>
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/495477 mentions this issue: doc/go1.21: document that -pgo=auto enabled by default

gopherbot pushed a commit that referenced this issue May 17, 2023
Updates #58099.

Change-Id: I95c0397add696f677c86ab7618482e07eb4e9fda
Reviewed-on: https://go-review.googlesource.com/c/go/+/495477
Run-TryBot: Cherry Mui <cherryyz@google.com>
Reviewed-by: Austin Clements <austin@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
@golang golang locked and limited conversation to collaborators May 15, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. FrozenDueToAge GoCommand cmd/go NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

6 participants