Unused variable in compiled Twig templates
In many compiled templates I see this instruction:
$context = array_intersect_key($context, $_parent) + $_parent;
Problem is that many times that variable at the very end of some method and it's never used:
Two quick questions:
- Is setting this variable needed ... or is this indeed a case of "unused variable" that we should optimize?
- If it's a truly unused variable ... should we care about it or will PHP itself remove this variable (and avoid computing its value) when optimizing code to generate the bytecode?
Thanks!
Optimizing this would require checking the following parts of the compiled template to ensure that the Twig context is not used anymore. This would add a huge amount of complexity to the Twig compiler as it would require doing multiple passes (as Twig allows hooking custom nodes in its AST, there is no way to know easily whether compiling a node will include accessing the context without actually compiling the nodes).
If the last thing using the context in your template or block is a loop, the cleanup of the context at the end of the loop might indeed be unnecessary, but it might be hard to optimize it away from the compiled template.
Thanks for the insights. I see that this is much more difficult than I thought. Let's close this as won't fix and let's hope that PHP removes this unused variable instead of calculating its value, which is probably what's already happening.
let's hope that PHP removes this unused variable instead of calculating its value
Sadly that's not the case 😅
PHP can't just remove a statement as
there is "no way" for the engine to tell if it has side effects or not ( at least now ), so this still computes.
In a template using symfony forms, where there is 1000s of forms being rendered, we end up with the above, a huge amount of memory being consumed here.
This is not the main issue with our memory usage, but it is significant.
@stof can we get this issue re-opened to explore how we can check if context is used later or not?
Note, we managed to cut down memory from:
To:
By replacing all $context = array_intersect_key($context, $_parent) + $_parent; statements with the following ( including unused ones ):
foreach ($_parent as $key => $_) {
if (isset($context[$key])) {
$_parent[$key] = $context[$key];
}
}
$context = $_parent;
Let's reopen this issue to think if we can fix it in some other way as @azjezz said. Thanks!
@azjezz your logic should use array_key_exists instead of isset to avoid issues with null values in variables.
Open a PR if there is a measurable performance improvement.