8000 bug #38882 [DependencyInjection] Improve performances in CircualRefer… · symfony/symfony@8375eee · GitHub
[go: up one dir, main page]
Skip to content

Commit 8375eee

Browse files
nicolas-grekasnicolas-grekas
committed
bug #38882 [DependencyInjection] Improve performances in CircualReference detection (jderusse)
This PR was merged into the 4.4 branch. Discussion ---------- [DependencyInjection] Improve performances in CircualReference detection | Q | A | ------------- | --- | Branch? | 4.4 | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | Fix #37850 | License | MIT | Doc PR | - This PR change the way circular references are detected. And improve the project submitted in #37850 [by **86%**](https://blackfire.io/profiles/compare/cc4aa41d-5b63-4caa-9a1b-fc282dc5d64a/graph) (Build the container in 1.1 sec instead of 10) Issue is: When a project contains a lot Circular Reference, the Dumper [spend a lot of time](https://blackfire.io/profiles/54f6eba2-b93c-4f2b-a268-1e58883faecb/graph) in merging those circular references. note: a circular reference is not an issue when an service is not injected by constructor, but this Can not be known until all references are resolved (performed previously by connectCircularReferences) This PR removed the connectCircularReferences and generate a flatten tree of dependencies: - the key is the service ID - the value is the list of direct **AND** indirect dependency + path to join the dependency I also [benched the PR with a project with few references](https://blackfire.io/profiles/compare/2f9902e6-3347-40b3-8421-e1fd09c067d2/graph) and result are almost the same before/after. Commits ------- d4db756 Improve performances in CircualReference detection
2 parents ed217a1 + d4db756 commit 8375eee

File tree

1 file changed

+71
-55
lines changed

1 file changed

+71
-55
lines changed

‎src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php

Lines changed: 71 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -180,25 +180,7 @@ public function dump(array $options = [])
180180
}
181181
}
182182

183-
(new AnalyzeServiceReferencesPass(false, !$this->getProxyDumper() instanceof NullDumper))->process($this->container);
184-
$checkedNodes = [];
185-
$this->circularReferences = [];
186-
$this->singleUsePrivateIds = [];
187-
foreach ($this->container->getCompiler()->getServiceReferenceGraph()->getNodes() as $id => $node) {
188-
if (!$node->getValue() instanceof Definition) {
189-
continue;
190-
}
191-
if (!isset($checkedNodes[$id])) {
192-
$this->analyzeCircularReferences($id, $node->getOutEdges(), $checkedNodes);
193-
}
194-
if ($this->isSingleUsePrivateNode($node)) {
195-
$this->singleUsePrivateIds[$id] = $id;
196-
}
197-
}
198-
$this->container->getCompiler()->getServiceReferenceGraph()->clear();
199-
$checkedNodes = [];
200-
$this->singleUsePrivateIds = array_diff_key($this->singleUsePrivateIds, $this->circularReferences);
201-
183+
$this->analyzeReferences();
202184
$this->docStar = $options['debug'] ? '*' : '';
203185

204186
if (!empty($options['file']) && is_dir($dir = \dirname($options['file']))) {
@@ -409,58 +391,92 @@ private function getProxyDumper(): ProxyDumper
409391
return $this->proxyDumper;
410392
}
411393

