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

cmd/compile: unclear escape analysis #36504

Open
FiloSottile opened this issue Jan 10, 2020 · 4 comments
Open

cmd/compile: unclear escape analysis #36504

FiloSottile opened this issue Jan 10, 2020 · 4 comments

Comments

@FiloSottile
Copy link
Member

@FiloSottile FiloSottile commented Jan 10, 2020

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

$ go version
go version devel +198f0452b0 Thu Nov 7 02:33:31 2019 +0000 darwin/amd64

Does this issue reproduce with the latest release?

Yep.

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

go env Output
$ go env
GO111MODULE="on"
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/filippo/Library/Caches/go-build"
GOENV="/Users/filippo/Library/Application Support/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GONOPROXY=""
GONOSUMDB=""
GOOS="darwin"
GOPATH="/Users/filippo"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org"
GOROOT="/Users/filippo/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/Users/filippo/go/pkg/tool/darwin_amd64"
GCCGO="gccgo"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/Users/filippo/src/golang.org/x/crypto/go.mod"
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 -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/cm/zzcl1fjx27sc8sf3s6sd_0vw0000gn/T/go-build168572639=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

I have a function that (also thanks to inlining) I'd expect not to allocate anything to the heap.

https://go.googlesource.com/crypto/+/c8d9389ed30fa12bbe2123256b81a26dd5cbe7b8/chacha20poly1305/chacha20poly1305_generic.go (from CL 206977)

What did you see instead?

  Total:     6340801   13065057 (flat, cum) 49.99%
     26            .          .           	binary.LittleEndian.PutUint64(buf[:], uint64(n)) 
     27            .          .           	p.Write(buf[:]) 
     28            .          .           } 
     29            .          .            
     30            .          .           func (c *chacha20poly1305) sealGeneric(dst, nonce, plaintext, additionalData []byte) []byte { 
     31            .        740           	ret, out := sliceForAppend(dst, len(plaintext)+poly1305.TagSize) 
     32            .          .           	ciphertext, tag := out[:len(plaintext)], out[len(plaintext):] 
     33            .          .           	if subtle.InexactOverlap(out, plaintext) { 
     34            .          .           		panic("chacha20poly1305: invalid buffer overlap") 
     35            .          .           	} 
     36            .          .            
     37      6340801    6340801           	var polyKey [32]byte 
     38            .          .           	s, _ := chacha20.NewUnauthenticatedCipher(c.key[:], nonce) 
     39            .          .           	s.XORKeyStream(polyKey[:], polyKey[:]) 
     40            .          .           	s.Advance(1) // set the counter to 1, skipping 32 bytes 
     41            .          .           	s.XORKeyStream(ciphertext, plaintext) 
     42            .          .            
     43            .    6723516           	p := poly1305.New(&polyKey) 
     44            .          .           	writeWithPadding(p, additionalData) 
     45            .          .           	writeWithPadding(p, ciphertext) 
     46            .          .           	writeUint64(p, len(additionalData)) 
     47            .          .           	writeUint64(p, len(plaintext)) 
     48            .          .           	p.Sum(tag[:0]) 

-gcflags=-m has nothing about neither polyKey nor p.

@toothrot toothrot added this to the Backlog milestone Jan 10, 2020
@josharian

This comment has been minimized.

Copy link
Contributor

@josharian josharian commented Jan 10, 2020

I patched in that CL and its parent and ran the package benchmarks. I see zero allocs, across the board, with Go 1.12 (old escape analysis) and with tip.

How did you get those profiling numbers?

@josharian

This comment has been minimized.

Copy link
Contributor

@josharian josharian commented Jan 10, 2020

(Incidentally, inlining should be completely independent of escape analysis.)

@FiloSottile

This comment has been minimized.

Copy link
Member Author

@FiloSottile FiloSottile commented Jan 10, 2020

Ah, sorry, I forgot to mention that this is the generic code which you can access on non-amd64 GOARCH or with the appengine tag.

(Incidentally, inlining should be completely independent of escape analysis.)

I meant that inlining prevents the values that can stay on the caller stack from escaping to the heap.

https://blog.filippo.io/efficient-go-apis-with-the-inliner/

@josharian

This comment has been minimized.

Copy link
Contributor

@josharian josharian commented Jan 13, 2020

I just ran $ go build -tags=appengine -gcflags=-m, and I get, among many other lines:

./chacha20poly1305_generic.go:37:6: moved to heap: polyKey
./chacha20poly1305_generic.go:57:6: moved to heap: polyKey

Running with -m -m yields more detail:

./chacha20poly1305_generic.go:37:6: polyKey escapes to heap:
./chacha20poly1305_generic.go:37:6:   flow: {heap} = &polyKey:
./chacha20poly1305_generic.go:37:6:     from polyKey (address-of) at ./chacha20poly1305_generic.go:39:24
./chacha20poly1305_generic.go:37:6:     from polyKey[:] (slice) at ./chacha20poly1305_generic.go:39:24
./chacha20poly1305_generic.go:37:6:     from s.XORKeyStream(polyKey[:], polyKey[:]) (call parameter) at ./chacha20poly1305_generic.go:39:16
./chacha20poly1305_generic.go:37:6: polyKey escapes to heap:
./chacha20poly1305_generic.go:37:6:   flow: {heap} = &polyKey:
./chacha20poly1305_generic.go:37:6:     from polyKey (address-of) at ./chacha20poly1305_generic.go:39:36
./chacha20poly1305_generic.go:37:6:     from polyKey[:] (slice) at ./chacha20poly1305_generic.go:39:36
./chacha20poly1305_generic.go:37:6:     from s.XORKeyStream(polyKey[:], polyKey[:]) (call parameter) at ./chacha20poly1305_generic.go:39:16
./chacha20poly1305_generic.go:57:6: polyKey escapes to heap:
./chacha20poly1305_generic.go:57:6:   flow: {heap} = &polyKey:
./chacha20poly1305_generic.go:57:6:     from polyKey (address-of) at ./chacha20poly1305_generic.go:59:24
./chacha20poly1305_generic.go:57:6:     from polyKey[:] (slice) at ./chacha20poly1305_generic.go:59:24
./chacha20poly1305_generic.go:57:6:     from s.XORKeyStream(polyKey[:], polyKey[:]) (call parameter) at ./chacha20poly1305_generic.go:59:16
./chacha20poly1305_generic.go:57:6: polyKey escapes to heap:
./chacha20poly1305_generic.go:57:6:   flow: {heap} = &polyKey:
./chacha20poly1305_generic.go:57:6:     from polyKey (address-of) at ./chacha20poly1305_generic.go:59:36
./chacha20poly1305_generic.go:57:6:     from polyKey[:] (slice) at ./chacha20poly1305_generic.go:59:36
./chacha20poly1305_generic.go:57:6:     from s.XORKeyStream(polyKey[:], polyKey[:]) (call parameter) at ./chacha20poly1305_generic.go:59:16

Is it possible that you forgot the appengine tag when running the compiler with -m?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.