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

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

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

bradfitz opened this issue May 1, 2020 · 8 comments
Labels
binary-size compiler/runtime Issues related to the Go compiler and/or runtime. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Backlog

Comments

@bradfitz
Copy link
Contributor

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

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

@randall77
Copy link
Contributor

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

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

@gopherbot
Copy link

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
@bradfitz
http2: mark some structs as non-comparable
e0ff5e5 
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
@bradfitz
net/http: remove badStringError, make some unexported structs non-com…
b8fd3ca 
…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
Contributor

martisch commented May 1, 2020 •
edited

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

@randall77
Copy link
Contributor

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
@cherrymui
cmd/link: don't mark a symbol's Gotype reachable
44d2286 
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>
@andybons andybons added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label May 1, 2020
@cuonglm
Copy link
Member

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 pushed a commit to xujianhai666/go-1 that referenced this issue May 21, 2020
@bradfitz @xujianhai666
net/http: remove badStringError, make some unexported structs non-com…
0ad8493 
…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 pushed a commit to xujianhai666/go-1 that referenced this issue May 21, 2020
@cherrymui @xujianhai666
cmd/link: don't mark a symbol's Gotype reachable
da624eb 
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>
@gopherbot
Copy link

Change https://golang.org/cl/258538 mentions this issue: [release-branch.go1.14] src, net/http: update vendor, regenerate h2_bundle.go

dteh pushed a commit to dteh/fhttp that referenced this issue Jun 22, 2022
@bradfitz
http2: mark some structs as non-comparable
11ff458 
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 gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Jul 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
binary-size compiler/runtime Issues related to the Go compiler and/or runtime. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
Go Compiler / Runtime
Status: Triage Backlog
Milestone
Backlog
Development

No branches or pull requests
6 participants
@bradfitz @martisch @andybons @cuonglm @randall77 @gopherbot