From 88ecd0dc9ac2a369a477dfb5558bc5229dc204f5 Mon Sep 17 00:00:00 2001 From: Nicolas Grekas Date: Sun, 13 May 2018 23:24:43 +0200 Subject: [PATCH] [DI] fine tune dumped factories --- .../DependencyInjection/Dumper/PhpDumper.php | 30 ++++++++++++++++--- .../Tests/Fixtures/php/services9_as_files.txt | 16 ++-------- .../Tests/Fixtures/php/services9_compiled.php | 6 ++-- .../php/services_errored_definition.php | 6 ++-- .../php/services_private_in_expression.php | 2 +- 5 files changed, 36 insertions(+), 24 deletions(-) diff --git a/src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php b/src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php index 8491f806b4b17..b12d885325723 100644 --- a/src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php +++ b/src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php @@ -17,6 +17,7 @@ use Symfony\Component\DependencyInjection\Variable; use Symfony\Component\DependencyInjection\Definition; use Symfony\Component\DependencyInjection\Compiler\AnalyzeServiceReferencesPass; +use Symfony\Component\DependencyInjection\Compiler\ServiceReferenceGraphNode; use Symfony\Component\DependencyInjection\ContainerBuilder; use Symfony\Component\DependencyInjection\Container; use Symfony\Component\DependencyInjection\ContainerInterface; @@ -68,6 +69,7 @@ class PhpDumper extends Dumper private $inlineRequires; private $inlinedRequires = array(); private $circularReferences = array(); + private $singleUsePrivateIds = array(); /** * @var ProxyDumper @@ -141,10 +143,14 @@ public function dump(array $options = array()) (new AnalyzeServiceReferencesPass())->process($this->container); $this->circularReferences = array(); + $this->singleUsePrivateIds = array(); $checkedNodes = array(); foreach ($this->container->getCompiler()->getServiceReferenceGraph()->getNodes() as $id => $node) { $currentPath = array($id => $id); $this->analyzeCircularReferences($node->getOutEdges(), $checkedNodes, $currentPath); + if ($this->isSingleUsePrivateNode($node)) { + $this->singleUsePrivateIds[$id] = $id; + } } $this->container->getCompiler()->getServiceReferenceGraph()->clear(); @@ -526,7 +532,7 @@ private function addServiceInstance(string $id, Definition $definition, string $ $isProxyCandidate = $this->getProxyDumper()->isProxyCandidate($definition); $instantiation = ''; - if (!$isProxyCandidate && $definition->isShared()) { + if (!$isProxyCandidate && $definition->isShared() && !isset($this->singleUsePrivateIds[$id])) { $instantiation = sprintf('$this->%s[\'%s\'] = %s', $this->container->getDefinition($id)->isPublic() ? 'services' : 'privates', $id, $isSimpleInstance ? '' : '$instance'); } elseif (!$isSimpleInstance) { $instantiation = '$instance'; @@ -819,7 +825,7 @@ private function generateServiceFiles() $definitions = $this->container->getDefinitions(); ksort($definitions); foreach ($definitions as $id => $definition) { - if (!$definition->isSynthetic() && !$this->isHotPath($definition)) { + if (!$definition->isSynthetic() && !$this->isHotPath($definition) && ($definition->isPublic() || !$this->isTrivialInstance($definition))) { $code = $this->addService($id, $definition, $file); if (!$definition->isShared()) { @@ -1662,7 +1668,7 @@ private function getServiceCall(string $id, Reference $reference = null): string $code = 'null'; } elseif ($this->isTrivialInstance($definition)) { $code = substr($this->addNewInstance($definition, '', '', $id), 8, -2); - if ($definition->isShared()) { + if ($definition->isShared() && !isset($this->singleUsePrivateIds[$id])) { $code = sprintf('$this->%s[\'%s\'] = %s', $definition->isPublic() ? 'services' : 'privates', $id, $code); } } elseif ($this->asFiles && !$this->isHotPath($definition)) { @@ -1674,7 +1680,7 @@ private function getServiceCall(string $id, Reference $reference = null): string } else { $code = sprintf('$this->%s()', $this->generateMethodName($id)); } - if ($definition->isShared()) { + if ($definition->isShared() && !isset($this->singleUsePrivateIds[$id])) { $code = sprintf('($this->%s[\'%s\'] ?? %s)', $definition->isPublic() ? 'services' : 'privates', $id, $code); } @@ -1798,6 +1804,22 @@ private function isHotPath(Definition $definition) return $this->hotPathTag && $definition->hasTag($this->hotPathTag) && !$definition->isDeprecated(); } + private function isSingleUsePrivateNode(ServiceReferenceGraphNode $node): bool + { + if ($node->getValue()->isPublic()) { + return false; + } + $ids = array(); + foreach ($node->getInEdges() as $edge) { + if ($edge->isLazy() || !$edge->getSourceNode()->getValue()->isShared()) { + return false; + } + $ids[$edge->getSourceNode()->getId()] = true; + } + + return 1 === \count($ids); + } + private function export($value) { if (null !== $this->targetDirRegex && is_string($value) && preg_match($this->targetDirRegex, $value, $matches, PREG_OFFSET_CAPTURE)) { diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services9_as_files.txt b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services9_as_files.txt index 2b92c5838ba15..3a092f7306d70 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services9_as_files.txt +++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services9_as_files.txt @@ -155,7 +155,7 @@ use Symfony\Component\DependencyInjection\Exception\RuntimeException; // This file has been auto-generated by the Symfony Dependency Injection Component for internal use. // Returns the public 'factory_service_simple' shared service. -return $this->services['factory_service_simple'] = ($this->privates['factory_simple'] ?? $this->load('getFactorySimpleService.php'))->getInstance(); +return $this->services['factory_service_simple'] = $this->load('getFactorySimpleService.php')->getInstance(); [Container%s/getFactorySimpleService.php] => privates['factory_simple'] = new \SimpleFactoryClass('foo'); +return new \SimpleFactoryClass('foo'); [Container%s/getFooService.php] => services['runtime_error'] = new \stdClass(($this->privates['errored_definition'] ?? $this->load('getErroredDefinitionService.php'))); +return $this->services['runtime_error'] = new \stdClass($this->load('getErroredDefinitionService.php')); [Container%s/getServiceFromStaticMethodService.php] => services['tagged_iterator'] = new \Bar(new RewindableGenerator(fun yield 1 => ($this->privates['tagged_iterator_foo'] ?? $this->privates['tagged_iterator_foo'] = new \Bar()); }, 2)); - [Container%s/getTaggedIteratorFooService.php] => privates['tagged_iterator_foo'] = new \Bar(); - [Container%s/ProjectServiceContainer.php] => services['factory_service_simple'] = ($this->privates['factory_simple'] ?? $this->getFactorySimpleService())->getInstance(); + return $this->services['factory_service_simple'] = $this->getFactorySimpleService()->getInstance(); } /** @@ -381,7 +381,7 @@ protected function getNewFactoryServiceService() */ protected function getRuntimeErrorService() { - return $this->services['runtime_error'] = new \stdClass(($this->privates['errored_definition'] ?? $this->getErroredDefinitionService())); + return $this->services['runtime_error'] = new \stdClass($this->getErroredDefinitionService()); } /** @@ -428,7 +428,7 @@ protected function getFactorySimpleService() { @trigger_error('The "factory_simple" service is deprecated. You should stop using it, as it will soon be removed.', E_USER_DEPRECATED); - return $this->privates['factory_simple'] = new \SimpleFactoryClass('foo'); + return new \SimpleFactoryClass('foo'); } public function getParameter($name) diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services_errored_definition.php b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services_errored_definition.php index 34a38dfc40274..9ff22bf4d43d8 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services_errored_definition.php +++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services_errored_definition.php @@ -243,7 +243,7 @@ protected function getFactoryServiceService() */ protected function getFactoryServiceSimpleService() { - return $this->services['factory_service_simple'] = ($this->privates['factory_simple'] ?? $this->getFactorySimpleService())->getInstance(); + return $this->services['factory_service_simple'] = $this->getFactorySimpleService()->getInstance(); } /** @@ -381,7 +381,7 @@ protected function getNewFactoryServiceService() */ protected function getRuntimeErrorService() { - return $this->services['runtime_error'] = new \stdClass(($this->privates['errored_definition'] ?? $this->getErroredDefinitionService())); + return $this->services['runtime_error'] = new \stdClass($this->getErroredDefinitionService()); } /** @@ -428,7 +428,7 @@ protected function getFactorySimpleService() { @trigger_error('The "factory_simple" service is deprecated. You should stop using it, as it will soon be removed.', E_USER_DEPRECATED); - return $this->privates['factory_simple'] = new \SimpleFactoryClass('foo'); + return new \SimpleFactoryClass('foo'); } public function getParameter($name) diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services_private_in_expression.php b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services_private_in_expression.php index 5caf9104dd34d..2ae36458036d9 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services_private_in_expression.php +++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services_private_in_expression.php @@ -67,6 +67,6 @@ public function getRemovedIds() */ protected function getPublicFooService() { - return $this->services['public_foo'] = new \stdClass(($this->privates['private_foo'] ?? $this->privates['private_foo'] = new \stdClass())); + return $this->services['public_foo'] = new \stdClass(new \stdClass()); } }