feature #27268 [DI] fine tune dumped factories (nicolas-grekas)

This PR was merged into the 4.2-dev branch.

Discussion
----------

[DI] fine tune dumped factories

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | -
| License       | MIT
| Doc PR        | -

Here is an optimization we forgot in the dumped container: when a private service is referenced once, there is no need to keep it in the internal registry as it will never be reused. This should help a bit the garbage collection process.

Commits
-------

88ecd0dc9a [DI] fine tune dumped factories

Author: fabpot

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)) {

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] => <?php
 
@@ -167,7 +167,7 @@ use Symfony\Component\DependencyInjection\Exception\RuntimeException;
 
 @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');
 
     [Container%s/getFooService.php] => <?php
 
@@ -326,7 +326,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 'runtime_error' shared service.
 
-return $this->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] => <?php
 
@@ -351,16 +351,6 @@ return $this->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] => <?php
-
-use Symfony\Component\DependencyInjection\Argument\RewindableGenerator;
-use Symfony\Component\DependencyInjection\Exception\RuntimeException;
-
-// This file has been auto-generated by the Symfony Dependency Injection Component for internal use.
-// Returns the private 'tagged_iterator_foo' shared service.
-
-return $this->privates['tagged_iterator_foo'] = new \Bar();
-
     [Container%s/ProjectServiceContainer.php] => <?php
 
 namespace Container%s;

Tests/Fixtures/php/services9_compiled.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)

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)

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());
     }
 }