mirror of
https://github.com/twigphp/Twig.git
synced 2026-10-02 18:07:35 +00:00
minor #4976 Speed up traversing nodes with node visitors (nicolas-grekas)
This PR was merged into the 3.x branch.
Discussion
----------
Speed up traversing nodes with node visitors
`NodeTraverser` walks the whole tree once per node visitor, and it iterated over the children of each node with `foreach ($node as ...)`, which allocates an `ArrayIterator` per node and per visitor. It now reads the children array through a new internal `Node::getNodes()` method. On the Symfony Demo, where six visitors are registered, walking the 21k nodes of its 52 templates takes half the instructions it took, and warming up its Twig cache runs 15% fewer instructions and takes 520ms instead of 566ms.
The traverser doesn't go through `getIterator()` anymore, so a subclass overriding it to hide children from visitors would be bypassed, and one declaring its own `getNodes()` would clash; I found none in Twig, Symfony or UX. Calling `getArrayCopy()` on the iterator returned by `getIterator()` would avoid the new method, but it saves 13% of the traversal instructions instead of 50%: `new \ArrayIterator()` and `getArrayCopy()` both copy the array.
<details>
<summary>Merge-up notes</summary>
To 4.x: the patch applies as is (I ran the 4.x suite with it), the CHANGELOG entry doesn't go to 4.x.
</details>
Commits
-------
69fefec125 Speed up traversing nodes with node visitors
This commit is contained in:
@@ -9,6 +9,7 @@
|
||||
* Fix the spread operator compiling to invalid PHP outside sequences, mappings, and call arguments
|
||||
* Fix the `deprecated` tag generating invalid PHP for an integer message
|
||||
* Add a third argument to `FilesystemLoader::addPath()` and `FilesystemLoader::prependPath()` to skip checking that the directory exists
|
||||
* Speed up traversing nodes with node visitors
|
||||
|
||||
# 3.30.0 (2026-09-25)
|
||||
|
||||
|
||||
@@ -292,6 +292,16 @@ class Node implements \Countable, \IteratorAggregate
|
||||
return new \ArrayIterator($this->nodes);
|
||||
}
|
||||
|
||||
/**
|
||||
* @internal
|
||||
*
|
||||
* @return array<string|int, Node>
|
||||
*/
|
||||
public function getNodes(): array
|
||||
{
|
||||
return $this->nodes;
|
||||
}
|
||||
|
||||
public function getTemplateName(): ?string
|
||||
{
|
||||
return $this->sourceContext ? $this->sourceContext->getName() : null;
|
||||
|
||||
@@ -61,7 +61,7 @@ final class NodeTraverser
|
||||
{
|
||||
$node = $visitor->enterNode($node, $this->env);
|
||||
|
||||
foreach ($node as $k => $n) {
|
||||
foreach ($node->getNodes() as $k => $n) {
|
||||
if (null !== $m = $this->traverseForVisitor($visitor, $n)) {
|
||||
if ($m !== $n) {
|
||||
$node->setNode($k, $m);
|
||||
|
||||
@@ -0,0 +1,145 @@
|
||||
<?php
|
||||
|
||||
/*
|
||||
* This file is part of Twig.
|
||||
*
|
||||
* (c) Fabien Potencier
|
||||
*
|
||||
* For the full copyright and license information, please view the LICENSE
|
||||
* file that was distributed with this source code.
|
||||
*/
|
||||
|
||||
namespace Twig\Tests;
|
||||
|
||||
use PHPUnit\Framework\TestCase;
|
||||
use Twig\Environment;
|
||||
use Twig\Loader\ArrayLoader;
|
||||
use Twig\Node\Node;
|
||||
use Twig\NodeTraverser;
|
||||
use Twig\NodeVisitor\NodeVisitorInterface;
|
||||
|
||||
class NodeTraverserTest extends TestCase
|
||||
{
|
||||
public function testVisitorsTraverseTheWholeTreeOneAfterTheOtherByPriority(): void
|
||||
{
|
||||
$log = [];
|
||||
$tree = new NodeTraverserTestNode('root', [
|
||||
'a' => new NodeTraverserTestNode('a'),
|
||||
'b' => new NodeTraverserTestNode('b', [new NodeTraverserTestNode('c')]),
|
||||
]);
|
||||
|
||||
$traverser = new NodeTraverser(new Environment(new ArrayLoader()), [
|
||||
new NodeTraverserTestVisitor('late', 10, $log),
|
||||
new NodeTraverserTestVisitor('early', -10, $log),
|
||||
new NodeTraverserTestVisitor('middle', 0, $log),
|
||||
]);
|
||||
|
||||
$this->assertSame($tree, $traverser->traverse($tree));
|
||||
$this->assertSame([
|
||||
'early enters root', 'early enters a', 'early leaves a', 'early enters b', 'early enters c', 'early leaves c', 'early leaves b', 'early leaves root',
|
||||
'middle enters root', 'middle enters a', 'middle leaves a', 'middle enters b', 'middle enters c', 'middle leaves c', 'middle leaves b', 'middle leaves root',
|
||||
'late enters root', 'late enters a', 'late leaves a', 'late enters b', 'late enters c', 'late leaves c', 'late leaves b', 'late leaves root',
|
||||
], $log);
|
||||
}
|
||||
|
||||
public function testVisitorsCanReplaceAndRemoveNodes(): void
|
||||
{
|
||||
$log = [];
|
||||
$tree = new NodeTraverserTestNode('root', [
|
||||
'replacedOnEnter' => new NodeTraverserTestNode('old', [new NodeTraverserTestNode('old child')]),
|
||||
'removed' => new NodeTraverserTestNode('removed'),
|
||||
'replacedOnLeave' => new NodeTraverserTestNode('leaving'),
|
||||
]);
|
||||
|
||||
$visitor = new NodeTraverserTestVisitor('visitor', 0, $log);
|
||||
$visitor->onEnter = static fn (Node $node) => 'old' === $node->getAttribute('name') ? new NodeTraverserTestNode('new', [new NodeTraverserTestNode('new child')]) : $node;
|
||||
$visitor->onLeave = static fn (Node $node) => match ($node->getAttribute('name')) {
|
||||
'removed' => null,
|
||||
'leaving' => new NodeTraverserTestNode('left'),
|
||||
default => $node,
|
||||
};
|
||||
|
||||
(new NodeTraverser(new Environment(new ArrayLoader()), [$visitor]))->traverse($tree);
|
||||
|
||||
$this->assertSame([
|
||||
'visitor enters root',
|
||||
'visitor enters old', 'visitor enters new child', 'visitor leaves new child', 'visitor leaves new',
|
||||
'visitor enters removed', 'visitor leaves removed',
|
||||
'visitor enters leaving', 'visitor leaves leaving',
|
||||
'visitor leaves root',
|
||||
], $log);
|
||||
$this->assertSame(['replacedOnEnter' => 'new', 'replacedOnLeave' => 'left'], array_map(static fn (Node $node) => $node->getAttribute('name'), iterator_to_array($tree)));
|
||||
}
|
||||
|
||||
public function testChildrenAreTheOnesTheNodeHadAfterBeingEntered(): void
|
||||
{
|
||||
$log = [];
|
||||
$tree = new NodeTraverserTestNode('root', [
|
||||
'a' => new NodeTraverserTestNode('a'),
|
||||
'b' => new NodeTraverserTestNode('b'),
|
||||
]);
|
||||
|
||||
$visitor = new NodeTraverserTestVisitor('visitor', 0, $log);
|
||||
$visitor->onEnter = static function (Node $node) use ($tree) {
|
||||
if ($node === $tree) {
|
||||
$tree->setNode('c', new NodeTraverserTestNode('c'));
|
||||
} elseif ('a' === $node->getAttribute('name')) {
|
||||
$tree->removeNode('b');
|
||||
$tree->setNode('d', new NodeTraverserTestNode('d'));
|
||||
}
|
||||
|
||||
return $node;
|
||||
};
|
||||
|
||||
(new NodeTraverser(new Environment(new ArrayLoader()), [$visitor]))->traverse($tree);
|
||||
|
||||
$this->assertSame([
|
||||
'visitor enters root',
|
||||
'visitor enters a', 'visitor leaves a',
|
||||
'visitor enters b', 'visitor leaves b',
|
||||
'visitor enters c', 'visitor leaves c',
|
||||
'visitor leaves root',
|
||||
], $log);
|
||||
$this->assertSame(['a', 'c', 'd'], array_keys(iterator_to_array($tree)));
|
||||
}
|
||||
}
|
||||
|
||||
class NodeTraverserTestNode extends Node
|
||||
{
|
||||
public function __construct(string $name, array $nodes = [])
|
||||
{
|
||||
parent::__construct($nodes, ['name' => $name]);
|
||||
}
|
||||
}
|
||||
|
||||
class NodeTraverserTestVisitor implements NodeVisitorInterface
|
||||
{
|
||||
public ?\Closure $onEnter = null;
|
||||
public ?\Closure $onLeave = null;
|
||||
|
||||
public function __construct(
|
||||
private string $name,
|
||||
private int $priority,
|
||||
private array &$log,
|
||||
) {
|
||||
}
|
||||
|
||||
public function enterNode(Node $node, Environment $env): Node
|
||||
{
|
||||
$this->log[] = $this->name.' enters '.$node->getAttribute('name');
|
||||
|
||||
return $this->onEnter ? ($this->onEnter)($node) : $node;
|
||||
}
|
||||
|
||||
public function leaveNode(Node $node, Environment $env): ?Node
|
||||
{
|
||||
$this->log[] = $this->name.' leaves '.$node->getAttribute('name');
|
||||
|
||||
return $this->onLeave ? ($this->onLeave)($node) : $node;
|
||||
}
|
||||
|
||||
public function getPriority(): int
|
||||
{
|
||||
return $this->priority;
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user