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

Only process templates with presentation model through pipeline #22

Closed
wants to merge 1 commit into from

Conversation

RaoulMeyer
Copy link

Right now Shoot will add a wrapper around every Twig node that is rendered. Even if those nodes do not have a presentation model associated to it. This has a significant impact on render time, especially when rendering a lot of small templates without presentation models.

This PR will make sure Shoot only adds the wrapper when it needs to do that, so only when there is a presentation model present.

I've measured the time it takes to render an empty template 100.000 times. Below are three runs of that test per scenario. The results are the time it takes in seconds to render the empty template once.

Current:
1.6900370121002E-5
1.8089079856873E-5
1.6964430809021E-5

Without any middleware
1.3252210617065E-5
1.3505561351776E-5
1.347021818161E-5

After this change:
1.1957528591156E-5
1.1988968849182E-5
1.1879818439484E-5

@victorwelling
Copy link
Member

This would result in not being able to use any of the other middleware in templates without a presentation model, such as the optional tag. I'm not sure that’s an improvement.

Are there any other ways we could get the performance boost without impacting functionality?

@RaoulMeyer
Copy link
Author

Yes you're right, this would break the optional tag behavior, I didn't think about that.

I don't see any obvious ways to improve performance here, because all middleware should always be executed to keep everything working as it does now.

@RaoulMeyer RaoulMeyer closed this Sep 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants