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

x/oauth2/google: very fat binaries produced just by linking it #19523

Closed
rasky opened this issue Mar 12, 2017 · 11 comments
Closed

x/oauth2/google: very fat binaries produced just by linking it #19523

rasky opened this issue Mar 12, 2017 · 11 comments

Comments

@rasky
Copy link
Member

rasky commented Mar 12, 2017

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

go version go1.8 darwin/amd64

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

GOARCH="amd64"
GOBIN=""
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPATH="/Users/rasky/Sources/go"
GORACE=""
GOROOT="/usr/local/Cellar/go/1.8/libexec"
GOTOOLDIR="/usr/local/Cellar/go/1.8/libexec/pkg/tool/darwin_amd64"
GCCGO="gccgo"
CC="clang"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/q5/4h90rf793dz456wy2tmfl7_80000gn/T/go-build946763060=/tmp/go-build -gno-record-gcc-switches -fno-common"
CXX="clang++"
CGO_ENABLED="1"
PKG_CONFIG="pkg-config"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"

What did you do?

Build this program:

package main

import (
	"fmt"

	"golang.org/x/oauth2/google"
)

func main() {
	fmt.Println(google.Endpoint)
}

What did you expect to see?

A small binary is produced.

What did you see instead?

A 9Mb binary is produced.

@bradfitz
Copy link
Contributor

This bug is too vague to be actionable, especially since your actual problem wasn't described. The general problem of binary size is already known and tracked. (#6853)

If you want to file concrete bugs about packages having init-time dependencies on other packages unnecessarily, feel free. But we don't need another general size bug, so I'm closing this one.

Some details on this case, though:

$ go tool nm -size size |sort -k2 -n
...
  505470       5922 T math/big.nat.probablyPrimeLucas
  449e50       5952 T runtime.gentraceback
  a66ee0       5968 D runtime.memstats
  5a2420       6034 T vendor/golang_org/x/crypto/curve25519.ladderstep
  a2c600       6080 D unicode.statictmp_584
  51db60       6360 T encoding/asn1.makeBody
  5aeb00       6481 T crypto/tls.(*Conn).clientHandshake
  5cacc0       6640 T crypto/tls.X509KeyPair
  5a9d70       6675 T crypto/tls.(*Conn).readRecord
  735af0       6696 T google.golang.org/grpc.(*Server).processUnaryRPC
  702200       7346 T golang.org/x/net/http2.init
  4e5050       7479 T crypto/sha1.blockAVX2
  5be1e0       8030 T crypto/tls.(*serverHandshakeState).doFullHandshake
  a4a160       8040 D runtime.cpuprof
  a4c0e0       8072 D runtime.hash
  438fc0       8128 T runtime.selectgo
  a68640       8192 D runtime.pdesc
  8676c0       8552 R html.statictmp_84
  607270       8593 T vendor/golang_org/x/net/proxy.(*socks5).connect
  474c20       8872 T time.Time.AppendFormat
  6596c0       8975 T net/http.(*Transport).dialConn
  5b0460       9057 T crypto/tls.(*clientHandshakeState).doFullHandshake
  6931e0       9128 T html.init
  72b730       9801 T github.com/golang/protobuf/proto.(*Properties).setEncAndDec
  58edd0      10808 T crypto/x509.parseCertificate
  4b4460      11701 T fmt.(*pp).printValue
  a2ddc0      11776 D vendor/golang_org/x/text/unicode/norm.nfkcValues
  5ee8f0      12231 T vendor/golang_org/x/net/http2/hpack.init
  a30bc0      12672 D vendor/golang_org/x/text/width.widthValues
  664a90      12840 T net/http.init
  a4e080      13264 D runtime.mheap_
  477600      13741 T time.parse
  5169c0      14014 T encoding/asn1.parseField
  4e7910      14647 T crypto/sha256.block
  876a40      15732 r runtime.typelink
  a51460      16064 D runtime.semtable
  5371c0      16152 T crypto/sha512.blockAMD64
  869840      16587 r runtime.findfunctab
  595f30      18600 T vendor/golang_org/x/crypto/chacha20poly1305.chacha20Poly1305Open
  a33d40      19075 D vendor/golang_org/x/text/unicode/norm.decomps
  59a7e0      20998 T vendor/golang_org/x/crypto/chacha20poly1305.chacha20Poly1305Seal
  4c2790      27303 T unicode.init
  86d920      34208 R html.statictmp_83
  6d5be0      53596 T golang.org/x/net/http2/hpack.newStaticTable
  a55320      65736 D runtime.trace
  87b4e0    1645034 r runtime.pclntab

