Skip to content

Commit 7180a09

Browse files
branczprattmic
authored andcommitted
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. Fixes #58292 Change-Id: Ia6c94f0793f6ca9b0882b5e2c4d34f38e600c1e3 Reviewed-on: https://go-review.googlesource.com/c/go/+/466675 Run-TryBot: Michael Pratt <mpratt@google.com> Reviewed-by: Austin Clements <austin@google.com> TryBot-Result: Gopher Robot <gobot@golang.org>
1 parent ae2f120 commit 7180a09

File tree

2 files changed

+90
-1
lines changed

2 files changed

+90
-1
lines changed

src/cmd/compile/internal/pgo/irgraph.go

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -140,9 +140,30 @@ func New(profileFile string) *Profile {
140140
return nil
141141
}
142142

143+
if len(profile.Sample) == 0 {
144+
// We accept empty profiles, but there is nothing to do.
145+
return nil
146+
}
147+
148+
valueIndex := -1
149+
for i, s := range profile.SampleType {
150+
// Samples count is the raw data collected, and CPU nanoseconds is just
151+
// a scaled version of it, so either one we can find is fine.
152+
if (s.Type == "samples" && s.Unit == "count") ||
153+
(s.Type == "cpu" && s.Unit == "nanoseconds") {
154+
valueIndex = i
155+
break
156+
}
157+
}
158+
159+
if valueIndex == -1 {
160+
log.Fatal("failed to find CPU samples count or CPU nanoseconds value-types in profile.")
161+
return nil
162+
}
163+
143164
g := newGraph(profile, &Options{
144165
CallTree: false,
145-
SampleValue: func(v []int64) int64 { return v[1] },
166+
SampleValue: func(v []int64) int64 { return v[valueIndex] },
146167
})
147168

148169
p := &Profile{

src/cmd/compile/internal/test/pgo_inl_test.go

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ package test
77
import (
88
"bufio"
99
"fmt"
10+
"internal/profile"
1011
"internal/testenv"
1112
"io"
1213
"os"
@@ -213,6 +214,73 @@ func TestPGOIntendedInliningShiftedLines(t *testing.T) {
213214
testPGOIntendedInlining(t, dir)
214215
}
215216

217+
// TestPGOSingleIndex tests that the sample index can not be 1 and compilation
218+
// will not fail. All it should care about is that the sample type is either
219+
// CPU nanoseconds or samples count, whichever it finds first.
220+
func TestPGOSingleIndex(t *testing.T) {
221+
for _, tc := range []struct {
222+
originalIndex int
223+
}{{
224+
// The `testdata/pgo/inline/inline_hot.pprof` file is a standard CPU
225+
// profile as the runtime would generate. The 0 index contains the
226+
// value-type samples and value-unit count. The 1 index contains the
227+
// value-type cpu and value-unit nanoseconds. These tests ensure that
228+
// the compiler can work with profiles that only have a single index,
229+
// but are either samples count or CPU nanoseconds.
230+
originalIndex: 0,
231+
}, {
232+
originalIndex: 1,
233+
}} {
234+
t.Run(fmt.Sprintf("originalIndex=%d", tc.originalIndex), func(t *testing.T) {
235+
wd, err := os.Getwd()
236+
if err != nil {
237+
t.Fatalf("error getting wd: %v", err)
238+
}
239+
srcDir := filepath.Join(wd, "testdata/pgo/inline")
240+
241+
// Copy the module to a scratch location so we can add a go.mod.
242+
dir := t.TempDir()
243+
244+
originalPprofFile, err := os.Open(filepath.Join(srcDir, "inline_hot.pprof"))
245+
if err != nil {
246+
t.Fatalf("error opening inline_hot.pprof: %v", err)
247+
}
248+
defer originalPprofFile.Close()
249+
250+
p, err := profile.Parse(originalPprofFile)
251+
if err != nil {
252+
t.Fatalf("error parsing inline_hot.pprof: %v", err)
253+
}
254+
255+
// Move the samples count value-type to the 0 index.
256+
p.SampleType = []*profile.ValueType{p.SampleType[tc.originalIndex]}
257+
258+
// Ensure we only have a single set of sample values.
259+
for _, s := range p.Sample {
260+
s.Value = []int64{s.Value[tc.originalIndex]}
261+
}
262+
263+
modifiedPprofFile, err := os.Create(filepath.Join(dir, "inline_hot.pprof"))
264+
if err != nil {
265+
t.Fatalf("error creating inline_hot.pprof: %v", err)
266+
}
267+
defer modifiedPprofFile.Close()
268+
269+
if err := p.Write(modifiedPprofFile); err != nil {
270+
t.Fatalf("error writing inline_hot.pprof: %v", err)
271+
}
272+
273+
for _, file := range []string{"inline_hot.go", "inline_hot_test.go"} {
274+
if err := copyFile(filepath.Join(dir, file), filepath.Join(srcDir, file)); err != nil {
275+
t.Fatalf("error copying %s: %v", file, err)
276+
}
277+
}
278+
279+
testPGOIntendedInlining(t, dir)
280+
})
281+
}
282+
}
283+
216284
func copyFile(dst, src string) error {
217285
s, err := os.Open(src)
218286
if err != nil {

0 commit comments

Comments
 (0)