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: dramatic growth in binary size #42729

Closed
robpike opened this issue Nov 19, 2020 · 9 comments
Closed

cmd/compile: dramatic growth in binary size #42729

robpike opened this issue Nov 19, 2020 · 9 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@robpike
Copy link
Contributor

robpike commented Nov 19, 2020

Found on golang-nuts, worth reporting. It's a serious change that needs to be understood, although it might well be a problem outside the Go implementation. Although it's capture well in a single repo, it's a huge repo as far as reproducible cases go.

Original message follows.

From stephen.t....@gmail.com stephen.t.illingworth@gmail.com

https://groups.google.com/d/msgid/golang-nuts/9006c660-bd92-4293-87fc-fab8af1aca26n%40googlegroups.com?utm_medium=email&utm_source=footer

I have a reasonably sized project that produces executables that have ballooned in size between two relatively simple commits.

I've tested with three compiler versions and in each case the executable size is significantly larger in one case than the other.

The changes between the two versions amount to 172 insertions and 210 deletions. No module has changed version. No binary data is embedded in either version. The only additional import is to a file within the project and which is imported from many other places.

I can live with the new file size if that's the way it has to be, but it's one heck of leap in size for a seemingly simple change.

I've isolated the change and uploaded to a new repository for easy comparison. I would be very appreciative if someone could help me understand what is happening.

https://github.com/JetSetIlly/Gopher2600-filesize-test

Regards

Stephen Illingworth

@ianlancetaylor ianlancetaylor changed the title dramatic growth in binary size cmd/link: dramatic growth in binary size Nov 19, 2020
@ianlancetaylor ianlancetaylor added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Nov 19, 2020
@ianlancetaylor ianlancetaylor added this to the Backlog milestone Nov 19, 2020
@randall77
Copy link
Contributor

[To reproduce I needed to brew install sdl2.]

Not many symbols changed in the two binaries. I think most of the size is attributable to _go.func.* getting much bigger. If you look at go tool nm <binary> | sort you can see this:

old

 45c5eb9 R _go.func.*
 4641a2a R _runtime.gcbits.*

new

 45c6759 R _go.func.*
 8cfb6ee R _runtime.gcbits.*

This means there's a whole lot more data in the symbol table. Not sure why that would be.

@randall77
Copy link
Contributor

My current guess is this added line:

+       signalHistory [MaxSignalHistory]signal.SignalAttributes

MaxSignalHistory is 228 * 400 = 91200, and each entry is 40 bytes, for 3.6MB total.
If there were a composite literal making the containing type (a Television) that had non-zero but constant entries, then we'd put an instance of that type in read-only memory to construct one by copying from that read-only copy.
That doesn't account for all the space, but maybe there are several instances? At 3.6MB per it could add up quickly.

@randall77
Copy link
Contributor

Changing MaxSignalHistory to 100 fixes the file size.

@JetSetIlly
Copy link

Changing MaxSignalHistory to 100 fixes the file size.

You're right, it does. I've replaced it with a make() in the latest code and that also fixes the problem and work perfectly. I would never have expected that to affect the executable size.

Thanks.

@randall77
Copy link
Contributor

I'm still not sure exactly why it bloats the binary. There's only one &Television{} and it doesn't look like there is a static allocation there.

@fzipp
Copy link
Contributor

fzipp commented Nov 19, 2020

There are multiple non-pointer receivers for Television. It's also directly embedded in digest.Audio.

@ianlancetaylor ianlancetaylor changed the title cmd/link: dramatic growth in binary size cmd/compile: dramatic growth in binary size Nov 19, 2020
JetSetIlly added a commit to JetSetIlly/Gopher2600 that referenced this issue Nov 19, 2020
@randall77
Copy link
Contributor

Ah, ok. Functions like this:

func NewAudio(tv television.Television) (*Audio, error) {

Require the compiler to describe to the runtime where all the pointers are in the arguments. The arguments are 3.6MB big, so that's lots of bits. And it isn't just once - it is multiple functions taking large types, times every callsite in those functions. It adds up quickly.

Conceivably we could figure out how to encode those bitmaps more succinctly (ala gcprograms, but for stack pointer maps), but generally passing around megabytes of arguments is probably going to suck for other reasons as well, so maybe not worth doing.

@JetSetIlly
Copy link

Interesting. I've genuinely learnt something here. I honestly wouldn't have thought that would affect executable size. Thanks for helping.

@randall77
Copy link
Contributor

All right, I'll close this issue then. I think there are some possible ideas here (e.g. make binary space issues more discoverable) but nothing that wouldn't probably require a proposal first.

@golang golang locked and limited conversation to collaborators Nov 19, 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.
Projects
None yet
Development

No branches or pull requests

6 participants