Skip to content

Commit 4084bc1

Browse files
prattmicgopherbot
authored andcommitted
cmd/go: inital plumbing for PGO profiles preprocessing
The new go tool preprofile preprocesses a PGO pprof profile into an intermediate representation that is more efficient for the compiler to consume. Performing preprocessing avoids having every single compile process from duplicating the same processing. This CL prepares the initial plumbing to support automatic preprocessing by cmd/go. Each compile action takes a new dependency on a new "preprocess PGO profile" action. The same action instance is shared by all compile actions (assuming they have the same input profile), so the action only executes once. Builder.build retrieves the file to pass to -pgofile from the output of the preprocessing action, rather than directly from p.Internal.PGOProfile. Builder.buildActionID also uses the preprocess output as the PGO component of the cache key, rather than the original source. This doesn't matter for normal toolchain releases, as the two files are semantically equivalent, but it is useful for correct cache invalidation in development. For example, if _only_ go tool preprofile changes (potentially changing the output), then we must regenerate the output and then rebuild all packages. This CL does not actually invoke go tool preprocess. That will come in the next CL. For now, it just copies the input pprof profile. This CL shouldn't be submitted on its own, only with the children. Since the new action doesn't yet use the build cache, every build (even fully cached builds) unconditionally run the PGO action. For #58102. Cq-Include-Trybots: luci.golang.try:gotip-linux-amd64-longtest Change-Id: I594417cfb0164cd39439a03977c904e4c0c83b8b Reviewed-on: https://go-review.googlesource.com/c/go/+/569423 Auto-Submit: Michael Pratt <[email protected]> Reviewed-by: Cherry Mui <[email protected]> LUCI-TryBot-Result: Go LUCI <[email protected]>
1 parent e718aee commit 4084bc1

File tree

7 files changed

+96
-44
lines changed

7 files changed

+96
-44
lines changed

src/cmd/go/internal/work/action.go

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -461,6 +461,34 @@ func (ba *buildActor) Act(b *Builder, ctx context.Context, a *Action) error {
461461
return b.build(ctx, a)
462462
}
463463

464+
// pgoActor implements the Actor interface for preprocessing PGO profiles.
465+
type pgoActor struct {
466+
// input is the path to the original pprof profile.
467+
input string
468+
}
469+
470+
func (p *pgoActor) Act(b *Builder, ctx context.Context, a *Action) error {
471+
// TODO(prattmic): Integrate with build cache to cache output.
472+
473+
sh := b.Shell(a)
474+
475+
if err := sh.Mkdir(a.Objdir); err != nil {
476+
return err
477+
}
478+
479+
// TODO(prattmic): This should use go tool preprofile to actually
480+
// preprocess the profile. For now, this is a dummy implementation that
481+
// simply copies the input to the output. This is technically a valid
482+
// implementation because go tool compile -pgofile accepts either a
483+
// pprof file or preprocessed file.
484+
if err := sh.CopyFile(a.Target, p.input, 0644, false); err != nil {
485+
return err
486+
}
487+
488+
a.built = a.Target
489+
return nil
490+
}
491+
464492
// CompileAction returns the action for compiling and possibly installing
465493
// (according to mode) the given package. The resulting action is only
466494
// for building packages (archives), never for linking executables.
@@ -494,6 +522,20 @@ func (b *Builder) CompileAction(mode, depMode BuildMode, p *load.Package) *Actio
494522
}
495523
}
496524

