Skip to content

Commit

Permalink
Merge #34257
Browse files Browse the repository at this point in the history
34257: pretty: make union and concat pointer receivers r=jordanlewis a=jordanlewis

Previously, union and concat were struct receivers in the Doc type,
which led to pathological hashing behavior when inserting into the
memoiDoc map. This commit switches them to pointer receivers, which
makes everything a lot faster.


```
name          old time/op    new time/op    delta
PrettyData-8     202ms ± 4%      66ms ± 2%  -67.35%  (p=0.000 n=10+10)

name          old alloc/op   new alloc/op   delta
PrettyData-8    24.7MB ± 0%    30.7MB ± 0%  +24.10%  (p=0.000 n=10+10)

name          old allocs/op  new allocs/op  delta
PrettyData-8     8.29k ± 0%     9.17k ± 0%  +10.59%  (p=0.000 n=10+10)
```

Helps #33649.

Release note: None

Co-authored-by: Jordan Lewis <[email protected]>
  • Loading branch information
craig[bot] and jordanlewis committed Jan 26, 2019
2 parents 2890b9a + 101676d commit 8cbeb53
Show file tree
Hide file tree
Showing 3 changed files with 10 additions and 10 deletions.
12 changes: 6 additions & 6 deletions pkg/util/pretty/document.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,10 +47,10 @@ func (text) isDoc() {}
func (line) isDoc() {}
func (softbreak) isDoc() {}
func (nilDoc) isDoc() {}
func (concat) isDoc() {}
func (*concat) isDoc() {}
func (nestt) isDoc() {}
func (nests) isDoc() {}
func (union) isDoc() {}
func (*union) isDoc() {}
func (*scolumn) isDoc() {}
func (*snesting) isDoc() {}
func (pad) isDoc() {}
Expand Down Expand Up @@ -105,7 +105,7 @@ type concat struct {
// This uses simplifyNil to avoid actually inserting NIL docs
// in the abstract tree.
func Concat(a, b Doc) Doc {
return simplifyNil(a, b, func(a, b Doc) Doc { return concat{a, b} })
return simplifyNil(a, b, func(a, b Doc) Doc { return &concat{a, b} })
}

// nests represents (NESTS Int DOC) :: DOC -- nesting a doc "under" another.
Expand Down Expand Up @@ -149,7 +149,7 @@ type union struct {

// Group will format d on one line if possible.
func Group(d Doc) Doc {
return union{flatten(d), d}
return &union{flatten(d), d}
}

var textSpace = Text(" ")
Expand All @@ -158,7 +158,7 @@ func flatten(d Doc) Doc {
switch t := d.(type) {
case nilDoc:
return Nil
case concat:
case *concat:
return Concat(flatten(t.a), flatten(t.b))
case nestt:
return NestT(flatten(t.d))
Expand All @@ -170,7 +170,7 @@ func flatten(d Doc) Doc {
return textSpace
case softbreak:
return Nil
case union:
case *union:
return flatten(t.x)
case *scolumn:
return &scolumn{f: func(c int16) Doc { return flatten(t.f(c)) }}
Expand Down
4 changes: 2 additions & 2 deletions pkg/util/pretty/pretty.go
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ func (b *beExec) be(k docPos, xlist *iDoc) *docBest {
switch t := d.d.(type) {
case nilDoc:
res = b.be(k, z)
case concat:
case *concat:
res = b.be(k, b.iDoc(d.i, t.a, b.iDoc(d.i, t.b, z)))
case nests:
res = b.be(k, b.iDoc(docPos{d.i.tabs, d.i.spaces + t.n}, t.d, z))
Expand All @@ -141,7 +141,7 @@ func (b *beExec) be(k docPos, xlist *iDoc) *docBest {
i: d.i,
d: b.be(d.i, z),
})
case union:
case *union:
res = b.better(k,
b.be(k, b.iDoc(d.i, t.x, z)),
// We eta-lift the second argument to avoid eager evaluation.
Expand Down
4 changes: 2 additions & 2 deletions pkg/util/pretty/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ func AlignUnder(head, nested Doc) Doc {
g := Group(nested)
a := ConcatSpace(head, Align(g))
b := Concat(head, NestT(Concat(Line, g)))
return Group(union{a, b})
return Group(&union{a, b})
}

// Fold applies f recursively to all Docs in d.
Expand Down Expand Up @@ -282,7 +282,7 @@ func RLTable(align bool, rows ...RLTableRow) Doc {
}
alignedTable := Stack(items...)

finalDoc = union{alignedTable, nestedSections}
finalDoc = &union{alignedTable, nestedSections}
}

return Group(finalDoc)
Expand Down

0 comments on commit 8cbeb53

Please sign in to comment.