From: Michael Pratt Date: Mon, 31 Oct 2022 16:20:09 +0000 (-0400) Subject: cmd/compile/internal/pgo: remove ListOfHotCallSites X-Git-Tag: go1.20rc1~467 X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=d73885588ac0a7520ac3e848112e44f5dbe97006;p=gostls13.git 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 Reviewed-by: Cherry Mui Run-TryBot: Michael Pratt TryBot-Result: Gopher Robot --- diff --git a/src/cmd/compile/internal/inline/inl.go b/src/cmd/compile/internal/inline/inl.go index 75f3a4b907..335bb23ecb 100644 --- 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 8fb256739e..56cfebf85e 100644 --- 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