cmd/compile: expected to inline a closure with a superficially low inline budget

[jech]

Using go1.11 linux/amd64.

I've got this function that logs debugging information:

	debugf := func(format string, v ...interface{}) {
		if l != nil {
			l.Printf(format, v...)
		}
	}

I was expecting the compiler to inline debugf and avoid consing the varargs when l is nil, but apparently that's not the case:

BenchmarkDebugf/No_Logger-8             30000000                45.4 ns/op            24 B/op          2 allocs/op
BenchmarkDebugf/Logger-8                 5000000               291 ns/op              38 B/op          3 allocs/op

Here's a complete test:

package debugf

import (
	"log"
	"testing"
	"io/ioutil"
)

func BenchmarkDebugf(b *testing.B) {
	var l *log.Logger

	debugf := func(format string, v ...interface{}) {
		if l != nil {
			l.Printf(format, v...)
		}
	}

	bench := func(b *testing.B) {
		for i := 0; i < b.N; i++ {
			debugf("i=%v", i)
		}
	}

	b.Run("No Logger", bench)
	l = log.New(ioutil.Discard, "", 0)
	b.Run("Logger", bench)
}

[odeke-em]

Hello @jech, thank you filing this bug!

So I could be mistaken, but believe that it isn't inlined because (*log.Logger).Printf has a high inlining budget. Superficially, it currently looks like the inline budget is low for debugf itself.

Kindly paging @josharian @randall77 @dr2chase to ask for ideas of we could do here. Could the inliner make a compromise here for escape analysis? That is, despite l.Printf having a high inline budget, since the arguments are being passed "verbatim" to l.Printf, hence they shouldn't be marked as escaping.

[randall77]

We don't inline debugf because it has a non-inlineable call in it. We're not doing much mid-stack inlining at the moment (#19348), so the only calls allowed in an inlinee are other inlineable calls, or some special things like panic.

debugf is, of course, a model example of why mid-stack inlining is good.

I'm not sure why the [1]interface{} must be heap allocated. As far as I can tell it doesn't escape. It might have to do with limitations of escape analysis with closures.
The contents of that [1]interface{} do escape though, so they must be heap allocated. So i must be converted to an interface{} and that (usually) needs an allocation.

[bcmills]

@randall77, is there anything more to be done for this issue?

[randall77]

General improvements like #19348 and escape analysis improvements would help.
But I don't see anything specific here. Closing.