From f91c850be62416d0aaa70e77831c8ba3e1ee2b1e Mon Sep 17 00:00:00 2001 From: Pantelis Sampaziotis Date: Mon, 30 Sep 2019 21:37:40 +0000 Subject: text/template/parse: specify slice capacity in Pipenode.CopyPipe() The required vars slice capacity is known so it can be specified before appending. Change-Id: Ifa2fe97602e84198c4d01e5a1b0529f3f65f2df1 GitHub-Last-Rev: a0580df208a1d498968138d63508ae4e30df2ec5 GitHub-Pull-Request: golang/go#34613 Reviewed-on: https://go-review.googlesource.com/c/go/+/197997 Reviewed-by: Andrew Bonventre --- src/text/template/parse/node.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/text/template/parse') diff --git a/src/text/template/parse/node.go b/src/text/template/parse/node.go index 1174a4b970..74552c293f 100644 --- a/src/text/template/parse/node.go +++ b/src/text/template/parse/node.go @@ -187,7 +187,7 @@ func (p *PipeNode) CopyPipe() *PipeNode { if p == nil { return p } - var vars []*VariableNode + vars := make([]*VariableNode, 0, len(p.Decl)) for _, d := range p.Decl { vars = append(vars, d.Copy().(*VariableNode)) } -- cgit v1.3 From 27cf81e1b48efe6a6387f34c7114766c7b0d4d73 Mon Sep 17 00:00:00 2001 From: Rob Pike Date: Tue, 1 Oct 2019 09:54:30 +1000 Subject: text/template: further simplify building the vars list Followup to https://golang.org/cl/197997 If you know the number of elements, you don't need append at all. Either use append to grow, or allocate and index. Here we choose number 2. Change-Id: Ic58637231789640ff7b293ece04a95a8de7ccf8f Reviewed-on: https://go-review.googlesource.com/c/go/+/198097 Reviewed-by: Ian Lance Taylor --- src/text/template/parse/node.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'src/text/template/parse') diff --git a/src/text/template/parse/node.go b/src/text/template/parse/node.go index 74552c293f..2f921be2ec 100644 --- a/src/text/template/parse/node.go +++ b/src/text/template/parse/node.go @@ -187,9 +187,9 @@ func (p *PipeNode) CopyPipe() *PipeNode { if p == nil { return p } - vars := make([]*VariableNode, 0, len(p.Decl)) - for _, d := range p.Decl { - vars = append(vars, d.Copy().(*VariableNode)) + vars := make([]*VariableNode, len(p.Decl)) + for i, d := range p.Decl { + vars[i] = d.Copy().(*VariableNode) } n := p.tr.newPipeline(p.Pos, p.Line, vars) n.IsAssign = p.IsAssign -- cgit v1.3 From 4f13a9c5b1bfc9ec2213d9ee7d9df49661b119dd Mon Sep 17 00:00:00 2001 From: Ariel Mashraki Date: Tue, 1 Oct 2019 01:10:44 +0300 Subject: text/template/parse: use strings.Builder in String methods As mentioned in godoc, strings.Builder is more efficient for concatenating and building strings. Running a simple bench test on VariableNode.String() gives: benchmark old ns/op new ns/op delta BenchmarkParseLarge-8 25676831 24453285 -4.77% BenchmarkVariableString-8 296 115 -61.15% benchmark old allocs new allocs delta BenchmarkVariableString-8 8 3 -62.50% benchmark old bytes new bytes delta BenchmarkVariableString-8 112 72 -35.71% Change-Id: I13c9340080738fcad1edeed859d33ba608e4b05a Reviewed-on: https://go-review.googlesource.com/c/go/+/198078 Reviewed-by: Emmanuel Odeke Run-TryBot: Emmanuel Odeke TryBot-Result: Gobot Gobot --- src/text/template/parse/node.go | 41 +++++++++++++++++++---------------- src/text/template/parse/parse_test.go | 16 ++++++++++++++ 2 files changed, 38 insertions(+), 19 deletions(-) (limited to 'src/text/template/parse') diff --git a/src/text/template/parse/node.go b/src/text/template/parse/node.go index 2f921be2ec..2eb1af0a95 100644 --- a/src/text/template/parse/node.go +++ b/src/text/template/parse/node.go @@ -160,23 +160,23 @@ func (p *PipeNode) append(command *CommandNode) { } func (p *PipeNode) String() string { - s := "" + var sb strings.Builder if len(p.Decl) > 0 { for i, v := range p.Decl { if i > 0 { - s += ", " + sb.WriteString(", ") } - s += v.String() + sb.WriteString(v.String()) } - s += " := " + sb.WriteString(" := ") } for i, c := range p.Cmds { if i > 0 { - s += " | " + sb.WriteString(" | ") } - s += c.String() + sb.WriteString(c.String()) } - return s + return sb.String() } func (p *PipeNode) tree() *Tree { @@ -249,18 +249,20 @@ func (c *CommandNode) append(arg Node) { } func (c *CommandNode) String() string { - s := "" + var sb strings.Builder for i, arg := range c.Args { if i > 0 { - s += " " + sb.WriteByte(' ') } if arg, ok := arg.(*PipeNode); ok { - s += "(" + arg.String() + ")" + sb.WriteByte('(') + sb.WriteString(arg.String()) + sb.WriteByte(')') continue } - s += arg.String() + sb.WriteString(arg.String()) } - return s + return sb.String() } func (c *CommandNode) tree() *Tree { @@ -333,14 +335,14 @@ func (t *Tree) newVariable(pos Pos, ident string) *VariableNode { } func (v *VariableNode) String() string { - s := "" + var sb strings.Builder for i, id := range v.Ident { if i > 0 { - s += "." + sb.WriteByte('.') } - s += id + sb.WriteString(id) } - return s + return sb.String() } func (v *VariableNode) tree() *Tree { @@ -426,11 +428,12 @@ func (t *Tree) newField(pos Pos, ident string) *FieldNode { } func (f *FieldNode) String() string { - s := "" + var sb strings.Builder for _, id := range f.Ident { - s += "." + id + sb.WriteByte('.') + sb.WriteString(id) } - return s + return sb.String() } func (f *FieldNode) tree() *Tree { diff --git a/src/text/template/parse/parse_test.go b/src/text/template/parse/parse_test.go index 6932cf232e..371de5d67c 100644 --- a/src/text/template/parse/parse_test.go +++ b/src/text/template/parse/parse_test.go @@ -553,3 +553,19 @@ func BenchmarkParseLarge(b *testing.B) { } } } + +var sink string + +func BenchmarkVariableString(b *testing.B) { + v := &VariableNode{ + Ident: []string{"$", "A", "BB", "CCC", "THIS_IS_THE_VARIABLE_BEING_PROCESSED"}, + } + b.ResetTimer() + b.ReportAllocs() + for i := 0; i < b.N; i++ { + sink = v.String() + } + if sink == "" { + b.Fatal("Benchmark was not run") + } +} -- cgit v1.3 From 86cd6c2ee5c5e4c5b5edf4ea8d1c85f80d9706a8 Mon Sep 17 00:00:00 2001 From: Ariel Mashraki Date: Tue, 1 Oct 2019 18:39:29 +0300 Subject: text/template/parse: use strings.Builder in Chain and List nodes This CL is a continuation of 198078. Benchmark output: benchmark old ns/op new ns/op delta BenchmarkParseLarge-8 24759165 24516563 -0.98% BenchmarkVariableString-8 115 115 +0.00% BenchmarkListString-8 924 680 -26.41% benchmark old allocs new allocs delta BenchmarkVariableString-8 3 3 +0.00% BenchmarkListString-8 14 13 -7.14% benchmark old bytes new bytes delta BenchmarkVariableString-8 72 72 +0.00% BenchmarkListString-8 512 424 -17.19% Change-Id: I9ec48fe4832437c556a5fa94d4cbf6e29e28d944 Reviewed-on: https://go-review.googlesource.com/c/go/+/198080 Reviewed-by: Emmanuel Odeke Run-TryBot: Emmanuel Odeke TryBot-Result: Gobot Gobot --- src/text/template/parse/node.go | 20 ++++++++++++-------- src/text/template/parse/parse_test.go | 22 +++++++++++++++++++--- 2 files changed, 31 insertions(+), 11 deletions(-) (limited to 'src/text/template/parse') diff --git a/src/text/template/parse/node.go b/src/text/template/parse/node.go index 2eb1af0a95..61c6853679 100644 --- a/src/text/template/parse/node.go +++ b/src/text/template/parse/node.go @@ -7,7 +7,6 @@ package parse import ( - "bytes" "fmt" "strconv" "strings" @@ -94,11 +93,11 @@ func (l *ListNode) tree() *Tree { } func (l *ListNode) String() string { - b := new(bytes.Buffer) + var sb strings.Builder for _, n := range l.Nodes { - fmt.Fprint(b, n) + sb.WriteString(n.String()) } - return b.String() + return sb.String() } func (l *ListNode) CopyList() *ListNode { @@ -472,14 +471,19 @@ func (c *ChainNode) Add(field string) { } func (c *ChainNode) String() string { - s := c.Node.String() + var sb strings.Builder if _, ok := c.Node.(*PipeNode); ok { - s = "(" + s + ")" + sb.WriteByte('(') + sb.WriteString(c.Node.String()) + sb.WriteByte(')') + } else { + sb.WriteString(c.Node.String()) } for _, field := range c.Field { - s += "." + field + sb.WriteByte('.') + sb.WriteString(field) } - return s + return sb.String() } func (c *ChainNode) tree() *Tree { diff --git a/src/text/template/parse/parse_test.go b/src/text/template/parse/parse_test.go index 371de5d67c..86a100bb5f 100644 --- a/src/text/template/parse/parse_test.go +++ b/src/text/template/parse/parse_test.go @@ -554,7 +554,7 @@ func BenchmarkParseLarge(b *testing.B) { } } -var sink string +var sinkv, sinkl string func BenchmarkVariableString(b *testing.B) { v := &VariableNode{ @@ -563,9 +563,25 @@ func BenchmarkVariableString(b *testing.B) { b.ResetTimer() b.ReportAllocs() for i := 0; i < b.N; i++ { - sink = v.String() + sinkv = v.String() } - if sink == "" { + if sinkv == "" { + b.Fatal("Benchmark was not run") + } +} + +func BenchmarkListString(b *testing.B) { + text := `{{ (printf .Field1.Field2.Field3).Value }}` + tree, err := New("bench").Parse(text, "", "", make(map[string]*Tree), builtins) + if err != nil { + b.Fatal(err) + } + b.ResetTimer() + b.ReportAllocs() + for i := 0; i < b.N; i++ { + sinkl = tree.Root.String() + } + if sinkl == "" { b.Fatal("Benchmark was not run") } } -- cgit v1.3 From 93a79bbcc0de229679ddeb2ad662ec8cea5b3de6 Mon Sep 17 00:00:00 2001 From: Ariel Mashraki Date: Tue, 1 Oct 2019 21:12:59 +0300 Subject: text/template/parse: remove duplication in peekNonSpace nextNonSpace has an identical code except the call to backup at the end. Change-Id: Iefa5b13950007da38323a800fb6b0ce3d436254b Reviewed-on: https://go-review.googlesource.com/c/go/+/198277 Run-TryBot: Rob Pike TryBot-Result: Gobot Gobot Reviewed-by: Rob Pike --- src/text/template/parse/parse.go | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) (limited to 'src/text/template/parse') diff --git a/src/text/template/parse/parse.go b/src/text/template/parse/parse.go index 7c35b0ff3d..c9b80f4a24 100644 --- a/src/text/template/parse/parse.go +++ b/src/text/template/parse/parse.go @@ -108,13 +108,8 @@ func (t *Tree) nextNonSpace() (token item) { } // peekNonSpace returns but does not consume the next non-space token. -func (t *Tree) peekNonSpace() (token item) { - for { - token = t.next() - if token.typ != itemSpace { - break - } - } +func (t *Tree) peekNonSpace() item { + token := t.nextNonSpace() t.backup() return token } -- cgit v1.3 From debbb1e78d08b201313c83f2d236de90d8444c8e Mon Sep 17 00:00:00 2001 From: Ariel Mashraki Date: Tue, 1 Oct 2019 22:40:20 +0300 Subject: text/template/parse: speed up nodes printing This CL is a follow up for 198080. Added a private writeTo method to the Node interface, in order to use the same builder for printing all nodes in the tree. Benchmark output against master: benchmark old ns/op new ns/op delta BenchmarkParseLarge-8 24594994 25292054 +2.83% BenchmarkVariableString-8 117 118 +0.85% BenchmarkListString-8 10475 3353 -67.99% benchmark old allocs new allocs delta BenchmarkVariableString-8 3 3 +0.00% BenchmarkListString-8 149 31 -79.19% benchmark old bytes new bytes delta BenchmarkVariableString-8 72 72 +0.00% BenchmarkListString-8 5698 1608 -71.78% Change-Id: I2b1cf07cda65c1b80083fb99671289423700feba Reviewed-on: https://go-review.googlesource.com/c/go/+/198278 Reviewed-by: Rob Pike Run-TryBot: Rob Pike TryBot-Result: Gobot Gobot --- src/text/template/parse/node.go | 129 +++++++++++++++++++++++++++++----- src/text/template/parse/parse_test.go | 22 +++++- 2 files changed, 130 insertions(+), 21 deletions(-) (limited to 'src/text/template/parse') diff --git a/src/text/template/parse/node.go b/src/text/template/parse/node.go index 61c6853679..1c116ea6fa 100644 --- a/src/text/template/parse/node.go +++ b/src/text/template/parse/node.go @@ -28,6 +28,8 @@ type Node interface { // tree returns the containing *Tree. // It is unexported so all implementations of Node are in this package. tree() *Tree + // writeTo writes the String output to the builder. + writeTo(*strings.Builder) } // NodeType identifies the type of a parse tree node. @@ -94,10 +96,14 @@ func (l *ListNode) tree() *Tree { func (l *ListNode) String() string { var sb strings.Builder + l.writeTo(&sb) + return sb.String() +} + +func (l *ListNode) writeTo(sb *strings.Builder) { for _, n := range l.Nodes { - sb.WriteString(n.String()) + n.writeTo(sb) } - return sb.String() } func (l *ListNode) CopyList() *ListNode { @@ -131,6 +137,10 @@ func (t *TextNode) String() string { return fmt.Sprintf(textFormat, t.Text) } +func (t *TextNode) writeTo(sb *strings.Builder) { + sb.WriteString(t.String()) +} + func (t *TextNode) tree() *Tree { return t.tr } @@ -160,12 +170,17 @@ func (p *PipeNode) append(command *CommandNode) { func (p *PipeNode) String() string { var sb strings.Builder + p.writeTo(&sb) + return sb.String() +} + +func (p *PipeNode) writeTo(sb *strings.Builder) { if len(p.Decl) > 0 { for i, v := range p.Decl { if i > 0 { sb.WriteString(", ") } - sb.WriteString(v.String()) + v.writeTo(sb) } sb.WriteString(" := ") } @@ -173,9 +188,8 @@ func (p *PipeNode) String() string { if i > 0 { sb.WriteString(" | ") } - sb.WriteString(c.String()) + c.writeTo(sb) } - return sb.String() } func (p *PipeNode) tree() *Tree { @@ -218,8 +232,15 @@ func (t *Tree) newAction(pos Pos, line int, pipe *PipeNode) *ActionNode { } func (a *ActionNode) String() string { - return fmt.Sprintf("{{%s}}", a.Pipe) + var sb strings.Builder + a.writeTo(&sb) + return sb.String() +} +func (a *ActionNode) writeTo(sb *strings.Builder) { + sb.WriteString("{{") + a.Pipe.writeTo(sb) + sb.WriteString("}}") } func (a *ActionNode) tree() *Tree { @@ -249,19 +270,23 @@ func (c *CommandNode) append(arg Node) { func (c *CommandNode) String() string { var sb strings.Builder + c.writeTo(&sb) + return sb.String() +} + +func (c *CommandNode) writeTo(sb *strings.Builder) { for i, arg := range c.Args { if i > 0 { sb.WriteByte(' ') } if arg, ok := arg.(*PipeNode); ok { sb.WriteByte('(') - sb.WriteString(arg.String()) + arg.writeTo(sb) sb.WriteByte(')') continue } - sb.WriteString(arg.String()) + arg.writeTo(sb) } - return sb.String() } func (c *CommandNode) tree() *Tree { @@ -312,6 +337,10 @@ func (i *IdentifierNode) String() string { return i.Ident } +func (i *IdentifierNode) writeTo(sb *strings.Builder) { + sb.WriteString(i.String()) +} + func (i *IdentifierNode) tree() *Tree { return i.tr } @@ -335,13 +364,17 @@ func (t *Tree) newVariable(pos Pos, ident string) *VariableNode { func (v *VariableNode) String() string { var sb strings.Builder + v.writeTo(&sb) + return sb.String() +} + +func (v *VariableNode) writeTo(sb *strings.Builder) { for i, id := range v.Ident { if i > 0 { sb.WriteByte('.') } sb.WriteString(id) } - return sb.String() } func (v *VariableNode) tree() *Tree { @@ -374,6 +407,10 @@ func (d *DotNode) String() string { return "." } +func (d *DotNode) writeTo(sb *strings.Builder) { + sb.WriteString(d.String()) +} + func (d *DotNode) tree() *Tree { return d.tr } @@ -404,6 +441,10 @@ func (n *NilNode) String() string { return "nil" } +func (n *NilNode) writeTo(sb *strings.Builder) { + sb.WriteString(n.String()) +} + func (n *NilNode) tree() *Tree { return n.tr } @@ -428,11 +469,15 @@ func (t *Tree) newField(pos Pos, ident string) *FieldNode { func (f *FieldNode) String() string { var sb strings.Builder + f.writeTo(&sb) + return sb.String() +} + +func (f *FieldNode) writeTo(sb *strings.Builder) { for _, id := range f.Ident { sb.WriteByte('.') sb.WriteString(id) } - return sb.String() } func (f *FieldNode) tree() *Tree { @@ -472,18 +517,22 @@ func (c *ChainNode) Add(field string) { func (c *ChainNode) String() string { var sb strings.Builder + c.writeTo(&sb) + return sb.String() +} + +func (c *ChainNode) writeTo(sb *strings.Builder) { if _, ok := c.Node.(*PipeNode); ok { sb.WriteByte('(') - sb.WriteString(c.Node.String()) + c.Node.writeTo(sb) sb.WriteByte(')') } else { - sb.WriteString(c.Node.String()) + c.Node.writeTo(sb) } for _, field := range c.Field { sb.WriteByte('.') sb.WriteString(field) } - return sb.String() } func (c *ChainNode) tree() *Tree { @@ -513,6 +562,10 @@ func (b *BoolNode) String() string { return "false" } +func (b *BoolNode) writeTo(sb *strings.Builder) { + sb.WriteString(b.String()) +} + func (b *BoolNode) tree() *Tree { return b.tr } @@ -646,6 +699,10 @@ func (n *NumberNode) String() string { return n.Text } +func (n *NumberNode) writeTo(sb *strings.Builder) { + sb.WriteString(n.String()) +} + func (n *NumberNode) tree() *Tree { return n.tr } @@ -673,6 +730,10 @@ func (s *StringNode) String() string { return s.Quoted } +func (s *StringNode) writeTo(sb *strings.Builder) { + sb.WriteString(s.String()) +} + func (s *StringNode) tree() *Tree { return s.tr } @@ -697,6 +758,10 @@ func (e *endNode) String() string { return "{{end}}" } +func (e *endNode) writeTo(sb *strings.Builder) { + sb.WriteString(e.String()) +} + func (e *endNode) tree() *Tree { return e.tr } @@ -725,6 +790,10 @@ func (e *elseNode) String() string { return "{{else}}" } +func (e *elseNode) writeTo(sb *strings.Builder) { + sb.WriteString(e.String()) +} + func (e *elseNode) tree() *Tree { return e.tr } @@ -745,6 +814,12 @@ type BranchNode struct { } func (b *BranchNode) String() string { + var sb strings.Builder + b.writeTo(&sb) + return sb.String() +} + +func (b *BranchNode) writeTo(sb *strings.Builder) { name := "" switch b.NodeType { case NodeIf: @@ -756,10 +831,17 @@ func (b *BranchNode) String() string { default: panic("unknown branch type") } + sb.WriteString("{{") + sb.WriteString(name) + sb.WriteByte(' ') + b.Pipe.writeTo(sb) + sb.WriteString("}}") + b.List.writeTo(sb) if b.ElseList != nil { - return fmt.Sprintf("{{%s %s}}%s{{else}}%s{{end}}", name, b.Pipe, b.List, b.ElseList) + sb.WriteString("{{else}}") + b.ElseList.writeTo(sb) } - return fmt.Sprintf("{{%s %s}}%s{{end}}", name, b.Pipe, b.List) + sb.WriteString("{{end}}") } func (b *BranchNode) tree() *Tree { @@ -833,10 +915,19 @@ func (t *Tree) newTemplate(pos Pos, line int, name string, pipe *PipeNode) *Temp } func (t *TemplateNode) String() string { - if t.Pipe == nil { - return fmt.Sprintf("{{template %q}}", t.Name) + var sb strings.Builder + t.writeTo(&sb) + return sb.String() +} + +func (t *TemplateNode) writeTo(sb *strings.Builder) { + sb.WriteString("{{template ") + sb.WriteString(strconv.Quote(t.Name)) + if t.Pipe != nil { + sb.WriteByte(' ') + t.Pipe.writeTo(sb) } - return fmt.Sprintf("{{template %q %s}}", t.Name, t.Pipe) + sb.WriteString("}}") } func (t *TemplateNode) tree() *Tree { diff --git a/src/text/template/parse/parse_test.go b/src/text/template/parse/parse_test.go index 86a100bb5f..4e09a7852c 100644 --- a/src/text/template/parse/parse_test.go +++ b/src/text/template/parse/parse_test.go @@ -304,7 +304,8 @@ var parseTests = []parseTest{ } var builtins = map[string]interface{}{ - "printf": fmt.Sprintf, + "printf": fmt.Sprintf, + "contains": strings.Contains, } func testParse(doCopy bool, t *testing.T) { @@ -571,7 +572,24 @@ func BenchmarkVariableString(b *testing.B) { } func BenchmarkListString(b *testing.B) { - text := `{{ (printf .Field1.Field2.Field3).Value }}` + text := ` +{{(printf .Field1.Field2.Field3).Value}} +{{$x := (printf .Field1.Field2.Field3).Value}} +{{$y := (printf $x.Field1.Field2.Field3).Value}} +{{$z := $y.Field1.Field2.Field3}} +{{if contains $y $z}} + {{printf "%q" $y}} +{{else}} + {{printf "%q" $x}} +{{end}} +{{with $z.Field1 | contains "boring"}} + {{printf "%q" . | printf "%s"}} +{{else}} + {{printf "%d %d %d" 11 11 11}} + {{printf "%d %d %s" 22 22 $x.Field1.Field2.Field3 | printf "%s"}} + {{printf "%v" (contains $z.Field1.Field2 $y)}} +{{end}} +` tree, err := New("bench").Parse(text, "", "", make(map[string]*Tree), builtins) if err != nil { b.Fatal(err) -- cgit v1.3