From: Robert Griesemer Date: Tue, 22 Mar 2011 00:15:59 +0000 (-0700) Subject: go/printer, gofmt: avoid exponential layout algorithm X-Git-Tag: weekly.2011-03-28~76 X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=4a33d440b891c4aaff5769b72c3d5edcc65d42dd;p=gostls13.git go/printer, gofmt: avoid exponential layout algorithm 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 --- diff --git a/src/cmd/godoc/godoc.go b/src/cmd/godoc/godoc.go index 41bd37ad66..71be62fb47 100644 --- a/src/cmd/godoc/godoc.go +++ b/src/cmd/godoc/godoc.go @@ -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) } diff --git a/src/cmd/gofix/main.go b/src/cmd/gofix/main.go index 9ca2ddb461..ca4becfa75 100644 --- a/src/cmd/gofix/main.go +++ b/src/cmd/gofix/main.go @@ -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 } diff --git a/src/cmd/gofix/main_test.go b/src/cmd/gofix/main_test.go index e4d0f60cce..ffcbe7cb09 100644 --- a/src/cmd/gofix/main_test.go +++ b/src/cmd/gofix/main_test.go @@ -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 diff --git a/src/cmd/gofmt/gofmt.go b/src/cmd/gofmt/gofmt.go index a688c8184c..943877aa27 100644 --- a/src/cmd/gofmt/gofmt.go +++ b/src/cmd/gofmt/gofmt.go @@ -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 } diff --git a/src/pkg/go/printer/nodes.go b/src/pkg/go/printer/nodes.go index 23da6c8b1e..5ab9a8bb86 100644 --- a/src/pkg/go/printer/nodes.go +++ b/src/pkg/go/printer/nodes.go @@ -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 } diff --git a/src/pkg/go/printer/printer.go b/src/pkg/go/printer/printer.go index 90d9784ac9..6c925ad4fd 100644 --- a/src/pkg/go/printer/printer.go +++ b/src/pkg/go/printer/printer.go @@ -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 diff --git a/src/pkg/go/printer/printer_test.go b/src/pkg/go/printer/printer_test.go index 62b7269131..debd2d336e 100644 --- a/src/pkg/go/printer/printer_test.go +++ b/src/pkg/go/printer/printer_test.go @@ -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 index 0000000000..43a15cb1d0 --- /dev/null +++ b/src/pkg/go/printer/testdata/slow.golden @@ -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 index 0000000000..0e5a23d886 --- /dev/null +++ b/src/pkg/go/printer/testdata/slow.input @@ -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) + } +}