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: non-inlineable allocating function in closure causes closure to allocate #25769

Closed
twmb opened this issue Jun 7, 2018 · 5 comments
Closed

cmd/compile: non-inlineable allocating function in closure causes closure to allocate #25769

twmb opened this issue Jun 7, 2018 · 5 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Performance
Milestone
Go1.13

Comments

@twmb
Copy link
Contributor

twmb commented Jun 7, 2018

It appears that inside a closure, if a function cannot be inlined and that function allocates, the closure will be allocated. This may be as intended (if so, sorry!), but it feels as if allocations in functions inside closures should be an orthogonal concern to the allocation of the closure itself.

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

go version devel +ca8ec69ce6 Wed Jun 6 19:20:12 2018 +0000 linux/amd64

Does this issue reproduce with the latest release?

Yes

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

GOARCH="amd64"
GOBIN=""
GOCACHE="/home/twmb/.cache/go-build"
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/home/twmb/go"
GORACE=""
GOROOT="/home/twmb/go/go"
GOTMPDIR=""
GOTOOLDIR="/home/twmb/go/go/pkg/tool/linux_amd64"
GCCGO="gccgo"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
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-build866772824=/tmp/go-build -gno-record-gcc-switches"

What did you do?

https://play.golang.org/p/5brgC6eRn-c (more representative)
https://play.golang.org/p/A0-tIgewmhU (more minimal, in return position)
https://play.golang.org/p/0bg2yuZA6gc (more minimal, in middle of func)

What did you expect to see?

No func literal escapes to heap in go build -gcflags '-m -m' foo.go

What did you see instead?

For the first play link:

$ go build -gcflags '-m -m' foo.go 
...
./foo.go:14:7: func literal escapes to heap
./foo.go:14:7: 	from &(func literal) (address-of) at ./foo.go:14:7
./foo.go:14:7: 	from c (assigned) at ./foo.go:14:2
./foo.go:14:7: 	from *c (indirection) at ./foo.go:14:2
./foo.go:14:7: 	from .out0 (captured by called closure) at ./foo.go:15:10
./foo.go:14:7: 	from ~r0 (assigned) at ./foo.go:13:11
@ianlancetaylor ianlancetaylor added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jun 7, 2018
@ianlancetaylor ianlancetaylor added this to the Go1.12 milestone Jun 7, 2018
@ianlancetaylor
Copy link
Contributor

CC @dr2chase

@twmb
Copy link
Contributor Author

twmb commented Jun 18, 2018

After benchmarking do in the links above, I found that they don't allocate in benchmarks (I'm not sure why, especially since the gcflag output indicates they would).

The allocation seems to be dependent on the closure capturing a variable and a function in the closure potentially allocating. This code, ran in a benchmark, allocates:

package foo

import "math/rand"

//go:noinline
func f() *int { return nil }

type T struct {
	i1 int
	i2 *int
}

func do() *T {
	n := rand.Intn(1) // always zero
	c := func() *T {
		return &T{
			i1: n,
			i2: f(),
		}
	}
	if n == 1 { // never
		return c()
	}
	return nil
}

Deleting setting either i1 or i2 from T inside the closure removes the allocation.

@randall77
Copy link
Contributor

Punting to 1.13, too late for anything major in 1.12.

@randall77 randall77 modified the milestones: Go1.12, Go1.13 Dec 12, 2018
@mariecurried
Copy link

I think this issue has been fixed by @mdempsky's escape analysis rewrite.

@twmb
Copy link
Contributor Author

twmb commented Apr 16, 2019

I believe so as well.

@twmb twmb closed this as completed Apr 16, 2019
@golang golang locked and limited conversation to collaborators Apr 15, 2020
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
Go1.13
Development

No branches or pull requests
6 participants
@twmb @agnivade @ianlancetaylor @randall77 @gopherbot @mariecurried