Twig icon indicating copy to clipboard operation
Twig copied to clipboard

Unused variable in compiled Twig templates

Open javiereguiluz opened this issue 1 year ago • 6 comments

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:

image

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!

javiereguiluz avatar Apr 15 '24 08:04 javiereguiluz

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.

stof avatar Apr 15 '24 10:04 stof

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.

javiereguiluz avatar Apr 19 '24 15:04 javiereguiluz

let's hope that PHP removes this unused variable instead of calculating its value

Sadly that's not the case 😅

Image

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:

Image

To:

Image

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;

azjezz avatar Nov 17 '25 18:11 azjezz

Let's reopen this issue to think if we can fix it in some other way as @azjezz said. Thanks!

javiereguiluz avatar Nov 18 '25 11:11 javiereguiluz

@azjezz your logic should use array_key_exists instead of isset to avoid issues with null values in variables.

stof avatar Nov 18 '25 12:11 stof

Open a PR if there is a measurable performance improvement.

GromNaN avatar Nov 18 '25 12:11 GromNaN