summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorFrederic Branczyk <fbranczyk@gmail.com>2023-02-08 17:59:27 +0000
committerDavid Chase <drchase@google.com>2023-02-10 19:24:45 +0000
commit3a04b6e12ef0e5a0c608f82051943408bd6f28bd (patch)
treeb43e5304dd123ca4ac79463f3a83ac0e18b38e98
parent00f5d3001a7e684263307ab39c64eba3c79f279c (diff)
downloadgo-git-3a04b6e12ef0e5a0c608f82051943408bd6f28bd.tar.gz
[release-branch.go1.20] 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. This is a redo of CL 465135, now allowing empty profiles. Note that preprocessProfileGraph will already cause pgo.New to return nil for empty profiles. For #58292 For #58309 Change-Id: Ia6c94f0793f6ca9b0882b5e2c4d34f38e600c1e3 Reviewed-on: https://go-review.googlesource.com/c/go/+/467375 Run-TryBot: Michael Pratt <mpratt@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Austin Clements <austin@google.com>
-rw-r--r--src/cmd/compile/internal/pgo/irgraph.go23
-rw-r--r--src/cmd/compile/internal/test/pgo_inl_test.go68
2 files changed, 90 insertions, 1 deletions
diff --git a/src/cmd/compile/internal/pgo/irgraph.go b/src/cmd/compile/internal/pgo/irgraph.go
index bf11e365f1..bb5df50e3a 100644
--- a/src/cmd/compile/internal/pgo/irgraph.go
+++ b/src/cmd/compile/internal/pgo/irgraph.go
@@ -140,9 +140,30 @@ func New(profileFile string) *Profile {
return nil
}
+ if len(profile.Sample) == 0 {
+ // We accept empty profiles, but there is nothing to do.
+ return nil
+ }
+
+ valueIndex := -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") {
+ valueIndex = i
+ break
+ }
+ }
+
+ if valueIndex == -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[valueIndex] },
})
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 2f6391fded..4d6b5a134a 100644
--- 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 {