feature #23143 [DI] Reference instead of inline for array-params (nicolas-grekas)

This PR was merged into the 3.4 branch. Discussion ---------- [DI] Reference instead of inline for array-params | Q | A | ------------- | --- | Branch? | 3.4 | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | - - [x] Tests to be written. This PR is part of my "container on a diet" quest. When big array parameters are resolved, they create data duplication in the dumped container. This is even worse when the same big array parameters are used several times. Even though OPcache stores static arrays in shared memory (php7), it does not deduplicate them (it does for strings.) Instead of inlining arrays, this PR leverages the `$this->parameters` property when possible. Commits ------- 7c3d0c7a46 [DI] Reference instead of inline for array-params
2017-06-14 12:08:03 -07:00 · 2017-06-14 12:08:03 -07:00 · a03e19424b
commit a03e19424b
parent 2fe6e69528 7c3d0c7a46
10 changed files with 252 additions and 28 deletions
--- a/src/Symfony/Component/DependencyInjection/Compiler/PassConfig.php
+++ b/src/Symfony/Component/DependencyInjection/Compiler/PassConfig.php
@ -51,7 +51,7 @@ class PassConfig
            new ResolveDefinitionTemplatesPass(),
            new ServiceLocatorTagPass(),
            new DecoratorServicePass(),
-            new ResolveParameterPlaceHoldersPass(),
+            new ResolveParameterPlaceHoldersPass(false),
            new ResolveFactoryClassPass(),
            new FactoryReturnTypePass($resolveClassPass),
            new CheckDefinitionValidityPass(),
