From ae991463017cad680e4b43b23df0cbe53c21bab1 Mon Sep 17 00:00:00 2001 From: Adrian Hesketh Date: Sun, 18 Aug 2024 23:10:29 +0100 Subject: [PATCH] refactor: improve void element parsing perf, fixes #886 and #857 (#887) --- generator/test-html/template.templ | 2 +- generator/test-html/template_templ.go | 2 +- parser/v2/benchmarks_test.go | 34 ++++++++++++++ parser/v2/benchmarktestdata/benchmark.txt | 18 ++++++++ parser/v2/diagnostics.go | 19 -------- parser/v2/diagnostics_test.go | 15 ------- parser/v2/elementparser.go | 45 ++++++++++++++----- parser/v2/elementparser_test.go | 40 +++++++++-------- ...are_converted_to_self_closing_elements.txt | 4 +- parser/v2/templateparser.go | 16 +++++++ parser/v2/templateparser_test.go | 38 ++++++++++++++++ parser/v2/textparser.go | 5 +++ 12 files changed, 171 insertions(+), 67 deletions(-) create mode 100644 parser/v2/benchmarks_test.go create mode 100644 parser/v2/benchmarktestdata/benchmark.txt diff --git a/generator/test-html/template.templ b/generator/test-html/template.templ index 8afe58f9b..4f338ca1d 100644 --- a/generator/test-html/template.templ +++ b/generator/test-html/template.templ @@ -10,5 +10,5 @@ templ render(p person) {
- Text + Text } diff --git a/generator/test-html/template_templ.go b/generator/test-html/template_templ.go index 59b15ad0f..4143b563a 100644 --- a/generator/test-html/template_templ.go +++ b/generator/test-html/template_templ.go @@ -103,7 +103,7 @@ func render(p person) templ.Component { return templ_7745c5c3_Err } } - _, templ_7745c5c3_Err = templ_7745c5c3_Buffer.WriteString(">
Text") + _, templ_7745c5c3_Err = templ_7745c5c3_Buffer.WriteString(">
Text") if templ_7745c5c3_Err != nil { return templ_7745c5c3_Err } diff --git a/parser/v2/benchmarks_test.go b/parser/v2/benchmarks_test.go new file mode 100644 index 000000000..1ac7b28bd --- /dev/null +++ b/parser/v2/benchmarks_test.go @@ -0,0 +1,34 @@ +package parser + +import ( + _ "embed" + "strings" + "testing" +) + +//go:embed benchmarktestdata/benchmark.txt +var benchmarkTemplate string + +func BenchmarkParse(b *testing.B) { + b.ReportAllocs() + for i := 0; i < b.N; i++ { + if _, err := ParseString(benchmarkTemplate); err != nil { + b.Fatal(err) + } + } +} + +func BenchmarkFormat(b *testing.B) { + b.ReportAllocs() + sb := new(strings.Builder) + for i := 0; i < b.N; i++ { + tf, err := ParseString(benchmarkTemplate) + if err != nil { + b.Fatal(err) + } + if err = tf.Write(sb); err != nil { + b.Fatal(err) + } + sb.Reset() + } +} diff --git a/parser/v2/benchmarktestdata/benchmark.txt b/parser/v2/benchmarktestdata/benchmark.txt new file mode 100644 index 000000000..e7c7ffc54 --- /dev/null +++ b/parser/v2/benchmarktestdata/benchmark.txt @@ -0,0 +1,18 @@ +package benchmarktestdata + +templ Benchmark() { +
Hello
+
+
+
+
+
Lorem ipsum dolor sit amet, consectetur adipiscing elit.
+
+
+ +} + diff --git a/parser/v2/diagnostics.go b/parser/v2/diagnostics.go index a8ee5e985..8f3665363 100644 --- a/parser/v2/diagnostics.go +++ b/parser/v2/diagnostics.go @@ -2,7 +2,6 @@ package parser import ( "errors" - "fmt" ) type diagnoser func(Node) ([]Diagnostic, error) @@ -35,7 +34,6 @@ func walkNodes(t []Node, f func(Node) bool) { var diagnosers = []diagnoser{ useOfLegacyCallSyntaxDiagnoser, - voidElementWithChildrenDiagnoser, } func Diagnose(t TemplateFile) ([]Diagnostic, error) { @@ -64,20 +62,3 @@ func useOfLegacyCallSyntaxDiagnoser(n Node) ([]Diagnostic, error) { } return nil, nil } - -func voidElementWithChildrenDiagnoser(n Node) (d []Diagnostic, err error) { - e, ok := n.(Element) - if !ok { - return - } - if !e.IsVoidElement() { - return - } - if len(e.Children) == 0 { - return - } - return []Diagnostic{{ - Message: fmt.Sprintf("void element <%s> should not have child content", e.Name), - Range: e.NameRange, - }}, nil -} diff --git a/parser/v2/diagnostics_test.go b/parser/v2/diagnostics_test.go index 3a7cea5be..e0a7b45f2 100644 --- a/parser/v2/diagnostics_test.go +++ b/parser/v2/diagnostics_test.go @@ -134,21 +134,6 @@ templ template () { }`, want: nil, }, - { - name: "voidElementWithChildrenDiagnoser: with diagnostics", - template: ` -package main - -templ template () { -
- Child content -
-}`, - want: []Diagnostic{{ - Message: "void element should not have child content", - Range: Range{Position{46, 5, 4}, Position{51, 5, 9}}, - }}, - }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { diff --git a/parser/v2/elementparser.go b/parser/v2/elementparser.go index ae71d5b8c..3c3e73d3f 100644 --- a/parser/v2/elementparser.go +++ b/parser/v2/elementparser.go @@ -375,6 +375,34 @@ var ( }) ) +// Void element closer. +var voidElementCloser voidElementCloserParser + +type voidElementCloserParser struct{} + +var voidElementCloseTags = []string{"", "", "
", "", "", "", "", "", "", "", "", "", "", "", "", ""} + +func (voidElementCloserParser) Parse(pi *parse.Input) (n Node, ok bool, err error) { + var ve string + for _, ve = range voidElementCloseTags { + s, canPeekLen := pi.Peek(len(ve)) + if !canPeekLen { + continue + } + if !strings.EqualFold(s, ve) { + continue + } + // Found a match. + ok = true + break + } + if !ok { + return nil, false, nil + } + pi.Take(len(ve)) + return nil, true, nil +} + // Element. var element elementParser @@ -396,28 +424,19 @@ func (elementParser) Parse(pi *parse.Input) (n Node, ok bool, err error) { // Once we've got an open tag, the rest must be present. l := pi.Position().Line - endOfOpenTag := pi.Index() // If the element is self-closing, even if it's not really a void element (br, hr etc.), we can return early. - if ot.Void { + if ot.Void || r.IsVoidElement() { // Escape early, no need to try to parse children for self-closing elements. return addTrailingSpaceAndValidate(start, r, pi) } - // Void elements _might_ have children, even though it's invalid. - // We want to allow this to be parsed. + // Parse children. closer := StripType(parse.All(parse.String("'))) tnp := newTemplateNodeParser(closer, fmt.Sprintf("<%s>: close tag", ot.Name)) nodes, _, err := tnp.Parse(pi) if err != nil { notFoundErr, isNotFoundError := err.(UntilNotFoundError) - if r.IsVoidElement() && isNotFoundError { - // Void elements shouldn't have children, or a close tag, so this is expected. - // When the template is reformatted, we won't reach here, because
will be converted to
. - // Return the element as we have it. - pi.Seek(endOfOpenTag) - return addTrailingSpaceAndValidate(start, r, pi) - } if isNotFoundError { err = notFoundErr.ParseError } @@ -443,6 +462,10 @@ func (elementParser) Parse(pi *parse.Input) (n Node, ok bool, err error) { } func addTrailingSpaceAndValidate(start parse.Position, e Element, pi *parse.Input) (n Node, ok bool, err error) { + // Elide any void close tags. + if _, _, err = voidElementCloser.Parse(pi); err != nil { + return e, false, err + } // Add trailing space. ws, _, err := parse.Whitespace.Parse(pi) if err != nil { diff --git a/parser/v2/elementparser_test.go b/parser/v2/elementparser_test.go index 764483661..0c7796dc7 100644 --- a/parser/v2/elementparser_test.go +++ b/parser/v2/elementparser_test.go @@ -550,6 +550,20 @@ if test { } } +func TestVoidElementCloserParser(t *testing.T) { + t.Run("all void elements are parsed", func(t *testing.T) { + for _, input := range voidElementCloseTags { + _, ok, err := voidElementCloser.Parse(parse.NewInput(input)) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if !ok { + t.Fatalf("failed to parse %q", input) + } + } + }) +} + func TestElementParser(t *testing.T) { tests := []struct { name string @@ -662,15 +676,9 @@ func TestElementParser(t *testing.T) { From: Position{Index: 1, Line: 0, Col: 1}, To: Position{Index: 6, Line: 0, Col: 6}, }, - Children: []Node{ - Text{ - Value: "Text", - Range: Range{ - From: Position{Index: 7, Line: 0, Col: 7}, - To: Position{Index: 11, Line: 0, Col: 11}, - }, - }, - }, + // is a void element, so text is not a child of the input. + // is ignored. + Children: nil, }, }, { @@ -736,7 +744,7 @@ func TestElementParser(t *testing.T) { }, }, { - name: "element: void nesting others is OK (br/hr)", + name: "element: void nesting is ignored", input: `

`, expected: Element{ Name: "br", @@ -744,15 +752,9 @@ func TestElementParser(t *testing.T) { From: Position{Index: 1, Line: 0, Col: 1}, To: Position{Index: 3, Line: 0, Col: 3}, }, - Children: []Node{ - Element{ - Name: "hr", - NameRange: Range{ - From: Position{Index: 5, Line: 0, Col: 5}, - To: Position{Index: 7, Line: 0, Col: 7}, - }, - }, - }, + //
is a void element, so
is not a child of the
. + //
is ignored. + Children: nil, }, }, { diff --git a/parser/v2/formattestdata/void_elements_are_converted_to_self_closing_elements.txt b/parser/v2/formattestdata/void_elements_are_converted_to_self_closing_elements.txt index 16cf47d2f..92caeb228 100644 --- a/parser/v2/formattestdata/void_elements_are_converted_to_self_closing_elements.txt +++ b/parser/v2/formattestdata/void_elements_are_converted_to_self_closing_elements.txt @@ -20,6 +20,7 @@ templ input(value, validation string) { + Text Text @@ -59,7 +60,8 @@ templ input(value, validation string) { - Text + Text + Text diff --git a/parser/v2/templateparser.go b/parser/v2/templateparser.go index a553ecae4..196563768 100644 --- a/parser/v2/templateparser.go +++ b/parser/v2/templateparser.go @@ -61,6 +61,10 @@ type templateNodeParser[TUntil any] struct { var rawElements = parse.Any(styleElement, scriptElement) +var templateNodeSkipParsers = []parse.Parser[Node]{ + voidElementCloser, //
, etc. - should be ignored. +} + var templateNodeParsers = []parse.Parser[Node]{ docType, // htmlComment, //