cmd/compile: reduce allocs some more

Also: update fmt_test.go.

Together with the previous commits, we are now at or below c85b77c
levels in terms of allocation for the benchmark described in #16897
(old = c85b77c, new = this commit):

name       old time/op     new time/op     delta
Template       297ms ± 5%      284ms ± 3%  -4.53%  (p=0.000 n=27+29)
Unicode        159ms ± 5%      151ms ± 5%  -4.91%  (p=0.000 n=28+30)
GoTypes        985ms ± 5%      935ms ± 2%  -5.13%  (p=0.000 n=28+29)

name       old alloc/op    new alloc/op    delta
Template      46.8MB ± 0%     45.7MB ± 0%  -2.37%  (p=0.000 n=30+30)
Unicode       37.8MB ± 0%     37.9MB ± 0%  +0.29%  (p=0.000 n=29+30)
GoTypes        143MB ± 0%      138MB ± 0%  -3.64%  (p=0.000 n=29+30)

name       old allocs/op   new allocs/op   delta
Template        444k ± 0%       440k ± 0%  -0.94%  (p=0.000 n=30+30)
Unicode         369k ± 0%       369k ± 0%  +0.19%  (p=0.000 n=29+30)
GoTypes        1.35M ± 0%      1.34M ± 0%  -1.24%  (p=0.000 n=30+30)

For #16897.

Change-Id: Iedbeb408e2f1e68dd4a3201bf8813c8066ebf7ed
Reviewed-on: https://go-review.googlesource.com/29089
Reviewed-by: Matthew Dempsky <mdempsky@google.com>

diff --git a/src/cmd/compile/fmt_test.go b/src/cmd/compile/fmt_test.go
index 89a5a684652deef8873207ee8c9408c4c25e8120..48999aec156f6450802b69aa2b3da68d92f41669 100644 (file)
--- a/src/cmd/compile/fmt_test.go
+++ b/src/cmd/compile/fmt_test.go
@@ -552,19 +552,15 @@ var knownFormats = map[string]string{
        "*cmd/compile/internal/gc.Node %j":                "",
        "*cmd/compile/internal/gc.Node %p":                "",
        "*cmd/compile/internal/gc.Node %v":                "",
-       "*cmd/compile/internal/gc.Sym % v":                "",
        "*cmd/compile/internal/gc.Sym %+v":                "",
        "*cmd/compile/internal/gc.Sym %-v":                "",
        "*cmd/compile/internal/gc.Sym %0S":                "",
        "*cmd/compile/internal/gc.Sym %S":                 "",
        "*cmd/compile/internal/gc.Sym %p":                 "",
        "*cmd/compile/internal/gc.Sym %v":                 "",
-       "*cmd/compile/internal/gc.Type % -v":              "",
        "*cmd/compile/internal/gc.Type %#v":               "",
        "*cmd/compile/internal/gc.Type %+v":               "",
-       "*cmd/compile/internal/gc.Type %- v":              "",
        "*cmd/compile/internal/gc.Type %-S":               "",
-       "*cmd/compile/internal/gc.Type %-v":               "",
        "*cmd/compile/internal/gc.Type %0S":               "",
        "*cmd/compile/internal/gc.Type %L":                "",
        "*cmd/compile/internal/gc.Type %S":                "",

diff --git a/src/cmd/compile/internal/gc/fmt.go b/src/cmd/compile/internal/gc/fmt.go
index c65c382ae481bebc2fbcd2a236c310c9c7771bd0..0563d88b49b087f8cdaa965876c79c56fdc194fe 100644 (file)
--- a/src/cmd/compile/internal/gc/fmt.go
+++ b/src/cmd/compile/internal/gc/fmt.go
@@ -17,6 +17,9 @@ import (
 // See the respective function's documentation for details.
 type FmtFlag int
 
+// TODO(gri) The ' ' flag is not used anymore in %-formats.
+//           Eliminate eventually.
+
 const ( //                                 fmt.Format flag/prec or verb
        FmtLeft     FmtFlag = 1 << iota // '-'
        FmtSharp                        // '#'
@@ -1556,6 +1559,7 @@ func (n *Node) nodedump(s fmt.State, flag FmtFlag) {
        }
 }
 
+// "%S" suppresses qualifying with package
 func (s *Sym) Format(f fmt.State, verb rune) {
        switch verb {
        case 'v', 'S':
@@ -1570,7 +1574,7 @@ func (s *Sym) String() string {
        return s.sconv(0)
 }
 
-// "%S" suppresses qualifying with package
+// See #16897 before changing the implementation of sconv.
 func (s *Sym) sconv(flag FmtFlag) string {
        if flag&FmtLong != 0 {
                panic("linksymfmt")
@@ -1671,6 +1675,9 @@ func Fldconv(f *Field, flag FmtFlag) string {
        return str
 }
 
+// "%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) Format(s fmt.State, verb rune) {
        switch verb {
        case 'v', 'S', 'L':
@@ -1681,9 +1688,7 @@ 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)
+// See #16897 before changing the implementation of tconv.
 func (t *Type) tconv(flag FmtFlag) string {
        if t == nil {
                return "<T>"

diff --git a/src/cmd/compile/internal/gc/subr.go b/src/cmd/compile/internal/gc/subr.go
index 409ea3da0b45d90209847b425d60272631693ea0..fe8f820c5f3ed7b5c037aa76e101a41cd1ef00ba 100644 (file)
--- a/src/cmd/compile/internal/gc/subr.go
+++ b/src/cmd/compile/internal/gc/subr.go
@@ -1147,10 +1147,11 @@ func syslook(name string) *Node {
 // typehash computes a hash value for type t to use in type switch
 // statements.
 func typehash(t *Type) uint32 {
-       // fmt.Sprintf("%- v", t) already contains all the necessary logic to generate
-       // a representation that completely describes the type, so using
+       // t.tconv(FmtLeft | FmtUnsigned) already contains all the necessary logic
+       // to generate a representation that completely describes the type, so using
        // it here avoids duplicating that code.
-       p := fmt.Sprintf("%- v", t)
+       // See the comments in exprSwitch.checkDupCases.
+       p := t.tconv(FmtLeft | FmtUnsigned)
 
        // Using MD5 is overkill, but reduces accidental collisions.
        h := md5.Sum([]byte(p))

diff --git a/src/cmd/compile/internal/gc/swt.go b/src/cmd/compile/internal/gc/swt.go
index a5dda732252c1b59848da00249dfc5d699c910de..bfe5c1fb23fe2d991bb670c5d0a6d3ff226de1f4 100644 (file)
--- a/src/cmd/compile/internal/gc/swt.go
+++ b/src/cmd/compile/internal/gc/swt.go
@@ -4,10 +4,7 @@
 
 package gc
 
-import (
-       "fmt"
-       "sort"
-)
+import "sort"
 
 const (
        // expression switch
@@ -647,9 +644,9 @@ func (s *exprSwitch) checkDupCases(cc []caseClause) {
                }
                n := c.node.Left
                tv := typeVal{
-                       // fmt.Sprintf("% -v", n.Type) here serves to completely describe the type.
+                       // n.Type.tconv(FmtLeft | FmtUnsigned) here serves to completely describe the type.
                        // See the comments in func typehash.
-                       typ: fmt.Sprintf("% -v", n.Type),
+                       typ: n.Type.tconv(FmtLeft | FmtUnsigned),
                        val: n.Val().Interface(),
                }
                prev, dup := seen[tv]