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: binary size explodes for huge return value #38554

Closed
typeless opened this issue Apr 21, 2020 · 5 comments
Closed

cmd/compile: binary size explodes for huge return value #38554

typeless opened this issue Apr 21, 2020 · 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

Comments

@typeless
Copy link

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

$ go version
go version go1.14.2 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
GO111MODULE="on"
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/user/.cache/go-build"
GOENV="/home/user/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/user"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/home/user/src/golang.org/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/home/user/src/golang.org/go/pkg/tool/linux_amd64"
GCCGO="/usr/bin/gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/dev/null"
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-build845352725=/tmp/go-build -gno-record-gcc-switches"

What did you do?

$ cat <<EOF > a.go
package main

import (
	"fmt"
)

type foo struct {
	data [256 * 256 * 256 * 32]byte
}

func f() foo {
	return foo{}
}
func main() {
	x := f()
	fmt.Println("%v", x.data[0])
}
EOF

$ go build a.go

https://play.golang.org/p/OZpuS_MxVNx

What did you expect to see?

A binary file less than 2MB.

What did you see instead?

A binary file of ~514MiB size.

For what it's worth, gccgo produces a sane binary (38KiB, dynamic linking).

@randall77
Copy link
Contributor

""..stmp_0 SRODATA size=536870912
""..stmp_2 SRODATA size=536870912

There are some really big static temps used to initialize instances of type foo. One at the return site of foo (which goes away because foo is inlined?) and one in main to initialize the heap-allocated foo (the compiler heap allocates x because it is too large).

We shouldn't be copying from zeroed data to initialize anything - we should just zero it directly.

An easier fix would be to allocated all-zero static temps in BSS, so at least they don't bloat the binary.

@randall77 randall77 added this to the Unplanned milestone Apr 21, 2020
@typeless
Copy link
Author

It seems that disabling this section of code can avoid the problem.

@andybons andybons added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Apr 21, 2020
@andybons andybons changed the title binary size explodes for huge return value cmd/compile: binary size explodes for huge return value Apr 21, 2020
@josharian
Copy link
Contributor

I have a fix in the works.

@gopherbot
Copy link

Change https://golang.org/cl/229704 mentions this issue: cmd/compile: optimize Move with all-zero ro sym src to Zero

@gopherbot
Copy link

Change https://golang.org/cl/229707 mentions this issue: cmd/compile: avoid double-zeroing

gopherbot pushed a commit that referenced this issue Apr 24, 2020
This triggers in 131 functions in std+cmd.
In those functions, it often helps considerably
(2-10% text size reduction).

Noticed while working on #38554.

Change-Id: Id0dbb8e7cb21d469ec08ec3d5be9beb9e8291e9c
Reviewed-on: https://go-review.googlesource.com/c/go/+/229707
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
xujianhai666 pushed a commit to xujianhai666/go-1 that referenced this issue May 21, 2020
We set up static symbols during walk that
we later make copies of to initialize local variables.
It is difficult to ascertain at that time exactly
when copying a symbol is profitable vs locally
initializing an autotmp.

During SSA, we are much better placed to optimize.
This change recognizes when we are copying from a
global readonly all-zero symbol and replaces it with
direct zeroing.

This often allows the all-zero symbol to be
deadcode eliminated at link time.
This is not ideal--it makes for large object files,
and longer link times--but it is the cleanest fix I could find.

This makes the final binary for the program in golang#38554
shrink from >500mb to ~2.2mb.

It also shrinks the standard binaries:

file      before    after     Δ       %
addr2line 4412496   4404304   -8192   -0.186%
buildid   2893816   2889720   -4096   -0.142%
cgo       4841048   4832856   -8192   -0.169%
compile   19926480  19922432  -4048   -0.020%
cover     5281816   5277720   -4096   -0.078%
link      6734648   6730552   -4096   -0.061%
nm        4366240   4358048   -8192   -0.188%
objdump   4755968   4747776   -8192   -0.172%
pprof     14653060  14612100  -40960  -0.280%
trace     11805940  1177726  -28672  -0.243%
vet       7185560   7181416   -4144   -0.058%
total     113588440 113465560 -122880 -0.108%

And not just by removing unnecessary symbols;
the program text shrinks a bit as well.

Fixes golang#38554

Change-Id: I8381ae6084ae145a5e0cd9410c451e52c0dc51c8
Reviewed-on: https://go-review.googlesource.com/c/go/+/229704
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
Reviewed-by: Keith Randall <khr@golang.org>
xujianhai666 pushed a commit to xujianhai666/go-1 that referenced this issue May 21, 2020
This triggers in 131 functions in std+cmd.
In those functions, it often helps considerably
(2-10% text size reduction).

Noticed while working on golang#38554.

Change-Id: Id0dbb8e7cb21d469ec08ec3d5be9beb9e8291e9c
Reviewed-on: https://go-review.googlesource.com/c/go/+/229707
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
@golang golang locked and limited conversation to collaborators Apr 25, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
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
Development

No branches or pull requests

5 participants