[dev.typeparams] cmd/compile: avoid redundant method wrappers in unified IR

Currently, unified IR takes a simple approach of generating method
wrappers for every anonymous type that it sees. This is correct, but
spends a lot of time in code generation and bloats the object files
with duplicate method wrappers that the linker discards.

This CL changes it to distinguish anonymous types that were found in
imported packages vs the local package. The simple win here is that
now we stop emitting wrappers for imported types; but by keeping track
of them and marking them as "have" instead of "need", we can avoid
emitting wrappers for types that appear in both the local package and
imported packages.

This can be improved further, but this is a simple first step that
prevents large protobuf projects from blowing up build cache limits.

Change-Id: Ia65e8981cb1f067eca2bd072b9bbb77c27b95207
Reviewed-on: https://go-review.googlesource.com/c/go/+/339411
Trust: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>

diff --git a/src/cmd/compile/internal/noder/reader.go b/src/cmd/compile/internal/noder/reader.go
index 83979a91c8abdc24a41370e977f2b5541d9da927..5481812b18d05c2f096797de8c91ad5499c876c9 100644 (file)
--- a/src/cmd/compile/internal/noder/reader.go
+++ b/src/cmd/compile/internal/noder/reader.go
@@ -2125,16 +2125,32 @@ func usedLocals(body []ir.Node) ir.NameSet {
 // method wrappers.
 var needWrapperTypes []*types.Type
 
-func (r *reader) needWrapper(typ *types.Type) *types.Type {
-       // TODO(mdempsky): Be more judicious about generating wrappers.
-       // For now, generating all possible wrappers is simple and correct,
-       // but potentially wastes a lot of time/space.
+// haveWrapperTypes lists types for which we know we already have
+// method wrappers, because we found the type in an imported package.
+var haveWrapperTypes []*types.Type
 
+func (r *reader) needWrapper(typ *types.Type) *types.Type {
        if typ.IsPtr() {
                base.Fatalf("bad pointer type: %v", typ)
        }
 
-       needWrapperTypes = append(needWrapperTypes, typ)
+       // If a type was found in an imported package, then we can assume
+       // that package (or one of its transitive dependencies) already
+       // generated method wrappers for it.
+       //
+       // Exception: If we're instantiating an imported generic type or
+       // function, we might be instantiating it with type arguments not
+       // previously seen before.
+       //
+       // TODO(mdempsky): Distinguish when a generic function or type was
+       // instantiated in an imported package so that we can add types to
+       // haveWrapperTypes instead.
+       if r.p != localPkgReader && !r.hasTypeParams() {
+               haveWrapperTypes = append(haveWrapperTypes, typ)
+       } else {
+               needWrapperTypes = append(needWrapperTypes, typ)
+       }
+
        return typ
 }
 
@@ -2143,21 +2159,33 @@ func (r *reader) wrapTypes(target *ir.Package) {
        r.needWrapper(types.ErrorType)
 
        seen := make(map[string]*types.Type)
-       for _, typ := range needWrapperTypes {
-               if typ.Sym() == nil {
-                       key := typ.LinkString()
-                       if prev := seen[key]; prev != nil {
-                               if !types.Identical(typ, prev) {
-                                       base.Fatalf("collision: types %v and %v have short string %q", typ, prev, key)
-                               }
-                               continue
+       addType := func(typ *types.Type) bool {
+               if typ.Sym() != nil {
+                       return true
+               }
+
+               key := typ.LinkString()
+               if prev := seen[key]; prev != nil {
+                       if !types.Identical(typ, prev) {
+                               base.Fatalf("collision: types %v and %v have short string %q", typ, prev, key)
                        }
-                       seen[key] = typ
+                       return false
                }
 
-               r.wrapType(typ, target)
+               seen[key] = typ
+               return true
        }
 
+       for _, typ := range haveWrapperTypes {
+               addType(typ)
+       }
+       haveWrapperTypes = nil
+
+       for _, typ := range needWrapperTypes {
+               if addType(typ) {
+                       r.wrapType(typ, target)
+               }
+       }
        needWrapperTypes = nil
 }
 
@@ -2243,12 +2271,6 @@ func (r *reader) methodValueWrapper(tbase *types.Type, method *types.Field, targ
        assert(!sym.Uniq())
        sym.SetUniq(true)
 
-       // TODO(mdempsky): Fix typecheck to not depend on creation of
-       // imported method value wrappers.
-       if false && !reflectdata.NeedEmit(tbase) {
-               return
-       }
-
        // TODO(mdempsky): Use method.Pos instead?
        pos := base.AutogeneratedPos
 
@@ -2258,6 +2280,11 @@ func (r *reader) methodValueWrapper(tbase *types.Type, method *types.Field, targ
        // Declare and initialize variable holding receiver.
        recv := ir.NewHiddenParam(pos, fn, typecheck.Lookup(".this"), recvType)
 
+       if !reflectdata.NeedEmit(tbase) {
+               typecheck.Func(fn)
+               return
+       }
+
        addTailCall(pos, fn, recv, method)
 
        r.finishWrapperFunc(fn, target)

diff --git a/src/cmd/compile/internal/reflectdata/reflect.go b/src/cmd/compile/internal/reflectdata/reflect.go
index dca8de74f303ff508164a4a60ab0c1beeef4861c..19cf2a0a1254abba2dcce03b8a6b6ffb5362b05c 100644 (file)
--- a/src/cmd/compile/internal/reflectdata/reflect.go
+++ b/src/cmd/compile/internal/reflectdata/reflect.go
@@ -1809,9 +1809,8 @@ func methodWrapper(rcvr *types.Type, method *types.Field, forItab bool) *obj.LSy
        newnam.SetSiggen(true)
 
        // Except in quirks mode, unified IR creates its own wrappers.
-       // Complain loudly if it missed any.
        if base.Debug.Unified != 0 && base.Debug.UnifiedQuirks == 0 {
-               base.FatalfAt(method.Pos, "missing wrapper for %+v (%+v, %v) / %+v / %+v", rcvr, orig, types.IsDirectIface(orig), method.Sym, newnam)
+               return lsym
        }
 
        if !generic && types.Identical(rcvr, method.Type.Recv().Type) {