Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

cmd/compile: support inlining of functions containing function literals #28727

Closed
tdewolff opened this issue Nov 11, 2018 · 4 comments
Closed

cmd/compile: support inlining of functions containing function literals #28727

tdewolff opened this issue Nov 11, 2018 · 4 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Performance
Milestone
Backlog

Comments

@tdewolff
Copy link

tdewolff commented Nov 11, 2018 •
edited

What version of Go are you using (go version)?

$ go version
go version go1.11.1 linux/amd64

Does this issue reproduce with the latest release?

Yes.

What operating system and processor architecture are you using (go env)?

go env Output
$ go env
GOARCH="amd64"
GOBIN=""
GOCACHE="/tmp/go-build"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/people/tdew803/go"
GOPROXY=""
GORACE=""
GOROOT="/usr/lib/go-1.11"
GOTMPDIR=""
GOTOOLDIR="/usr/lib/go-1.11/pkg/tool/linux_amd64"
GCCGO="gccgo"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build626602379=/tmp/go-build -gno-record-gcc-switches"

What did you do?

Looking at the following benchmarks, returning a func literal always gets allocated on the heap while a similar approach using a struct can be allocated on the stack. I propose that func literals are also assessed whether they can be put on the stack or on the heap.

package main

import "testing"

func FuncA(x *int) func() {
    return func() { (*x)++ }
}

var A int

func BenchmarkTestA(b *testing.B) {
    for n := 0; n < b.N; n++ {
        f := FuncA(&A)
        f()
    }
}

////////////////

type Increaser struct {
    x *int
}

func (inc Increaser) Increase() {
    (*inc.x)++
}

func FuncB(x *int) Increaser {
    return Increaser{x}
}

var B int

func BenchmarkTestB(b *testing.B) {
    for n := 0; n < b.N; n++ {
        inc := FuncB(&B)
        inc.Increase()
    }
}

giving

$ go test -bench=. -benchmem -gcflags '-m -l'
# test [test]
./run_test.go:6:12: func literal escapes to heap
./run_test.go:6:12: func literal escapes to heap
./run_test.go:5:12: leaking param: x to result ~r1 level=-1
./run_test.go:13:7: Increaser.Increase inc does not escape
./run_test.go:17:12: leaking param: x to result ~r1 level=0
./run_test.go:25:20: &A escapes to heap
./run_test.go:23:21: BenchmarkTestA b does not escape
./run_test.go:30:21: BenchmarkTestB b does not escape
./run_test.go:32:22: BenchmarkTestB &B does not escape
<autogenerated>:1: (*Increaser).Increase .this does not escape
# test
/tmp/go-build496566533/b001/_testmain.go:42:42: testdeps.TestDeps literal escapes to heap
goos: linux
goarch: amd64
pkg: test
BenchmarkTestA-8   	50000000	        26.9 ns/op	      16 B/op	       1 allocs/op
BenchmarkTestB-8   	1000000000	         2.41 ns/op	       0 B/op	       0 allocs/op
PASS
ok  	test	4.041s

When this func literal would be allocated on the stack, we would see a ~10x performance increase.
@randall77
Copy link
Contributor

We do allocate function literals on the stack, if possible.
We can't currently return the address of stack-allocated objects, function literals or otherwise.
That's what is happening in benchmark A, the stack frame that the closure would be allocated on disappears on the return.
Your benchmark B is misleading I think because everything inlines.

@ianlancetaylor ianlancetaylor changed the title cmd/gc: allocate func literal on stack if possible cmd/compile: allocate func literal on stack if possible Nov 12, 2018
@ianlancetaylor ianlancetaylor added Performance NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Nov 12, 2018
@ianlancetaylor ianlancetaylor added this to the Go1.13 milestone Nov 12, 2018
@mdempsky
Copy link
Member

The crux of this issue is that functions that contain closures aren't currently inlineable. If FuncA was inlined into BenchmarkTestA, then I expect escape analysis would detect that it can be stack allocated.

@mdempsky mdempsky changed the title cmd/compile: allocate func literal on stack if possible cmd/compile: support inlining of functions containing function literals Apr 16, 2019
@mdempsky mdempsky mentioned this issue Apr 22, 2019
Closed
@andybons andybons modified the milestones: Go1.13, Go1.14 Jul 8, 2019
@rsc rsc modified the milestones: Go1.14, Backlog Oct 9, 2019
@danscales danscales self-assigned this Nov 5, 2020
@gopherbot
Copy link

Change https://golang.org/cl/267880 mentions this issue: Exporting, importing, and inlining functions with OCLOSURE

@randall77 randall77 mentioned this issue Dec 16, 2020
Closed
@gopherbot
Copy link

Change https://golang.org/cl/283112 mentions this issue: [dev.regabi] cmd/compile: Exporting, importing, and inlining functions with OCLOSURE

@golang golang locked and limited conversation to collaborators Feb 16, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Performance
Projects
None yet
Milestone
Backlog
Development

No branches or pull requests
8 participants
@mdempsky @rsc @andybons @danscales @ianlancetaylor @tdewolff @randall77 @gopherbot