Skip to content

Commit 5fc70b6

Browse files
committed
cmd/compile: set stricter inlining threshold in large functions
If we're compiling a large function, be more picky about how big the function we're inlining is. If the function is >5000 nodes, we lower the inlining threshold from a cost of 80 to 20. Turns out reflect.Value's cost is exactly 80. That's the function at issue in golang#26546. 20 was chosen as a proxy for "inlined body is smaller than the call would be". Simple functions still get inlined, like this one at cost 7: func ifaceIndir(t *rtype) bool { return t.kind&kindDirectIface == 0 } 5000 nodes was chosen as the big function size. Here are all the 5000+ node (~~1000+ lines) functions in the stdlib: 5187 cmd/internal/obj/arm (*ctxt5).asmout 6879 cmd/internal/obj/s390x (*ctxtz).asmout 6567 cmd/internal/obj/ppc64 (*ctxt9).asmout 9643 cmd/internal/obj/arm64 (*ctxt7).asmout 5042 cmd/internal/obj/x86 (*AsmBuf).doasm 8768 cmd/compile/internal/ssa rewriteBlockAMD64 8878 cmd/compile/internal/ssa rewriteBlockARM 8344 cmd/compile/internal/ssa rewriteValueARM64_OpARM64OR_20 7916 cmd/compile/internal/ssa rewriteValueARM64_OpARM64OR_30 5427 cmd/compile/internal/ssa rewriteBlockARM64 5126 cmd/compile/internal/ssa rewriteValuePPC64_OpPPC64OR_50 6152 cmd/compile/internal/ssa rewriteValuePPC64_OpPPC64OR_60 6412 cmd/compile/internal/ssa rewriteValuePPC64_OpPPC64OR_70 6486 cmd/compile/internal/ssa rewriteValuePPC64_OpPPC64OR_80 6534 cmd/compile/internal/ssa rewriteValuePPC64_OpPPC64OR_90 6534 cmd/compile/internal/ssa rewriteValuePPC64_OpPPC64OR_100 6534 cmd/compile/internal/ssa rewriteValuePPC64_OpPPC64OR_110 6675 cmd/compile/internal/gc typecheck1 5433 cmd/compile/internal/gc walkexpr 14070 cmd/vendor/golang.org/x/arch/arm64/arm64asm decodeArg There are a lot more smaller (~1000 node) functions in the stdlib. The function in golang#26546 has 12477 nodes. At some point it might be nice to have a better heuristic for "inlined body is smaller than the call", a non-cliff way to scale down the cost as the function gets bigger, doing cheaper inlined calls first, etc. All that can wait for another release. I'd like to do this CL for 1.11. Fixes golang#26546 Update golang#17566 Change-Id: Idda13020e46ec2b28d79a17217f44b189f8139ac Reviewed-on: https://go-review.googlesource.com/125516 Run-TryBot: Keith Randall <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: David Chase <[email protected]>
1 parent 90066bf commit 5fc70b6

File tree

2 files changed

+1082
-19
lines changed

2 files changed

+1082
-19
lines changed

src/cmd/compile/internal/gc/inl.go

+53-19
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,9 @@ const (
4141
inlineExtraCallCost = inlineMaxBudget // default is do not inline, -l=4 enables by using 1 instead.
4242
inlineExtraPanicCost = 1 // do not penalize inlining panics.
4343
inlineExtraThrowCost = inlineMaxBudget // with current (2018-05/1.11) code, inlining runtime.throw does not help.
44+
45+
inlineBigFunctionNodes = 5000 // Functions with this many nodes are considered "big".
46+
inlineBigFunctionMaxCost = 20 // Max cost of inlinee when inlining into a "big" function.
4447
)
4548

4649
// Get the function's package. For ordinary functions it's on the ->sym, but for imported methods
@@ -459,12 +462,38 @@ func inlcopy(n *Node) *Node {
459462
return m
460463
}
461464

