diff --git a/src/cmd/compile/internal/pgo/irgraph.go b/src/cmd/compile/internal/pgo/irgraph.go index 224f14368f..9ed16d224b 100644 --- a/src/cmd/compile/internal/pgo/irgraph.go +++ b/src/cmd/compile/internal/pgo/irgraph.go @@ -108,7 +108,6 @@ type NamedCallEdge struct { CallerName string CalleeName string CallSiteOffset int // Line offset from function start line. - CallStartLine int // Start line of the function. Can be 0 which means missing. } // NamedEdgeMap contains all unique call edges in the profile and their @@ -336,20 +335,19 @@ func createNamedEdgeMapFromPreprocess(r io.Reader) (edgeMap NamedEdgeMap, totalW split := strings.Split(readStr, " ") - if len(split) != 5 { - return NamedEdgeMap{}, 0, fmt.Errorf("preprocessed profile entry got %v want 5 fields", split) + if len(split) != 2 { + return NamedEdgeMap{}, 0, fmt.Errorf("preprocessed profile entry got %v want 2 fields", split) } co, _ := strconv.Atoi(split[0]) - cs, _ := strconv.Atoi(split[1]) namedEdge := NamedCallEdge{ CallerName: callerName, - CallSiteOffset: co - cs, + CalleeName: calleeName, + CallSiteOffset: co, } - namedEdge.CalleeName = calleeName - EWeight, _ := strconv.ParseInt(split[4], 10, 64) + EWeight, _ := strconv.ParseInt(split[1], 10, 64) weight[namedEdge] += EWeight totalWeight += EWeight diff --git a/src/cmd/compile/internal/test/pgo_devirtualize_test.go b/src/cmd/compile/internal/test/pgo_devirtualize_test.go index f451243683..af09107dc0 100644 --- a/src/cmd/compile/internal/test/pgo_devirtualize_test.go +++ b/src/cmd/compile/internal/test/pgo_devirtualize_test.go @@ -19,8 +19,11 @@ type devirtualization struct { callee string } +const profFileName = "devirt.pprof" +const preProfFileName = "devirt.pprof.node_map" + // testPGODevirtualize tests that specific PGO devirtualize rewrites are performed. -func testPGODevirtualize(t *testing.T, dir string, want []devirtualization) { +func testPGODevirtualize(t *testing.T, dir string, want []devirtualization, pgoProfileName string) { testenv.MustHaveGoRun(t) t.Parallel() @@ -45,7 +48,7 @@ go 1.21 } // Build the test with the profile. - pprof := filepath.Join(dir, "devirt.pprof") + pprof := filepath.Join(dir, pgoProfileName) gcflag := fmt.Sprintf("-gcflags=-m=2 -pgoprofile=%s -d=pgodebug=3", pprof) out := filepath.Join(dir, "test.exe") cmd = testenv.CleanCmdEnv(testenv.Command(t, testenv.GoToolPath(t), "test", "-o", out, gcflag, ".")) @@ -126,7 +129,7 @@ func TestPGODevirtualize(t *testing.T) { if err := os.Mkdir(filepath.Join(dir, "mult.pkg"), 0755); err != nil { t.Fatalf("error creating dir: %v", err) } - for _, file := range []string{"devirt.go", "devirt_test.go", "devirt.pprof", filepath.Join("mult.pkg", "mult.go")} { + for _, file := range []string{"devirt.go", "devirt_test.go", profFileName, filepath.Join("mult.pkg", "mult.go")} { if err := copyFile(filepath.Join(dir, file), filepath.Join(srcDir, file)); err != nil { t.Fatalf("error copying %s: %v", file, err) } @@ -172,7 +175,70 @@ func TestPGODevirtualize(t *testing.T) { //}, } - testPGODevirtualize(t, dir, want) + testPGODevirtualize(t, dir, want, profFileName) +} + +// TestPGOPreprocessDevirtualize tests that specific functions are devirtualized when PGO +// is applied to the exact source that was profiled. The input profile is PGO preprocessed file. +func TestPGOPreprocessDevirtualize(t *testing.T) { + wd, err := os.Getwd() + if err != nil { + t.Fatalf("error getting wd: %v", err) + } + srcDir := filepath.Join(wd, "testdata", "pgo", "devirtualize") + + // Copy the module to a scratch location so we can add a go.mod. + dir := t.TempDir() + if err := os.Mkdir(filepath.Join(dir, "mult.pkg"), 0755); err != nil { + t.Fatalf("error creating dir: %v", err) + } + for _, file := range []string{"devirt.go", "devirt_test.go", preProfFileName, filepath.Join("mult.pkg", "mult.go")} { + if err := copyFile(filepath.Join(dir, file), filepath.Join(srcDir, file)); err != nil { + t.Fatalf("error copying %s: %v", file, err) + } + } + + want := []devirtualization{ + // ExerciseIface + { + pos: "./devirt.go:101:20", + callee: "mult.Mult.Multiply", + }, + { + pos: "./devirt.go:101:39", + callee: "Add.Add", + }, + // ExerciseFuncConcrete + { + pos: "./devirt.go:173:36", + callee: "AddFn", + }, + { + pos: "./devirt.go:173:15", + callee: "mult.MultFn", + }, + // ExerciseFuncField + { + pos: "./devirt.go:207:35", + callee: "AddFn", + }, + { + pos: "./devirt.go:207:19", + callee: "mult.MultFn", + }, + // ExerciseFuncClosure + // TODO(prattmic): Closure callees not implemented. + //{ + // pos: "./devirt.go:249:27", + // callee: "AddClosure.func1", + //}, + //{ + // pos: "./devirt.go:249:15", + // callee: "mult.MultClosure.func1", + //}, + } + + testPGODevirtualize(t, dir, want, preProfFileName) } // Regression test for https://go.dev/issue/65615. If a target function changes @@ -190,7 +256,7 @@ func TestLookupFuncGeneric(t *testing.T) { if err := os.Mkdir(filepath.Join(dir, "mult.pkg"), 0755); err != nil { t.Fatalf("error creating dir: %v", err) } - for _, file := range []string{"devirt.go", "devirt_test.go", "devirt.pprof", filepath.Join("mult.pkg", "mult.go")} { + for _, file := range []string{"devirt.go", "devirt_test.go", profFileName, filepath.Join("mult.pkg", "mult.go")} { if err := copyFile(filepath.Join(dir, file), filepath.Join(srcDir, file)); err != nil { t.Fatalf("error copying %s: %v", file, err) } @@ -238,7 +304,7 @@ func TestLookupFuncGeneric(t *testing.T) { //}, } - testPGODevirtualize(t, dir, want) + testPGODevirtualize(t, dir, want, profFileName) } var multFnRe = regexp.MustCompile(`func MultFn\(a, b int64\) int64`) diff --git a/src/cmd/compile/internal/test/pgo_inl_test.go b/src/cmd/compile/internal/test/pgo_inl_test.go index 3aafaee197..7d665655d5 100644 --- a/src/cmd/compile/internal/test/pgo_inl_test.go +++ b/src/cmd/compile/internal/test/pgo_inl_test.go @@ -18,6 +18,9 @@ import ( "testing" ) +const profFile = "inline_hot.pprof" +const preProfFile = "inline_hot.pprof.node_map" + func buildPGOInliningTest(t *testing.T, dir string, gcflag string) []byte { const pkg = "example.com/pgo/inline" @@ -43,12 +46,7 @@ go 1.19 } // testPGOIntendedInlining tests that specific functions are inlined. -func testPGOIntendedInlining(t *testing.T, dir string, preprocessed ...bool) { - defaultPGOPackValue := false - if len(preprocessed) > 0 { - defaultPGOPackValue = preprocessed[0] - } - +func testPGOIntendedInlining(t *testing.T, dir string, profFile string) { testenv.MustHaveGoRun(t) t.Parallel() @@ -91,13 +89,7 @@ func testPGOIntendedInlining(t *testing.T, dir string, preprocessed ...bool) { // Build the test with the profile. Use a smaller threshold to test. // TODO: maybe adjust the test to work with default threshold. - var pprof string - if defaultPGOPackValue == false { - pprof = filepath.Join(dir, "inline_hot.pprof") - } else { - pprof = filepath.Join(dir, "inline_hot.pprof.node_map") - } - gcflag := fmt.Sprintf("-m -m -pgoprofile=%s -d=pgoinlinebudget=160,pgoinlinecdfthreshold=90", pprof) + gcflag := fmt.Sprintf("-m -m -pgoprofile=%s -d=pgoinlinebudget=160,pgoinlinecdfthreshold=90", profFile) out := buildPGOInliningTest(t, dir, gcflag) scanner := bufio.NewScanner(bytes.NewReader(out)) @@ -165,13 +157,13 @@ func TestPGOIntendedInlining(t *testing.T) { // Copy the module to a scratch location so we can add a go.mod. dir := t.TempDir() - for _, file := range []string{"inline_hot.go", "inline_hot_test.go", "inline_hot.pprof"} { + for _, file := range []string{"inline_hot.go", "inline_hot_test.go", profFile} { if err := copyFile(filepath.Join(dir, file), filepath.Join(srcDir, file)); err != nil { t.Fatalf("error copying %s: %v", file, err) } } - testPGOIntendedInlining(t, dir) + testPGOIntendedInlining(t, dir, profFile) } // TestPGOIntendedInlining tests that specific functions are inlined when PGO @@ -186,13 +178,13 @@ func TestPGOPreprocessInlining(t *testing.T) { // Copy the module to a scratch location so we can add a go.mod. dir := t.TempDir() - for _, file := range []string{"inline_hot.go", "inline_hot_test.go", "inline_hot.pprof.node_map"} { + for _, file := range []string{"inline_hot.go", "inline_hot_test.go", preProfFile} { if err := copyFile(filepath.Join(dir, file), filepath.Join(srcDir, file)); err != nil { t.Fatalf("error copying %s: %v", file, err) } } - testPGOIntendedInlining(t, dir, true) + testPGOIntendedInlining(t, dir, preProfFile) } // TestPGOIntendedInlining tests that specific functions are inlined when PGO @@ -208,7 +200,7 @@ func TestPGOIntendedInliningShiftedLines(t *testing.T) { dir := t.TempDir() // Copy most of the files unmodified. - for _, file := range []string{"inline_hot_test.go", "inline_hot.pprof"} { + for _, file := range []string{"inline_hot_test.go", profFile} { if err := copyFile(filepath.Join(dir, file), filepath.Join(srcDir, file)); err != nil { t.Fatalf("error copying %s : %v", file, err) } @@ -240,7 +232,7 @@ func TestPGOIntendedInliningShiftedLines(t *testing.T) { dst.Close() - testPGOIntendedInlining(t, dir) + testPGOIntendedInlining(t, dir, profFile) } // TestPGOSingleIndex tests that the sample index can not be 1 and compilation @@ -270,15 +262,15 @@ func TestPGOSingleIndex(t *testing.T) { // 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")) + originalPprofFile, err := os.Open(filepath.Join(srcDir, profFile)) if err != nil { - t.Fatalf("error opening inline_hot.pprof: %v", err) + t.Fatalf("error opening %v: %v", profFile, err) } defer originalPprofFile.Close() p, err := profile.Parse(originalPprofFile) if err != nil { - t.Fatalf("error parsing inline_hot.pprof: %v", err) + t.Fatalf("error parsing %v: %v", profFile, err) } // Move the samples count value-type to the 0 index. @@ -289,14 +281,14 @@ func TestPGOSingleIndex(t *testing.T) { s.Value = []int64{s.Value[tc.originalIndex]} } - modifiedPprofFile, err := os.Create(filepath.Join(dir, "inline_hot.pprof")) + modifiedPprofFile, err := os.Create(filepath.Join(dir, profFile)) if err != nil { - t.Fatalf("error creating inline_hot.pprof: %v", err) + t.Fatalf("error creating %v: %v", profFile, err) } defer modifiedPprofFile.Close() if err := p.Write(modifiedPprofFile); err != nil { - t.Fatalf("error writing inline_hot.pprof: %v", err) + t.Fatalf("error writing %v: %v", profFile, err) } for _, file := range []string{"inline_hot.go", "inline_hot_test.go"} { @@ -305,7 +297,7 @@ func TestPGOSingleIndex(t *testing.T) { } } - testPGOIntendedInlining(t, dir) + testPGOIntendedInlining(t, dir, profFile) }) } } @@ -343,13 +335,13 @@ func TestPGOHash(t *testing.T) { // Copy the module to a scratch location so we can add a go.mod. dir := t.TempDir() - for _, file := range []string{"inline_hot.go", "inline_hot_test.go", "inline_hot.pprof"} { + for _, file := range []string{"inline_hot.go", "inline_hot_test.go", profFile} { if err := copyFile(filepath.Join(dir, file), filepath.Join(srcDir, file)); err != nil { t.Fatalf("error copying %s: %v", file, err) } } - pprof := filepath.Join(dir, "inline_hot.pprof") + pprof := filepath.Join(dir, profFile) // build with -trimpath so the source location (thus the hash) // does not depend on the temporary directory path. gcflag0 := fmt.Sprintf("-pgoprofile=%s -trimpath %s=>%s -d=pgoinlinebudget=160,pgoinlinecdfthreshold=90,pgodebug=1", pprof, dir, pkg) diff --git a/src/cmd/compile/internal/test/testdata/pgo/devirtualize/devirt.pprof.node_map b/src/cmd/compile/internal/test/testdata/pgo/devirtualize/devirt.pprof.node_map new file mode 100644 index 0000000000..c55f990e84 --- /dev/null +++ b/src/cmd/compile/internal/test/testdata/pgo/devirtualize/devirt.pprof.node_map @@ -0,0 +1,52 @@ +GO PREPROFILE V1 +example.com/pgo/devirtualize.ExerciseFuncClosure +example.com/pgo/devirtualize/mult%2epkg.MultClosure.func1 +18 93 +example.com/pgo/devirtualize.ExerciseIface +example.com/pgo/devirtualize/mult%2epkg.NegMult.Multiply +49 4 +example.com/pgo/devirtualize.ExerciseFuncConcrete +example.com/pgo/devirtualize.AddFn +48 103 +example.com/pgo/devirtualize.ExerciseFuncField +example.com/pgo/devirtualize/mult%2epkg.NegMultFn +23 8 +example.com/pgo/devirtualize.ExerciseFuncField +example.com/pgo/devirtualize/mult%2epkg.MultFn +23 94 +example.com/pgo/devirtualize.ExerciseIface +example.com/pgo/devirtualize/mult%2epkg.Mult.Multiply +49 40 +example.com/pgo/devirtualize.ExerciseIface +example.com/pgo/devirtualize.Add.Add +49 55 +example.com/pgo/devirtualize.ExerciseFuncConcrete +example.com/pgo/devirtualize/mult%2epkg.NegMultFn +48 8 +example.com/pgo/devirtualize.ExerciseFuncClosure +example.com/pgo/devirtualize/mult%2epkg.NegMultClosure.func1 +18 10 +example.com/pgo/devirtualize.ExerciseIface +example.com/pgo/devirtualize.Sub.Add +49 7 +example.com/pgo/devirtualize.ExerciseFuncField +example.com/pgo/devirtualize.AddFn +23 101 +example.com/pgo/devirtualize.ExerciseFuncField +example.com/pgo/devirtualize.SubFn +23 12 +example.com/pgo/devirtualize.BenchmarkDevirtFuncConcrete +example.com/pgo/devirtualize.ExerciseFuncConcrete +1 2 +example.com/pgo/devirtualize.ExerciseFuncConcrete +example.com/pgo/devirtualize/mult%2epkg.MultFn +48 91 +example.com/pgo/devirtualize.ExerciseFuncConcrete +example.com/pgo/devirtualize.SubFn +48 5 +example.com/pgo/devirtualize.ExerciseFuncClosure +example.com/pgo/devirtualize.Add.Add +18 92 +example.com/pgo/devirtualize.ExerciseFuncClosure +example.com/pgo/devirtualize.Sub.Add +18 14 diff --git a/src/cmd/compile/internal/test/testdata/pgo/inline/inline_hot.pprof.node_map b/src/cmd/compile/internal/test/testdata/pgo/inline/inline_hot.pprof.node_map index bc5bc66b61..6e5f937a50 100644 --- a/src/cmd/compile/internal/test/testdata/pgo/inline/inline_hot.pprof.node_map +++ b/src/cmd/compile/internal/test/testdata/pgo/inline/inline_hot.pprof.node_map @@ -1,13 +1,13 @@ GO PREPROFILE V1 example.com/pgo/inline.benchmarkB example.com/pgo/inline.A -18 17 0 1 1 +18 1 example.com/pgo/inline.(*BS).NS example.com/pgo/inline.T -13 53 124 129 2 +8 3 example.com/pgo/inline.(*BS).NS example.com/pgo/inline.T -8 53 124 129 3 +13 2 example.com/pgo/inline.A example.com/pgo/inline.(*BS).NS -7 74 1 130 129 +7 129 diff --git a/src/cmd/preprofile/main.go b/src/cmd/preprofile/main.go index cf747266ca..806f25fee8 100644 --- a/src/cmd/preprofile/main.go +++ b/src/cmd/preprofile/main.go @@ -35,11 +35,11 @@ import ( // Header // caller_name // callee_name -// "call site offset" "caller's start line number" "flat" "cum" "call edge weight" +// "call site offset" "call edge weight" // ... // caller_name // callee_name -// "call site offset" "caller's start line number" "flat" "cum" "call edge weight" +// "call site offset" "call edge weight" func usage() { fmt.Fprintf(os.Stderr, "MUST have (pprof) input file \n") @@ -52,13 +52,6 @@ type NodeMapKey struct { CallerName string CalleeName string CallSiteOffset int // Line offset from function start line. - CallStartLine int // Start line of the function. Can be 0 which means missing. -} - -type Weights struct { - NFlat int64 - NCum int64 - EWeight int64 } func readPprofFile(profileFile string, outputFile string, verbose bool) bool { @@ -101,33 +94,19 @@ func readPprofFile(profileFile string, outputFile string, verbose bool) bool { SampleValue: func(v []int64) int64 { return v[valueIndex] }, }) - nFlat := make(map[string]int64) - nCum := make(map[string]int64) - - // Accummulate weights for the same node. - for _, n := range g.Nodes { - canonicalName := n.Info.Name - nFlat[canonicalName] += n.FlatValue() - nCum[canonicalName] += n.CumValue() - } - - TotalNodeWeight := int64(0) TotalEdgeWeight := int64(0) - NodeMap := make(map[NodeMapKey]*Weights) - NodeWeightMap := make(map[string]int64) + NodeMap := make(map[NodeMapKey]int64) for _, n := range g.Nodes { - TotalNodeWeight += n.FlatValue() canonicalName := n.Info.Name // Create the key to the nodeMapKey. nodeinfo := NodeMapKey{ CallerName: canonicalName, CallSiteOffset: n.Info.Lineno - n.Info.StartLine, - CallStartLine: n.Info.StartLine, } - if nodeinfo.CallStartLine == 0 { + if n.Info.StartLine == 0 { if verbose { log.Println("[PGO] warning: " + canonicalName + " relative line number is missing from the profile") } @@ -137,27 +116,14 @@ func readPprofFile(profileFile string, outputFile string, verbose bool) bool { TotalEdgeWeight += e.WeightValue() nodeinfo.CalleeName = e.Dest.Info.Name if w, ok := NodeMap[nodeinfo]; ok { - w.EWeight += e.WeightValue() + w += e.WeightValue() } else { - weights := new(Weights) - weights.NFlat = nFlat[canonicalName] - weights.NCum = nCum[canonicalName] - weights.EWeight = e.WeightValue() - NodeMap[nodeinfo] = weights + w = e.WeightValue() + NodeMap[nodeinfo] = w } } } - for _, n := range g.Nodes { - lineno := fmt.Sprintf("%v", n.Info.Lineno) - canonicalName := n.Info.Name + "-" + lineno - if _, ok := (NodeWeightMap)[canonicalName]; ok { - (NodeWeightMap)[canonicalName] += n.CumValue() - } else { - (NodeWeightMap)[canonicalName] = n.CumValue() - } - } - var fNodeMap *os.File if outputFile == "" { fNodeMap = os.Stdout @@ -190,16 +156,13 @@ func readPprofFile(profileFile string, outputFile string, verbose bool) bool { line = key.CalleeName + "\n" w.WriteString(line) line = strconv.Itoa(key.CallSiteOffset) - line = line + separator + strconv.Itoa(key.CallStartLine) - line = line + separator + strconv.FormatInt(element.NFlat, 10) - line = line + separator + strconv.FormatInt(element.NCum, 10) - line = line + separator + strconv.FormatInt(element.EWeight, 10) + "\n" + line = line + separator + strconv.FormatInt(element, 10) + "\n" w.WriteString(line) w.Flush() count += 1 } - if TotalNodeWeight == 0 || TotalEdgeWeight == 0 { + if TotalEdgeWeight == 0 { return false }