diff options
author | Frederic Branczyk <fbranczyk@gmail.com> | 2023-02-08 17:59:27 +0000 |
---|---|---|
committer | David Chase <drchase@google.com> | 2023-02-10 19:24:45 +0000 |
commit | 3a04b6e12ef0e5a0c608f82051943408bd6f28bd (patch) | |
tree | b43e5304dd123ca4ac79463f3a83ac0e18b38e98 | |
parent | 00f5d3001a7e684263307ab39c64eba3c79f279c (diff) | |
download | go-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.go | 23 | ||||
-rw-r--r-- | src/cmd/compile/internal/test/pgo_inl_test.go | 68 |
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 { |