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

performance: templ parse takes a long time and uses high CPU when unclosed void elements are used #886

Closed
s00500 opened this issue Aug 17, 2024 · 8 comments

Comments

@s00500
Copy link

s00500 commented Aug 17, 2024

Describe the bug
With the code I show here templ takes VERY long to compile, about 29 seconds. If I add more cases to the switch case it takes even longer, to the point where it is completey unusable and locked.

If I just remove the inputs in the top of the ActionStep template it goes below one second again...

This seems like a really funkey scanner issue to me, but I am sure you are much faster in finding the actual issue...

the templ process also climbs to ridiculous CPU usage while trying to generate in this case...

To Reproduce
Repo: https://github.com/s00500/templ-switchcaseissue

Expected behavior
Fast compile, no lockup

Logs

❯ templ generate
(!) templ version check: failed to read go.mod file: open /go.mod: no such file or directory
(✓) Complete [ updates=1 duration=28.554246583s ]

templ info output

❯ templ info
(✓) os [ goos=darwin goarch=arm64 ]
(✓) go [ location=/usr/local/go/bin/go version=go version go1.22.1 darwin/arm64 ]
(✓) gopls [ location=/Users/lb/go/bin/gopls version=golang.org/x/tools/gopls v0.16.1 ]
(✓) templ [ location=/Users/lb/go/bin/templ version=v0.2.759 ]

Desktop (please complete the following information):
macOS (latest, sonoma 14.6.1)

❯ templ version
v0.2.759

(Masterbuild from today)

templ

@s00500
Copy link
Author

s00500 commented Aug 17, 2024

Here is the time it takes after adding one more case with a
to the template:

❯ templ generate
(!) templ version check: failed to read go.mod file: open /go.mod: no such file or directory
(✓) Complete [ updates=1 duration=48.298382166s ]

(increase by ~20 seconds)

@a-h
Copy link
Owner

a-h commented Aug 17, 2024

It's the unclosed <br> elements. The parser attempts to look for a close tag to decide what to do.

Once fmt has run, it doesn't need to do that any more. It replaces <br> with <br/> and the speed is back to normal.

A fix would be for the parser to give up trying to look for the </br> as soon as it sees another <br> in the doc.

@a-h
Copy link
Owner

a-h commented Aug 17, 2024

The parser is slow because, imagine this scenario ("element: void nesting same is OK" test).

People sometimes accidentally put elements inside void elements, so it's not clear what the intent is.

<div><br><br></br></div>

How should templ handle this?

<br>
  <br>
</br>

Or...

<br>
<br></br>

Or an error complaining about a void element having nested content?

Or strip the nested content?

Or ignore closers for void elements?

@s00500
Copy link
Author

s00500 commented Aug 17, 2024

Ah I see... ok...
Any chance this can lead to a warning ? the behavior is understandable, but it is not intuitive to see whats happening. So having a warning shown on every <br> feels like it would help a lot, what do you think ?

@a-h
Copy link
Owner

a-h commented Aug 17, 2024

If you run templ fmt it should auto-fix <br> to <br/> (although it would take a long time first time time).

If you do <input><br/></input> or similar, you should get a "void element should not have child content" warning in the LSP warnings, e.g.:

image

@a-h a-h changed the title Templ generate takes very long with very high CPU usage in strange edgecase performance: templ parse takes a long time and uses high CPU when unclosed void elements are used Aug 17, 2024
@s00500
Copy link
Author

s00500 commented Aug 18, 2024

Cool... I was able to get the message you show in the screenshot. but I dont get any if I just put a <br> without any closer...

Also templ fmt takes long as you mentioned
(never actually completed) so it is kind of not applicable here

Out of your comments from yesterday I feel that errors or warnings are the way to go. if the parser has difficulties with this kind of scenario that is fair, I just feel it would be great if the lsp could warn for that early :-)

(Amazing work on this project btw, finally a templating system for go that is mostly enjoyable to use)

@a-h
Copy link
Owner

a-h commented Aug 18, 2024

In the PR, I think I've sorted the performance out. If you have time to build templ in the PR and try it out, that would be great.

The "install-snapshot" task in the README does the required stuff.

@a-h a-h closed this as completed in ae99146 Aug 18, 2024
@s00500
Copy link
Author

s00500 commented Aug 19, 2024

Awesome, I tested this on my little repo with the snapshot and it performs as expected, thanks a lot for the quick fix :-)

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

No branches or pull requests

2 participants