Skip to content

Commit f506ad2

Browse files
committed
cmd/compile/internal/escape: speed up analyzing some functions with many closures
Escape analysis examines functions in batches. In some cases, closures are in the same batch as their parent function, but in other cases, the closures are in different batches. This can mean the per-batch ir.ReassignOracle cache is not as effective. For example, #74615 has 4,000 closures in a single function that are all in different batches. For that example, these caches had an ~80% hit rate. This CL makes the ir.ReassignOracle cache more broadly scoped, instead of per batch. This speeds up escape analysis when a function has many closures that end up in different batches, including this resolves #74615. For that example, this cache now has a ~100% hit rate. In addition, in (*batch).rewriteWithLiterals, we also slightly delay checking the ir.ReassignOracle cache, which is more natural to do now compared to when rewriteWithLiterals was first merged. This means we can avoid consulting or populating the cache in more cases. (We also leave a new type-related TODO there. If we were to also implement that TODO, a quick test suggests we could independently resolve the specific example in #74615 even without making the cache more broadly scoped, though other conceivable examples would not be helped; the scoping of the cache is the more fundamental improvement.) If we look at cumulative time spent via pprof for the #74615 example using this CL, the work of ir.ReassignOracle escape analysis cache now typically shows zero cpu samples. This CL passes "go build -toolexec 'toolstash -cmp' -a std cmd". Fixes #74615 Change-Id: I3c17c527fbb546ffb8a4fa52cd61e41ff3cdb869 Reviewed-on: https://go-review.googlesource.com/c/go/+/688075 Auto-Submit: Keith Randall <[email protected]> Reviewed-by: Cherry Mui <[email protected]> Reviewed-by: Keith Randall <[email protected]> Reviewed-by: Keith Randall <[email protected]> LUCI-TryBot-Result: Go LUCI <[email protected]>
1 parent 9c507e7 commit f506ad2

File tree

1 file changed

+21
-12
lines changed

1 file changed

+21
-12
lines changed

src/cmd/compile/internal/escape/escape.go

Lines changed: 21 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -122,17 +122,24 @@ type escape struct {
122122
}
123123

124124
func Funcs(all []*ir.Func) {
125-
ir.VisitFuncsBottomUp(all, Batch)
125+
// Make a cache of ir.ReassignOracles. The cache is lazily populated.
126+
// TODO(thepudds): consider adding a field on ir.Func instead. We might also be able
127+
// to use that field elsewhere, like in walk. See discussion in https://go.dev/cl/688075.
128+
reassignOracles := make(map[*ir.Func]*ir.ReassignOracle)
129+
130+
ir.VisitFuncsBottomUp(all, func(list []*ir.Func, recursive bool) {
131+
Batch(list, reassignOracles)
132+
})
126133
}
127134

128135
// Batch performs escape analysis on a minimal batch of
129136
// functions.
130-
func Batch(fns []*ir.Func, recursive bool) {
137+
func Batch(fns []*ir.Func, reassignOracles map[*ir.Func]*ir.ReassignOracle) {
131138
var b batch
132139
b.heapLoc.attrs = attrEscapes | attrPersists | attrMutates | attrCalls
133140
b.mutatorLoc.attrs = attrMutates
134141
b.calleeLoc.attrs = attrCalls
135-
b.reassignOracles = make(map[*ir.Func]*ir.ReassignOracle)
142+
b.reassignOracles = reassignOracles
136143

137144
// Construct data-flow graph from syntax trees.
138145
for _, fn := range fns {
@@ -531,15 +538,6 @@ func (b *batch) rewriteWithLiterals(n ir.Node, fn *ir.Func) {
531538
if n == nil || fn == nil {
532539
return
533540
}
534-
if n.Op() != ir.OMAKESLICE && n.Op() != ir.OCONVIFACE {
535-
return
536-
}
537-
538-
// Look up a cached ReassignOracle for the function, lazily computing one if needed.
539-
ro := b.reassignOracle(fn)
540-
if ro == nil {
541-
base.Fatalf("no ReassignOracle for function %v with closure parent %v", fn, fn.ClosureParent)
542-
}
543541

544542
assignTemp := func(n ir.Node, init *ir.Nodes) {
545543
// Preserve any side effects of n by assigning it to an otherwise unused temp.
@@ -561,6 +559,11 @@ func (b *batch) rewriteWithLiterals(n ir.Node, fn *ir.Func) {
561559
}
562560

563561
if (*r).Op() != ir.OLITERAL {
562+
// Look up a cached ReassignOracle for the function, lazily computing one if needed.
563+
ro := b.reassignOracle(fn)
564+
if ro == nil {
565+
base.Fatalf("no ReassignOracle for function %v with closure parent %v", fn, fn.ClosureParent)
566+
}
564567
if s := ro.StaticValue(*r); s.Op() == ir.OLITERAL {
565568
lit, ok := s.(*ir.BasicLit)
566569
if !ok || lit.Val().Kind() != constant.Int {
@@ -582,6 +585,12 @@ func (b *batch) rewriteWithLiterals(n ir.Node, fn *ir.Func) {
582585
// a literal to avoid heap allocating the underlying interface value.
583586
conv := n.(*ir.ConvExpr)
584587
if conv.X.Op() != ir.OLITERAL && !conv.X.Type().IsInterface() {
588+
// TODO(thepudds): likely could avoid some work by tightening the check of conv.X's type.
589+
// Look up a cached ReassignOracle for the function, lazily computing one if needed.
590+
ro := b.reassignOracle(fn)
591+
if ro == nil {
592+
base.Fatalf("no ReassignOracle for function %v with closure parent %v", fn, fn.ClosureParent)
593+
}
585594
v := ro.StaticValue(conv.X)
586595
if v != nil && v.Op() == ir.OLITERAL && ir.ValidTypeForConst(conv.X.Type(), v.Val()) {
587596
if !base.LiteralAllocHash.MatchPos(n.Pos(), nil) {

0 commit comments

Comments
 (0)