Skip to content

Commit

Permalink
Revert "cmd/preprofile: Add preprocess tool to pre-parse the profile …
Browse files Browse the repository at this point in the history 
…file."

This reverts CL 529738.

Reason for revert: Breaking longtest builders

For golang#58102.
Fixes golang#65220.

Change-Id: Id295e3249da9d82f6a9e4fc571760302a1362def
Reviewed-on: https://go-review.googlesource.com/c/go/+/557460
Auto-Submit: Michael Pratt <mpratt@google.com>
Reviewed-by: Bryan Mills <bcmills@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
  • Loading branch information
@prattmic @ezz-no
prattmic authored and ezz-no committed Feb 17, 2024
1 parent b3b3ad4 commit 26b2a04
Show file tree
Hide file tree
Showing 6 changed files with 44 additions and 450 deletions.
2 changes: 1 addition & 1 deletion src/cmd/compile/internal/base/flag.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ type CmdFlags struct {
TraceProfile string "help:\"write an execution trace to `file`\""
TrimPath string "help:\"remove `prefix` from recorded source file paths\""
WB bool "help:\"enable write barrier\"" // TODO: remove
PgoProfile string "help:\"read profile or pre-process profile from `file`\""
PgoProfile string "help:\"read profile from `file`\""
ErrorURL bool "help:\"print explanatory URL with error message if applicable\""

// Configuration derived from flags; not a flag itself.
Expand Down
13 changes: 7 additions & 6 deletions src/internal/profile/graph.go → ...pile/internal/pgo/internal/graph/graph.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,11 @@
// Package graph represents a pprof profile as a directed graph.
//
// This package is a simplified fork of github.com/google/pprof/internal/graph.
package profile
package graph

import (
"fmt"
"internal/profile"
"sort"
"strings"
)
Expand Down Expand Up @@ -240,7 +241,7 @@ func (e *Edge) WeightValue() int64 {
}

// NewGraph computes a graph from a profile.
func NewGraph(prof *Profile, o *Options) *Graph {
func NewGraph(prof *profile.Profile, o *Options) *Graph {
nodes, locationMap := CreateNodes(prof, o)
seenNode := make(map[*Node]bool)
seenEdge := make(map[nodePair]bool)
Expand Down Expand Up @@ -367,13 +368,13 @@ func (l locationMap) get(id uint64) Nodes {
// CreateNodes creates graph nodes for all locations in a profile. It
// returns set of all nodes, plus a mapping of each location to the
// set of corresponding nodes (one per location.Line).
func CreateNodes(prof *Profile, o *Options) (Nodes, locationMap) {
func CreateNodes(prof *profile.Profile, o *Options) (Nodes, locationMap) {
locations := locationMap{make([]Nodes, len(prof.Location)+1), make(map[uint64]Nodes)}
nm := make(NodeMap, len(prof.Location))
for _, l := range prof.Location {
lines := l.Line
if len(lines) == 0 {
lines = []Line{{}} // Create empty line to include location info.
lines = []profile.Line{{}} // Create empty line to include location info.
}
nodes := make(Nodes, len(lines))
for ln := range lines {
Expand All @@ -392,7 +393,7 @@ func (nm NodeMap) nodes() Nodes {
return nodes
}

func (nm NodeMap) findOrInsertLine(l *Location, li Line, o *Options) *Node {
func (nm NodeMap) findOrInsertLine(l *profile.Location, li profile.Line, o *Options) *Node {
var objfile string
if m := l.Mapping; m != nil && m.File != "" {
objfile = m.File
Expand All @@ -404,7 +405,7 @@ func (nm NodeMap) findOrInsertLine(l *Location, li Line, o *Options) *Node {
return nil
}

func nodeInfo(l *Location, line Line, objfile string, o *Options) *NodeInfo {
func nodeInfo(l *profile.Location, line profile.Line, objfile string, o *Options) *NodeInfo {
if line.Function == nil {
return &NodeInfo{Address: l.Address}
}
Expand Down
207 changes: 34 additions & 173 deletions src/cmd/compile/internal/pgo/irgraph.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,20 +41,16 @@
package pgo

import (
"bufio"
"cmd/compile/internal/base"
"cmd/compile/internal/ir"
"cmd/compile/internal/pgo/internal/graph"
"cmd/compile/internal/typecheck"
"cmd/compile/internal/types"
"cmd/internal/bio"
"errors"
"fmt"
"internal/profile"
"log"
"os"
"sort"
"strconv"
"strings"
)

// IRGraph is a call graph with nodes pointing to IRs of functions and edges
Expand Down Expand Up @@ -109,7 +105,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
Expand Down Expand Up @@ -144,52 +139,8 @@ type Profile struct {
WeightedCG *IRGraph
}

var wantHdr = "GO PREPROFILE V1\n"

func isPreProfileFile(filename string) (bool, error) {
file, err := bio.Open(filename)
if err != nil {
return false, err
}
defer file.Close()

/* check the header */
line, err := file.ReadString('\n')
if err != nil {
return false, err
}

if wantHdr == line {
return true, nil
}
return false, nil
}

// New generates a profile-graph from the profile or pre-processed profile.
// New generates a profile-graph from the profile.
func New(profileFile string) (*Profile, error) {
var profile *Profile
var err error
isPreProf, err := isPreProfileFile(profileFile)
if err != nil {
return nil, fmt.Errorf("error opening profile: %w", err)
}
if !isPreProf {
profile, err = processProto(profileFile)
if err != nil {
log.Fatalf("%s: PGO error: %v", profileFile, err)
}
} else {
profile, err = processPreprof(profileFile)
if err != nil {
log.Fatalf("%s: Preprocessed PGO error: %v", profileFile, err)
}
}
return profile, nil

}

// processProto generates a profile-graph from the profile.
func processProto(profileFile string) (*Profile, error) {
f, err := os.Open(profileFile)
if err != nil {
return nil, fmt.Errorf("error opening profile: %w", err)
Expand Down Expand Up @@ -224,7 +175,7 @@ func processProto(profileFile string) (*Profile, error) {
return nil, fmt.Errorf(`profile does not contain a sample index with value/type "samples/count" or cpu/nanoseconds"`)
}

g := profile.NewGraph(p, &profile.Options{
g := graph.NewGraph(p, &graph.Options{
SampleValue: func(v []int64) int64 { return v[valueIndex] },
})

Expand All @@ -247,130 +198,11 @@ func processProto(profileFile string) (*Profile, error) {
}, nil
}

// processPreprof generates a profile-graph from the pre-procesed profile.
func processPreprof(preprofileFile string) (*Profile, error) {
namedEdgeMap, totalWeight, err := createNamedEdgeMapFromPreprocess(preprofileFile)
if err != nil {
return nil, err
}

if totalWeight == 0 {
return nil, nil // accept but ignore profile with no samples.
}

// Create package-level call graph with weights from profile and IR.
wg := createIRGraph(namedEdgeMap)

return &Profile{
TotalWeight: totalWeight,
NamedEdgeMap: namedEdgeMap,
WeightedCG: wg,
}, nil
}

func postProcessNamedEdgeMap(weight map[NamedCallEdge]int64, weightVal int64) (edgeMap NamedEdgeMap, totalWeight int64, err error) {
if weightVal == 0 {
return NamedEdgeMap{}, 0, nil // accept but ignore profile with no samples.
}
byWeight := make([]NamedCallEdge, 0, len(weight))
for namedEdge := range weight {
byWeight = append(byWeight, namedEdge)
}
sort.Slice(byWeight, func(i, j int) bool {
ei, ej := byWeight[i], byWeight[j]
if wi, wj := weight[ei], weight[ej]; wi != wj {
return wi > wj // want larger weight first
}
// same weight, order by name/line number
if ei.CallerName != ej.CallerName {
return ei.CallerName < ej.CallerName
}
if ei.CalleeName != ej.CalleeName {
return ei.CalleeName < ej.CalleeName
}
return ei.CallSiteOffset < ej.CallSiteOffset
})

edgeMap = NamedEdgeMap{
Weight: weight,
ByWeight: byWeight,
}

totalWeight = weightVal

return edgeMap, totalWeight, nil
}

// restore NodeMap information from a preprocessed profile.
// The reader can refer to the format of preprocessed profile in cmd/preprofile/main.go.
func createNamedEdgeMapFromPreprocess(preprofileFile string) (edgeMap NamedEdgeMap, totalWeight int64, err error) {
readFile, err := os.Open(preprofileFile)
if err != nil {
log.Fatal("preprofile: failed to open file " + preprofileFile)
return
}
defer readFile.Close()

fileScanner := bufio.NewScanner(readFile)
fileScanner.Split(bufio.ScanLines)
weight := make(map[NamedCallEdge]int64)

if !fileScanner.Scan() {
log.Fatal("fail to parse preprocessed profile: missing header")
return
}
if fileScanner.Text()+"\n" != wantHdr {
log.Fatal("fail to parse preprocessed profile: mismatched header")
return
}

for fileScanner.Scan() {
readStr := fileScanner.Text()

callerName := readStr

if !fileScanner.Scan() {
log.Fatal("fail to parse preprocessed profile: missing callee")
return
}
calleeName := fileScanner.Text()

if !fileScanner.Scan() {
log.Fatal("fail to parse preprocessed profile: missing weight")
return
}
readStr = fileScanner.Text()

split := strings.Split(readStr, " ")

if len(split) == 5 {
co, _ := strconv.Atoi(split[0])
cs, _ := strconv.Atoi(split[1])

namedEdge := NamedCallEdge{
CallerName: callerName,
CallSiteOffset: co - cs,
}

namedEdge.CalleeName = calleeName
EWeight, _ := strconv.ParseInt(split[4], 10, 64)

weight[namedEdge] += EWeight
totalWeight += EWeight
} else {
log.Fatal("fail to parse preprocessed profile: mismatched fields.\n")
}
}

return postProcessNamedEdgeMap(weight, totalWeight)

}

// createNamedEdgeMap builds a map of callsite-callee edge weights from the
// profile-graph.
//
// Caller should ignore the profile if totalWeight == 0.
func createNamedEdgeMap(g *profile.Graph) (edgeMap NamedEdgeMap, totalWeight int64, err error) {
func createNamedEdgeMap(g *graph.Graph) (edgeMap NamedEdgeMap, totalWeight int64, err error) {
seenStartLine := false

// Process graph and build various node and edge maps which will
Expand All @@ -394,13 +226,42 @@ func createNamedEdgeMap(g *profile.Graph) (edgeMap NamedEdgeMap, totalWeight int
}
}

if totalWeight == 0 {
return NamedEdgeMap{}, 0, nil // accept but ignore profile with no samples.
}

if !seenStartLine {
// TODO(prattmic): If Function.start_line is missing we could
// fall back to using absolute line numbers, which is better
// than nothing.
return NamedEdgeMap{}, 0, fmt.Errorf("profile missing Function.start_line data (Go version of profiled application too old? Go 1.20+ automatically adds this to profiles)")
}
return postProcessNamedEdgeMap(weight, totalWeight)

byWeight := make([]NamedCallEdge, 0, len(weight))
for namedEdge := range weight {
byWeight = append(byWeight, namedEdge)
}
sort.Slice(byWeight, func(i, j int) bool {
ei, ej := byWeight[i], byWeight[j]
if wi, wj := weight[ei], weight[ej]; wi != wj {
return wi > wj // want larger weight first
}
// same weight, order by name/line number
if ei.CallerName != ej.CallerName {
return ei.CallerName < ej.CallerName
}
if ei.CalleeName != ej.CalleeName {
return ei.CalleeName < ej.CalleeName
}
return ei.CallSiteOffset < ej.CallSiteOffset
})

edgeMap = NamedEdgeMap{
Weight: weight,
ByWeight: byWeight,
}

return edgeMap, totalWeight, nil
}

// initializeIRGraph builds the IRGraph by visiting all the ir.Func in decl list
Expand Down
35 changes: 2 additions & 33 deletions src/cmd/compile/internal/test/pgo_inl_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,12 +43,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) {
testenv.MustHaveGoRun(t)
t.Parallel()

Expand Down Expand Up @@ -91,12 +86,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")
}
pprof := filepath.Join(dir, "inline_hot.pprof")
gcflag := fmt.Sprintf("-m -m -pgoprofile=%s -d=pgoinlinebudget=160,pgoinlinecdfthreshold=90", pprof)
out := buildPGOInliningTest(t, dir, gcflag)

Expand Down Expand Up @@ -174,27 +164,6 @@ func TestPGOIntendedInlining(t *testing.T) {
testPGOIntendedInlining(t, dir)
}

// TestPGOIntendedInlining tests that specific functions are inlined when PGO
// is applied to the exact source that was profiled.
func TestPGOPreprocessInlining(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()

for _, file := range []string{"inline_hot.go", "inline_hot_test.go", "inline_hot.pprof.node_map"} {
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 tests that specific functions are inlined when PGO
// is applied to the modified source.
func TestPGOIntendedInliningShiftedLines(t *testing.T) {
Expand Down
Loading

0 comments on commit 26b2a04

Please sign in to comment.