From 39da9ae5130afa58f8b9e4ea609a57d516bd78db Mon Sep 17 00:00:00 2001 From: Rob Findley Date: Thu, 6 May 2021 22:28:37 -0400 Subject: [PATCH] go/types: ensure that Named.check is nilled out once it is expanded To support lazy expansion of defined types, *Named holds on to a *Checker field, which can pin the *Checker in memory. This can have meaningful memory implications for applications that keep type information around. Ensure that the Checker field is nilled out for any Named types that are instantiated during the type checking pass, by deferring a clean up to 'later' boundaries. In testing this almost exactly offset the ~6% memory footprint increase I observed with 1.17. Fixes #45580 Change-Id: I8aa5bb777573a924afe36e79fa65f8729336bceb Reviewed-on: https://go-review.googlesource.com/c/go/+/318849 Trust: Robert Findley Trust: Robert Griesemer Run-TryBot: Robert Findley TryBot-Result: Go Bot Reviewed-by: Robert Griesemer --- src/go/types/decl.go | 53 ++++++++++++++++++++++++++++++---------- src/go/types/sanitize.go | 3 +++ src/go/types/type.go | 17 ++++++++++++- 3 files changed, 59 insertions(+), 14 deletions(-) diff --git a/src/go/types/decl.go b/src/go/types/decl.go index 5f38a346ce..9211febc6d 100644 --- a/src/go/types/decl.go +++ b/src/go/types/decl.go @@ -577,15 +577,37 @@ func (check *Checker) varDecl(obj *Var, lhs []*Var, typ, init ast.Expr) { // n0.check != nil, the cycle is reported. func (n0 *Named) under() Type { u := n0.underlying - if u == nil { - return Typ[Invalid] + + if u == Typ[Invalid] { + return u } // If the underlying type of a defined type is not a defined - // type, then that is the desired underlying type. + // (incl. instance) type, then that is the desired underlying + // type. + switch u.(type) { + case nil: + return Typ[Invalid] + default: + // common case + return u + case *Named, *instance: + // handled below + } + + if n0.check == nil { + panic("internal error: Named.check == nil but type is incomplete") + } + + // Invariant: after this point n0 as well as any named types in its + // underlying chain should be set up when this function exits. + check := n0.check + + // If we can't expand u at this point, it is invalid. n := asNamed(u) if n == nil { - return u // common case + n0.underlying = Typ[Invalid] + return n0.underlying } // Otherwise, follow the forward chain. @@ -597,7 +619,16 @@ func (n0 *Named) under() Type { u = Typ[Invalid] break } - n1 := asNamed(u) + var n1 *Named + switch u1 := u.(type) { + case *Named: + n1 = u1 + case *instance: + n1, _ = u1.expand().(*Named) + if n1 == nil { + u = Typ[Invalid] + } + } if n1 == nil { break // end of chain } @@ -608,11 +639,7 @@ func (n0 *Named) under() Type { if i, ok := seen[n]; ok { // cycle - // TODO(rFindley) revert this to a method on Checker. Having a possibly - // nil Checker on Named and TypeParam is too subtle. - if n0.check != nil { - n0.check.cycleError(path[i:]) - } + check.cycleError(path[i:]) u = Typ[Invalid] break } @@ -622,8 +649,8 @@ func (n0 *Named) under() Type { // We should never have to update the underlying type of an imported type; // those underlying types should have been resolved during the import. // Also, doing so would lead to a race condition (was issue #31749). - // Do this check always, not just in debug more (it's cheap). - if n0.check != nil && n.obj.pkg != n0.check.pkg { + // Do this check always, not just in debug mode (it's cheap). + if n.obj.pkg != check.pkg { panic("internal error: imported type with unresolved underlying type") } n.underlying = u @@ -665,7 +692,7 @@ func (check *Checker) typeDecl(obj *TypeName, tdecl *ast.TypeSpec, def *Named) { } else { // defined type declaration - named := &Named{check: check, obj: obj} + named := check.newNamed(obj, nil, nil) def.setUnderlying(named) obj.typ = named // make sure recursive type declarations terminate diff --git a/src/go/types/sanitize.go b/src/go/types/sanitize.go index 5970ab38c7..727ec173ea 100644 --- a/src/go/types/sanitize.go +++ b/src/go/types/sanitize.go @@ -135,6 +135,9 @@ func (s sanitizer) typ(typ Type) Type { } case *Named: + if debug && t.check != nil { + panic("internal error: Named.check != nil") + } if orig := s.typ(t.orig); orig != t.orig { t.orig = orig } diff --git a/src/go/types/type.go b/src/go/types/type.go index 3303cfc077..3fdb2365a0 100644 --- a/src/go/types/type.go +++ b/src/go/types/type.go @@ -644,7 +644,7 @@ func (c *Chan) Elem() Type { return c.elem } // A Named represents a named (defined) type. type Named struct { - check *Checker // for Named.under implementation + check *Checker // for Named.under implementation; nilled once under has been called info typeInfo // for cycle detection obj *TypeName // corresponding declared object orig Type // type (on RHS of declaration) this *Named type is derived of (for cycle reporting) @@ -673,6 +673,21 @@ func (check *Checker) newNamed(obj *TypeName, underlying Type, methods []*Func) if obj.typ == nil { obj.typ = typ } + // Ensure that typ is always expanded, at which point the check field can be + // nilled out. + // + // Note that currently we cannot nil out check inside typ.under(), because + // it's possible that typ is expanded multiple times. + // + // TODO(rFindley): clean this up so that under is the only function mutating + // named types. + check.later(func() { + switch typ.under().(type) { + case *Named, *instance: + panic("internal error: unexpanded underlying type") + } + typ.check = nil + }) return typ }