]> Cypherpunks repositories - gostls13.git/commitdiff
go/printer, gofmt: avoid exponential layout algorithm
authorRobert Griesemer <gri@golang.org>
Tue, 22 Mar 2011 00:15:59 +0000 (17:15 -0700)
committerRobert Griesemer <gri@golang.org>
Tue, 22 Mar 2011 00:15:59 +0000 (17:15 -0700)
Use memoization to avoid repeated recomputation of nested
node sizes. Speeds up testdata/slow.input by several orders
of magnitude.

- added respective test case
- added timeout to test code
- deleted some unrelated unused code

Fixes #1628.

R=rsc, r
CC=golang-dev
https://golang.org/cl/4274075

src/cmd/godoc/godoc.go
src/cmd/gofix/main.go
src/cmd/gofix/main_test.go
src/cmd/gofmt/gofmt.go
src/pkg/go/printer/nodes.go
src/pkg/go/printer/printer.go
src/pkg/go/printer/printer_test.go
src/pkg/go/printer/testdata/slow.golden [new file with mode: 0644]
src/pkg/go/printer/testdata/slow.input [new file with mode: 0644]

index 41bd37ad665044e88fc3995cab3aab25e6c7179c..71be62fb476fcf1ec175bc83376b0451c2b56ae1 100644 (file)
@@ -374,7 +374,7 @@ func writeNode(w io.Writer, fset *token.FileSet, x interface{}) {
        //           with an another printer mode (which is more efficiently
        //           implemented in the printer than here with another layer)
        mode := printer.TabIndent | printer.UseSpaces
-       (&printer.Config{mode, *tabwidth}).Fprint(&tconv{output: w}, fset, x)
+       (&printer.Config{Mode: mode, Tabwidth: *tabwidth}).Fprint(&tconv{output: w}, fset, x)
 }
 
 