It seems something in the deps of the x/oauth2/google package is brining in http2 and friends:

        "Imports": [
                "cloud.google.com/go/compute/metadata",
                "crypto/rsa",
                "encoding/json",
                "errors",
                "fmt",
                "golang.org/x/net/context",
                "golang.org/x/oauth2",
                "golang.org/x/oauth2/internal",
                "golang.org/x/oauth2/jws",
                "golang.org/x/oauth2/jwt",
                "io/ioutil",
                "net/http",
                "os",
                "os/user",
                "path/filepath",
                "runtime",
                "sort",
                "strings",
                "sync",
                "time"
        ],      

But you'll need all those once you do any real work anyway, so this is somewhat of a non-issue.

If you want to file a bug about slimming the x/oauth2/google package, though, that repo uses its own bug tracker: https://github.com/golang/oauth2/issues

Closing this one.

@rasky
Copy link
Member Author

rasky commented Mar 13, 2017

Why the linker is not able to remove all that cruft if I'm only referencing google.Endpoint?

@bradfitz
Copy link
Contributor

Generally because of init funcs in packages. That is one of the big reasons we push back against unnecessary init-time work. You could file a bug against x/oauth2/google to do more work lazily, as needed, which means the linker would be able to discard more.

@bradfitz
Copy link
Contributor

Btw, last year I added the -dumpdep linker flag to debug cases like this.

Try:

$ go build -ldflags=-dumpdep size.go 2>&1 | grep oauth2/google
main.main -> golang.org/x/oauth2/google.Endpoint
main.init -> golang.org/x/oauth2/google.init
golang.org/x/oauth2/google.Endpoint -> go.string."https://accounts.google.com/o/oauth2/auth"
golang.org/x/oauth2/google.Endpoint -> go.string."https://accounts.google.com/o/oauth2/token"
golang.org/x/oauth2/google.init -> golang.org/x/oauth2/google.initdone·
golang.org/x/oauth2/google.init -> sort.init
golang.org/x/oauth2/google.init -> strings.init
golang.org/x/oauth2/google.init -> time.init
golang.org/x/oauth2/google.init -> golang.org/x/net/context.init
golang.org/x/oauth2/google.init -> golang.org/x/oauth2.init
golang.org/x/oauth2/google.init -> encoding/json.init
golang.org/x/oauth2/google.init -> io/ioutil.init
golang.org/x/oauth2/google.init -> net/http.init
golang.org/x/oauth2/google.init -> path/filepath.init
golang.org/x/oauth2/google.init -> cloud.google.com/go/compute/metadata.init
golang.org/x/oauth2/google.init -> golang.org/x/oauth2/jwt.init
golang.org/x/oauth2/google.init -> crypto/rsa.init
golang.org/x/oauth2/google.init -> golang.org/x/oauth2/internal.init
golang.org/x/oauth2/google.init -> golang.org/x/oauth2/jws.init
golang.org/x/oauth2/google.init -> os/user.init
golang.org/x/oauth2/google.init -> type.map[string]*golang.org/x/oauth2/google.tokenLock
golang.org/x/oauth2/google.init -> golang.org/x/oauth2/google.aeTokens
type.map[string]*golang.org/x/oauth2/google.tokenLock -> type..namedata.*map[string]*google.tokenLock.
type.map[string]*golang.org/x/oauth2/google.tokenLock -> type.*golang.org/x/oauth2/google.tokenLock
type.map[string]*golang.org/x/oauth2/google.tokenLock -> type.noalg.map.bucket[string]*golang.org/x/oauth2/google.tokenLock
type.map[string]*golang.org/x/oauth2/google.tokenLock -> type.noalg.map.hdr[string]*golang.org/x/oauth2/google.tokenLock
type.*golang.org/x/oauth2/google.tokenLock -> type..namedata.*google.tokenLock.
type.*golang.org/x/oauth2/google.tokenLock -> type.golang.org/x/oauth2/google.tokenLock
type.noalg.map.bucket[string]*golang.org/x/oauth2/google.tokenLock -> runtime.gcbits.aaaafe03
type.noalg.map.bucket[string]*golang.org/x/oauth2/google.tokenLock -> type..namedata.*map.bucket[string]*google.tokenLock.
type.noalg.map.bucket[string]*golang.org/x/oauth2/google.tokenLock -> type.noalg.[8]*golang.org/x/oauth2/google.tokenLock
type.noalg.map.bucket[string]*golang.org/x/oauth2/google.tokenLock -> type.*map.bucket[string]*golang.org/x/oauth2/google.tokenLock
type.noalg.map.hdr[string]*golang.org/x/oauth2/google.tokenLock -> type..namedata.*map.hdr[string]*google.tokenLock.
type.golang.org/x/oauth2/google.tokenLock -> type..importpath.golang.org/x/oauth2/google.
type.golang.org/x/oauth2/google.tokenLock -> type..namedata.mu.
type.golang.org/x/oauth2/google.tokenLock -> type.*golang.org/x/oauth2.Token
type.noalg.[8]*golang.org/x/oauth2/google.tokenLock -> type..namedata.*[8]*google.tokenLock.
type.noalg.[8]*golang.org/x/oauth2/google.tokenLock -> type.[]*golang.org/x/oauth2/google.tokenLock
type.[]*golang.org/x/oauth2/google.tokenLock -> type..namedata.*[]*google.tokenLock.

