From 37d452c3e9c1fe354375ad41ae0b952b563dfbe4 Mon Sep 17 00:00:00 2001 From: Robert Griesemer Date: Mon, 12 Sep 2016 13:44:43 -0700 Subject: [PATCH] cmd/compile: reduce allocs to c85b77c (pre-fmt.go change) levels MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit 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 TryBot-Result: Gobot Gobot Reviewed-by: Matthew Dempsky --- src/cmd/compile/internal/gc/fmt.go | 207 ++++++++++++------------- src/cmd/compile/internal/gc/reflect.go | 14 +- 2 files changed, 103 insertions(+), 118 deletions(-) diff --git a/src/cmd/compile/internal/gc/fmt.go b/src/cmd/compile/internal/gc/fmt.go index 9032e1711f..b430283838 100644 --- a/src/cmd/compile/internal/gc/fmt.go +++ b/src/cmd/compile/internal/gc/fmt.go @@ -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, "") - return + return "" } 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, "") - return + return "" } 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, "") - return + return "" } 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 { diff --git a/src/cmd/compile/internal/gc/reflect.go b/src/cmd/compile/internal/gc/reflect.go index 58edd32766..c579bfe826 100644 --- a/src/cmd/compile/internal/gc/reflect.go +++ b/src/cmd/compile/internal/gc/reflect.go @@ -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)) } -- 2.48.1