Skip to content

Commit

Permalink
minor #28737 [DependencyInjection] Optimize exporting variables (Sand…
Browse files Browse the repository at this point in the history
…er van der Vlugt)

This PR was squashed before being merged into the 4.2-dev branch (closes #28737).

Discussion
----------

[DependencyInjection] Optimize exporting variables

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

Store previously resolved variables so that the relatively expensive function [resolveEnvPlaceholders](https://github.com/symfony/symfony/blob/3e7b029524bf724abfd603c17795af39bb289615/src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php#L1776) is not called unnecessarily.

The effect on running ```bin/console``` with an empty cache in a big application (200+ env variables) in seconds:

| Before     | After      |
|------------|------------|
| 24.1       | 15.8       |
| 24.5       | 16.0       |
| 24.7       | 16.3       |
| 24.2       | 16.0       |
|  Avg: 24.4 | Avg:16.0   |

Commits
-------

768de2f [DependencyInjection] Optimize exporting variables
  • Loading branch information
nicolas-grekas committed Oct 23, 2018
2 parents 60394bc + 768de2f commit 717ff2d
Showing 1 changed file with 11 additions and 0 deletions.
11 changes: 11 additions & 0 deletions src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php
Expand Up @@ -80,6 +80,7 @@ class PhpDumper extends Dumper
private $addGetService = false;
private $locatedIds = array();
private $serviceLocatorTag;
private $exportedVariables = array();

/**
* @var ProxyDumper
Expand Down Expand Up @@ -125,6 +126,7 @@ public function dump(array $options = array())
$this->locatedIds = array();
$this->targetDirRegex = null;
$this->inlinedRequires = array();
$this->exportedVariables = array();
$options = array_merge(array(
'class' => 'ProjectServiceContainer',
'base_class' => 'Container',
Expand Down Expand Up @@ -304,6 +306,7 @@ public function dump(array $options = array())
$this->inlinedRequires = array();
$this->circularReferences = array();
$this->locatedIds = array();
$this->exportedVariables = array();

$unusedEnvs = array();
foreach ($this->container->getEnvCounters() as $env => $use) {
Expand Down Expand Up @@ -1768,6 +1771,10 @@ private function export($value)

private function doExport($value, $resolveEnv = false)
{
$shouldCacheValue = $resolveEnv && \is_string($value);
if ($shouldCacheValue && isset($this->exportedVariables[$value])) {
return $this->exportedVariables[$value];
}
if (\is_string($value) && false !== strpos($value, "\n")) {
$cleanParts = explode("\n", $value);
$cleanParts = array_map(function ($part) { return var_export($part, true); }, $cleanParts);
Expand All @@ -1789,6 +1796,10 @@ private function doExport($value, $resolveEnv = false)
}
}

if ($shouldCacheValue) {
$this->exportedVariables[$value] = $export;
}

return $export;
}
}

0 comments on commit 717ff2d

Please sign in to comment.