]> Cypherpunks repositories - gostls13.git/commitdiff
cmd/compile: reduce allocs to c85b77c (pre-fmt.go change) levels
authorRobert Griesemer <gri@golang.org>
Mon, 12 Sep 2016 20:44:43 +0000 (13:44 -0700)
committerRobert Griesemer <gri@golang.org>
Tue, 13 Sep 2016 00:02:54 +0000 (00:02 +0000)
Linker and reflect info generation (reflect.go) relies on formatting
of types (tconv). The fmt.Format based approach introduces extra
allocations, which matter in those cases. Resurrected sconv and tconv
code from commit c85b77c (fmt.go only); and adjusted it slightly.
The formatter-based approach is still used throughout the rest of the
compiler, but reflect.go now uses the tconv method that simply returns
the desired string.

(The timing data below may not be accurate; I've included it only for
comparison with the numbers in issue #16897).

name       old time/op     new time/op     delta
Template       297ms ± 2%      288ms ± 3%  -3.12%        (p=0.000 n=27+29)
Unicode        155ms ± 5%      150ms ± 5%  -3.26%        (p=0.000 n=30+30)
GoTypes        1.00s ± 3%      0.95s ± 3%  -4.51%        (p=0.000 n=28+29)

name       old alloc/op    new alloc/op    delta
Template      46.8MB ± 0%     46.5MB ± 0%  -0.65%        (p=0.000 n=28+30)
Unicode       37.9MB ± 0%     37.8MB ± 0%  -0.24%        (p=0.000 n=29+30)
GoTypes        144MB ± 0%      143MB ± 0%  -0.68%        (p=0.000 n=30+30)

name       old allocs/op   new allocs/op   delta
Template        469k ± 0%       446k ± 0%  -5.01%        (p=0.000 n=29+30)
Unicode         375k ± 0%       369k ± 0%  -1.62%        (p=0.000 n=30+28)
GoTypes        1.47M ± 0%      1.37M ± 0%  -6.29%        (p=0.000 n=30+30)

The code for sconv/tconv in fmt.go now closely match the code from c85b77c
again; except that the functions are now methods. Removing the use of
the bytes.Buffer in tconv and special-caseing interface{} has helped a
small amount as well:

name       old time/op     new time/op     delta
Template       299ms ± 3%      288ms ± 3%  -3.83%        (p=0.000 n=29+29)
Unicode        156ms ± 5%      150ms ± 5%  -3.56%        (p=0.000 n=30+30)
GoTypes        960ms ± 2%      954ms ± 3%  -0.58%        (p=0.037 n=26+29)

name       old alloc/op    new alloc/op    delta
Template      46.6MB ± 0%     46.5MB ± 0%  -0.22%        (p=0.000 n=30+30)
Unicode       37.8MB ± 0%     37.8MB ± 0%    ~           (p=0.075 n=30+30)
GoTypes        143MB ± 0%      143MB ± 0%  -0.31%        (p=0.000 n=30+30)

name       old allocs/op   new allocs/op   delta
Template        447k ± 0%       446k ± 0%  -0.28%        (p=0.000 n=30+30)
Unicode         369k ± 0%       369k ± 0%  -0.03%        (p=0.032 n=30+28)
GoTypes        1.38M ± 0%      1.37M ± 0%  -0.35%        (p=0.000 n=29+30)

Comparison between c85b77c and now (see issue #16897):

name       old time/op     new time/op     delta
Template       307ms ± 4%      288ms ± 3%  -6.24%  (p=0.000 n=29+29)
Unicode        164ms ± 4%      150ms ± 5%  -8.20%  (p=0.000 n=30+30)
GoTypes        1.01s ± 3%      0.95s ± 3%  -5.72%  (p=0.000 n=30+29)

name       old alloc/op    new alloc/op    delta
Template      46.8MB ± 0%     46.5MB ± 0%  -0.66%  (p=0.000 n=29+30)
Unicode       37.8MB ± 0%     37.8MB ± 0%  -0.13%  (p=0.000 n=30+30)
GoTypes        143MB ± 0%      143MB ± 0%  -0.11%  (p=0.000 n=30+30)

name       old allocs/op   new allocs/op   delta
Template        444k ± 0%       446k ± 0%  +0.48%  (p=0.000 n=30+30)
Unicode         369k ± 0%       369k ± 0%  +0.09%  (p=0.000 n=30+28)
GoTypes        1.35M ± 0%      1.37M ± 0%  +1.47%  (p=0.000 n=30+30)

There's still a small increase (< 1.5%) for GoTypes but pending a complete
rewrite of fmt.go, this seems ok again.

Fixes #16897.

Change-Id: I7e0e56cd1b9f981252eded917f5752259d402354
Reviewed-on: https://go-review.googlesource.com/29087
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
src/cmd/compile/internal/gc/fmt.go
src/cmd/compile/internal/gc/reflect.go

index 9032e1711f68794d87aa7b982c78d5b13be5e12c..b43028383848766f1edc76c58f346a2942a913b5 100644 (file)
@@ -5,6 +5,7 @@
 package gc
 
 import (
+       "bytes"
        "cmd/internal/obj"
        "fmt"
        "strconv"
@@ -506,34 +507,28 @@ func (et EType) String() string {
        return fmt.Sprintf("E-%d", et)
 }
 
-func (s *Sym) symfmt(f fmt.State, flag FmtFlag) {
+func (s *Sym) symfmt(flag FmtFlag) string {
        if s.Pkg != nil && flag&FmtShort == 0 {
                switch fmtmode {
                case FErr: // This is for the user
                        if s.Pkg == builtinpkg || s.Pkg == localpkg {
-                               fmt.Fprint(f, s.Name)
-                               return
+                               return s.Name
                        }
 
                        // If the name was used by multiple packages, display the full path,
                        if s.Pkg.Name != "" && numImport[s.Pkg.Name] > 1 {
-                               fmt.Fprintf(f, "%q.%s", s.Pkg.Path, s.Name)
-                               return
+                               return fmt.Sprintf("%q.%s", s.Pkg.Path, s.Name)
                        }
-                       fmt.Fprint(f, s.Pkg.Name+"."+s.Name)
-                       return
+                       return s.Pkg.Name + "." + s.Name
 
                case FDbg:
-                       fmt.Fprint(f, s.Pkg.Name+"."+s.Name)
-                       return
+                       return s.Pkg.Name + "." + s.Name
 
                case FTypeId:
                        if flag&FmtUnsigned != 0 {
-                               fmt.Fprint(f, s.Pkg.Name+"."+s.Name) // dcommontype, typehash
-                               return
+                               return s.Pkg.Name + "." + s.Name // dcommontype, typehash
                        }
-                       fmt.Fprint(f, s.Pkg.Prefix+"."+s.Name) // (methodsym), typesym, weaksym
-                       return
+                       return s.Pkg.Prefix + "." + s.Name // (methodsym), typesym, weaksym
                }
        }
 
@@ -546,15 +541,13 @@ func (s *Sym) symfmt(f fmt.State, flag FmtFlag) {
                }
 
                if fmtmode == FDbg {
-                       fmt.Fprintf(f, "@%q.%s", s.Pkg.Path, name)
-                       return
+                       return fmt.Sprintf("@%q.%s", s.Pkg.Path, name)
                }
 
-               fmt.Fprint(f, name)
-               return
+               return name
        }
 
-       fmt.Fprint(f, s.Name)
+       return s.Name
 }
 
 var basicnames = []string{
@@ -581,24 +574,21 @@ var basicnames = []string{
        TBLANK:      "blank",
 }
 
-func (t *Type) typefmt(s fmt.State, flag FmtFlag) {
+func (t *Type) typefmt(flag FmtFlag) string {
        if t == nil {
-               fmt.Fprint(s, "<T>")
-               return
+               return "<T>"
        }
 
        if t == bytetype || t == runetype {
                // in %-T mode collapse rune and byte with their originals.
                if fmtmode != FTypeId {
-                       fmt.Fprintf(s, "%S", t.Sym)
-                       return
+                       return t.Sym.sconv(FmtShort)
                }
                t = Types[t.Etype]
        }
 
        if t == errortype {
-               fmt.Fprint(s, "error")
-               return
+               return "error"
        }
 
        // Unless the 'l' flag was specified, if the type has a name, just print that name.
@@ -607,127 +597,127 @@ func (t *Type) typefmt(s fmt.State, flag FmtFlag) {
                case FTypeId:
                        if flag&FmtShort != 0 {
                                if t.Vargen != 0 {
-                                       fmt.Fprintf(s, "%S·%d", t.Sym, t.Vargen)
-                                       return
+                                       return fmt.Sprintf("%v·%d", t.Sym.sconv(FmtShort), t.Vargen)
                                }
-                               fmt.Fprintf(s, "%S", t.Sym)
-                               return
+                               return t.Sym.sconv(FmtShort)
                        }
 
                        if flag&FmtUnsigned != 0 {
-                               fmt.Fprintf(s, "% v", t.Sym)
-                               return
+                               return t.Sym.sconv(FmtUnsigned)
                        }
 
                        if t.Sym.Pkg == localpkg && t.Vargen != 0 {
-                               fmt.Fprintf(s, "%v·%d", t.Sym, t.Vargen)
-                               return
+                               return fmt.Sprintf("%v·%d", t.Sym, t.Vargen)
                        }
                }
 
-               fmt.Fprint(s, t.Sym)
-               return
+               return t.Sym.String()
        }
 
        if int(t.Etype) < len(basicnames) && basicnames[t.Etype] != "" {
+               prefix := ""
                if fmtmode == FErr && (t == idealbool || t == idealstring) {
-                       fmt.Fprint(s, "untyped ")
+                       prefix = "untyped "
                }
-               fmt.Fprint(s, basicnames[t.Etype])
-               return
+               return prefix + basicnames[t.Etype]
        }
 
        if fmtmode == FDbg {
                fmtmode = 0
-               fmt.Fprintf(s, "%v-", t.Etype)
-               t.typefmt(s, flag)
+               str := t.Etype.String() + "-" + t.typefmt(flag)
                fmtmode = FDbg
-               return
+               return str
        }
 
        switch t.Etype {
        case TPTR32, TPTR64:
                if fmtmode == FTypeId && (flag&FmtShort != 0) {
-                       fmt.Fprintf(s, "*%S", t.Elem())
-                       return
+                       return "*" + t.Elem().tconv(FmtShort)
                }
-               fmt.Fprint(s, "*"+t.Elem().String())
+               return "*" + t.Elem().String()
 
        case TARRAY:
                if t.isDDDArray() {
-                       fmt.Fprint(s, "[...]"+t.Elem().String())
-                       return
+                       return "[...]" + t.Elem().String()
                }
-               fmt.Fprintf(s, "[%d]%v", t.NumElem(), t.Elem())
+               return fmt.Sprintf("[%d]%v", t.NumElem(), t.Elem())
 
        case TSLICE:
-               fmt.Fprint(s, "[]"+t.Elem().String())
+               return "[]" + t.Elem().String()
 
        case TCHAN:
                switch t.ChanDir() {
                case Crecv:
-                       fmt.Fprint(s, "<-chan "+t.Elem().String())
-                       return
+                       return "<-chan " + t.Elem().String()
 
                case Csend:
-                       fmt.Fprint(s, "chan<- "+t.Elem().String())
-                       return
+                       return "chan<- " + t.Elem().String()
                }
 
                if t.Elem() != nil && t.Elem().IsChan() && t.Elem().Sym == nil && t.Elem().ChanDir() == Crecv {
-                       fmt.Fprint(s, "chan ("+t.Elem().String()+")")
-                       return
+                       return "chan (" + t.Elem().String() + ")"
                }
-               fmt.Fprint(s, "chan "+t.Elem().String())
+               return "chan " + t.Elem().String()
 
        case TMAP:
-               fmt.Fprint(s, "map["+t.Key().String()+"]"+t.Val().String())
+               return "map[" + t.Key().String() + "]" + t.Val().String()
 
        case TINTER:
-               fmt.Fprint(s, "interface {")
+               if t.IsEmptyInterface() {
+                       return "interface {}"
+               }
+               buf := make([]byte, 0, 64)
+               buf = append(buf, "interface {"...)
                for i, f := range t.Fields().Slice() {
                        if i != 0 {
-                               fmt.Fprint(s, ";")
+                               buf = append(buf, ';')
                        }
-                       fmt.Fprint(s, " ")
+                       buf = append(buf, ' ')
                        switch {
                        case f.Sym == nil:
                                // Check first that a symbol is defined for this type.
                                // Wrong interface definitions may have types lacking a symbol.
                                break
                        case exportname(f.Sym.Name):
-                               fmt.Fprintf(s, "%S", f.Sym)
+                               buf = append(buf, f.Sym.sconv(FmtShort)...)
                        default:
-                               fmt.Fprintf(s, "% v", f.Sym)
+                               buf = append(buf, f.Sym.sconv(FmtUnsigned)...)
                        }
-                       fmt.Fprintf(s, "%S", f.Type)
+                       buf = append(buf, f.Type.tconv(FmtShort)...)
                }
                if t.NumFields() != 0 {
-                       fmt.Fprint(s, " ")
+                       buf = append(buf, ' ')
                }
-               fmt.Fprint(s, "}")
+               buf = append(buf, '}')
+               return string(buf)
 
        case TFUNC:
+               buf := make([]byte, 0, 64)
                if flag&FmtShort != 0 {
                        // no leading func
                } else {
                        if t.Recv() != nil {
-                               fmt.Fprintf(s, "method %v ", t.Recvs())
+                               buf = append(buf, "method"...)
+                               buf = append(buf, t.Recvs().String()...)
+                               buf = append(buf, ' ')
                        }
-                       fmt.Fprint(s, "func")
+                       buf = append(buf, "func"...)
                }
-               fmt.Fprintf(s, "%v", t.Params())
+               buf = append(buf, t.Params().String()...)
 
                switch t.Results().NumFields() {
                case 0:
                        // nothing to do
 
                case 1:
-                       fmt.Fprintf(s, " %v", t.Results().Field(0).Type) // struct->field->field's type
+                       buf = append(buf, ' ')
+                       buf = append(buf, t.Results().Field(0).Type.String()...) // struct->field->field's type
 
                default:
-                       fmt.Fprintf(s, " %v", t.Results())
+                       buf = append(buf, ' ')
+                       buf = append(buf, t.Results().String()...)
                }
+               return string(buf)
 
        case TSTRUCT:
                if m := t.StructType().Map; m != nil {
@@ -735,71 +725,68 @@ func (t *Type) typefmt(s fmt.State, flag FmtFlag) {
                        // Format the bucket struct for map[x]y as map.bucket[x]y.
                        // This avoids a recursive print that generates very long names.
                        if mt.Bucket == t {
-                               fmt.Fprint(s, "map.bucket["+m.Key().String()+"]"+m.Val().String())
-                               return
+                               return "map.bucket[" + m.Key().String() + "]" + m.Val().String()
                        }
 
                        if mt.Hmap == t {
-                               fmt.Fprint(s, "map.hdr["+m.Key().String()+"]"+m.Val().String())
-                               return
+                               return "map.hdr[" + m.Key().String() + "]" + m.Val().String()
                        }
 
                        if mt.Hiter == t {
-                               fmt.Fprint(s, "map.iter["+m.Key().String()+"]"+m.Val().String())
-                               return
+                               return "map.iter[" + m.Key().String() + "]" + m.Val().String()
                        }
 
                        Yyerror("unknown internal map type")
                }
 
+               var buf bytes.Buffer
                if t.IsFuncArgStruct() {
-                       fmt.Fprint(s, "(")
+                       buf.WriteString("(")
                        var flag1 FmtFlag
                        if fmtmode == FTypeId || fmtmode == FErr { // no argument names on function signature, and no "noescape"/"nosplit" tags
                                flag1 = FmtShort
                        }
                        for i, f := range t.Fields().Slice() {
                                if i != 0 {
-                                       fmt.Fprint(s, ", ")
+                                       buf.WriteString(", ")
                                }
-                               fmt.Fprint(s, Fldconv(f, flag1))
+                               buf.WriteString(Fldconv(f, flag1))
                        }
-                       fmt.Fprint(s, ")")
+                       buf.WriteString(")")
                } else {
-                       fmt.Fprint(s, "struct {")
+                       buf.WriteString("struct {")
                        for i, f := range t.Fields().Slice() {
                                if i != 0 {
-                                       fmt.Fprint(s, ";")
+                                       buf.WriteString(";")
                                }
-                               fmt.Fprint(s, " ")
-                               fmt.Fprint(s, Fldconv(f, FmtLong))
+                               buf.WriteString(" ")
+                               buf.WriteString(Fldconv(f, FmtLong))
                        }
                        if t.NumFields() != 0 {
-                               fmt.Fprint(s, " ")
+                               buf.WriteString(" ")
                        }
-                       fmt.Fprint(s, "}")
+                       buf.WriteString("}")
                }
+               return buf.String()
 
        case TFORW:
                if t.Sym != nil {
-                       fmt.Fprint(s, "undefined "+t.Sym.String())
-                       return
+                       return "undefined " + t.Sym.String()
                }
-               fmt.Fprint(s, "undefined")
+               return "undefined"
 
        case TUNSAFEPTR:
-               fmt.Fprint(s, "unsafe.Pointer")
+               return "unsafe.Pointer"
 
        case TDDDFIELD:
-               fmt.Fprintf(s, "%v <%v> %v", t.Etype, t.Sym, t.DDDField())
+               return fmt.Sprintf("%v <%v> %v", t.Etype, t.Sym, t.DDDField())
 
        case Txxx:
-               fmt.Fprint(s, "Txxx")
-
-       default:
-               // Don't know how to handle - fall back to detailed prints.
-               fmt.Fprintf(s, "%v <%v> %v", t.Etype, t.Sym, t.Elem())
+               return "Txxx"
        }
+
+       // Don't know how to handle - fall back to detailed prints.
+       return fmt.Sprintf("%v <%v> %v", t.Etype, t.Sym, t.Elem())
 }
 
 // Statements which may be rendered with a simplestmt as init.
@@ -1573,7 +1560,7 @@ func (n *Node) nodedump(s fmt.State, flag FmtFlag) {
 func (s *Sym) Format(f fmt.State, verb rune) {
        switch verb {
        case 'v', 'S':
-               s.sconv(f, fmtFlag(f, verb))
+               fmt.Fprint(f, s.sconv(fmtFlag(f, verb)))
 
        default:
                fmt.Fprintf(f, "%%!%c(*Sym=%p)", verb, s)
@@ -1581,34 +1568,33 @@ func (s *Sym) Format(f fmt.State, verb rune) {
 }
 
 func (s *Sym) String() string {
-       return fmt.Sprint(s)
+       return s.sconv(0)
 }
 
 // "%S" suppresses qualifying with package
-func (s *Sym) sconv(f fmt.State, flag FmtFlag) {
+func (s *Sym) sconv(flag FmtFlag) string {
        if flag&FmtLong != 0 {
                panic("linksymfmt")
        }
 
        if s == nil {
-               fmt.Fprint(f, "<S>")
-               return
+               return "<S>"
        }
 
        if s.Name == "_" {
-               fmt.Fprint(f, "_")
-               return
+               return "_"
        }
 
        sf := flag
        sm := setfmode(&flag)
-       s.symfmt(f, flag)
+       str := s.symfmt(flag)
        flag = sf
        fmtmode = sm
+       return str
 }
 
 func (t *Type) String() string {
-       return fmt.Sprint(t)
+       return t.tconv(0)
 }
 
 func Fldconv(f *Field, flag FmtFlag) string {
@@ -1689,7 +1675,7 @@ func Fldconv(f *Field, flag FmtFlag) string {
 func (t *Type) Format(s fmt.State, verb rune) {
        switch verb {
        case 'v', 'S', 'L':
-               t.tconv(s, fmtFlag(s, verb))
+               fmt.Fprint(s, t.tconv(fmtFlag(s, verb)))
 
        default:
                fmt.Fprintf(s, "%%!%c(*Type=%p)", verb, t)
@@ -1699,15 +1685,13 @@ func (t *Type) Format(s fmt.State, verb rune) {
 // "%L"  print definition, not name
 // "%S"  omit 'func' and receiver from function types, short type names
 // "% v" package name, not prefix (FTypeId mode, sticky)
-func (t *Type) tconv(s fmt.State, flag FmtFlag) {
+func (t *Type) tconv(flag FmtFlag) string {
        if t == nil {
-               fmt.Fprint(s, "<T>")
-               return
+               return "<T>"
        }
 
        if t.Trecur > 4 {
-               fmt.Fprint(s, "<...>")
-               return
+               return "<...>"
        }
 
        t.Trecur++
@@ -1721,7 +1705,7 @@ func (t *Type) tconv(s fmt.State, flag FmtFlag) {
                flag |= FmtUnsigned
        }
 
-       t.typefmt(s, flag)
+       str := t.typefmt(flag)
 
        if fmtmode == FTypeId && (sf&FmtUnsigned != 0) {
                fmtpkgpfx--
@@ -1730,6 +1714,7 @@ func (t *Type) tconv(s fmt.State, flag FmtFlag) {
        flag = sf
        fmtmode = sm
        t.Trecur--
+       return str
 }
 
 func (n *Node) String() string {
index 58edd327665cdea8ac6ef18643878ba6e1397634..c579bfe826ee0ea273cf6ac954b3f738acfd386c 100644 (file)
@@ -864,7 +864,7 @@ func dcommontype(s *Sym, ot int, t *Type) int {
        }
 
        exported := false
-       p := fmt.Sprintf("%- v", t)
+       p := t.tconv(FmtLeft | FmtUnsigned)
        // If we're writing out type T,
        // we are very likely to write out type *T as well.
        // Use the string "*T"[1:] for "T", so that the two
@@ -926,22 +926,22 @@ func dcommontype(s *Sym, ot int, t *Type) int {
 }
 
 func typesym(t *Type) *Sym {
-       return Pkglookup(fmt.Sprintf("%-v", t), typepkg)
+       return Pkglookup(t.tconv(FmtLeft), typepkg)
 }
 
 // tracksym returns the symbol for tracking use of field/method f, assumed
 // to be a member of struct/interface type t.
 func tracksym(t *Type, f *Field) *Sym {
-       return Pkglookup(fmt.Sprintf("%-v.%s", t, f.Sym.Name), trackpkg)
+       return Pkglookup(t.tconv(FmtLeft)+"."+f.Sym.Name, trackpkg)
 }
 
 func typelinkLSym(t *Type) *obj.LSym {
-       name := fmt.Sprintf("go.typelink.%-v", t) // complete, unambiguous type name
+       name := "go.typelink." + t.tconv(FmtLeft) // complete, unambiguous type name
        return obj.Linklookup(Ctxt, name, 0)
 }
 
 func typesymprefix(prefix string, t *Type) *Sym {
-       p := fmt.Sprintf("%s.%-v", prefix, t)
+       p := prefix + "." + t.tconv(FmtLeft)
        s := Pkglookup(p, typepkg)
 
        //print("algsym: %s -> %+S\n", p, s);
@@ -981,7 +981,7 @@ func itabname(t, itype *Type) *Node {
        if t == nil || (t.IsPtr() && t.Elem() == nil) || t.IsUntyped() || !itype.IsInterface() || itype.IsEmptyInterface() {
                Fatalf("itabname(%v, %v)", t, itype)
        }
-       s := Pkglookup(fmt.Sprintf("%-v,%-v", t, itype), itabpkg)
+       s := Pkglookup(t.tconv(FmtLeft)+","+itype.tconv(FmtLeft), itabpkg)
        if s.Def == nil {
                n := newname(s)
                n.Type = Types[TUINT8]
@@ -1406,7 +1406,7 @@ func dumptypestructs() {
                // method functions. None are allocated on heap, so we can use obj.NOPTR.
                ggloblsym(i.sym, int32(o), int16(obj.DUPOK|obj.NOPTR))
 
-               ilink := Pkglookup(fmt.Sprintf("%-v,%-v", i.t, i.itype), itablinkpkg)
+               ilink := Pkglookup(i.t.tconv(FmtLeft)+","+i.itype.tconv(FmtLeft), itablinkpkg)
                dsymptr(ilink, 0, i.sym, 0)
                ggloblsym(ilink, int32(Widthptr), int16(obj.DUPOK|obj.RODATA))
        }