From 7ab875402985ea5a31512fb9750dc0f809e06861 Mon Sep 17 00:00:00 2001 From: Matthew Dempsky Date: Mon, 2 Aug 2021 23:56:13 -0700 Subject: [PATCH] [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 Run-TryBot: Matthew Dempsky TryBot-Result: Go Bot Reviewed-by: Cuong Manh Le --- src/cmd/compile/internal/noder/reader.go | 71 +++++++++++++------ .../compile/internal/reflectdata/reflect.go | 3 +- 2 files changed, 50 insertions(+), 24 deletions(-) diff --git a/src/cmd/compile/internal/noder/reader.go b/src/cmd/compile/internal/noder/reader.go index 83979a91c8a..5481812b18d 100644 --- 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 - } - seen[key] = typ + addType := func(typ *types.Type) bool { + if typ.Sym() != nil { + return true } - r.wrapType(typ, target) + 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) + } + return false + } + + 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 dca8de74f30..19cf2a0a125 100644 --- 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) {