index 9ca2ddb461cb68ef34c685adc962c5603f2737f1..ca4becfa756ab9318a22cb35bb6c62c552dc7b20 100644 (file)
@@ -127,7 +127,7 @@ func processFile(filename string, useStdin bool) os.Error {
        fmt.Fprintf(os.Stderr, "%s: %s\n", filename, buf.String()[1:])
 
        buf.Reset()
-       _, err = (&printer.Config{printerMode, tabWidth}).Fprint(&buf, fset, file)
+       _, err = (&printer.Config{Mode: printerMode, Tabwidth: tabWidth}).Fprint(&buf, fset, file)
        if err != nil {
                return err
        }
index e4d0f60cce6c10c293057330adc2abd82c29a79b..ffcbe7cb0994cd44d17dee233804de0a1ea5bcb7 100644 (file)
@@ -37,7 +37,7 @@ func parseFixPrint(t *testing.T, fn func(*ast.File) bool, desc, in string) (out
 
        var buf bytes.Buffer
        buf.Reset()
-       _, err = (&printer.Config{printerMode, tabWidth}).Fprint(&buf, fset, file)
+       _, err = (&printer.Config{Mode: printerMode, Tabwidth: tabWidth}).Fprint(&buf, fset, file)
        if err != nil {
                t.Errorf("%s: printing: %v", desc, err)
                return
@@ -60,7 +60,7 @@ func parseFixPrint(t *testing.T, fn func(*ast.File) bool, desc, in string) (out
        }
 
        buf.Reset()
-       _, err = (&printer.Config{printerMode, tabWidth}).Fprint(&buf, fset, file)
+       _, err = (&printer.Config{Mode: printerMode, Tabwidth: tabWidth}).Fprint(&buf, fset, file)
        if err != nil {
                t.Errorf("%s: printing: %v", desc, err)
                return
index a688c8184c43ef289add02c3c14fadd1bec4f8ac..943877aa27b3cc34c62adc377b7284c2fcbdc2e1 100644 (file)
@@ -103,7 +103,7 @@ func processFile(f *os.File) os.Error {
        }
 
        var buf bytes.Buffer
-       _, err = (&printer.Config{printerMode, *tabWidth}).Fprint(&buf, fset, file)
+       _, err = (&printer.Config{Mode: printerMode, Tabwidth: *tabWidth}).Fprint(&buf, fset, file)
        if err != nil {
                return err
        }
index 23da6c8b1eae7ced10a32f50f6555d03de265cb0..5ab9a8bb869a8bb4c920d56fa562baa9eb275713 100644 (file)
@@ -108,17 +108,6 @@ func (p *printer) identList(list []*ast.Ident, indent bool, multiLine *bool) {
 }
 
 
-// Compute the key size of a key:value expression.
-// Returns 0 if the expression doesn't fit onto a single line.
-func (p *printer) keySize(pair *ast.KeyValueExpr) int {
-       if p.nodeSize(pair, infinity) <= infinity {
-               // entire expression fits on one line - return key size
-               return p.nodeSize(pair.Key, infinity)
-       }
-       return 0
-}
-
-
 // Print a list of expressions. If the list spans multiple
 // source lines, the original line breaks are respected between
 // expressions. Sets multiLine to true if the list spans multiple
@@ -1324,11 +1313,21 @@ func (p *printer) genDecl(d *ast.GenDecl, multiLine *bool) {
 // any control chars. Otherwise, the result is > maxSize.
 //
 func (p *printer) nodeSize(n ast.Node, maxSize int) (size int) {
+       // nodeSize invokes the printer, which may invoke nodeSize
+       // recursively. For deep composite literal nests, this can
+       // lead to an exponential algorithm. Remember previous
+       // results to prune the recursion (was issue 1628).
+       if size, found := p.nodeSizes[n]; found {
+               return size
+       }
+
        size = maxSize + 1 // assume n doesn't fit
+       p.nodeSizes[n] = size
+
        // nodeSize computation must be indendent of particular
        // style so that we always get the same decision; print
        // in RawFormat
-       cfg := Config{Mode: RawFormat}
+       cfg := Config{Mode: RawFormat, nodeSizes: p.nodeSizes}
        var buf bytes.Buffer
        if _, err := cfg.Fprint(&buf, p.fset, n); err != nil {
                return
@@ -1340,6 +1339,7 @@ func (p *printer) nodeSize(n ast.Node, maxSize int) (size int) {
                        }
                }
                size = buf.Len() // n fits
+               p.nodeSizes[n] = size
        }
        return
 }
index 90d9784ac97ab0ae3e0eaad72bac92391ddc7d8a..6c925ad4fd8f6df4187d921b0fbbbf2542113008 100644 (file)
@@ -94,9 +94,6 @@ type printer struct {
        // written using writeItem.
        last token.Position
 
-       // HTML support
-       lastTaggedLine int // last line for which a line tag was written
-
        // The list of all source comments, in order of appearance.
        comments        []*ast.CommentGroup // may be nil
        cindex          int                 // current comment index
@@ -989,8 +986,9 @@ const (
 
 // A Config node controls the output of Fprint.
 type Config struct {
-       Mode     uint // default: 0
-       Tabwidth int  // default: 8
+       Mode      uint             // default: 0
+       Tabwidth  int              // default: 8
+       nodeSizes map[ast.Node]int // memoized node sizes as computed by nodeSize
 }
 
 
@@ -1001,6 +999,12 @@ type Config struct {
 // ast.Decl, ast.Spec, or ast.Stmt.
 //
 func (cfg *Config) Fprint(output io.Writer, fset *token.FileSet, node interface{}) (int, os.Error) {
+       // only if Fprint is called recursively (via nodeSize)
+       // does cfg.nodeSizes exist - set it up otherwise
+       if cfg.nodeSizes == nil {
+               cfg.nodeSizes = make(map[ast.Node]int)
+       }
+
        // redirect output through a trimmer to eliminate trailing whitespace
        // (Input to a tabwriter must be untrimmed since trailing tabs provide
        // formatting information. The tabwriter could provide trimming
index 62b7269131cd87add5f5298eb69c5764bc5e09ee..debd2d336edc13f7e05f03e2b344a3273b171cb6 100644 (file)
@@ -13,6 +13,7 @@ import (
        "go/token"
        "path/filepath"
        "testing"
+       "time"
 )
 
 
@@ -45,7 +46,7 @@ const (
 )
 
 
-func check(t *testing.T, source, golden string, mode checkMode) {
+func runcheck(t *testing.T, source, golden string, mode checkMode) {
        // parse source
        prog, err := parser.ParseFile(fset, source, nil, parser.ParseComments)
        if err != nil {
@@ -109,6 +110,32 @@ func check(t *testing.T, source, golden string, mode checkMode) {
 }
 
 
+func check(t *testing.T, source, golden string, mode checkMode) {
+       // start a timer to produce a time-out signal
+       tc := make(chan int)
+       go func() {
+               time.Sleep(2e9) // plenty of a safety margin, even for very slow machines
+               tc <- 0
+       }()
+
+       // run the test
+       cc := make(chan int)
+       go func() {
+               runcheck(t, source, golden, mode)
+               cc <- 0
+       }()
+
+       // wait for the first finisher
+       select {
+       case <-tc:
+               // test running past time out
+               t.Errorf("%s: running too slowly", source)
+       case <-cc:
+               // test finished within alloted time margin
+       }
+}
+
+
 type entry struct {
        source, golden string
        mode           checkMode
@@ -124,6 +151,7 @@ var data = []entry{
        {"expressions.input", "expressions.raw", rawFormat},
        {"declarations.input", "declarations.golden", 0},
        {"statements.input", "statements.golden", 0},
+       {"slow.input", "slow.golden", 0},
 }
 
 
diff --git a/src/pkg/go/printer/testdata/slow.golden b/src/pkg/go/printer/testdata/slow.golden
new file mode 100644 (file)
index 0000000..43a15cb
--- /dev/null
@@ -0,0 +1,85 @@
+// Copyright 2011 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package deepequal_test
+
+import (
+       "testing"
+       "google3/spam/archer/frontend/deepequal"
+)
+
+func TestTwoNilValues(t *testing.T) {
+       if err := deepequal.Check(nil, nil); err != nil {
+               t.Errorf("expected nil, saw %v", err)
+       }
+}
+
+type Foo struct {
+       bar     *Bar
+       bang    *Bar
+}
+
+type Bar struct {
+       baz     *Baz
+       foo     []*Foo
+}
+
+type Baz struct {
+       entries         map[int]interface{}
+       whatever        string
+}
+
+func newFoo() *Foo {
+       return &Foo{bar: &Bar{baz: &Baz{
+               entries: map[int]interface{}{
+                       42:     &Foo{},
+                       21:     &Bar{},
+                       11:     &Baz{whatever: "it's just a test"}}}},
+               bang: &Bar{foo: []*Foo{
+                       &Foo{bar: &Bar{baz: &Baz{
+                               entries: map[int]interface{}{
+                                       43:     &Foo{},
+                                       22:     &Bar{},
+                                       13:     &Baz{whatever: "this is nuts"}}}},
+                               bang: &Bar{foo: []*Foo{
+                                       &Foo{bar: &Bar{baz: &Baz{
+                                               entries: map[int]interface{}{
+                                                       61:     &Foo{},
+                                                       71:     &Bar{},
+                                                       11:     &Baz{whatever: "no, it's Go"}}}},
+                                               bang: &Bar{foo: []*Foo{
+                                                       &Foo{bar: &Bar{baz: &Baz{
+                                                               entries: map[int]interface{}{
+                                                                       0:      &Foo{},
+                                                                       -2:     &Bar{},
+                                                                       -11:    &Baz{whatever: "we need to go deeper"}}}},
+                                                               bang: &Bar{foo: []*Foo{
+                                                                       &Foo{bar: &Bar{baz: &Baz{
+                                                                               entries: map[int]interface{}{
+                                                                                       -2:     &Foo{},
+                                                                                       -5:     &Bar{},
+                                                                                       -7:     &Baz{whatever: "are you serious?"}}}},
+                                                                               bang:   &Bar{foo: []*Foo{}}},
+                                                                       &Foo{bar: &Bar{baz: &Baz{
+                                                                               entries: map[int]interface{}{
+                                                                                       -100:   &Foo{},
+                                                                                       50:     &Bar{},
+                                                                                       20:     &Baz{whatever: "na, not really ..."}}}},
+                                                                               bang:   &Bar{foo: []*Foo{}}}}}}}}},
+                                       &Foo{bar: &Bar{baz: &Baz{
+                                               entries: map[int]interface{}{
+                                                       2:      &Foo{},
+                                                       1:      &Bar{},
+                                                       -1:     &Baz{whatever: "... it's just a test."}}}},
+                                               bang:   &Bar{foo: []*Foo{}}}}}}}}}
+}
+
+func TestElaborate(t *testing.T) {
+       a := newFoo()
+       b := newFoo()
+
+       if err := deepequal.Check(a, b); err != nil {
+               t.Errorf("expected nil, saw %v", err)
+       }
+}
diff --git a/src/pkg/go/printer/testdata/slow.input b/src/pkg/go/printer/testdata/slow.input
new file mode 100644 (file)
index 0000000..0e5a23d
--- /dev/null
@@ -0,0 +1,85 @@
+// Copyright 2011 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package deepequal_test
+
+import (
+        "testing"
+        "google3/spam/archer/frontend/deepequal"
+)
+
+func TestTwoNilValues(t *testing.T) {
+        if err := deepequal.Check(nil, nil); err != nil {
+                t.Errorf("expected nil, saw %v", err)
+        }
+}
+
+type Foo struct {
+        bar *Bar
+        bang *Bar
+}
+
+type Bar struct {
+        baz *Baz
+        foo []*Foo
+}
+
+type Baz struct {
+        entries  map[int]interface{}
+        whatever string
+}
+
+func newFoo() (*Foo) {
+return &Foo{bar: &Bar{ baz: &Baz{
+entries: map[int]interface{}{
+42: &Foo{},
+21: &Bar{},
+11: &Baz{ whatever: "it's just a test" }}}},
+        bang: &Bar{foo: []*Foo{
+&Foo{bar: &Bar{ baz: &Baz{
+entries: map[int]interface{}{
+43: &Foo{},
+22: &Bar{},
+13: &Baz{ whatever: "this is nuts" }}}},
+        bang: &Bar{foo: []*Foo{
+&Foo{bar: &Bar{ baz: &Baz{
+entries: map[int]interface{}{
+61: &Foo{},
+71: &Bar{},
+11: &Baz{ whatever: "no, it's Go" }}}},
+        bang: &Bar{foo: []*Foo{
+&Foo{bar: &Bar{ baz: &Baz{
+entries: map[int]interface{}{
+0: &Foo{},
+-2: &Bar{},
+-11: &Baz{ whatever: "we need to go deeper" }}}},
+        bang: &Bar{foo: []*Foo{
+&Foo{bar: &Bar{ baz: &Baz{
+entries: map[int]interface{}{
+-2: &Foo{},
+-5: &Bar{},
+-7: &Baz{ whatever: "are you serious?" }}}},
+        bang: &Bar{foo: []*Foo{}}},
+&Foo{bar: &Bar{ baz: &Baz{
+entries: map[int]interface{}{
+-100: &Foo{},
+50: &Bar{},
+20: &Baz{ whatever: "na, not really ..." }}}},
+        bang: &Bar{foo: []*Foo{}}}}}}}}},
+&Foo{bar: &Bar{ baz: &Baz{
+entries: map[int]interface{}{
+2: &Foo{},
+1: &Bar{},
+-1: &Baz{ whatever: "... it's just a test." }}}},
+        bang: &Bar{foo: []*Foo{}}}}}}}}}
+}
+
+func TestElaborate(t *testing.T) {
+        a := newFoo()
+        b := newFoo()
+
+        if err := deepequal.Check(a, b); err != nil {
+                t.Errorf("expected nil, saw %v", err)
+        }
+}