-
-
Notifications
You must be signed in to change notification settings - Fork 361
[TwigComponent] Improve performance for complex use-cases #2812
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
Comments
@Kocal I see you’re busy with building components like I did. I guess I don’t do something completely wrong 😉 |
Thanks for the reproducer :) I will try to give Blackfire a try when having some free time. @smnandre already worked on TwigComponent optimization, and IIRC, major performance impacts were found due to how event listeners worked. |
Thanks all for the comments! No components testI've started with migrating to HTML elements with classes to see what a potential optimum could be. On the Twig Components with Tailwind classes HTML elements with extracted Tailwind classes SVGIn that branch I also switched to UX Icons, but I could not see a difference. They also render as SVG elements. Anonymous ComponentsIs it an idea to treat anonymous components differently? More like functions instead of services. |
I'm sorry I don't get this... 🤔 How can the HTML be different if you are writing the same HTML ? TwigComponent does not add a single tag or attribute you do not render yourself. So it's either a question of ....or am i missing something here ? |
Sorry I should have been clearer! What this commit does is:
I will continue doing the same for other components than Button. In terms of solutions, I'm thinking:
Curious what you think? Blackfire seems paid now? I'll see if I can run SPX later for better data. |
Tailwind Merge ![]() Found a few unnecessary instantiations of data in TailwindMerge, see When I make them static I get a nice reduction in the reproducer: Response time Response time |
Twig already differenciates a static string and an evaluated expression. And cannot return only a part of it so i'm not sure it would change anything here.
There is nothing related to CSS in TwigComponent, so this is not something we have any power.
Not now sadly, that would break the BC promise. But this is something we will work on in 3.0 (and I have several POC / prototypes ready for this) 😄 |
@smnandre thanks for the comments! Yes got (local) fixes for the Tailwind stuff and hope to contribute it back upstream 👍 The profiler is disabled, thanks! I do see indeed a big junk for event listener tracking in my real project where the profiler is enabled. I forgot to say, but I also looked into event listeners as it was hinted on before here #2812 (comment). Got a nice performance boost by disabling event dispatching in My components look very similar to what @Kocal is building (also using |
Going to close this as I think we discussed what's possible and I have a few improvements to do in other repos.
Looking forward to this! |
If you want to get a big performance improvement for this scenario (a lot of embedding anonymous component with big graph): try uninstalling live component. And tell me what metrics you get :) You are 100% right about the events, it's one of the things we cannot move for now without massive BC break... but something we need to change... without what any optimization is just very, very hard to get. |
Opened gehrisandro/tailwind-merge-php#19 and #2821 |
Uh oh!
There was an error while loading. Please reload this page.
Firstly, great work and nice to see a component based way of working for Symfony.
Scenario
Creating components like shadcn with Twig Components is great. However, when using a lot of them, the performance degrades quickly. Especially when adding popovers with buttons in them for each cell.
Reproducer
composer install
symfony server:start
Solutions
I'm probably switching back to simple HTML elements again (
<button class="ghost">
) and use Tailwinds@apply
for different variants. Of course, I'd prefer to keep using the component approach.Phoenix LiveView solved the 2. problem with an optimization, see Optimization #1: splitting statics from dynamics. This way the response size doesn't grow very large.
The text was updated successfully, but these errors were encountered: