cmd/compile/internal/pgo: fix hard-coded PGO sample data position

This patch detects at which index position profiling samples that have the value-type samples count are, instead of the previously hard-coded position of index 1. Runtime generated profiles always generate CPU profiling data with the 0 index being CPU nanoseconds, and samples count at index 1, which is why this previously hasn't come up.

Fixes #58292

Change-Id: Idde761d53b02259f3076c4e5dcb4a96a9d53df0e
GitHub-Last-Rev: dabbf9f88c560286e150e9b136a27c3ac23c5ec1
GitHub-Pull-Request: golang/go#58294
Reviewed-on: https://go-review.googlesource.com/c/go/+/465135
Auto-Submit: Michael Pratt <mpratt@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Michael Pratt <mpratt@google.com>
Run-TryBot: Michael Pratt <mpratt@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>

diff --git a/src/cmd/compile/internal/pgo/irgraph.go b/src/cmd/compile/internal/pgo/irgraph.go
index ca9e2f3b5adde3917c91d84e390594c5bcc410c9..a0319f3962b2f388c87056b4bade45d59da42794 100644 (file)
--- a/src/cmd/compile/internal/pgo/irgraph.go
+++ b/src/cmd/compile/internal/pgo/irgraph.go
@@ -140,9 +140,25 @@ func New(profileFile string) *Profile {
                return nil
        }
 
+       samplesCountIndex := -1
+       for i, s := range profile.SampleType {
+               // Samples count is the raw data collected, and CPU nanoseconds is just
+               // a scaled version of it, so either one we can find is fine.
+               if (s.Type == "samples" && s.Unit == "count") ||
+                       (s.Type == "cpu" && s.Unit == "nanoseconds") {
+                       samplesCountIndex = i
+                       break
+               }
+       }
+
+       if samplesCountIndex == -1 {
+               log.Fatal("failed to find CPU samples count or CPU nanoseconds value-types in profile.")
+               return nil
+       }
+
        g := newGraph(profile, &Options{
                CallTree:    false,
-               SampleValue: func(v []int64) int64 { return v[1] },
+               SampleValue: func(v []int64) int64 { return v[samplesCountIndex] },
        })
 
        p := &Profile{

diff --git a/src/cmd/compile/internal/test/pgo_inl_test.go b/src/cmd/compile/internal/test/pgo_inl_test.go
index 2f6391fded265dddc4fd4464ad3d6441b86aa196..4d6b5a134a0e2808ae25da3f2f06bece30b50a19 100644 (file)
--- a/src/cmd/compile/internal/test/pgo_inl_test.go
+++ b/src/cmd/compile/internal/test/pgo_inl_test.go
@@ -7,6 +7,7 @@ package test
 import (
        "bufio"
        "fmt"
+       "internal/profile"
        "internal/testenv"
        "io"
        "os"
@@ -213,6 +214,73 @@ func TestPGOIntendedInliningShiftedLines(t *testing.T) {
        testPGOIntendedInlining(t, dir)
 }
 
+// TestPGOSingleIndex tests that the sample index can not be 1 and compilation
+// will not fail. All it should care about is that the sample type is either
+// CPU nanoseconds or samples count, whichever it finds first.
+func TestPGOSingleIndex(t *testing.T) {
+       for _, tc := range []struct {
+               originalIndex int
+       }{{
+               // The `testdata/pgo/inline/inline_hot.pprof` file is a standard CPU
+               // profile as the runtime would generate. The 0 index contains the
+               // value-type samples and value-unit count. The 1 index contains the
+               // value-type cpu and value-unit nanoseconds. These tests ensure that
+               // the compiler can work with profiles that only have a single index,
+               // but are either samples count or CPU nanoseconds.
+               originalIndex: 0,
+       }, {
+               originalIndex: 1,
+       }} {
+               t.Run(fmt.Sprintf("originalIndex=%d", tc.originalIndex), func(t *testing.T) {
+                       wd, err := os.Getwd()
+                       if err != nil {
+                               t.Fatalf("error getting wd: %v", err)
+                       }
+                       srcDir := filepath.Join(wd, "testdata/pgo/inline")
+
+                       // Copy the module to a scratch location so we can add a go.mod.
+                       dir := t.TempDir()
+
+                       originalPprofFile, err := os.Open(filepath.Join(srcDir, "inline_hot.pprof"))
+                       if err != nil {
+                               t.Fatalf("error opening inline_hot.pprof: %v", err)
+                       }
+                       defer originalPprofFile.Close()
+
+                       p, err := profile.Parse(originalPprofFile)
+                       if err != nil {
+                               t.Fatalf("error parsing inline_hot.pprof: %v", err)
+                       }
+
+                       // Move the samples count value-type to the 0 index.
+                       p.SampleType = []*profile.ValueType{p.SampleType[tc.originalIndex]}
+
+                       // Ensure we only have a single set of sample values.
+                       for _, s := range p.Sample {
+                               s.Value = []int64{s.Value[tc.originalIndex]}
+                       }
+
+                       modifiedPprofFile, err := os.Create(filepath.Join(dir, "inline_hot.pprof"))
+                       if err != nil {
+                               t.Fatalf("error creating inline_hot.pprof: %v", err)
+                       }
+                       defer modifiedPprofFile.Close()
+
+                       if err := p.Write(modifiedPprofFile); err != nil {
+                               t.Fatalf("error writing inline_hot.pprof: %v", err)
+                       }
+
+                       for _, file := range []string{"inline_hot.go", "inline_hot_test.go"} {
+                               if err := copyFile(filepath.Join(dir, file), filepath.Join(srcDir, file)); err != nil {
+                                       t.Fatalf("error copying %s: %v", file, err)
+                               }
+                       }
+
+                       testPGOIntendedInlining(t, dir)
+               })
+       }
+}
+
 func copyFile(dst, src string) error {
        s, err := os.Open(src)
        if err != nil {