412-
private function analyzeCircularReferences(string $sourceId, array $edges, array &$checkedNodes, array &$currentPath = [], bool $byConstructor = true)
394+
private function analyzeReferences()
413395
{
414-
$checkedNodes[$sourceId] = true;
415-
$currentPath[$sourceId] = $byConstructor;
396+
(new AnalyzeServiceReferencesPass(false, !$this->getProxyDumper() instanceof NullDumper))->process($this->container);
397+
$checkedNodes = [];
398+
$this->circularReferences = [];
399+
$this->singleUsePrivateIds = [];
400+
foreach ($this->container->getCompiler()->getServiceReferenceGraph()->getNodes() as $id => $node) {
401+
if (!$node->getValue() instanceof Definition) {
402+
continue;
403+
}
416404

417-
foreach ($edges as $edge) {
418-
$node = $edge->getDestNode();
419-
$id = $node->getId();
405+
if ($this->isSingleUsePrivateNode($node)) {
406+
$this->singleUsePrivateIds[$id] = $id;
407+
}
420408

421-
if (!$node->getValue() instanceof Definition || $sourceId === $id || $edge->isLazy() || $edge->isWeak()) {
422-
// no-op
423-
} elseif (isset($currentPath[$id])) {
424-
$this->addCircularReferences($id, $currentPath, $edge->isReferencedByConstructor());
425-
} elseif (!isset($checkedNodes[$id])) {
426-
$this->analyzeCircularReferences($id, $node->getOutEdges(), $checkedNodes, $currentPath, $edge->isReferencedByConstructor());
427-
} elseif (isset($this->circularReferences[$id])) {
428-
$this->connectCircularReferences($id, $currentPath, $edge->isReferencedByConstructor());
409+
$newNodes = [];
410+
if (!$this->collectCircularReferences($id, $node->getOutEdges(), $checkedNodes, $newNodes)) {
411+
foreach ($newNodes as $newNodeId => $_) {
412+
$checkedNodes[$newNodeId] = [];
413+
}
414+
continue;
429415
}
430-
}
431-
unset($currentPath[$sourceId]);
432-
}
433416

434-
private function connectCircularReferences(string $sourceId, array &$currentPath, bool $byConstructor, array &$subPath = [])
435-
{
436-
$currentPath[$sourceId] = $subPath[$sourceId] = $byConstructor;
417+
$nodesToFlatten = $newNodes;
418+
do {
419+
$changedNodes = [];
420+
foreach ($nodesToFlatten as $newNodeId => $_) {
421+
$deps = &$checkedNodes[$newNodeId];
422+
foreach ($deps as $id => [$path, $depsByConstructor]) {
423+
foreach ($checkedNodes[$id] as $depsId => [$subPath, $subDepsByConstructor]) {
424+
if (!isset($deps[$depsId]) || ($depsByConstructor && $subDepsByConstructor && !$deps[$depsId][1])) {
425+
array_unshift($subPath, $id);
426+
$deps[$depsId] = [$subPath, $depsByConstructor && $subDepsByConstructor];
427+
$changedNodes += $newNodes[$newNodeId] ?? [];
428+
}
429+
}
430+
}
431+
}
432+
} while ($nodesToFlatten = $changedNodes);
437433

438-
foreach ($this->circularReferences[$sourceId] as $id => $byConstructor) {
439-
if (isset($currentPath[$id])) {
440-
$this->addCircularReferences($id, $currentPath, $byConstructor);
441-
} elseif (!isset($subPath[$id]) && isset($this->circularReferences[$id])) {
442-
$this->connectCircularReferences($id, $currentPath, $byConstructor, $subPath);
434+
foreach ($newNodes as $newNodeId => $_) {
435+
if (null !== $n = $checkedNodes[$newNodeId][$newNodeId] ?? null) {
436+
$this->addCircularReferences($newNodeId, $n[0], $n[1]);
437+
}
443438
}
444439
}
445-
unset($currentPath[$sourceId], $subPath[$sourceId]);
440+
441+
$this->container->getCompiler()->getServiceReferenceGraph()->clear();
442+
$this->singleUsePrivateIds = array_diff_key($this->singleUsePrivateIds, $this->circularReferences);
446443
}
447444

448-
private function addCircularReferences(string $id, array $currentPath, bool $byConstructor)
445+
private function collectCircularReferences(string $sourceId, array $edges, array &$checkedNodes, array &$newNodes, array $path = []): bool
449446
{
450-
$currentPath[$id] = $byConstructor;
451-
$circularRefs = [];
447+
$path[$sourceId] = true;
448+
$checkedNodes[$sourceId] = [];
449+
$newNodes[$sourceId] = [];
450+
$circular = false;
451+
foreach ($edges as $edge) {
452+
$node = $edge->getDestNode();
453+
$id = $node->getId();
454+
if (!$node->getValue() instanceof Definition || $sourceId === $id || $edge->isLazy() || $edge->isWeak()) {
455+
continue;
456+
}
452457

453-
foreach (array_reverse($currentPath) as $parentId => $v) {
454-
$byConstructor = $byConstructor && $v;
455-
$circularRefs[] = $parentId;
458+
if (isset($path[$id])) {
459+
$circular = true;
460+
} elseif (!isset($checkedNodes[$id])) {
461+
$circular = $this->collectCircularReferences($id, $node->getOutEdges(), $checkedNodes, $newNodes, $path) || $circular;
462+
}
456463

457-
if ($parentId === $id) {
458-
break;
464+
$checkedNodes[$sourceId][$id] = [[], $edge->isReferencedByConstructor()];
465+
if (isset($newNodes[$id])) {
466+
$newNodes[$id][$sourceId] = true;
459467
}
460468
}
469+
unset($path[$sourceId]);
461470

462-
$currentId = $id;
463-
foreach ($circularRefs as $parentId) {
471+
return $circular;
472+
}
473+
474+
private function addCircularReferences(string $sourceId, array $currentPath, bool $byConstructor)
475+
{
476+
$currentId = $sourceId;
477+
$currentPath = array_reverse($currentPath);
478+
$currentPath[] = $currentId;
479+
foreach ($currentPath as $parentId) {
464480
if (empty($this->circularReferences[$parentId][$currentId])) {
465481
$this->circularReferences[$parentId][$currentId] = $byConstructor;
466482
}

0 commit comments

Comments
 (0)
0