cmd/compile: make typenamesym do less work

This is a re-roll of CL 39710,
which broke deterministic builds.

typenamesym is called from three places:
typename, ngotype, and Type.Symbol.
Only in typename do we actually need a Node.
ngotype and Type.Symbol require only a Sym.
And writing the newly created Node to
Sym.Def is unsafe in a concurrent backend.
Rather than use a mutex protect to Sym.Def,
make typenamesym not touch Sym.Def.

The assignment to Sym.Def was serving a second purpose,
namely to prevent duplicate entries on signatlist.
Preserve that functionality by switching signatlist to a map.
This in turn requires that we sort signatlist
when exporting it, to preserve reproducibility.

We sort using exactly the same mechanism
that the export code (dtypesym) uses.
Failure to do that led to non-deterministic builds (#19872).
Since we've already calculated the Type's export name,
we could pass it to dtypesym, sparing it a bit of work.
That can be done as a future optimization.

Updates #15756

name       old alloc/op      new alloc/op      delta
Template        39.2MB ± 0%       39.3MB ± 0%    ~     (p=0.075 n=10+10)
Unicode         29.8MB ± 0%       29.8MB ± 0%    ~     (p=0.393 n=10+10)
GoTypes          113MB ± 0%        113MB ± 0%  +0.06%  (p=0.027 n=10+8)
SSA             1.25GB ± 0%       1.25GB ± 0%  +0.05%  (p=0.000 n=8+10)
Flate           25.3MB ± 0%       25.3MB ± 0%    ~     (p=0.105 n=10+10)
GoParser        31.7MB ± 0%       31.8MB ± 0%    ~     (p=0.165 n=10+10)
Reflect         78.2MB ± 0%       78.2MB ± 0%    ~     (p=0.190 n=10+10)
Tar             26.6MB ± 0%       26.6MB ± 0%    ~     (p=0.481 n=10+10)
XML             42.2MB ± 0%       42.2MB ± 0%    ~     (p=0.968 n=10+9)

name       old allocs/op     new allocs/op     delta
Template          384k ± 1%         386k ± 1%  +0.43%  (p=0.019 n=10+10)
Unicode           320k ± 0%         321k ± 0%  +0.36%  (p=0.015 n=10+10)
GoTypes          1.14M ± 0%        1.14M ± 0%  +0.33%  (p=0.000 n=10+8)
SSA              9.69M ± 0%        9.71M ± 0%  +0.18%  (p=0.000 n=10+9)
Flate             233k ± 1%         233k ± 1%    ~     (p=0.481 n=10+10)
GoParser          315k ± 1%         316k ± 1%    ~     (p=0.113 n=9+10)
Reflect           979k ± 0%         979k ± 0%    ~     (p=0.971 n=10+10)
Tar               250k ± 1%         250k ± 1%    ~     (p=0.481 n=10+10)
XML               391k ± 1%         392k ± 0%    ~     (p=1.000 n=10+9)

Change-Id: Ia9f21cc29c047021fa8a18c2a3d861a5146aefac
Reviewed-on: https://go-review.googlesource.com/39915
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>

diff --git a/src/cmd/compile/internal/gc/reflect.go b/src/cmd/compile/internal/gc/reflect.go
index 3b39b6b1288bc5b32de90332869e235c3605d108..eb89789a2fe94e78803a2e9f1e849b51a33432cf 100644 (file)
--- a/src/cmd/compile/internal/gc/reflect.go
+++ b/src/cmd/compile/internal/gc/reflect.go
@@ -35,7 +35,7 @@ type ptabEntry struct {
 }
 
 // runtime interface and reflection data structures
-var signatlist []*types.Type
+var signatlist = make(map[*types.Type]bool)
 var itabs []itabEntry
 var ptabs []ptabEntry
 
@@ -906,15 +906,17 @@ func dcommontype(s *types.Sym, ot int, t *types.Type) int {
        return ot
 }
 
-func typesym(t *types.Type) *types.Sym {
+func typesymname(t *types.Type) string {
        name := t.ShortString()
-
        // Use a separate symbol name for Noalg types for #17752.
        if a, bad := algtype1(t); a == ANOEQ && bad.Noalg() {
                name = "noalg." + name
        }
+       return name
+}
 
-       return typepkg.Lookup(name)
+func typesym(t *types.Type) *types.Sym {
+       return typepkg.Lookup(typesymname(t))
 }
 
 // tracksym returns the symbol for tracking use of field/method f, assumed
@@ -934,24 +936,23 @@ func typesymprefix(prefix string, t *types.Type) *types.Sym {
 
 func typenamesym(t *types.Type) *types.Sym {
        if t == nil || (t.IsPtr() && t.Elem() == nil) || t.IsUntyped() {
-               Fatalf("typename %v", t)
+               Fatalf("typenamesym %v", t)
        }
        s := typesym(t)
+       addsignat(t)
+       return s
+}
+
+func typename(t *types.Type) *Node {
+       s := typenamesym(t)
        if s.Def == nil {
                n := newnamel(src.NoXPos, s)
                n.Type = types.Types[TUINT8]
                n.Class = PEXTERN
                n.Typecheck = 1
                s.Def = asTypesNode(n)
-
-               signatlist = append(signatlist, t)
        }
 
-       return asNode(s.Def).Sym
-}
-
-func typename(t *types.Type) *Node {
-       s := typenamesym(t)
        n := nod(OADDR, asNode(s.Def), nil)
        n.Type = types.NewPtr(asNode(s.Def).Type)
        n.SetAddable(true)
@@ -1075,14 +1076,18 @@ func needkeyupdate(t *types.Type) bool {
        }
 }
 
-func dtypesym(t *types.Type) *types.Sym {
-       // Replace byte, rune aliases with real type.
-       // They've been separate internally to make error messages
-       // better, but we have to merge them in the reflect tables.
+// formalType replaces byte and rune aliases with real types.
+// They've been separate internally to make error messages
+// better, but we have to merge them in the reflect tables.
+func formalType(t *types.Type) *types.Type {
        if t == types.Bytetype || t == types.Runetype {
-               t = types.Types[t.Etype]
+               return types.Types[t.Etype]
        }
+       return t
+}
 
+func dtypesym(t *types.Type) *types.Sym {
+       t = formalType(t)
        if t.IsUntyped() {
                Fatalf("dtypesym %v", t)
        }
@@ -1418,21 +1423,35 @@ func itabsym(it *obj.LSym, offset int64) *obj.LSym {
        return syms[methodnum]
 }
 
+func addsignat(t *types.Type) {
+       signatlist[formalType(t)] = true
+}
+
 func dumptypestructs() {
        // copy types from externdcl list to signatlist
        for _, n := range externdcl {
                if n.Op == OTYPE {
-                       signatlist = append(signatlist, n.Type)
+                       addsignat(n.Type)
                }
        }
 
-       // Process signatlist.  This can't use range, as entries are
-       // added to the list while it is being processed.
-       for i := 0; i < len(signatlist); i++ {
-               t := signatlist[i]
-               dtypesym(t)
-               if t.Sym != nil {
-                       dtypesym(types.NewPtr(t))
+       // Process signatlist. Use a loop, as dtypesym adds
+       // entries to signatlist while it is being processed.
+       signats := make([]typeAndStr, len(signatlist))
+       for len(signatlist) > 0 {
+               signats = signats[:0]
+               // Transfer entries to a slice and sort, for reproducible builds.
+               for t := range signatlist {
+                       signats = append(signats, typeAndStr{t: t, s: typesymname(t)})
+                       delete(signatlist, t)
+               }
+               sort.Sort(typesByString(signats))
+               for _, ts := range signats {
+                       t := ts.t
+                       dtypesym(t)
+                       if t.Sym != nil {
+                               dtypesym(types.NewPtr(t))
+                       }
                }
        }
 
@@ -1528,6 +1547,17 @@ func dumptypestructs() {
        }
 }
 
+type typeAndStr struct {
+       t *types.Type
+       s string
+}
+
+type typesByString []typeAndStr
+
+func (a typesByString) Len() int           { return len(a) }
+func (a typesByString) Less(i, j int) bool { return a[i].s < a[j].s }
+func (a typesByString) Swap(i, j int)      { a[i], a[j] = a[j], a[i] }
+
 type pkgByPath []*types.Pkg
 
 func (a pkgByPath) Len() int           { return len(a) }

diff --git a/src/cmd/compile/internal/types/type.go b/src/cmd/compile/internal/types/type.go
index 196b7b751f6ad2de5f0736cc39587e0051b81085..b1903f22ec6de6476058c0f304701ffb8f749d55 100644 (file)
--- a/src/cmd/compile/internal/types/type.go
+++ b/src/cmd/compile/internal/types/type.go
@@ -927,6 +927,8 @@ func (r *Sym) cmpsym(s *Sym) ssa.Cmp {
 // cmp compares two *Types t and x, returning ssa.CMPlt,
 // ssa.CMPeq, ssa.CMPgt as t<x, t==x, t>x, for an arbitrary
 // and optimizer-centric notion of comparison.
+// TODO(josharian): make this safe for recursive interface types
+// and use in signatlist sorting. See issue 19869.
 func (t *Type) cmp(x *Type) ssa.Cmp {
        // This follows the structure of eqtype in subr.go
        // with two exceptions.