[DI] Reference instead of inline for array-params #23143
Conversation
|
couldn't this cause issue if we keep some parameters unresolved in the container ? Are we already unescaping string values containing a |
f01b2df to
cf755be
Compare
|
Tests added, PR is RFR. |
cf755be to
48c6631
Compare
| @@ -80,10 +80,12 @@ protected function getTestService() | |||
| */ | |||
| 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); | |||
There was a problem hiding this comment.
µoptim here: conditionally call strtolower. This is a bit more important now that this will use getParameter internally a few more times.
There was a problem hiding this comment.
should we deprecate the case insensitivity of parameter names ? This would be consistent with service ids.
There was a problem hiding this comment.
+1,000 !!! We should stop changing the user config "magically" behind the scenes ... and that includes service IDs, param IDs, etc. ... unless there are strong technical reasons to do so. Thanks!
There was a problem hiding this comment.
we may, in another PR :)
96a8c32 to
ee9ebc2
Compare
|
@nicolas-grekas if I have an argument configured as |
ee9ebc2 to
1ef5105
Compare
|
@stof test added, looks all good (the PhpDumper handles that logic) |
| $container->setParameter('array_1', array(123)); | ||
| $container->setParameter('array_2', array(__DIR__)); | ||
| $container->register('bar', 'BarClass') | ||
| ->addMethodCall('setBaz', array('%array_1%', '%array_2%', '%%string%%')); |
There was a problem hiding this comment.
please make the content between the %% match the name of an existing parameter though (to ensure that working properly is not just due to the hasParameter check)
1ef5105 to
7c3d0c7
Compare
|
Thank you @nicolas-grekas. |
…olas-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 ------- 7c3d0c7 [DI] Reference instead of inline for array-params
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->parametersproperty when possible.