525+
if p.Internal.PGOProfile != "" {
526+
pgoAction := b.cacheAction("preprocess PGO profile "+p.Internal.PGOProfile, nil, func() *Action {
527+
a := &Action{
528+
Mode: "preprocess PGO profile",
529+
Actor: &pgoActor{input: p.Internal.PGOProfile},
530+
Objdir: b.NewObjdir(),
531+
}
532+
a.Target = filepath.Join(a.Objdir, "pgo.preprofile")
533+
534+
return a
535+
})
536+
a.Deps = append(a.Deps, pgoAction)
537+
}
538+
497539
if p.Standard {
498540
switch p.ImportPath {
499541
case "builtin", "unsafe":

src/cmd/go/internal/work/exec.go

Lines changed: 19 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -395,14 +395,14 @@ func (b *Builder) buildActionID(a *Action) cache.ActionID {
395395
for _, file := range inputFiles {
396396
fmt.Fprintf(h, "file %s %s\n", file, b.fileHash(filepath.Join(p.Dir, file)))
397397
}
398-
if p.Internal.PGOProfile != "" {
399-
fmt.Fprintf(h, "pgofile %s\n", b.fileHash(p.Internal.PGOProfile))
400-
}
401398
for _, a1 := range a.Deps {
402399
p1 := a1.Package
403400
if p1 != nil {
404401
fmt.Fprintf(h, "import %s %s\n", p1.ImportPath, contentID(a1.buildID))
405402
}
403+
if a1.Mode == "preprocess PGO profile" {
404+
fmt.Fprintf(h, "pgofile %s\n", b.fileHash(a1.built))
405+
}
406406
}
407407

408408
return h.Sum()
@@ -864,6 +864,18 @@ OverlayLoop:
864864
embedcfg = js
865865
}
866866

867+
// Find PGO profile if needed.
868+
var pgoProfile string
869+
for _, a1 := range a.Deps {
870+
if a1.Mode != "preprocess PGO profile" {
871+
continue
872+
}
873+
if pgoProfile != "" {
874+
return fmt.Errorf("action contains multiple PGO profile dependencies")
875+
}
876+
pgoProfile = a1.built
877+
}
878+
867879
if p.Internal.BuildInfo != nil && cfg.ModulesEnabled {
868880
prog := modload.ModInfoProg(p.Internal.BuildInfo.String(), cfg.BuildToolchainName == "gccgo")
869881
if len(prog) > 0 {
@@ -876,7 +888,7 @@ OverlayLoop:
876888

877889
// Compile Go.
878890
objpkg := objdir + "_pkg_.a"
879-
ofile, out, err := BuildToolchain.gc(b, a, objpkg, icfg.Bytes(), embedcfg, symabis, len(sfiles) > 0, gofiles)
891+
ofile, out, err := BuildToolchain.gc(b, a, objpkg, icfg.Bytes(), embedcfg, symabis, len(sfiles) > 0, pgoProfile, gofiles)
880892
if err := sh.reportCmd("", "", out, err); err != nil {
881893
return err
882894
}
@@ -2041,7 +2053,7 @@ func mkAbs(dir, f string) string {
20412053
type toolchain interface {
20422054
// gc runs the compiler in a specific directory on a set of files
20432055
// and returns the name of the generated output file.
2044-
gc(b *Builder, a *Action, archive string, importcfg, embedcfg []byte, symabis string, asmhdr bool, gofiles []string) (ofile string, out []byte, err error)
2056+
gc(b *Builder, a *Action, archive string, importcfg, embedcfg []byte, symabis string, asmhdr bool, pgoProfile string, gofiles []string) (ofile string, out []byte, err error)
20452057
// cc runs the toolchain's C compiler in a directory on a C file
20462058
// to produce an output file.
20472059
cc(b *Builder, a *Action, ofile, cfile string) error
@@ -2081,7 +2093,7 @@ func (noToolchain) linker() string {
20812093
return ""
20822094
}
20832095

2084-
func (noToolchain) gc(b *Builder, a *Action, archive string, importcfg, embedcfg []byte, symabis string, asmhdr bool, gofiles []string) (ofile string, out []byte, err error) {
2096+
func (noToolchain) gc(b *Builder, a *Action, archive string, importcfg, embedcfg []byte, symabis string, asmhdr bool, pgoProfile string, gofiles []string) (ofile string, out []byte, err error) {
20852097
return "", nil, noCompiler()
20862098
}
20872099

@@ -3252,7 +3264,7 @@ func (b *Builder) swigDoIntSize(objdir string) (intsize string, err error) {
32523264

32533265
p := load.GoFilesPackage(context.TODO(), load.PackageOpts{}, srcs)
32543266

3255-
if _, _, e := BuildToolchain.gc(b, &Action{Mode: "swigDoIntSize", Package: p, Objdir: objdir}, "", nil, nil, "", false, srcs); e != nil {
3267+
if _, _, e := BuildToolchain.gc(b, &Action{Mode: "swigDoIntSize", Package: p, Objdir: objdir}, "", nil, nil, "", false, "", srcs); e != nil {
32563268
return "32", nil
32573269
}
32583270
return "64", nil

src/cmd/go/internal/work/gc.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ func pkgPath(a *Action) string {
5353
return ppath
5454
}
5555

56-
func (gcToolchain) gc(b *Builder, a *Action, archive string, importcfg, embedcfg []byte, symabis string, asmhdr bool, gofiles []string) (ofile string, output []byte, err error) {
56+
func (gcToolchain) gc(b *Builder, a *Action, archive string, importcfg, embedcfg []byte, symabis string, asmhdr bool, pgoProfile string, gofiles []string) (ofile string, output []byte, err error) {
5757
p := a.Package
5858
sh := b.Shell(a)
5959
objdir := a.Objdir
@@ -112,8 +112,8 @@ func (gcToolchain) gc(b *Builder, a *Action, archive string, importcfg, embedcfg
112112
if p.Internal.Cover.Cfg != "" {
113113
defaultGcFlags = append(defaultGcFlags, "-coveragecfg="+p.Internal.Cover.Cfg)
114114
}
115-
if p.Internal.PGOProfile != "" {
116-
defaultGcFlags = append(defaultGcFlags, "-pgoprofile="+p.Internal.PGOProfile)
115+
if pgoProfile != "" {
116+
defaultGcFlags = append(defaultGcFlags, "-pgoprofile="+pgoProfile)
117117
}
118118
if symabis != "" {
119119
defaultGcFlags = append(defaultGcFlags, "-symabis", symabis)

src/cmd/go/internal/work/gccgo.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ func checkGccgoBin() {
5959
base.Exit()
6060
}
6161

62-
func (tools gccgoToolchain) gc(b *Builder, a *Action, archive string, importcfg, embedcfg []byte, symabis string, asmhdr bool, gofiles []string) (ofile string, output []byte, err error) {
62+
func (tools gccgoToolchain) gc(b *Builder, a *Action, archive string, importcfg, embedcfg []byte, symabis string, asmhdr bool, pgoProfile string, gofiles []string) (ofile string, output []byte, err error) {
6363
p := a.Package
6464
sh := b.Shell(a)
6565
objdir := a.Objdir

src/cmd/go/testdata/script/build_pgo.txt

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,8 @@ go build triv.go
99
# build with PGO, should trigger rebuild
1010
# starting with an empty profile (the compiler accepts it)
1111
go build -x -pgo=prof -o triv.exe triv.go
12-
stderr 'compile.*-pgoprofile=.*prof.*triv.go'
12+
stderr 'cp.*prof' # preprocess PGO profile
13+
stderr 'compile.*-pgoprofile=.*triv.go'
1314

1415
# check that PGO appears in build info
1516
# N.B. we can't start the stdout check with -pgo because the script assumes that
@@ -33,9 +34,11 @@ cmp stdout list.out
3334
# overwrite the prof
3435
go run overwrite.go
3536

36-
# build again, profile content changed, should trigger rebuild
37+
# build again, profile content changed, should trigger rebuild, including std
3738
go build -n -pgo=prof triv.go
38-
stderr 'compile.*-pgoprofile=.*prof.*p.go'
39+
stderr 'cp.*prof' # preprocess PGO profile
40+
stderr 'compile.*-pgoprofile=.*triv.go'
41+
stderr 'compile.*-p runtime.*-pgoprofile=.*'
3942

4043
# check that the build ID is different
4144
go list -export -json=BuildID -pgo=prof triv.go

src/cmd/go/testdata/script/build_pgo_auto.txt

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,11 @@
44

55
# use default.pgo for a single main package
66
go build -n -pgo=auto -o a1.exe ./a/a1
7-
stderr 'compile.*-pgoprofile=.*default\.pgo.*a1.go'
7+
stderr 'cp.*default\.pgo' # preprocess PGO profile
8+
stderr 'compile.*-pgoprofile=.*a1.go'
89

910
# check that pgo applied to dependencies
10-
stderr 'compile.*-p test/dep.*-pgoprofile=.*default\.pgo'
11+
stderr 'compile.*-p test/dep.*-pgoprofile=.*'
1112

1213
# check that pgo appears in build info
1314
# N.B. we can't start the stdout check with -pgo because the script assumes that
@@ -19,7 +20,7 @@ stderr 'build\\t-pgo=.*default\.pgo'
1920

2021
# use default.pgo for ... with a single main package
2122
go build -n -pgo=auto ./a/...
22-
stderr 'compile.*-pgoprofile=.*default\.pgo.*a1.go'
23+
stderr 'compile.*-pgoprofile=.*a1.go'
2324

2425
# check that pgo appears in build info
2526
stderr 'build\\t-pgo=.*default\.pgo'
@@ -34,14 +35,14 @@ stderr 'compile.*nopgo.go'
3435

3536
# other build-related commands
3637
go install -a -n -pgo=auto ./a/a1
37-
stderr 'compile.*-pgoprofile=.*default\.pgo.*a1.go'
38+
stderr 'compile.*-pgoprofile=.*a1.go'
3839

3940
go run -a -n -pgo=auto ./a/a1
40-
stderr 'compile.*-pgoprofile=.*default\.pgo.*a1.go'
41+
stderr 'compile.*-pgoprofile=.*a1.go'
4142

4243
go test -a -n -pgo=auto ./a/a1
43-
stderr 'compile.*-pgoprofile=.*default\.pgo.*a1.go.*a1_test.go'
44-
stderr 'compile.*-pgoprofile=.*default\.pgo.*external_test.go'
44+
stderr 'compile.*-pgoprofile=.*a1.go.*a1_test.go'
45+
stderr 'compile.*-pgoprofile=.*external_test.go'
4546

4647
# go list commands should succeed as usual
4748
go list -pgo=auto ./a/a1
@@ -53,8 +54,8 @@ go list -deps -pgo=auto ./a/a1
5354
# -pgo=auto is the default. Commands without explicit -pgo=auto
5455
# should work as -pgo=auto.
5556
go build -a -n -o a1.exe ./a/a1
56-
stderr 'compile.*-pgoprofile=.*default\.pgo.*a1.go'
57-
stderr 'compile.*-p test/dep.*-pgoprofile=.*default\.pgo'
57+
stderr 'compile.*-pgoprofile=.*a1.go'
58+
stderr 'compile.*-p test/dep.*-pgoprofile=.*'
5859

5960
# check that pgo appears in build info
6061
stderr 'build\\t-pgo=.*default\.pgo'

src/cmd/go/testdata/script/build_pgo_auto_multi.txt

Lines changed: 15 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -2,24 +2,21 @@
22

33
go install -a -n -pgo=auto ./a ./b ./nopgo
44

5-
# a/default.pgo applies to package a and (transitive)
6-
# dependencies.
7-
stderr 'compile.*-pgoprofile=.*a(/|\\\\)default\.pgo.*a(/|\\\\)a\.go'
8-
stderr 'compile.*-pgoprofile=.*a(/|\\\\)default\.pgo.*dep(/|\\\\)dep\.go'
9-
stderr 'compile.*-pgoprofile=.*a(/|\\\\)default\.pgo.*dep2(/|\\\\)dep2\.go'
10-
stderr -count=1 'compile.*-pgoprofile=.*a(/|\\\\)default\.pgo.*dep3(/|\\\\)dep3\.go'
5+
# a/default.pgo and b/default.pgo are both preprocessed
6+
stderr 'cp.*a(/|\\)default\.pgo'
7+
stderr 'cp.*b(/|\\)default\.pgo'
118

12-
# b/default.pgo applies to package b and (transitive)
13-
# dependencies.
14-
stderr 'compile.*-pgoprofile=.*b(/|\\\\)default\.pgo.*b(/|\\\\)b\.go'
15-
stderr 'compile.*-pgoprofile=.*b(/|\\\\)default\.pgo.*dep(/|\\\\)dep\.go'
16-
stderr 'compile.*-pgoprofile=.*b(/|\\\\)default\.pgo.*dep2(/|\\\\)dep2\.go'
17-
stderr -count=1 'compile.*-pgoprofile=.*b(/|\\\\)default\.pgo.*dep3(/|\\\\)dep3\.go'
9+
# a and b built once each with PGO.
10+
# Ideally we would check that the passed profile is the expected profile (here
11+
# and for dependencies). Unfortunately there is no nice way to map the expected
12+
# paths after preprocessing.
13+
stderr -count=1 'compile.*-pgoprofile=.*a(/|\\\\)a\.go'
14+
stderr -count=1 'compile.*-pgoprofile=.*b(/|\\\\)b\.go'
1815

1916
# nopgo should be built without PGO.
2017
! stderr 'compile.*-pgoprofile=.*nopgo(/|\\\\)nopgo\.go'
2118

22-
# Dependencies should also be built without PGO.
19+
# Dependencies should also be built with and without PGO.
2320
# Here we want to match a compile action without -pgoprofile,
2421
# by matching 3 occurrences of "compile dep.go", among which
2522
# 2 of them have -pgoprofile (therefore one without).
@@ -39,17 +36,14 @@ stderr 'path\\ttest/b\\n.*build\\t-pgo=.*b(/|\\\\)default\.pgo'
3936

4037
# go test works the same way
4138
go test -a -n -pgo=auto ./a ./b ./nopgo
42-
stderr 'compile.*-pgoprofile=.*a(/|\\\\)default\.pgo.*a(/|\\\\)a_test\.go'
43-
stderr 'compile.*-pgoprofile=.*a(/|\\\\)default\.pgo.*dep(/|\\\\)dep\.go'
44-
stderr 'compile.*-pgoprofile=.*b(/|\\\\)default\.pgo.*b(/|\\\\)b_test\.go'
45-
stderr 'compile.*-pgoprofile=.*b(/|\\\\)default\.pgo.*dep(/|\\\\)dep\.go'
39+
stderr -count=1 'compile.*-pgoprofile=.*a(/|\\\\)a_test\.go'
40+
stderr -count=1 'compile.*-pgoprofile=.*b(/|\\\\)b_test\.go'
41+
stderr -count=2 'compile.*-pgoprofile=.*dep(/|\\\\)dep\.go'
4642
! stderr 'compile.*-pgoprofile=.*nopgo(/|\\\\)nopgo_test\.go'
4743

4844
# test-only dependencies also have profiles attached
49-
stderr 'compile.*-pgoprofile=.*a(/|\\\\)default\.pgo.*testdep(/|\\\\)testdep\.go'
50-
stderr 'compile.*-pgoprofile=.*b(/|\\\\)default\.pgo.*testdep(/|\\\\)testdep\.go'
51-
stderr 'compile.*-pgoprofile=.*a(/|\\\\)default\.pgo.*testdep2(/|\\\\)testdep2\.go'
52-
stderr 'compile.*-pgoprofile=.*b(/|\\\\)default\.pgo.*testdep2(/|\\\\)testdep2\.go'
45+
stderr -count=2 'compile.*-pgoprofile=.*testdep(/|\\\\)testdep\.go'
46+
stderr -count=2 'compile.*-pgoprofile=.*testdep2(/|\\\\)testdep2\.go'
5347

5448
# go list -deps prints packages built multiple times.
5549
go list -pgo=auto -deps ./a ./b ./nopgo

0 commit comments

Comments
 (0)