Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: improve void element parsing perf, fixes #886 and #857 #887

Merged
merged 3 commits into from
Aug 18, 2024
Merged

Conversation

a-h
Copy link
Owner

@a-h a-h commented Aug 17, 2024

This PR changes the behaviour of templ's void element handling.

Previously, you could do <input>Text</input> and templ would allow it, despite it not being valid HTML. The LSP would emit a warning, but it would render.

However, this caused a performance issue for unclosed void elements. templ was parsing the rest of the file to look for a closing element, just in case it was present, so that the elements could be nested in templ's object model.

This operation wasn't memoized, so it ran every time a void element was encountered. In files with lots of void elements, it would cause a long delay!

The formatting operation rewrites <br> to <br/> which vastly reduces the time, but it's not a great experience that it takes a long time. It seems that some users don't use the editor extensions, or are unaware of the fmt tool, so they just live with the known bad performance.

This PR changes the templ behaviour so that it doesn't have nested content for all known void HTML elements, e.g.:

<input>Text</input>

Becomes:

<input/>Text

This improves the performance, see benchmark results:

Before

pkg: github.com/a-h/templ/parser/v2
BenchmarkParse-16           3538            387117 ns/op          194075 B/op       4684 allocs/op
BenchmarkFormat-16          3138            400745 ns/op          197759 B/op       4765 allocs/op

After

pkg: github.com/a-h/templ/parser/v2
BenchmarkParse-16          23718             46576 ns/op           23478 B/op        544 allocs/op
BenchmarkFormat-16         19960             60805 ns/op           26874 B/op        625 allocs/op

Comparison

The important part is the ns/op figure - how many nanoseconds each operation takes.

46576 / 387117=12%, so the new version is about 8 times faster in the given scenario.

@a-h a-h requested a review from joerdav August 17, 2024 15:07
@a-h a-h marked this pull request as ready for review August 17, 2024 15:10
Copy link
Collaborator

@joerdav joerdav left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, this makes a lot of sense!

@a-h a-h merged commit ae99146 into main Aug 18, 2024
6 of 7 checks passed
@a-h a-h deleted the issue_886 branch August 18, 2024 22:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants