Optimize circular collection by removing flattening

jderusse · jderusse · commit d3b951b767e7 · 2020-11-08T11:46:48.000+01:00
diff --git a/src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php b/src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php
@@ -406,64 +406,44 @@ private function analyzeReferences()
                 $this->singleUsePrivateIds[$id] = $id;
             }
 
-            $newNodes = [];
-            if (!$this->collectCircularReferences($id, $node->getOutEdges(), $checkedNodes, $newNodes)) {
-                foreach ($newNodes as $newNodeId => $_) {
-                    $checkedNodes[$newNodeId] = [];
-                }
-                continue;
-            }
-
-            $nodesToFlatten = $newNodes;
-            do {
-                $changedNodes = [];
-                foreach ($nodesToFlatten as $newNodeId => $_) {
-                    $deps = &$checkedNodes[$newNodeId];
-                    foreach ($deps as $id => [$path, $depsByConstructor]) {
-                        foreach ($checkedNodes[$id] as $depsId => [$subPath, $subDepsByConstructor]) {
-                            if (!isset($deps[$depsId]) || ($depsByConstructor && $subDepsByConstructor && !$deps[$depsId][1])) {
-                                array_unshift($subPath, $id);
-                                $deps[$depsId] = [$subPath, $depsByConstructor && $subDepsByConstructor];
-                                $changedNodes += $newNodes[$newNodeId] ?? [];
-                            }
-                        }
-                    }
-                }
-            } while ($nodesToFlatten = $changedNodes);
-
-            foreach ($newNodes as $newNodeId => $_) {
-                if (null !== $n = $checkedNodes[$newNodeId][$newNodeId] ?? null) {
-                    $this->addCircularReferences($newNodeId, $n[0], $n[1]);
-                }
+            $circular = $this->collectCircularReferences($id, $node->getOutEdges(), $checkedNodes);
+            foreach ($circular as [$loop, $byConstructor]) {
+                $sourceId = array_shift($loop);
+                $this->addCircularReferences($sourceId, $loop, $byConstructor);
             }
         }
 
         $this->container->getCompiler()->getServiceReferenceGraph()->clear();
         $this->singleUsePrivateIds = array_diff_key($this->singleUsePrivateIds, $this->circularReferences);
     }
 
-    private function collectCircularReferences(string $sourceId, array $edges, array &$checkedNodes, array &$newNodes, array $path = []): bool
+    private function collectCircularReferences(string $sourceId, array $edges, array &$checkedNodes, array $path = [], bool $byConstructor = true): array
     {
-        $path[$sourceId] = true;
-        $checkedNodes[$sourceId] = [];
-        $newNodes[$sourceId] = [];
-        $circular = false;
+        $path[$sourceId] = $byConstructor;
+        $checkedNodes[$sourceId] = true;
+        $circular = [];
         foreach ($edges as $edge) {
             $node = $edge->getDestNode();
             $id = $node->getId();
-            if (!$node->getValue() instanceof Definition || $sourceId === $id || $edge->isWeak()) {
+
+            if (!($definition = $node->getValue()) instanceof Definition || $sourceId === $id || ($edge->isLazy() && ($this->proxyDumper ?? $this->getProxyDumper())->isProxyCandidate($definition)) || $edge->isWeak()) {
                 continue;
             }
 
             if (isset($path[$id])) {
-                $circular = true;
+                $loop = null;
+                $loopByConstructor = $edge->isReferencedByConstructor();
+                foreach ($path as $k => $pathByConstructor) {
+                    if (null !== $loop) {
+                        $loop[] = $k;
+                        $loopByConstructor = $loopByConstructor && $pathByConstructor;
+                    } elseif ($k === $id) {
+                        $loop = [$k];
+                    }
+                }
+                $circular[] = [$loop, $loopByConstructor];
             } elseif (!isset($checkedNodes[$id])) {
-                $circular = $this->collectCircularReferences($id, $node->getOutEdges(), $checkedNodes, $newNodes, $path) || $circular;
-            }
-
-            $checkedNodes[$sourceId][$id] = [[], $edge->isReferencedByConstructor()];
-            if (isset($newNodes[$id])) {
-                $newNodes[$id][$sourceId] = true;
+                $circular = array_merge($circular, $this->collectCircularReferences($id, $node->getOutEdges(), $checkedNodes, $path, $edge->isReferencedByConstructor()));
             }
         }
         unset($path[$sourceId]);
diff --git a/src/Symfony/Component/DependencyInjection/Tests/Dumper/PhpDumperTest.php b/src/Symfony/Component/DependencyInjection/Tests/Dumper/PhpDumperTest.php
@@ -1070,6 +1070,10 @@ public function testAlmostCircular($visibility)
 
         $manager = $container->get('manager');
         $this->assertEquals(new \stdClass(), $manager);
+        $subscriber = $container->get('subscriber');
+        $this->assertEquals(new \stdClass(), $subscriber);
+        $connection = $container->get('connection');
+        $this->assertEquals(new \stdClass(), $connection);
 
         $manager = $container->get('manager2');
         $this->assertEquals(new \stdClass(), $manager);
