-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Swap BC layer for yield-ready and reclaim perf loss #4216
Conversation
c2a9e53
to
5d1a19a
Compare
@nicolas-grekas Can you add a note in CHANGELOG? |
I've just tested on https://github.com/j0r1s/twig-performance-loss-reproducer, and it indeed fixes the issue with the default Before: https://blackfire.io/profiles/a94a955c-b84d-4dfb-94f0-89300e0aa4a3/graph I've also tested on a simpler test script and it works great as well. |
Thank you @nicolas-grekas. |
Oops, forgot about the CHANGELOG. Fixed here: 3a307cd |
Wow thank you @nicolas-grekas for this! 👏 🙇 |
Follows #3999
Fix #4146
Fix #4103
When
use_yield
is set to false (the default), this PR reverts the implementation of therender()
method to use a wrapping output buffer instead of hooking between each steps of generators. In this mode, the behavior of the yield method is not "pure": it triggers a mix of yield and echo. But this is fine for render and display methods.When
use_yield
is set totrue
, we skip that wrapping output buffer. This makes twig compatible with fibers (and this also makes compilation fail if a non-YieldReady extension is found.)That makes the name of the option not ideal, but BC rulez FTW.