qbit/go
1
0
Fork 0
mirror of https://github.com/golang/go synced 2024-11-23 00:30:07 -07:00
Code Releases Activity

cmd/compile: update the incorrect assignment of call site offset.

Browse Source 
The call site calculation in the previous version is incorrect. For
the PGO preprocess file, the compiler should directly use the call
site offset value. Additionly, this change refactors the preprocess
tool to clean up unused fields including startline, the flat and the
cum.

Change-Id: I7bffed3215d4c016d9a9e4034bfd373bf50ab43f
Reviewed-on: https://go-review.googlesource.com/c/go/+/562795
Reviewed-by: Cherry Mui <cherryyz@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
This commit is contained in:
Jin Lin 2024-02-08 12:52:07 -08:00 committed by Cherry Mui
parent b6ca586181
commit 185f31bf30
6 changed files with 162 additions and 91 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

12
src/cmd/compile/internal/pgo/irgraph.go
View File

@ -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

78
src/cmd/compile/internal/test/pgo_devirtualize_test.go
View File

@ -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`)

48
src/cmd/compile/internal/test/pgo_inl_test.go
View File

@ -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)

52
src/cmd/compile/internal/test/testdata/pgo/devirtualize/devirt.pprof.node_map vendored Normal file
View File

@ -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

8
src/cmd/compile/internal/test/testdata/pgo/inline/inline_hot.pprof.node_map vendored
View File

@ -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

55
src/cmd/preprofile/main.go
View File

@ -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
}