]> Cypherpunks repositories - gostls13.git/commitdiff
cmd/go: use cache for PGO preprocessing
authorMichael Pratt <mpratt@google.com>
Thu, 29 Feb 2024 21:23:03 +0000 (16:23 -0500)
committerGopher Robot <gobot@golang.org>
Thu, 18 Apr 2024 15:39:17 +0000 (15:39 +0000)
This is the final CL in the series adding PGO preprocessing support to
cmd/go. Now that the tool is hooked up, we integrate with the build
cache to cache the result.

This is fairly straightforward. One difference is that the compile and
link do caching through updateBuildID. However, preprocessed PGO files
don't have a build ID, so it doesn't make much sense to hack our way
through that function when it is simple to just add to the cache
ourselves.

As as aside, we could add a build ID to the preproccessed file format,
though it is not clear if it is worthwhile. The one place a build ID
could be used is in buildActionID, which currently compute the file hash
of the preprocessed profile. With a build ID it could simply read the
build ID. This would save one complete read of the file per build
(cmd/go caches the hash), but each compile process also reads the entire
file, so this is a small change overall.

Fixes #58102.

Cq-Include-Trybots: luci.golang.try:gotip-linux-amd64-longtest
Change-Id: I86e2999a08ccd264230fbb1c983192259b7288e9
Reviewed-on: https://go-review.googlesource.com/c/go/+/569425
Auto-Submit: Michael Pratt <mpratt@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>

doc/next/5-toolchain.md
src/cmd/go/internal/work/action.go
src/cmd/go/internal/work/buildid.go
src/cmd/go/testdata/script/build_cache_pgo.txt [new file with mode: 0644]
src/cmd/go/testdata/script/build_pgo.txt
src/cmd/go/testdata/script/build_pgo_auto.txt
src/cmd/preprofile/main.go

index 0f4a816479754c2c55ce9ead3ac3581bb1f4453c..ce763f1b93827b02806f56aca501977a4c3a20fd 100644 (file)
@@ -1,5 +1,9 @@
 ## Compiler {#compiler}
 
+The build time overhead to building with [Profile Guided Optimization](/doc/pgo) has been reduced significantly.
+Previously, large builds could see 100%+ build time increase from enabling PGO.
+In Go 1.23, overhead should be in the single digit percentages.
+
 ## Assembler {#assembler}
 
 ## Linker {#linker}
index 249c80226980b685181f9387929344a57852fcb1..5e83f1ebfd3a74ccfb1a717c8ede1ad077c1a4bc 100644 (file)
@@ -461,6 +461,17 @@ func (ba *buildActor) Act(b *Builder, ctx context.Context, a *Action) error {
        return b.build(ctx, a)
 }
 
