1
0
mirror of https://github.com/golang/go synced 2024-11-17 10:14:46 -07:00

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>
This commit is contained in:
Michael Pratt 2022-10-31 12:20:09 -04:00 committed by Gopher Robot
parent 1ae93e4c20
commit d73885588a
2 changed files with 14 additions and 22 deletions

View File

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

View File

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