etc.

@rasky
Copy link
Member Author

rasky commented Mar 13, 2017 via email

@bradfitz
Copy link
Contributor

We replied at the same time.

The linker's doing almost everything it can do already. Anything more requires a lot of work (to prove to itself that things are safe and preserve language semantics) for rare gains that are usually only beneficial in somewhat contrived cases like this one.

Some init funcs are implicit, like top-level var assignments, dependencies on other packages (and thus to their init funcs), etc.

@davecheney
Copy link
Contributor

davecheney commented Mar 13, 2017 via email

@bradfitz
Copy link
Contributor

It's 9.5 MB on mine. Dave, I think your deps are out of date.

@bradfitz
Copy link
Contributor

As one example of the sorts of init-reductions you can do to save space for simple programs like this (but are useless for large programs):

This saves 12835 bytes:

diff --git a/google/appengine.go b/google/appengine.go
index 4243f4c..74418f9 100644
--- a/google/appengine.go
+++ b/google/appengine.go
@@ -45,7 +45,7 @@ func AppEngineTokenSource(ctx context.Context, scope ...string) oauth2.TokenSour
 // aeTokens helps the fetched tokens to be reused until their expiration.
 var (
        aeTokensMu sync.Mutex
-       aeTokens   = make(map[string]*tokenLock) // key is space-separated scopes
+       aeTokens   map[string]*tokenLock // key is space-separated scopes
 )
 
 type tokenLock struct {
@@ -67,6 +67,9 @@ func (ts *appEngineTokenSource) Token() (*oauth2.Token, error) {
        aeTokensMu.Lock()
        tok, ok := aeTokens[ts.key]
        if !ok {
+               if aeTokens == nil {
+                       aeTokens = make(map[string]*tokenLock)
+               }
                tok = &tokenLock{}
                aeTokens[ts.key] = tok
        }

I found that using the output above.

Then you just keep doing that repeatedly, pruning the dep graph, and then your "Hello, World" program is small again. But your real program is still exactly the same size.

@rasky
Copy link
Member Author

rasky commented Mar 13, 2017

FWIW, the example is not made up so much. I'm writing a basic oauth2 client, and to be able to login with Google and access the basic profile, I only need google.Endpoint out of the whole package, but this brings in lots of unneeded dependencies (at least in my case, I don't use grpc or appengine or GCE). My real-world binary went from 5Mb to 11Mb just by using google.Endpoint. It looks like you're saying it's more of a bug in google itself (which might be well true), but I think it also points towards a general deficiency in the toolchain.

For instance, in the case you showed, the toolchain doesn't realize that, since aeTokens is unused, it can dispose the generated init function as well because a simple allocation doesn't have side effects (as far as I can see). I can't find a tracking bug for this issue; should I open one and call it "generated init functions without side effects are not discarded when symbols are unused"?

@bradfitz
Copy link
Contributor

If you want to file such a linker bug, feel free. But you should include concrete examples in your bug report of cases it should handle.

Filing an x/oauth2/google and other bugs for related packages would pay off quicker, though.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants