Skip to content

Commit

Permalink
minor #45184 [DependencyInjection][EventDispatcher] Avoid instantiati…
Browse files Browse the repository at this point in the history
…ng not called listeners when tracing (Jean-Beru)

This PR was squashed before being merged into the 6.1 branch.

Discussion
----------

[DependencyInjection][EventDispatcher] Avoid instantiating not called listeners when tracing

| Q             | A
| ------------- | ---
| Branch?       | 6.1
| Bug fix?      | no
| New feature?  | no
| Deprecations? | no
| Tickets       | Fix #43658
| License       | MIT
| Doc PR        |

This PR removes useless listeners instantiation when `TraceableEventDispatcher::getNotCalledListeners()` is called.

Even if this PR is in WIP, I created it to allow comments from `@nicolas`-grekas (and anyone interested in of course).

Commits
-------

d4e60e9 [DependencyInjection][EventDispatcher] Avoid instantiating not called listeners when tracing
  • Loading branch information
nicolas-grekas committed Feb 18, 2022
2 parents 0294c11 + d4e60e9 commit 2d723e1
Show file tree
Hide file tree
Showing 9 changed files with 98 additions and 20 deletions.
13 changes: 12 additions & 1 deletion src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php
Expand Up @@ -1739,7 +1739,18 @@ private function dumpValue(mixed $value, bool $interpolate = true): string

$code = sprintf('return %s;', $code);

return sprintf("function ()%s {\n %s\n }", $returnedType, $code);
$attribute = '';
if ($value) {
$attribute = 'name: '.$this->dumpValue((string) $value, $interpolate);

if ($this->container->hasDefinition($value) && ($class = $this->container->findDefinition($value)->getClass()) && $class !== (string) $value) {
$attribute .= ', class: '.$this->dumpValue($class, $interpolate);
}

$attribute = sprintf('#[\Closure(%s)] ', $attribute);
}

return sprintf("%sfunction ()%s {\n %s\n }", $attribute, $returnedType, $code);
}

if ($value instanceof IteratorArgument) {
Expand Down
Expand Up @@ -29,7 +29,7 @@ class getClosureService extends ProjectServiceContainer
{
$container->services['closure'] = $instance = new \stdClass();

$instance->closures = [0 => function () use ($container): ?\stdClass {
$instance->closures = [0 => #[\Closure(name: 'foo', class: 'FooClass')] function () use ($container): ?\stdClass {
return ($container->services['foo'] ?? null);
}];

Expand Down
Expand Up @@ -55,7 +55,7 @@ protected function getFooService()
*/
protected function getServiceClosureService()
{
return $this->services['service_closure'] = new \Bar(function () {
return $this->services['service_closure'] = new \Bar(#[\Closure(name: 'foo', class: 'Foo')] function () {
return ($this->services['foo'] ?? ($this->services['foo'] = new \Foo()));
});
}
Expand Down
Expand Up @@ -70,9 +70,9 @@ protected function getBarServiceService()
*/
protected function getFooServiceService()
{
return $this->services['foo_service'] = new \Symfony\Component\DependencyInjection\ServiceLocator(['bar' => function () {
return $this->services['foo_service'] = new \Symfony\Component\DependencyInjection\ServiceLocator(['bar' => #[\Closure(name: 'bar_service', class: 'stdClass')] function () {
return ($this->services['bar_service'] ?? $this->getBarServiceService());
}, 'baz' => function (): \stdClass {
}, 'baz' => #[\Closure(name: 'baz_service', class: 'stdClass')] function (): \stdClass {
return ($this->privates['baz_service'] ?? ($this->privates['baz_service'] = new \stdClass()));
}, 'nil' => function () {
return NULL;
Expand Down Expand Up @@ -116,7 +116,7 @@ protected function getTranslator_Loader3Service()
*/
protected function getTranslator1Service()
{
return $this->services['translator_1'] = new \Symfony\Component\DependencyInjection\Tests\Fixtures\StubbedTranslator(new \Symfony\Component\DependencyInjection\ServiceLocator(['translator.loader_1' => function () {
return $this->services['translator_1'] = new \Symfony\Component\DependencyInjection\Tests\Fixtures\StubbedTranslator(new \Symfony\Component\DependencyInjection\ServiceLocator(['translator.loader_1' => #[\Closure(name: 'translator.loader_1', class: 'stdClass')] function () {
return ($this->services['translator.loader_1'] ?? ($this->services['translator.loader_1'] = new \stdClass()));
}]));
}
Expand All @@ -128,7 +128,7 @@ protected function getTranslator1Service()
*/
protected function getTranslator2Service()
{
$this->services['translator_2'] = $instance = new \Symfony\Component\DependencyInjection\Tests\Fixtures\StubbedTranslator(new \Symfony\Component\DependencyInjection\ServiceLocator(['translator.loader_2' => function () {
$this->services['translator_2'] = $instance = new \Symfony\Component\DependencyInjection\Tests\Fixtures\StubbedTranslator(new \Symfony\Component\DependencyInjection\ServiceLocator(['translator.loader_2' => #[\Closure(name: 'translator.loader_2', class: 'stdClass')] function () {
return ($this->services['translator.loader_2'] ?? ($this->services['translator.loader_2'] = new \stdClass()));
}]));

Expand All @@ -144,7 +144,7 @@ protected function getTranslator2Service()
*/
protected function getTranslator3Service()
{
$this->services['translator_3'] = $instance = new \Symfony\Component\DependencyInjection\Tests\Fixtures\StubbedTranslator(new \Symfony\Component\DependencyInjection\ServiceLocator(['translator.loader_3' => function () {
$this->services['translator_3'] = $instance = new \Symfony\Component\DependencyInjection\Tests\Fixtures\StubbedTranslator(new \Symfony\Component\DependencyInjection\ServiceLocator(['translator.loader_3' => #[\Closure(name: 'translator.loader_3', class: 'stdClass')] function () {
return ($this->services['translator.loader_3'] ?? ($this->services['translator.loader_3'] = new \stdClass()));
}]));

Expand Down
Expand Up @@ -58,11 +58,11 @@ protected function getBarService()
$instance->foo1 = ($this->services['foo1'] ?? null);
$instance->foo2 = null;
$instance->foo3 = ($this->privates['foo3'] ?? null);
$instance->closures = [0 => function () {
$instance->closures = [0 => #[\Closure(name: 'foo1', class: 'stdClass')] function () {
return ($this->services['foo1'] ?? null);
}, 1 => function () {
}, 1 => #[\Closure(name: 'foo2')] function () {
return null;
}, 2 => function () {
}, 2 => #[\Closure(name: 'foo3', class: 'stdClass')] function () {
return ($this->privates['foo3'] ?? null);
}];
$instance->iter = new RewindableGenerator(function () {
Expand Down
Expand Up @@ -13,6 +13,7 @@

use Psr\EventDispatcher\StoppableEventInterface;
use Psr\Log\LoggerInterface;
use Symfony\Component\EventDispatcher\EventDispatcher;
use Symfony\Component\EventDispatcher\EventDispatcherInterface;
use Symfony\Component\EventDispatcher\EventSubscriberInterface;
use Symfony\Component\HttpFoundation\Request;
Expand Down Expand Up @@ -187,7 +188,7 @@ public function getCalledListeners(Request $request = null): array
public function getNotCalledListeners(Request $request = null): array
{
try {
$allListeners = $this->getListeners();
$allListeners = $this->dispatcher instanceof EventDispatcher ? $this->getListenersWithPriority() : $this->getListenersWithoutPriority();
} catch (\Exception $e) {
$this->logger?->info('An exception was thrown while getting the uncalled listeners.', ['exception' => $e]);

Expand All @@ -209,11 +210,12 @@ public function getNotCalledListeners(Request $request = null): array
}

$notCalled = [];

foreach ($allListeners as $eventName => $listeners) {
foreach ($listeners as $listener) {
foreach ($listeners as [$listener, $priority]) {
if (!\in_array($listener, $calledListeners, true)) {
if (!$listener instanceof WrappedListener) {
$listener = new WrappedListener($listener, null, $this->stopwatch, $this);
$listener = new WrappedListener($listener, null, $this->stopwatch, $this, $priority);
}
$notCalled[] = $listener->getInfo($eventName);
}
Expand Down Expand Up @@ -323,7 +325,7 @@ private function postProcess(string $eventName): void
}
}

private function sortNotCalledListeners(array $a, array $b)
private function sortNotCalledListeners(array $a, array $b): int
{
if (0 !== $cmp = strcmp($a['event'], $b['event'])) {
return $cmp;
Expand All @@ -347,4 +349,35 @@ private function sortNotCalledListeners(array $a, array $b)

return 1;
}

private function getListenersWithPriority(): array
{
$result = [];

$allListeners = new \ReflectionProperty(EventDispatcher::class, 'listeners');
$allListeners->setAccessible(true);

foreach ($allListeners->getValue($this->dispatcher) as $eventName => $listenersByPriority) {
foreach ($listenersByPriority as $priority => $listeners) {
foreach ($listeners as $listener) {
$result[$eventName][] = [$listener, $priority];
}
}
}

return $result;
}

private function getListenersWithoutPriority(): array
{
$result = [];

foreach ($this->getListeners() as $eventName => $listeners) {
foreach ($listeners as $listener) {
$result[$eventName][] = [$listener, null];
}
}

return $result;
}
}
31 changes: 26 additions & 5 deletions src/Symfony/Component/EventDispatcher/Debug/WrappedListener.php
Expand Up @@ -29,20 +29,23 @@ final class WrappedListener
private Stopwatch $stopwatch;
private ?EventDispatcherInterface $dispatcher;
private string $pretty;
private string $callableRef;
private ClassStub|string $stub;
private ?int $priority = null;
private static bool $hasClassStub;

public function __construct(callable|array $listener, ?string $name, Stopwatch $stopwatch, EventDispatcherInterface $dispatcher = null)
public function __construct(callable|array $listener, ?string $name, Stopwatch $stopwatch, EventDispatcherInterface $dispatcher = null, int $priority = null)
{
$this->listener = $listener;
$this->optimizedListener = $listener instanceof \Closure ? $listener : (\is_callable($listener) ? \Closure::fromCallable($listener) : null);
$this->stopwatch = $stopwatch;
$this->dispatcher = $dispatcher;
$this->priority = $priority;

if (\is_array($listener)) {
$this->name = \is_object($listener[0]) ? get_debug_type($listener[0]) : $listener[0];
[$this->name, $this->callableRef] = $this->parseListener($listener);
$this->pretty = $this->name.'::'.$listener[1];
$this->callableRef .= '::'.$listener[1];
} elseif ($listener instanceof \Closure) {
$r = new \ReflectionFunction($listener);
if (str_contains($r->name, '{closure}')) {
Expand All @@ -58,6 +61,7 @@ public function __construct(callable|array $listener, ?string $name, Stopwatch $
} else {
$this->name = get_debug_type($listener);
$this->pretty = $this->name.'::__invoke';
$this->callableRef = \get_class($listener).'::__invoke';
}

if (null !== $name) {
Expand Down Expand Up @@ -89,11 +93,11 @@ public function getPretty(): string

public function getInfo(string $eventName): array
{
$this->stub ??= self::$hasClassStub ? new ClassStub($this->pretty.'()', $this->listener) : $this->pretty.'()';
$this->stub ??= self::$hasClassStub ? new ClassStub($this->pretty.'()', $this->callableRef ?? $this->listener) : $this->pretty.'()';

return [
'event' => $eventName,
'priority' => $this->priority ?? $this->dispatcher?->getListenerPriority($eventName, $this->listener),
'priority' => $this->priority ??= $this->dispatcher?->getListenerPriority($eventName, $this->listener),
'pretty' => $this->pretty,
'stub' => $this->stub,
];
Expand All @@ -104,7 +108,7 @@ public function __invoke(object $event, string $eventName, EventDispatcherInterf
$dispatcher = $this->dispatcher ?: $dispatcher;

$this->called = true;
$this->priority = $dispatcher->getListenerPriority($eventName, $this->listener);
$this->priority ??= $dispatcher->getListenerPriority($eventName, $this->listener);

$e = $this->stopwatch->start($this->name, 'event_listener');

Expand All @@ -118,4 +122,21 @@ public function __invoke(object $event, string $eventName, EventDispatcherInterf
$this->stoppedPropagation = true;
}
}

private function parseListener(array $listener): array
{
if ($listener[0] instanceof \Closure) {
foreach ((new \ReflectionFunction($listener[0]))->getAttributes(\Closure::class) as $attribute) {
if ($name = $attribute->getArguments()['name'] ?? false) {
return [$name, $attribute->getArguments()['class'] ?? $name];
}
}
}

if (\is_object($listener[0])) {
return [get_debug_type($listener[0]), \get_class($listener[0])];
}

return [$listener[0], $listener[0]];
}
}
Expand Up @@ -125,6 +125,18 @@ public function testGetCalledListeners()
$this->assertEquals([], $tdispatcher->getNotCalledListeners());
}

public function testGetNotCalledClosureListeners()
{
$instantiationCount = 0;

$tdispatcher = new TraceableEventDispatcher(new EventDispatcher(), new Stopwatch());
$tdispatcher->addListener('foo', [static function () use (&$instantiationCount) { ++$instantiationCount; }, 'onFoo']);

$tdispatcher->getNotCalledListeners();

$this->assertSame(0, $instantiationCount);
}

public function testClearCalledListeners()
{
$tdispatcher = new TraceableEventDispatcher(new EventDispatcher(), new Stopwatch());
Expand Down
Expand Up @@ -40,6 +40,7 @@ public function provideListenersToDescribe()
[\Closure::fromCallable([new FooListener(), 'listen']), 'Symfony\Component\EventDispatcher\Tests\Debug\FooListener::listen'],
[\Closure::fromCallable(['Symfony\Component\EventDispatcher\Tests\Debug\FooListener', 'listenStatic']), 'Symfony\Component\EventDispatcher\Tests\Debug\FooListener::listenStatic'],
[\Closure::fromCallable(function () {}), 'closure'],
[[#[\Closure(name: FooListener::class)] static fn () => new FooListener(), 'listen'], 'Symfony\Component\EventDispatcher\Tests\Debug\FooListener::listen'],
];
}
}
Expand Down

0 comments on commit 2d723e1

Please sign in to comment.