cmd/compile/internal/pgo: remove ListOfHotCallSites

The global ListOfHotCallSites set is used to communicate between
CanInline and InlineCalls the set of call sites that InlineCalls may
increase the budget for.

CanInline clears this map on each call, thus assuming that
InlineCalls(x) is called immediately after CanInline(x). This assumption
is false, as CanInline (among other cases) is recursive (CanInline ->
hairyVisitor.doNode -> inlCallee -> CanInline).

When this assumption proves false, we will lose the opportunity to
inline hot calls.

This CL is the least invasive fix for this. ListOfHotCallSites is
actually just a subset of the candHotEdgeMap, with CallSiteInfo.Callee
cleared. candHotEdgeMap doesn't actually need to distinguish based on
Callee, so we can drop callee from candHotEdgeMap as well and just use
that directly [1].

Later CLs should do more work to remove the globals entirely.

For cmd/compile, this inceases the number of PGO inlined functions by
~50% for one set of PGO parameters. I have no evaluated performance
impact.

[1] This is something that we likely want to change in the future.

For #55022.

Change-Id: I57735958d651f6dfa9bd296499841213d20e1706
Reviewed-on: https://go-review.googlesource.com/c/go/+/446755
Auto-Submit: Michael Pratt <mpratt@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Run-TryBot: Michael Pratt <mpratt@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>

diff --git a/src/cmd/compile/internal/inline/inl.go b/src/cmd/compile/internal/inline/inl.go
index 75f3a4b9070a12a1c7d9da8cb5aaa11e608f7bef..335bb23ecb91fa3236b69b4a75e5900bf14becd8 100644 (file)
--- a/src/cmd/compile/internal/inline/inl.go
+++ b/src/cmd/compile/internal/inline/inl.go
@@ -56,13 +56,16 @@ const (
 )
 
 var (
-       // List of all hot ndes.
+       // List of all hot nodes.
+       // TODO(prattmic): Make this non-global.
        candHotNodeMap = make(map[*pgo.IRNode]struct{})
 
-       // List of all hot call sites.
+       // List of all hot call sites. CallSiteInfo.Callee is always nil.
+       // TODO(prattmic): Make this non-global.
        candHotEdgeMap = make(map[pgo.CallSiteInfo]struct{})
 
-       // List of inlined call sites.
+       // List of inlined call sites. CallSiteInfo.Callee is always nil.
+       // TODO(prattmic): Make this non-global.
        inlinedCallSites = make(map[pgo.CallSiteInfo]struct{})
 
        // Threshold in percentage for hot function inlining.
@@ -107,7 +110,7 @@ func pgoInlinePrologue(p *pgo.Profile) {
                                        if e.Weight != 0 {
                                                edgeweightpercent := pgo.WeightInPercentage(e.Weight, p.TotalEdgeWeight)
                                                if edgeweightpercent > inlineHotCallSiteThresholdPercent {
-                                                       csi := pgo.CallSiteInfo{Line: e.CallSite, Caller: n.AST, Callee: e.Dst.AST}
+                                                       csi := pgo.CallSiteInfo{Line: e.CallSite, Caller: n.AST}
                                                        if _, ok := candHotEdgeMap[csi]; !ok {
                                                                candHotEdgeMap[csi] = struct{}{}
                                                        }
@@ -176,9 +179,6 @@ func CanInline(fn *ir.Func, profile *pgo.Profile) {
                base.Fatalf("CanInline no nname %+v", fn)
        }
 
-       // Initialize an empty list of hot callsites for this caller.
-       pgo.ListOfHotCallSites = make(map[pgo.CallSiteInfo]struct{})
-
        var reason string // reason, if any, that the function was not inlined
        if base.Flag.LowerM > 1 || logopt.Enabled() {
                defer func() {
@@ -448,13 +448,12 @@ func (v *hairyVisitor) doNode(n ir.Node) bool {
                        }
                }
 
-               // Determine if the callee edge is a for hot callee or not.
+               // Determine if the callee edge is for an inlinable hot callee or not.
                if v.profile != nil && v.curFunc != nil {
                        if fn := inlCallee(n.X, v.profile); fn != nil && typecheck.HaveInlineBody(fn) {
                                line := int(base.Ctxt.InnermostPos(n.Pos()).RelLine())
-                               csi := pgo.CallSiteInfo{Line: line, Caller: v.curFunc, Callee: fn}
+                               csi := pgo.CallSiteInfo{Line: line, Caller: v.curFunc}
                                if _, o := candHotEdgeMap[csi]; o {
-                                       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))
                                        }
@@ -885,7 +884,7 @@ func mkinlcall(n *ir.CallExpr, fn *ir.Func, maxCost int32, inlCalls *[]*ir.Inlin
                // If the callsite is hot and it is under the inlineHotMaxBudget budget, then try to inline it, or else bail.
                line := int(base.Ctxt.InnermostPos(n.Pos()).RelLine())
                csi := pgo.CallSiteInfo{Line: line, Caller: ir.CurFunc}
-               if _, ok := pgo.ListOfHotCallSites[csi]; ok {
+               if _, ok := candHotEdgeMap[csi]; ok {
                        if fn.Inl.Cost > inlineHotMaxBudget {
                                if logopt.Enabled() {
                                        logopt.LogOpt(n.Pos(), "cannotInlineCall", "inline", ir.FuncName(ir.CurFunc),

diff --git a/src/cmd/compile/internal/pgo/irgraph.go b/src/cmd/compile/internal/pgo/irgraph.go
index 8fb256739efd7293c9f11e0d0636765dcdadf598..56cfebf85e3ff9f92e54c8125b9e672a6af89943 100644 (file)
--- a/src/cmd/compile/internal/pgo/irgraph.go
+++ b/src/cmd/compile/internal/pgo/irgraph.go
@@ -128,16 +128,6 @@ type Profile struct {
        WeightedCG *IRGraph
 }
 
-var (
-       // Per-caller data structure to track the list of hot call sites. This
-       // gets rewritten every caller leaving it to GC for cleanup.
-       //
-       // TODO(prattmic): Make this non-global. Use of this seems to assume
-       // inline.CanInline is called immediately before inline.InlineCalls,
-       // which isn't necessarily true?
-       ListOfHotCallSites = make(map[CallSiteInfo]struct{})
-)
-
 // New generates a profile-graph from the profile.
 func New(profileFile string) *Profile {
        f, err := os.Open(profileFile)
@@ -424,7 +414,10 @@ func (p *Profile) PrintWeightedCallGraphDOT(nodeThreshold float64, edgeThreshold
        fmt.Printf("}\n")
 }
 
-// RedirectEdges deletes and redirects out-edges from node cur based on inlining information via inlinedCallSites.
+// RedirectEdges deletes and redirects out-edges from node cur based on
+// inlining information via inlinedCallSites.
+//
+// CallSiteInfo.Callee must be nil.
 func (p *Profile) RedirectEdges(cur *IRNode, inlinedCallSites map[CallSiteInfo]struct{}) {
        g := p.WeightedCG