diff --git a/src/Symfony/Component/OptionsResolver/Options.php b/src/Symfony/Component/OptionsResolver/Options.php index 1fb6057c30802..f1733524a9249 100644 --- a/src/Symfony/Component/OptionsResolver/Options.php +++ b/src/Symfony/Component/OptionsResolver/Options.php @@ -16,7 +16,10 @@ /** * Container for resolving inter-dependent options. * + * Once read, the options cannot be changed anymore. + * * @author Bernhard Schussek + * @author Tobias Schultze */ class Options implements \ArrayAccess, \Iterator, \Countable { @@ -32,18 +35,6 @@ class Options implements \ArrayAccess, \Iterator, \Countable */ private $normalizers = array(); - /** - * A list storing the names of all options that need to be normalized. - * @var array - */ - private $normalization = array(); - - /** - * A list storing the names of all LazyOption instances as keys. - * @var array - */ - private $lazy = array(); - /** * A list of Boolean locks for each LazyOption. * @var array @@ -125,10 +116,8 @@ public function setNormalizer($option, \Closure $normalizer) throw new OptionDefinitionException('Normalizers cannot be added anymore once options have been read.'); } - $this->normalizers[$option] = $normalizer; - // Each option for which a normalizer exists needs to be normalized - $this->normalization[$option] = true; + $this->normalizers[$option] = $normalizer; } /** @@ -192,19 +181,9 @@ public function overload($option, $value) if (isset($params[0]) && null !== ($class = $params[0]->getClass()) && __CLASS__ === $class->name) { $currentValue = isset($params[1]) && isset($this->options[$option]) ? $this->options[$option] : null; $value = new LazyOption($value, $currentValue); - - // Store which options are lazy for more efficient resolving - $this->lazy[$option] = true; - - $this->options[$option] = $value; - - return; } } - // Reset lazy flag and locks by default - unset($this->lazy[$option]); - $this->options[$option] = $value; } @@ -229,11 +208,11 @@ public function get($option) throw new \OutOfBoundsException('The option "' . $option . '" does not exist.'); } - if (isset($this->lazy[$option])) { + if ($this->options[$option] instanceof LazyOption) { $this->resolve($option); } - if (isset($this->normalization[$option])) { + if (isset($this->normalizers[$option])) { $this->normalize($option); } @@ -268,7 +247,6 @@ public function remove($option) } unset($this->options[$option]); - unset($this->lazy[$option]); } /** @@ -285,7 +263,6 @@ public function clear() } $this->options = array(); - $this->lazy = array(); } /** @@ -299,16 +276,19 @@ public function all() { $this->reading = true; - // Performance-wise this is slightly better than - // while (null !== $option = key($this->lazy)) - foreach ($this->lazy as $option => $isLazy) { - if (isset($this->lazy[$option])) { + foreach ($this->options as $option => $value) { + // We cannot use $value here because a lazy option might have + // invoked another lazy option and thus it might already be resolved + // during this loop. + if ($this->options[$option] instanceof LazyOption) { $this->resolve($option); } } - foreach ($this->normalization as $option => $normalize) { - if (isset($this->normalization[$option])) { + foreach ($this->normalizers as $option => $normalizer) { + // This test is necessary for the same reason as above (applied to + // normalizers). + if (isset($this->normalizers[$option])) { $this->normalize($option); } } @@ -443,28 +423,15 @@ public function count() */ private function resolve($option) { - if (isset($this->lock[$option])) { - $conflicts = array(); - - foreach ($this->lock as $option => $locked) { - if ($locked) { - $conflicts[] = $option; - } - } - - throw new OptionDefinitionException('The options "' . implode('", "', $conflicts) . '" have a cyclic dependency.'); - } + $this->checkCyclicDependency($option); $this->lock[$option] = true; $this->options[$option] = $this->options[$option]->evaluate($this); unset($this->lock[$option]); - - // The option now isn't lazy anymore - unset($this->lazy[$option]); } /** - * Normalizes the given option. + * Normalizes the given option. * * The evaluated value is written into the options array. * @@ -474,6 +441,28 @@ private function resolve($option) * on another option. */ private function normalize($option) + { + $this->checkCyclicDependency($option); + + /** @var \Closure $normalizer */ + $normalizer = $this->normalizers[$option]; + + $this->lock[$option] = true; + $this->options[$option] = $normalizer($this, $this->options[$option]); + unset($this->lock[$option]); + + // The option is now normalized + unset($this->normalizers[$option]); + } + + /** + * Checks if there is cyclic dependency for the option by examining the locks. + * + * @param string $option The option + * + * @throws OptionDefinitionException If there is a cyclic dependency for the option. + */ + private function checkCyclicDependency($option) { if (isset($this->lock[$option])) { $conflicts = array(); @@ -486,15 +475,5 @@ private function normalize($option) throw new OptionDefinitionException('The options "' . implode('", "', $conflicts) . '" have a cyclic dependency.'); } - - /** @var \Closure $normalizer */ - $normalizer = $this->normalizers[$option]; - - $this->lock[$option] = true; - $this->options[$option] = $normalizer($this, $this->options[$option]); - unset($this->lock[$option]); - - // The option is now normalized - unset($this->normalization[$option]); } } diff --git a/src/Symfony/Component/OptionsResolver/Tests/OptionsTest.php b/src/Symfony/Component/OptionsResolver/Tests/OptionsTest.php index f1cde68d15b46..ffb51b6d10956 100644 --- a/src/Symfony/Component/OptionsResolver/Tests/OptionsTest.php +++ b/src/Symfony/Component/OptionsResolver/Tests/OptionsTest.php @@ -93,7 +93,7 @@ public function testSetLazyOption() { $test = $this; - $this->options->set('foo', function (Options $options) use ($test) { + $this->options->set('foo', function (Options $options) { return 'dynamic'; }); @@ -166,7 +166,7 @@ public function testPreviousValueIsNotEvaluatedIfNoSecondArgument() }); // defined by subclass, no $previousValue argument defined! - $this->options->overload('foo', function (Options $options) use ($test) { + $this->options->overload('foo', function (Options $options) { return 'dynamic'; }); @@ -458,8 +458,58 @@ public function testOptionsIteration() { $this->options->set('foo', 'bar'); $this->options->set('foo1', 'bar1'); + $expectedResult = array('foo' => 'bar', 'foo1' => 'bar1'); $this->assertEquals($expectedResult, iterator_to_array($this->options, true)); } + + /** + * Calls the methods of the Options class many times. + * + * @group benchmark + */ + public function testPerformance() + { + $s = microtime(true); + + for ($n = 0; $n < 1000; $n++) { + for ($i = 0; $i < 50; $i++) { + $this->options->set('temp'.$i, 'foo'); + } + $this->options->clear(); + } + + for ($i = 0; $i < 5000; $i++) { + $this->options->overload('static', function (Options $options) { + return 'test'; + }); + $this->options->set('static', 'bar'); + $this->options->setNormalizer('static', function (Options $options) { + return 'BAR'; + }); + + $this->options->set('foo', 'bar'); + $this->options->remove('foo'); + + $opt = 'opt'.($i % 300); + $this->options->overload($opt, 'bar'); + $this->options->overload($opt, function (Options $options, $previousValue) { + return $previousValue . 'test'; + + }); + $this->options->setNormalizer($opt, function (Options $options, $previousValue) { + return $previousValue . $options->get('static'); + }); + } + + for ($i = 0; $i < 3000; $i++) { + $this->options->get('opt0'); + $this->options->all(); + $this->options->get('opt'.($i % 300)); + } + + $time = microtime(true) - $s; + $this->assertTrue($time < 1); + } }