From 5987f3c2715d93cb52f05dcb1c29825507e1d625 Mon Sep 17 00:00:00 2001 From: Cherry Mui Date: Mon, 27 Feb 2023 15:37:32 -0500 Subject: [PATCH] cmd/go: make PGO profile path per package 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 TryBot-Result: Gopher Robot Reviewed-by: Michael Pratt Reviewed-by: Bryan Mills --- src/cmd/go/internal/cfg/cfg.go | 1 - src/cmd/go/internal/load/pkg.go | 22 ++++++++++++++-------- src/cmd/go/internal/load/test.go | 3 +++ src/cmd/go/internal/work/exec.go | 4 ++-- src/cmd/go/internal/work/gc.go | 4 ++-- 5 files changed, 21 insertions(+), 13 deletions(-) diff --git a/src/cmd/go/internal/cfg/cfg.go b/src/cmd/go/internal/cfg/cfg.go index a6ad7390ef..2037e7cf06 100644 --- a/src/cmd/go/internal/cfg/cfg.go +++ b/src/cmd/go/internal/cfg/cfg.go @@ -81,7 +81,6 @@ var ( BuildO string // -o flag BuildP = runtime.GOMAXPROCS(0) // -p flag BuildPGO string // -pgo flag - BuildPGOFile string // profile selected by -pgo flag, an absolute path (if not empty) BuildPkgdir string // -pkgdir flag BuildRace bool // -race flag BuildToolexec []string // -toolexec flag diff --git a/src/cmd/go/internal/load/pkg.go b/src/cmd/go/internal/load/pkg.go index 799f7de85e..cfb853e979 100644 --- a/src/cmd/go/internal/load/pkg.go +++ b/src/cmd/go/internal/load/pkg.go @@ -232,6 +232,7 @@ type PackageInternal struct { TestmainGo *[]byte // content for _testmain.go Embed map[string][]string // //go:embed comment mapping OrigImportPath string // original import path before adding '_test' suffix + PGOProfile string // path to PGO profile Asmflags []string // -asmflags for this package Gcflags []string // -gcflags for this package @@ -2385,11 +2386,11 @@ func (p *Package) setBuildInfo(autoVCS bool) { appendSetting("-ldflags", ldflags) } } - if cfg.BuildPGOFile != "" { + if p.Internal.PGOProfile != "" { if cfg.BuildTrimpath { - appendSetting("-pgo", filepath.Base(cfg.BuildPGOFile)) + appendSetting("-pgo", filepath.Base(p.Internal.PGOProfile)) } else { - appendSetting("-pgo", cfg.BuildPGOFile) + appendSetting("-pgo", p.Internal.PGOProfile) } } if cfg.BuildMSan { @@ -2894,7 +2895,7 @@ func PackagesAndErrors(ctx context.Context, opts PackageOpts, patterns []string) return pkgs } -// setPGOProfilePath sets cfg.BuildPGOFile to the PGO profile path. +// setPGOProfilePath sets the PGO profile path for pkgs. // In -pgo=auto mode, it finds the default PGO profile. func setPGOProfilePath(pkgs []*Package) { switch cfg.BuildPGO { @@ -2929,16 +2930,21 @@ func setPGOProfilePath(pkgs []*Package) { } file := filepath.Join(mainpkg.Dir, "default.pgo") if fi, err := os.Stat(file); err == nil && !fi.IsDir() { - cfg.BuildPGOFile = file + for _, p := range PackageList(pkgs) { + p.Internal.PGOProfile = file + } } default: // Profile specified from the command line. // Make it absolute path, as the compiler runs on various directories. - if p, err := filepath.Abs(cfg.BuildPGO); err != nil { + file, err := filepath.Abs(cfg.BuildPGO) + if err != nil { base.Fatalf("fail to get absolute path of PGO file %s: %v", cfg.BuildPGO, err) - } else { - cfg.BuildPGOFile = p + } + + for _, p := range PackageList(pkgs) { + p.Internal.PGOProfile = file } } } diff --git a/src/cmd/go/internal/load/test.go b/src/cmd/go/internal/load/test.go index 38afd96aa4..64e5b74cc2 100644 --- a/src/cmd/go/internal/load/test.go +++ b/src/cmd/go/internal/load/test.go @@ -206,6 +206,7 @@ func TestPackagesAndErrors(ctx context.Context, opts PackageOpts, p *Package, co ptest.Internal.Embed = testEmbed ptest.EmbedFiles = str.StringList(p.EmbedFiles, p.TestEmbedFiles) ptest.Internal.OrigImportPath = p.Internal.OrigImportPath + ptest.Internal.PGOProfile = p.Internal.PGOProfile ptest.Internal.Build.Directives = append(slices.Clip(p.Internal.Build.Directives), p.Internal.Build.TestDirectives...) ptest.collectDeps() } else { @@ -243,6 +244,7 @@ func TestPackagesAndErrors(ctx context.Context, opts PackageOpts, p *Package, co Gccgoflags: p.Internal.Gccgoflags, Embed: xtestEmbed, OrigImportPath: p.Internal.OrigImportPath, + PGOProfile: p.Internal.PGOProfile, }, } if pxtestNeedsPtest { @@ -270,6 +272,7 @@ func TestPackagesAndErrors(ctx context.Context, opts PackageOpts, p *Package, co Ldflags: p.Internal.Ldflags, Gccgoflags: p.Internal.Gccgoflags, OrigImportPath: p.Internal.OrigImportPath, + PGOProfile: p.Internal.PGOProfile, }, } diff --git a/src/cmd/go/internal/work/exec.go b/src/cmd/go/internal/work/exec.go index 573a58627a..6a0a53429f 100644 --- a/src/cmd/go/internal/work/exec.go +++ b/src/cmd/go/internal/work/exec.go @@ -385,8 +385,8 @@ func (b *Builder) buildActionID(a *Action) cache.ActionID { for _, file := range inputFiles { fmt.Fprintf(h, "file %s %s\n", file, b.fileHash(filepath.Join(p.Dir, file))) } - if cfg.BuildPGOFile != "" { - fmt.Fprintf(h, "pgofile %s\n", b.fileHash(cfg.BuildPGOFile)) + if p.Internal.PGOProfile != "" { + fmt.Fprintf(h, "pgofile %s\n", b.fileHash(p.Internal.PGOProfile)) } for _, a1 := range a.Deps { p1 := a1.Package diff --git a/src/cmd/go/internal/work/gc.go b/src/cmd/go/internal/work/gc.go index 51d1760d9c..ec01798e09 100644 --- a/src/cmd/go/internal/work/gc.go +++ b/src/cmd/go/internal/work/gc.go @@ -144,8 +144,8 @@ func (gcToolchain) gc(b *Builder, a *Action, archive string, importcfg, embedcfg if p.Internal.CoverageCfg != "" { defaultGcFlags = append(defaultGcFlags, "-coveragecfg="+p.Internal.CoverageCfg) } - if cfg.BuildPGOFile != "" { - defaultGcFlags = append(defaultGcFlags, "-pgoprofile="+cfg.BuildPGOFile) + if p.Internal.PGOProfile != "" { + defaultGcFlags = append(defaultGcFlags, "-pgoprofile="+p.Internal.PGOProfile) } if symabis != "" { defaultGcFlags = append(defaultGcFlags, "-symabis", symabis) -- 2.48.1