Commit 91433eb5 authored by Josh Bleecher Snyder's avatar Josh Bleecher Snyder

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: default avatarMatthew Dempsky <mdempsky@google.com>
parent b599c1fe
......@@ -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) }
......
......@@ -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.
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment