diff --git a/src/cmd/compile/internal/inline/inl.go b/src/cmd/compile/internal/inline/inl.go index 4d942d037fc..7e1a9adae87 100644 --- a/src/cmd/compile/internal/inline/inl.go +++ b/src/cmd/compile/internal/inline/inl.go @@ -441,10 +441,10 @@ func (v *hairyVisitor) doNode(n ir.Node) bool { // Determine if the callee edge is a for hot callee or not. if base.Flag.PgoProfile != "" && pgo.WeightedCG != nil && v.curFunc != nil { if fn := inlCallee(n.X); fn != nil && typecheck.HaveInlineBody(fn) { - ln := pgo.ConvertLine2Int(ir.Line(n)) - csi := pgo.CallSiteInfo{Line: ln, Caller: v.curFunc, Callee: fn} + line := int(base.Ctxt.InnermostPos(n.Pos()).RelLine()) + csi := pgo.CallSiteInfo{Line: line, Caller: v.curFunc, Callee: fn} if _, o := candHotEdgeMap[csi]; o { - pgo.ListOfHotCallSites[pgo.CallSiteInfo{Line: ln, Caller: v.curFunc}] = struct{}{} + pgo.ListOfHotCallSites[pgo.CallSiteInfo{Line: line, Caller: v.curFunc}] = struct{}{} if base.Debug.PGOInline > 0 { fmt.Printf("hot-callsite identified at line=%v for func=%v\n", ir.Line(n), ir.PkgFuncName(v.curFunc)) } @@ -873,8 +873,8 @@ func mkinlcall(n *ir.CallExpr, fn *ir.Func, maxCost int32, inlCalls *[]*ir.Inlin } if fn.Inl.Cost > maxCost { // If the callsite is hot and it is under the inlineHotMaxBudget budget, then try to inline it, or else bail. - ln := pgo.ConvertLine2Int(ir.Line(n)) - csi := pgo.CallSiteInfo{Line: ln, Caller: ir.CurFunc} + line := int(base.Ctxt.InnermostPos(n.Pos()).RelLine()) + csi := pgo.CallSiteInfo{Line: line, Caller: ir.CurFunc} if _, ok := pgo.ListOfHotCallSites[csi]; ok { if fn.Inl.Cost > inlineHotMaxBudget { if logopt.Enabled() { @@ -1038,8 +1038,8 @@ func mkinlcall(n *ir.CallExpr, fn *ir.Func, maxCost int32, inlCalls *[]*ir.Inlin } if base.Debug.PGOInline > 0 { - ln := pgo.ConvertLine2Int(ir.Line(n)) - csi := pgo.CallSiteInfo{Line: ln, Caller: ir.CurFunc} + line := int(base.Ctxt.InnermostPos(n.Pos()).RelLine()) + csi := pgo.CallSiteInfo{Line: line, Caller: ir.CurFunc} if _, ok := inlinedCallSites[csi]; !ok { inlinedCallSites[csi] = struct{}{} } diff --git a/src/cmd/compile/internal/pgo/irgraph.go b/src/cmd/compile/internal/pgo/irgraph.go index 98238823f7f..e6e183cdd22 100644 --- a/src/cmd/compile/internal/pgo/irgraph.go +++ b/src/cmd/compile/internal/pgo/irgraph.go @@ -4,9 +4,45 @@ // WORK IN PROGRESS +// A note on line numbers: when working with line numbers, we always use the +// binary-visible relative line number. i.e., the line number as adjusted by +// //line directives (ctxt.InnermostPos(ir.Node.Pos()).RelLine()). +// +// If you are thinking, "wait, doesn't that just make things more complex than +// using the real line number?", then you are 100% correct. Unfortunately, +// pprof profiles generated by the runtime always contain line numbers as +// adjusted by //line directives (because that is what we put in pclntab). Thus +// for the best behavior when attempting to match the source with the profile +// it makes sense to use the same line number space. +// +// Some of the effects of this to keep in mind: +// +// - For files without //line directives there is no impact, as RelLine() == +// Line(). +// - For functions entirely covered by the same //line directive (i.e., a +// directive before the function definition and no directives within the +// function), there should also be no impact, as line offsets within the +// function should be the same as the real line offsets. +// - Functions containing //line directives may be impacted. As fake line +// numbers need not be monotonic, we may compute negative line offsets. We +// should accept these and attempt to use them for best-effort matching, as +// these offsets should still match if the source is unchanged, and may +// continue to match with changed source depending on the impact of the +// changes on fake line numbers. +// - Functions containing //line directives may also contain duplicate lines, +// making it ambiguous which call the profile is referencing. This is a +// similar problem to multiple calls on a single real line, as we don't +// currently track column numbers. +// +// Long term it would be best to extend pprof profiles to include real line +// numbers. Until then, we have to live with these complexities. Luckily, +// //line directives that change line numbers in strange ways should be rare, +// and failing PGO matching on these files is not too big of a loss. + package pgo import ( + "cmd/compile/internal/base" "cmd/compile/internal/ir" "cmd/compile/internal/typecheck" "cmd/compile/internal/types" @@ -14,8 +50,6 @@ import ( "internal/profile" "log" "os" - "strconv" - "strings" ) // IRGraph is the key datastrcture that is built from profile. It is essentially a call graph with nodes pointing to IRs of functions and edges carrying weights and callsite information. The graph is bidirectional that helps in removing nodes efficiently. @@ -131,13 +165,6 @@ func BuildWeightedCallGraph() { WeightedCG = createIRGraph() } -// ConvertLine2Int converts ir.Line string to integer. -func ConvertLine2Int(line string) int { - splits := strings.Split(line, ":") - cs, _ := strconv.ParseInt(splits[len(splits)-2], 0, 64) - return int(cs) -} - // preprocessProfileGraph builds various maps from the profile-graph. It builds GlobalNodeMap and other information based on the name and callsite to compute node and edge weights which will be used later on to create edges for WeightedCG. func preprocessProfileGraph() { nFlat := make(map[string]int64) @@ -284,7 +311,7 @@ func (g *IRGraph) createIRGraphEdge(fn *ir.Func, callernode *IRNode, name string ir.DoChildren(n, doNode) case ir.OCALLFUNC: call := n.(*ir.CallExpr) - line := ConvertLine2Int(ir.Line(n)) + line := int(base.Ctxt.InnermostPos(n.Pos()).RelLine()) // Find the callee function from the call site and add the edge. f := inlCallee(call.X) if f != nil { @@ -294,7 +321,7 @@ func (g *IRGraph) createIRGraphEdge(fn *ir.Func, callernode *IRNode, name string call := n.(*ir.CallExpr) // Find the callee method from the call site and add the edge. fn2 := ir.MethodExprName(call.X).Func - line := ConvertLine2Int(ir.Line(n)) + line := int(base.Ctxt.InnermostPos(n.Pos()).RelLine()) g.addEdge(callernode, fn2, &n, name, line) } return false