465+
func countNodes(n *Node) int {
466+
if n == nil {
467+
return 0
468+
}
469+
cnt := 1
470+
cnt += countNodes(n.Left)
471+
cnt += countNodes(n.Right)
472+
for _, n1 := range n.Ninit.Slice() {
473+
cnt += countNodes(n1)
474+
}
475+
for _, n1 := range n.Nbody.Slice() {
476+
cnt += countNodes(n1)
477+
}
478+
for _, n1 := range n.List.Slice() {
479+
cnt += countNodes(n1)
480+
}
481+
for _, n1 := range n.Rlist.Slice() {
482+
cnt += countNodes(n1)
483+
}
484+
return cnt
485+
}
486+
462487
// Inlcalls/nodelist/node walks fn's statements and expressions and substitutes any
463488
// calls made to inlineable functions. This is the external entry point.
464489
func inlcalls(fn *Node) {
465490
savefn := Curfn
466491
Curfn = fn
467-
fn = inlnode(fn)
492+
maxCost := int32(inlineMaxBudget)
493+
if countNodes(fn) >= inlineBigFunctionNodes {
494+
maxCost = inlineBigFunctionMaxCost
495+
}
496+
fn = inlnode(fn, maxCost)
468497
if fn != Curfn {
469498
Fatalf("inlnode replaced curfn")
470499
}
@@ -505,10 +534,10 @@ func inlconv2list(n *Node) []*Node {
505534
return s
506535
}
507536

508-
func inlnodelist(l Nodes) {
537+
func inlnodelist(l Nodes, maxCost int32) {
509538
s := l.Slice()
510539
for i := range s {
511-
s[i] = inlnode(s[i])
540+
s[i] = inlnode(s[i], maxCost)
512541
}
513542
}
514543

@@ -525,7 +554,7 @@ func inlnodelist(l Nodes) {
525554
// shorter and less complicated.
526555
// The result of inlnode MUST be assigned back to n, e.g.
527556
// n.Left = inlnode(n.Left)
528-
func inlnode(n *Node) *Node {
557+
func inlnode(n *Node, maxCost int32) *Node {
529558
if n == nil {
530559
return n
531560
}
@@ -547,19 +576,19 @@ func inlnode(n *Node) *Node {
547576

548577
lno := setlineno(n)
549578

550-
inlnodelist(n.Ninit)
579+
inlnodelist(n.Ninit, maxCost)
551580
for _, n1 := range n.Ninit.Slice() {
552581
if n1.Op == OINLCALL {
553582
inlconv2stmt(n1)
554583
}
555584
}
556585

557-
n.Left = inlnode(n.Left)
586+
n.Left = inlnode(n.Left, maxCost)
558587
if n.Left != nil && n.Left.Op == OINLCALL {
559588
n.Left = inlconv2expr(n.Left)
560589
}
561590

562-
n.Right = inlnode(n.Right)
591+
n.Right = inlnode(n.Right, maxCost)
563592
if n.Right != nil && n.Right.Op == OINLCALL {
564593
if n.Op == OFOR || n.Op == OFORUNTIL {
565594
inlconv2stmt(n.Right)
@@ -568,7 +597,7 @@ func inlnode(n *Node) *Node {
568597
}
569598
}
570599

571-
inlnodelist(n.List)
600+
inlnodelist(n.List, maxCost)
572601
switch n.Op {
573602
case OBLOCK:
574603
for _, n2 := range n.List.Slice() {
@@ -595,7 +624,7 @@ func inlnode(n *Node) *Node {
595624
}
596625
}
597626

598-
inlnodelist(n.Rlist)
627+
inlnodelist(n.Rlist, maxCost)
599628
if n.Op == OAS2FUNC && n.Rlist.First().Op == OINLCALL {
600629
n.Rlist.Set(inlconv2list(n.Rlist.First()))
601630
n.Op = OAS2
@@ -614,7 +643,7 @@ func inlnode(n *Node) *Node {
614643
}
615644
}
616645

617-
inlnodelist(n.Nbody)
646+
inlnodelist(n.Nbody, maxCost)
618647
for _, n := range n.Nbody.Slice() {
619648
if n.Op == OINLCALL {
620649
inlconv2stmt(n)
@@ -637,12 +666,12 @@ func inlnode(n *Node) *Node {
637666
fmt.Printf("%v:call to func %+v\n", n.Line(), n.Left)
638667
}
639668
if n.Left.Func != nil && n.Left.Func.Inl != nil && !isIntrinsicCall(n) { // normal case
640-
n = mkinlcall(n, n.Left)
669+
n = mkinlcall(n, n.Left, maxCost)
641670
} else if n.Left.isMethodExpression() && asNode(n.Left.Sym.Def) != nil {
642-
n = mkinlcall(n, asNode(n.Left.Sym.Def))
671+
n = mkinlcall(n, asNode(n.Left.Sym.Def), maxCost)
643672
} else if n.Left.Op == OCLOSURE {
644673
if f := inlinableClosure(n.Left); f != nil {
645-
n = mkinlcall(n, f)
674+
n = mkinlcall(n, f, maxCost)
646675
}
647676
} else if n.Left.Op == ONAME && n.Left.Name != nil && n.Left.Name.Defn != nil {
648677
if d := n.Left.Name.Defn; d.Op == OAS && d.Right.Op == OCLOSURE {
@@ -668,7 +697,7 @@ func inlnode(n *Node) *Node {
668697
}
669698
break
670699
}
671-
n = mkinlcall(n, f)
700+
n = mkinlcall(n, f, maxCost)
672701
}
673702
}
674703
}
@@ -687,7 +716,7 @@ func inlnode(n *Node) *Node {
687716
Fatalf("no function definition for [%p] %+v\n", n.Left.Type, n.Left.Type)
688717
}
689718

690-
n = mkinlcall(n, asNode(n.Left.Type.FuncType().Nname))
719+
n = mkinlcall(n, asNode(n.Left.Type.FuncType().Nname), maxCost)
691720
}
692721

693722
lineno = lno
@@ -788,7 +817,7 @@ func (v *reassignVisitor) visitList(l Nodes) *Node {
788817

789818
// The result of mkinlcall MUST be assigned back to n, e.g.
790819
// n.Left = mkinlcall(n.Left, fn, isddd)
791-
func mkinlcall(n *Node, fn *Node) *Node {
820+
func mkinlcall(n *Node, fn *Node, maxCost int32) *Node {
792821
save_safemode := safemode
793822

794823
// imported functions may refer to unsafe as long as the
@@ -798,7 +827,7 @@ func mkinlcall(n *Node, fn *Node) *Node {
798827
if pkg != localpkg && pkg != nil {
799828
safemode = false
800829
}
801-
n = mkinlcall1(n, fn)
830+
n = mkinlcall1(n, fn, maxCost)
802831
safemode = save_safemode
803832
return n
804833
}
@@ -824,11 +853,16 @@ var inlgen int
824853
// parameters.
825854
// The result of mkinlcall1 MUST be assigned back to n, e.g.
826855
// n.Left = mkinlcall1(n.Left, fn, isddd)
827-
func mkinlcall1(n, fn *Node) *Node {
856+
func mkinlcall1(n, fn *Node, maxCost int32) *Node {
828857
if fn.Func.Inl == nil {
829858
// No inlinable body.
830859
return n
831860
}
861+
if fn.Func.Inl.Cost > maxCost {
862+
// The inlined function body is too big. Typically we use this check to restrict
863+
// inlining into very big functions. See issue 26546 and 17566.
864+
return n
865+
}
832866

833867
if fn == Curfn || fn.Name.Defn == Curfn {
834868
// Can't recursively inline a function into itself.
@@ -1094,7 +1128,7 @@ func mkinlcall1(n, fn *Node) *Node {
10941128
// instead we emit the things that the body needs
10951129
// and each use must redo the inlining.
10961130
// luckily these are small.
1097-
inlnodelist(call.Nbody)
1131+
inlnodelist(call.Nbody, maxCost)
10981132
for _, n := range call.Nbody.Slice() {
10991133
if n.Op == OINLCALL {
11001134
inlconv2stmt(n)

0 commit comments

Comments
 (0)