+// pgoActionID computes the action ID for a preprocess PGO action.
+func (b *Builder) pgoActionID(input string) cache.ActionID {
+       h := cache.NewHash("preprocess PGO profile " + input)
+
+       fmt.Fprintf(h, "preprocess PGO profile\n")
+       fmt.Fprintf(h, "preprofile %s\n", b.toolID("preprofile"))
+       fmt.Fprintf(h, "input %q\n", b.fileHash(input))
+
+       return h.Sum()
+}
+
 // pgoActor implements the Actor interface for preprocessing PGO profiles.
 type pgoActor struct {
        // input is the path to the original pprof profile.
@@ -468,7 +479,10 @@ type pgoActor struct {
 }
 
 func (p *pgoActor) Act(b *Builder, ctx context.Context, a *Action) error {
-       // TODO(prattmic): Integrate with build cache to cache output.
+       if b.useCache(a, b.pgoActionID(p.input), a.Target, !b.IsCmdList) || b.IsCmdList {
+               return nil
+       }
+       defer b.flushOutput(a)
 
        sh := b.Shell(a)
 
@@ -480,7 +494,33 @@ func (p *pgoActor) Act(b *Builder, ctx context.Context, a *Action) error {
                return err
        }
 
+       // N.B. Builder.build looks for the out in a.built, regardless of
+       // whether this came from cache.
        a.built = a.Target
+
+       if !cfg.BuildN {
+               // Cache the output.
+               //
+               // N.B. We don't use updateBuildID here, as preprocessed PGO profiles
+               // do not contain a build ID. updateBuildID is typically responsible
+               // for adding to the cache, thus we must do so ourselves instead.
+
+               r, err := os.Open(a.Target)
+               if err != nil {
+                       return fmt.Errorf("error opening target for caching: %w", err)
+               }
+
+               c := cache.Default()
+               outputID, _, err := c.Put(a.actionID, r)
+               r.Close()
+               if err != nil {
+                       return fmt.Errorf("error adding target to cache: %w", err)
+               }
+               if cfg.BuildX {
+                       sh.ShowCmd("", "%s # internal", joinUnambiguously(str.StringList("cp", a.Target, c.OutputFile(outputID))))
+               }
+       }
+
        return nil
 }
 
index bf923d0d5e7d3274a52f83dc5b02fcf52aed8971..acbda1af5532ed56465ca6caf832ff51a1ed3551 100644 (file)
@@ -527,6 +527,14 @@ func (b *Builder) useCache(a *Action, actionHash cache.ActionID, target string,
 
        // Check to see if the action output is cached.
        if file, _, err := cache.GetFile(c, actionHash); err == nil {
+               if a.Mode == "preprocess PGO profile" {
+                       // Preprocessed PGO profiles don't embed a build ID, so
+                       // skip the build ID lookup.
+                       // TODO(prattmic): better would be to add a build ID to the format.
+                       a.built = file
+                       a.Target = "DO NOT USE - using cache"
+                       return true
+               }
                if buildID, err := buildid.ReadFile(file); err == nil {
                        if printOutput {
                                showStdout(b, c, a, "stdout")
diff --git a/src/cmd/go/testdata/script/build_cache_pgo.txt b/src/cmd/go/testdata/script/build_cache_pgo.txt
new file mode 100644 (file)
index 0000000..5efecab
--- /dev/null
@@ -0,0 +1,28 @@
+[short] skip
+
+# Set up fresh GOCACHE.
+env GOCACHE=$WORK/gocache
+mkdir $GOCACHE
+
+# Building trivial non-main package should run preprofile the first time.
+go build -x -pgo=default.pgo lib.go
+stderr 'preprofile.*default\.pgo'
+
+# ... but not again ...
+go build -x -pgo=default.pgo lib.go
+! stderr 'preprofile.*default\.pgo'
+
+# ... unless we use -a.
+go build -a -x -pgo=default.pgo lib.go
+stderr 'preprofile.*default\.pgo'
+
+# ... building a different package should not run preprofile again, instead using a profile from cache.
+go build -x -pgo=default.pgo lib2.go
+! stderr 'preprofile.*default\.pgo'
+stderr 'compile.*-pgoprofile=.*'$GOCACHE'.*lib2.go'
+
+-- lib.go --
+package lib
+-- lib2.go --
+package lib2
+-- default.pgo --
index 0ca2105f56ec8a05b11830d118706b6d8997a7f5..792d299ab18edb3e95a7134c99b21498d8b0b2ab 100644 (file)
@@ -3,6 +3,10 @@
 
 [short] skip 'compiles and links executables'
 
+# Set up fresh GOCACHE.
+env GOCACHE=$WORK/gocache
+mkdir $GOCACHE
+
 # build without PGO
 go build triv.go
 
index 1ae86d4e57dd072d0a0cfc0507aa2b3935924da8..dc2570272fe5332643841efd8aee5cc7af61d547 100644 (file)
@@ -2,6 +2,10 @@
 
 [short] skip 'compiles and links executables'
 
+# Set up fresh GOCACHE.
+env GOCACHE=$WORK/gocache
+mkdir $GOCACHE
+
 # use default.pgo for a single main package
 go build -n -pgo=auto -o a1.exe ./a/a1
 stderr 'preprofile.*-i.*default\.pgo'
index 4cb87f63c86284c97d741346d00bbeeacc72536e..f29b5279e20c4aaba3f85e616e8cef16c514d04a 100644 (file)
@@ -16,6 +16,7 @@ package main
 
 import (
        "bufio"
+       "cmd/internal/objabi"
        "cmd/internal/pgo"
        "flag"
        "fmt"
@@ -67,6 +68,8 @@ func preprocess(profileFile string, outputFile string) error {
 }
 
 func main() {
+       objabi.AddVersionFlag()
+
        log.SetFlags(0)
        log.SetPrefix("preprofile: ")