--- a/src/Symfony/Component/DependencyInjection/Compiler/ResolveParameterPlaceHoldersPass.php
+++ b/src/Symfony/Component/DependencyInjection/Compiler/ResolveParameterPlaceHoldersPass.php
@ -23,6 +23,12 @@ use Symfony\Component\DependencyInjection\Exception\ParameterNotFoundException;
 class ResolveParameterPlaceHoldersPass extends AbstractRecursivePass
 {
    private $bag;
+    private $resolveArrays;
+
+    public function __construct($resolveArrays = true)
+    {
+        $this->resolveArrays = $resolveArrays;
+    }

    /**
     * {@inheritdoc}
@ -55,7 +61,9 @@ class ResolveParameterPlaceHoldersPass extends AbstractRecursivePass
    protected function processValue($value, $isRoot = false)
    {
        if (is_string($value)) {
-            return $this->bag->resolveValue($value);
+            $v = $this->bag->resolveValue($value);
+
+            return $this->resolveArrays || !$v || !is_array($v) ? $v : $value;
        }
        if ($value instanceof Definition) {
            $changes = $value->getChanges();
--- a/src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php
+++ b/src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php
@ -1061,10 +1061,12 @@ EOF;
     */
    public function getParameter($name)
    {
-        $name = strtolower($name);
+        if (!(isset($this->parameters[$name]) || isset($this->loadedDynamicParameters[$name]) || array_key_exists($name, $this->parameters))) {
+            $name = strtolower($name);

-        if (!(isset($this->parameters[$name]) || array_key_exists($name, $this->parameters) || isset($this->loadedDynamicParameters[$name]))) {
-            throw new InvalidArgumentException(sprintf('The parameter "%s" must be defined.', $name));
+            if (!(isset($this->parameters[$name]) || isset($this->loadedDynamicParameters[$name]) || array_key_exists($name, $this->parameters))) {
+                throw new InvalidArgumentException(sprintf('The parameter "%s" must be defined.', $name));
+            }
        }
        if (isset($this->loadedDynamicParameters[$name])) {
            return $this->loadedDynamicParameters[$name] ? $this->dynamicParameters[$name] : $this->getDynamicParameter($name);
@ -1080,7 +1082,7 @@ EOF;
    {
        $name = strtolower($name);

-        return isset($this->parameters[$name]) || array_key_exists($name, $this->parameters) || isset($this->loadedDynamicParameters[$name]);
+        return isset($this->parameters[$name]) || isset($this->loadedDynamicParameters[$name]) || array_key_exists($name, $this->parameters);
    }

    /**
@ -1580,11 +1582,22 @@ EOF;
     */
    private function dumpParameter($name)
    {
+        $name = strtolower($name);
+
        if ($this->container->isCompiled() && $this->container->hasParameter($name)) {
-            return $this->dumpValue($this->container->getParameter($name), false);
+            $value = $this->container->getParameter($name);
+            $dumpedValue = $this->dumpValue($value, false);
+
+            if (!$value || !is_array($value)) {
+                return $dumpedValue;
+            }
+
+            if (!preg_match("/\\\$this->(?:getEnv\('\w++'\)|targetDirs\[\d++\])/", $dumpedValue)) {
+                return sprintf("\$this->parameters['%s']", $name);
+            }
        }

-        return sprintf("\$this->getParameter('%s')", strtolower($name));
+        return sprintf("\$this->getParameter('%s')", $name);
    }

    /**
--- a/src/Symfony/Component/DependencyInjection/Tests/Dumper/PhpDumperTest.php
+++ b/src/Symfony/Component/DependencyInjection/Tests/Dumper/PhpDumperTest.php
@ -584,4 +584,18 @@ class PhpDumperTest extends TestCase
        $container = new \Symfony_DI_PhpDumper_Test_Private_With_Ignore_On_Invalid_Reference();
        $this->assertInstanceOf('BazClass', $container->get('bar')->getBaz());
    }
+
+    public function testArrayParameters()
+    {
+        $container = new ContainerBuilder();
+        $container->setParameter('array_1', array(123));
+        $container->setParameter('array_2', array(__DIR__));
+        $container->register('bar', 'BarClass')
+            ->addMethodCall('setBaz', array('%array_1%', '%array_2%', '%%array_1%%'));
+        $container->compile();
+
+        $dumper = new PhpDumper($container);
+
+        $this->assertStringEqualsFile(self::$fixturesPath.'/php/services_array_params.php', str_replace('\\\\Dumper', '/Dumper', $dumper->dump(array('file' => self::$fixturesPath.'/php/services_array_params.php'))));
+    }
 }
--- a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services10.php
+++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services10.php
@ -80,10 +80,12 @@ class ProjectServiceContainer extends Container
     */
    public function getParameter($name)
    {
-        $name = strtolower($name);
+        if (!(isset($this->parameters[$name]) || isset($this->loadedDynamicParameters[$name]) || array_key_exists($name, $this->parameters))) {
+            $name = strtolower($name);

-        if (!(isset($this->parameters[$name]) || array_key_exists($name, $this->parameters) || isset($this->loadedDynamicParameters[$name]))) {
-            throw new InvalidArgumentException(sprintf('The parameter "%s" must be defined.', $name));
+            if (!(isset($this->parameters[$name]) || isset($this->loadedDynamicParameters[$name]) || array_key_exists($name, $this->parameters))) {
+                throw new InvalidArgumentException(sprintf('The parameter "%s" must be defined.', $name));
+            }
        }
        if (isset($this->loadedDynamicParameters[$name])) {
            return $this->loadedDynamicParameters[$name] ? $this->dynamicParameters[$name] : $this->getDynamicParameter($name);
@ -99,7 +101,7 @@ class ProjectServiceContainer extends Container
    {
        $name = strtolower($name);

-        return isset($this->parameters[$name]) || array_key_exists($name, $this->parameters) || isset($this->loadedDynamicParameters[$name]);
+        return isset($this->parameters[$name]) || isset($this->loadedDynamicParameters[$name]) || array_key_exists($name, $this->parameters);
    }

    /**
--- a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services12.php
+++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services12.php
@ -84,10 +84,12 @@ class ProjectServiceContainer extends Container
     */
    public function getParameter($name)
    {
-        $name = strtolower($name);
+        if (!(isset($this->parameters[$name]) || isset($this->loadedDynamicParameters[$name]) || array_key_exists($name, $this->parameters))) {
+            $name = strtolower($name);

-        if (!(isset($this->parameters[$name]) || array_key_exists($name, $this->parameters) || isset($this->loadedDynamicParameters[$name]))) {
-            throw new InvalidArgumentException(sprintf('The parameter "%s" must be defined.', $name));
+            if (!(isset($this->parameters[$name]) || isset($this->loadedDynamicParameters[$name]) || array_key_exists($name, $this->parameters))) {
+                throw new InvalidArgumentException(sprintf('The parameter "%s" must be defined.', $name));
+            }
        }
        if (isset($this->loadedDynamicParameters[$name])) {
            return $this->loadedDynamicParameters[$name] ? $this->dynamicParameters[$name] : $this->getDynamicParameter($name);
@ -103,7 +105,7 @@ class ProjectServiceContainer extends Container
    {
        $name = strtolower($name);

-        return isset($this->parameters[$name]) || array_key_exists($name, $this->parameters) || isset($this->loadedDynamicParameters[$name]);
+        return isset($this->parameters[$name]) || isset($this->loadedDynamicParameters[$name]) || array_key_exists($name, $this->parameters);
    }

    /**
--- a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services26.php
+++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services26.php
@ -82,10 +82,12 @@ class ProjectServiceContainer extends Container
     */
    public function getParameter($name)
    {
-        $name = strtolower($name);
+        if (!(isset($this->parameters[$name]) || isset($this->loadedDynamicParameters[$name]) || array_key_exists($name, $this->parameters))) {
+            $name = strtolower($name);

-        if (!(isset($this->parameters[$name]) || array_key_exists($name, $this->parameters) || isset($this->loadedDynamicParameters[$name]))) {
-            throw new InvalidArgumentException(sprintf('The parameter "%s" must be defined.', $name));
+            if (!(isset($this->parameters[$name]) || isset($this->loadedDynamicParameters[$name]) || array_key_exists($name, $this->parameters))) {
+                throw new InvalidArgumentException(sprintf('The parameter "%s" must be defined.', $name));
+            }
        }
        if (isset($this->loadedDynamicParameters[$name])) {
            return $this->loadedDynamicParameters[$name] ? $this->dynamicParameters[$name] : $this->getDynamicParameter($name);
@ -101,7 +103,7 @@ class ProjectServiceContainer extends Container
    {
        $name = strtolower($name);

-        return isset($this->parameters[$name]) || array_key_exists($name, $this->parameters) || isset($this->loadedDynamicParameters[$name]);
+        return isset($this->parameters[$name]) || isset($this->loadedDynamicParameters[$name]) || array_key_exists($name, $this->parameters);
    }

    /**
--- a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services8.php
+++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services8.php
@ -64,10 +64,12 @@ class ProjectServiceContainer extends Container
     */
    public function getParameter($name)
    {
-        $name = strtolower($name);
+        if (!(isset($this->parameters[$name]) || isset($this->loadedDynamicParameters[$name]) || array_key_exists($name, $this->parameters))) {
+            $name = strtolower($name);

-        if (!(isset($this->parameters[$name]) || array_key_exists($name, $this->parameters) || isset($this->loadedDynamicParameters[$name]))) {
-            throw new InvalidArgumentException(sprintf('The parameter "%s" must be defined.', $name));
+            if (!(isset($this->parameters[$name]) || isset($this->loadedDynamicParameters[$name]) || array_key_exists($name, $this->parameters))) {
+                throw new InvalidArgumentException(sprintf('The parameter "%s" must be defined.', $name));
+            }
        }
        if (isset($this->loadedDynamicParameters[$name])) {
            return $this->loadedDynamicParameters[$name] ? $this->dynamicParameters[$name] : $this->getDynamicParameter($name);
@ -83,7 +85,7 @@ class ProjectServiceContainer extends Container
    {
        $name = strtolower($name);

-        return isset($this->parameters[$name]) || array_key_exists($name, $this->parameters) || isset($this->loadedDynamicParameters[$name]);
+        return isset($this->parameters[$name]) || isset($this->loadedDynamicParameters[$name]) || array_key_exists($name, $this->parameters);
    }

    /**
--- a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services9_compiled.php
+++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services9_compiled.php
@ -391,10 +391,12 @@ class ProjectServiceContainer extends Container
     */
    public function getParameter($name)
    {
-        $name = strtolower($name);
+        if (!(isset($this->parameters[$name]) || isset($this->loadedDynamicParameters[$name]) || array_key_exists($name, $this->parameters))) {
+            $name = strtolower($name);

-        if (!(isset($this->parameters[$name]) || array_key_exists($name, $this->parameters) || isset($this->loadedDynamicParameters[$name]))) {
-            throw new InvalidArgumentException(sprintf('The parameter "%s" must be defined.', $name));
+            if (!(isset($this->parameters[$name]) || isset($this->loadedDynamicParameters[$name]) || array_key_exists($name, $this->parameters))) {
+                throw new InvalidArgumentException(sprintf('The parameter "%s" must be defined.', $name));
+            }
        }
        if (isset($this->loadedDynamicParameters[$name])) {
            return $this->loadedDynamicParameters[$name] ? $this->dynamicParameters[$name] : $this->getDynamicParameter($name);
@ -410,7 +412,7 @@ class ProjectServiceContainer extends Container
    {
        $name = strtolower($name);

-        return isset($this->parameters[$name]) || array_key_exists($name, $this->parameters) || isset($this->loadedDynamicParameters[$name]);
+        return isset($this->parameters[$name]) || isset($this->loadedDynamicParameters[$name]) || array_key_exists($name, $this->parameters);
    }

    /**
--- a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services_array_params.php
+++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services_array_params.php
@ -0,0 +1,179 @@
+<?php
+
+use Symfony\Component\DependencyInjection\Argument\RewindableGenerator;
+use Symfony\Component\DependencyInjection\ContainerInterface;
+use Symfony\Component\DependencyInjection\Container;
+use Symfony\Component\DependencyInjection\Exception\InvalidArgumentException;
+use Symfony\Component\DependencyInjection\Exception\LogicException;
+use Symfony\Component\DependencyInjection\Exception\RuntimeException;
+use Symfony\Component\DependencyInjection\ParameterBag\FrozenParameterBag;
+
+/**
+ * ProjectServiceContainer.
+ *
+ * This class has been auto-generated
+ * by the Symfony Dependency Injection Component.
+ *
+ * @final since Symfony 3.3
+ */
+class ProjectServiceContainer extends Container
+{
+    private $parameters;
+    private $targetDirs = array();
+
+    /**
+     * Constructor.
+     */
+    public function __construct()
+    {
+        $dir = __DIR__;
+        for ($i = 1; $i <= 5; ++$i) {
+            $this->targetDirs[$i] = $dir = dirname($dir);
+        }
+        $this->parameters = $this->getDefaultParameters();
+
+        $this->services = array();
+        $this->methodMap = array(
+            'bar' => 'getBarService',
+        );
+
+        $this->aliases = array();
+    }
+
+    /**
+     * {@inheritdoc}
+     */
+    public function compile()
+    {
+        throw new LogicException('You cannot compile a dumped container that was already compiled.');
+    }
+
+    /**
+     * {@inheritdoc}
+     */
+    public function isCompiled()
+    {
+        return true;
+    }
+
+    /**
+     * {@inheritdoc}
+     */
+    public function isFrozen()
+    {
+        @trigger_error(sprintf('The %s() method is deprecated since version 3.3 and will be removed in 4.0. Use the isCompiled() method instead.', __METHOD__), E_USER_DEPRECATED);
+
+        return true;
+    }
+
+    /**
+     * Gets the 'bar' service.
+     *
+     * This service is shared.
+     * This method always returns the same instance of the service.
+     *
+     * @return \BarClass A BarClass instance
+     */
+    protected function getBarService()
+    {
+        $this->services['bar'] = $instance = new \BarClass();
+
+        $instance->setBaz($this->parameters['array_1'], $this->getParameter('array_2'), '%array_1%');
+
+        return $instance;
+    }
+
+    /**
+     * {@inheritdoc}
+     */
+    public function getParameter($name)
+    {
+        if (!(isset($this->parameters[$name]) || isset($this->loadedDynamicParameters[$name]) || array_key_exists($name, $this->parameters))) {
+            $name = strtolower($name);
+
+            if (!(isset($this->parameters[$name]) || isset($this->loadedDynamicParameters[$name]) || array_key_exists($name, $this->parameters))) {
+                throw new InvalidArgumentException(sprintf('The parameter "%s" must be defined.', $name));
+            }
+        }
+        if (isset($this->loadedDynamicParameters[$name])) {
+            return $this->loadedDynamicParameters[$name] ? $this->dynamicParameters[$name] : $this->getDynamicParameter($name);
+        }
+
+        return $this->parameters[$name];
+    }
+
+    /**
+     * {@inheritdoc}
+     */
+    public function hasParameter($name)
+    {
+        $name = strtolower($name);
+
+        return isset($this->parameters[$name]) || isset($this->loadedDynamicParameters[$name]) || array_key_exists($name, $this->parameters);
+    }
+
+    /**
+     * {@inheritdoc}
+     */
+    public function setParameter($name, $value)
+    {
+        throw new LogicException('Impossible to call set() on a frozen ParameterBag.');
+    }
+
+    /**
+     * {@inheritdoc}
+     */
+    public function getParameterBag()
+    {
+        if (null === $this->parameterBag) {
+            $parameters = $this->parameters;
+            foreach ($this->loadedDynamicParameters as $name => $loaded) {
+                $parameters[$name] = $loaded ? $this->dynamicParameters[$name] : $this->getDynamicParameter($name);
+            }
+            $this->parameterBag = new FrozenParameterBag($parameters);
+        }
+
+        return $this->parameterBag;
+    }
+
+    private $loadedDynamicParameters = array(
+        'array_2' => false,
+    );
+    private $dynamicParameters = array();
+
+    /**
+     * Computes a dynamic parameter.
+     *
+     * @param string The name of the dynamic parameter to load
+     *
+     * @return mixed The value of the dynamic parameter
+     *
+     * @throws InvalidArgumentException When the dynamic parameter does not exist
+     */
+    private function getDynamicParameter($name)
+    {
+        switch ($name) {
+            case 'array_2': $value = array(
+                0 => ($this->targetDirs[2].'/Dumper'),
+            ); break;
+            default: throw new InvalidArgumentException(sprintf('The dynamic parameter "%s" must be defined.', $name));
+        }
+        $this->loadedDynamicParameters[$name] = true;
+
+        return $this->dynamicParameters[$name] = $value;
+    }
+
+    /**
+     * Gets the default parameters.
+     *
+     * @return array An array of the default parameters
+     */
+    protected function getDefaultParameters()
+    {
+        return array(
+            'array_1' => array(
+                0 => 123,
+            ),
+        );
+    }
+}