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: binaries contain many ..type.eq funcs #38782

Open
bradfitz opened this issue May 1, 2020 · 7 comments
Open

cmd/compile: binaries contain many ..type.eq funcs #38782

bradfitz opened this issue May 1, 2020 · 7 comments

Comments

@bradfitz
Copy link
Contributor

@bradfitz bradfitz commented May 1, 2020

We have a meta bug for binary size (#6853), but as one specific item:

Go binaries contain many ..type.eq funcs to support == on types at runtime.

The compiler & linker could probably omit provably unneeded ones. Or even do some with slower reflect if they're large & unlikely to be needed at runtime.

Background:

@bradfitz bradfitz added this to the Backlog milestone May 1, 2020
@gopherbot
Copy link

@gopherbot gopherbot commented May 1, 2020

Change https://golang.org/cl/231119 mentions this issue: http2: mark some structs as non-comparable

@randall77
Copy link
Contributor

@randall77 randall77 commented May 1, 2020

In https://go-review.googlesource.com/c/go/+/191198 I did a similar thing for hash functions. Only the ones we know are needed are generated. The rest are done reflect-like inside the runtime.
The slowdown wasn't too bad (20%) and it saved a lot of binary size (1%). The nice thing about maps is we know most of the hash functions we need, as interface-keyed maps are the only exception and they are fairly rare (rarer still, non-pointer interface keys). It's less clear for == because those comparison functions are used in more places.

@gopherbot
Copy link

@gopherbot gopherbot commented May 1, 2020

Change https://golang.org/cl/231118 mentions this issue: net/http: remove badStringError, make some unexported structs non-comparable

@gopherbot
Copy link

@gopherbot gopherbot commented May 1, 2020

Change https://golang.org/cl/231397 mentions this issue: cmd/link: don't mark a symbol's Gotype reachable

gopherbot pushed a commit to golang/net that referenced this issue May 1, 2020
Reduces binary size by not generating eq algs.

Also, remove the badStringError type that only had one use, and was
just copied from net/http where it's also not used much.

Updates golang/go#38782

Change-Id: I56bddde0bb500109e2c18bb1419e8a920a5bebf9
Reviewed-on: https://go-review.googlesource.com/c/net/+/231119
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
gopherbot pushed a commit that referenced this issue May 1, 2020
…parable

Reduces binary size by 4K, not counting the http2 changes (in CL
231119) that'll be bundled into this package in the future.

Updates #38782

Change-Id: Id360348707e076b8310a8f409e412d68dd2394b2
Reviewed-on: https://go-review.googlesource.com/c/go/+/231118
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
@martisch
Copy link
Member

@martisch martisch commented May 1, 2020

I saw some [x]interface{} algs in @bradfitz tweet which may be created as part of variadic functions.
We might have a few places left were we create algs for compiler generated arrays. I should do a pass to find out and then close the issue: #28639

@bradfitz bradfitz added the binary-size label May 1, 2020
@randall77
Copy link
Contributor

@randall77 randall77 commented May 1, 2020

I tried to noalg slice backing store arrays, but trybots didn't like me:
https://go-review.googlesource.com/c/go/+/151497
Never got around to figuring out why.

gopherbot pushed a commit that referenced this issue May 1, 2020
A symbol being reachable doesn't imply its type descriptor is
needed. Don't mark it.

If the type is converted to interface somewhere in the program,
there will be an explicit use of the type descriptor, which
will make it marked.

A println("hello") program before and after

-rwxr-xr-x  1 cherryyz  primarygroup  1259824 Apr 30 23:00 hello
-rwxr-xr-x  1 cherryyz  primarygroup  1169680 Apr 30 23:10 hello

Updates #38782.
Updates #6853.

Change-Id: I88884c126ce75ba073f1ba059c4b892c87d2ac96
Reviewed-on: https://go-review.googlesource.com/c/go/+/231397
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Alessandro Arzilli <alessandro.arzilli@gmail.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
@cuonglm
Copy link
Contributor

@cuonglm cuonglm commented May 2, 2020

I tried to noalg slice backing store arrays, but trybots didn't like me:
https://go-review.googlesource.com/c/go/+/151497
Never got around to figuring out why.

I think the reason is the same as 343b7fa

xujianhai666 added a commit to xujianhai666/go-1 that referenced this issue May 21, 2020
…parable

Reduces binary size by 4K, not counting the http2 changes (in CL
231119) that'll be bundled into this package in the future.

Updates golang#38782

Change-Id: Id360348707e076b8310a8f409e412d68dd2394b2
Reviewed-on: https://go-review.googlesource.com/c/go/+/231118
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
xujianhai666 added a commit to xujianhai666/go-1 that referenced this issue May 21, 2020
A symbol being reachable doesn't imply its type descriptor is
needed. Don't mark it.

If the type is converted to interface somewhere in the program,
there will be an explicit use of the type descriptor, which
will make it marked.

A println("hello") program before and after

-rwxr-xr-x  1 cherryyz  primarygroup  1259824 Apr 30 23:00 hello
-rwxr-xr-x  1 cherryyz  primarygroup  1169680 Apr 30 23:10 hello

Updates golang#38782.
Updates golang#6853.

Change-Id: I88884c126ce75ba073f1ba059c4b892c87d2ac96
Reviewed-on: https://go-review.googlesource.com/c/go/+/231397
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Alessandro Arzilli <alessandro.arzilli@gmail.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
6 participants
You can’t perform that action at this time.