cmd/compile: make typenamesym do less work

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'd like to use Type.cmp for sorting,
but that causes infinite recursion at the moment;
see #19869.

For now, use Type.LongString as the sort key,
which is a complete description of the type.
Type.LongString is relatively expensive,
but we calculate it only once per type,
and signatlist is generally fairly small,
so the performance impact is minimal.

Updates #15756

name       old alloc/op    new alloc/op    delta
Template      39.4MB ± 0%     39.4MB ± 0%    ~     (p=0.222 n=5+5)
Unicode       29.8MB ± 0%     29.8MB ± 0%    ~     (p=0.151 n=5+5)
GoTypes        113MB ± 0%      113MB ± 0%    ~     (p=0.095 n=5+5)
SSA           1.25GB ± 0%     1.25GB ± 0%  +0.04%  (p=0.008 n=5+5)
Flate         25.3MB ± 0%     25.4MB ± 0%    ~     (p=0.056 n=5+5)
GoParser      31.8MB ± 0%     31.8MB ± 0%    ~     (p=0.310 n=5+5)
Reflect       78.3MB ± 0%     78.3MB ± 0%    ~     (p=0.690 n=5+5)
Tar           26.7MB ± 0%     26.7MB ± 0%    ~     (p=0.548 n=5+5)
XML           42.2MB ± 0%     42.2MB ± 0%    ~     (p=0.222 n=5+5)

name       old allocs/op   new allocs/op   delta
Template        387k ± 0%       388k ± 0%    ~     (p=0.056 n=5+5)
Unicode         320k ± 0%       321k ± 0%  +0.32%  (p=0.032 n=5+5)
GoTypes        1.14M ± 0%      1.15M ± 0%    ~     (p=0.095 n=5+5)
SSA            9.70M ± 0%      9.72M ± 0%  +0.18%  (p=0.008 n=5+5)
Flate           234k ± 0%       235k ± 0%  +0.60%  (p=0.008 n=5+5)
GoParser        317k ± 0%       317k ± 0%    ~     (p=1.000 n=5+5)
Reflect         982k ± 0%       983k ± 0%    ~     (p=0.841 n=5+5)
Tar             252k ± 1%       252k ± 0%    ~     (p=0.310 n=5+5)
XML             393k ± 0%       392k ± 0%    ~     (p=0.548 n=5+5)

Change-Id: I53a3b95d19cf1a7b7511a94fba896706addf84fb
Reviewed-on: https://go-review.googlesource.com/39710
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 b11ca7082a8b75ca5cd4df2c5bc785a1db5ed11c..815086777a22880abcfbcfd1089c31aff1a5b660 100644 (file)
--- a/src/cmd/compile/internal/gc/reflect.go
+++ b/src/cmd/compile/internal/gc/reflect.go
@@ -34,7 +34,7 @@ type ptabEntry struct {
 }
 
 // runtime interface and reflection data structures
-var signatlist []*Type
+var signatlist = make(map[*Type]bool)
 var itabs []itabEntry
 var ptabs []ptabEntry
 
@@ -933,24 +933,22 @@ func typesymprefix(prefix string, t *Type) *Sym {
 
 func typenamesym(t *Type) *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 *Type) *Node {
+       s := typenamesym(t)
        if s.Def == nil {
                n := newnamel(src.NoXPos, s)
                n.Type = Types[TUINT8]
                n.Class = PEXTERN
                n.Typecheck = 1
                s.Def = n
-
-               signatlist = append(signatlist, t)
        }
-
-       return s.Def.Sym
-}
-
-func typename(t *Type) *Node {
-       s := typenamesym(t)
        n := nod(OADDR, s.Def, nil)
        n.Type = typPtr(s.Def.Type)
        n.SetAddable(true)
@@ -1417,21 +1415,35 @@ func itabsym(it *obj.LSym, offset int64) *obj.LSym {
        return syms[methodnum]
 }
 
+func addsignat(t *Type) {
+       signatlist[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(typPtr(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: t.LongString()})
+                       delete(signatlist, t)
+               }
+               sort.Sort(typesByLongString(signats))
+               for _, ts := range signats {
+                       t := ts.t
+                       dtypesym(t)
+                       if t.Sym != nil {
+                               dtypesym(typPtr(t))
+                       }
                }
        }
 
@@ -1532,6 +1544,18 @@ func dumptypestructs() {
        }
 }
 
+type typeAndStr struct {
+       t *Type
+       s string
+}
+
+// TODO(josharian): simplify this to just use Type.cmp once issue 19869 has been fixed.
+type typesByLongString []typeAndStr
+
+func (a typesByLongString) Len() int           { return len(a) }
+func (a typesByLongString) Less(i, j int) bool { return a[i].s < a[j].s }
+func (a typesByLongString) Swap(i, j int)      { a[i], a[j] = a[j], a[i] }
+
 type pkgByPath []*Pkg
 
 func (a pkgByPath) Len() int           { return len(a) }

diff --git a/src/cmd/compile/internal/gc/type.go b/src/cmd/compile/internal/gc/type.go
index b74111271125ffed04cc9a06b420b9dbd0555e06..61220648cdc75e95a2a5e58eb1b1cf0a5633b6b7 100644 (file)
--- a/src/cmd/compile/internal/gc/type.go
+++ b/src/cmd/compile/internal/gc/type.go
@@ -976,6 +976,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.