merged branch bschussek/normalizer-performance (PR #5067)
Commits -------d858f7b[OptionsResolver] Optimized previous values of a lazy option not to be evaluated if the second argument is not defined8a338cb[OptionsResolver] Micro-optimizatione659f0e[OptionsResolver] Improved the performance of normalizers Discussion ---------- [OptionsResolver] Improved the performance of normalizers Bug fix: no Feature addition: no Backwards compatibility break: no Symfony2 tests pass: yes Fixes the following tickets: - Todo: - Normalizers are now stored in the Options instance only once. Previously, normalizers were stored in Options upon resolving, which meant that they were added a lot of time if the same resolver was used for many different options arrays. This improvement led to an improvement of 30ms on http://advancedform.gpserver.dk/app_dev.php/taxclasses/1 --------------------------------------------------------------------------- by beberlei at 2012-07-26T13:34:23Z @bschussek do you have the code for this forms somewhere btw? --------------------------------------------------------------------------- by bschussek at 2012-07-26T13:54:52Z @beberlei https://github.com/stof/symfony-standard/tree/twig_forms
This commit is contained in:
Fabien Potencier
commit
77ce04d0b1
@ -26,6 +26,18 @@ class Options implements \ArrayAccess, \Iterator, \Countable
|
||||
*/
|
||||
private $options = array();
|
||||
|
||||
/**
|
||||
* A list of normalizer closures.
|
||||
* @var array
|
||||
*/
|
||||
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
|
||||
@ -87,6 +99,38 @@ class Options implements \ArrayAccess, \Iterator, \Countable
|
||||
$this->overload($option, $value);
|
||||
}
|
||||
|
||||
/**
|
||||
* Sets the normalizer for a given option.
|
||||
*
|
||||
* Normalizers should be closures with the following signature:
|
||||
*
|
||||
* <code>
|
||||
* function (Options $options, $value)
|
||||
* </code>
|
||||
*
|
||||
* This closure will be evaluated once the option is read using
|
||||
* {@link get()}. The closure has access to the resolved values of
|
||||
* other options through the passed {@link Options} instance.
|
||||
*
|
||||
* @param string $option The name of the option.
|
||||
* @param \Closure $normalizer The normalizer.
|
||||
*
|
||||
* @throws OptionDefinitionException If options have already been read.
|
||||
* Once options are read, the container
|
||||
* becomes immutable.
|
||||
*/
|
||||
public function setNormalizer($option, \Closure $normalizer)
|
||||
{
|
||||
if ($this->reading) {
|
||||
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;
|
||||
}
|
||||
|
||||
/**
|
||||
* Replaces the contents of the container with the given options.
|
||||
*
|
||||
@ -108,7 +152,7 @@ class Options implements \ArrayAccess, \Iterator, \Countable
|
||||
$this->options = array();
|
||||
|
||||
foreach ($options as $option => $value) {
|
||||
$this->set($option, $value);
|
||||
$this->overload($option, $value);
|
||||
}
|
||||
}
|
||||
|
||||
@ -146,12 +190,9 @@ class Options implements \ArrayAccess, \Iterator, \Countable
|
||||
$params = $reflClosure->getParameters();
|
||||
|
||||
if (isset($params[0]) && null !== ($class = $params[0]->getClass()) && __CLASS__ === $class->name) {
|
||||
$currentValue = isset($this->options[$option]) ? $this->options[$option] : null;
|
||||
$currentValue = isset($params[1]) && isset($this->options[$option]) ? $this->options[$option] : null;
|
||||
$value = new LazyOption($value, $currentValue);
|
||||
|
||||
// Store locks for lazy options to detect cyclic dependencies
|
||||
$this->lock[$option] = false;
|
||||
|
||||
// Store which options are lazy for more efficient resolving
|
||||
$this->lazy[$option] = true;
|
||||
|
||||
@ -162,7 +203,6 @@ class Options implements \ArrayAccess, \Iterator, \Countable
|
||||
}
|
||||
|
||||
// Reset lazy flag and locks by default
|
||||
unset($this->lock[$option]);
|
||||
unset($this->lazy[$option]);
|
||||
|
||||
$this->options[$option] = $value;
|
||||
@ -193,6 +233,10 @@ class Options implements \ArrayAccess, \Iterator, \Countable
|
||||
$this->resolve($option);
|
||||
}
|
||||
|
||||
if (isset($this->normalization[$option])) {
|
||||
$this->normalize($option);
|
||||
}
|
||||
|
||||
return $this->options[$option];
|
||||
}
|
||||
|
||||
@ -224,7 +268,6 @@ class Options implements \ArrayAccess, \Iterator, \Countable
|
||||
}
|
||||
|
||||
unset($this->options[$option]);
|
||||
unset($this->lock[$option]);
|
||||
unset($this->lazy[$option]);
|
||||
}
|
||||
|
||||
@ -242,7 +285,6 @@ class Options implements \ArrayAccess, \Iterator, \Countable
|
||||
}
|
||||
|
||||
$this->options = array();
|
||||
$this->lock = array();
|
||||
$this->lazy = array();
|
||||
}
|
||||
|
||||
@ -257,19 +299,20 @@ class Options implements \ArrayAccess, \Iterator, \Countable
|
||||
{
|
||||
$this->reading = true;
|
||||
|
||||
// Create a copy because resolve() modifies the array
|
||||
$lazy = $this->lazy;
|
||||
|
||||
// Performance-wise this is slightly better than
|
||||
// while (null !== $option = key($this->lazy))
|
||||
foreach ($lazy as $option => $isLazy) {
|
||||
// When resolve() is called, potentially multiple lazy options
|
||||
// are evaluated, so check again if the option is still lazy.
|
||||
foreach ($this->lazy as $option => $isLazy) {
|
||||
if (isset($this->lazy[$option])) {
|
||||
$this->resolve($option);
|
||||
}
|
||||
}
|
||||
|
||||
foreach ($this->normalization as $option => $normalize) {
|
||||
if (isset($this->normalization[$option])) {
|
||||
$this->normalize($option);
|
||||
}
|
||||
}
|
||||
|
||||
return $this->options;
|
||||
}
|
||||
|
||||
@ -388,7 +431,7 @@ class Options implements \ArrayAccess, \Iterator, \Countable
|
||||
}
|
||||
|
||||
/**
|
||||
* Evaluates the given option if it is a lazy option.
|
||||
* Evaluates the given lazy option.
|
||||
*
|
||||
* The evaluated value is written into the options array. The closure for
|
||||
* evaluating the option is discarded afterwards.
|
||||
@ -400,7 +443,7 @@ class Options implements \ArrayAccess, \Iterator, \Countable
|
||||
*/
|
||||
private function resolve($option)
|
||||
{
|
||||
if ($this->lock[$option]) {
|
||||
if (isset($this->lock[$option])) {
|
||||
$conflicts = array();
|
||||
|
||||
foreach ($this->lock as $option => $locked) {
|
||||
@ -414,9 +457,44 @@ class Options implements \ArrayAccess, \Iterator, \Countable
|
||||
|
||||
$this->lock[$option] = true;
|
||||
$this->options[$option] = $this->options[$option]->evaluate($this);
|
||||
$this->lock[$option] = false;
|
||||
unset($this->lock[$option]);
|
||||
|
||||
// The option now isn't lazy anymore
|
||||
unset($this->lazy[$option]);
|
||||
}
|
||||
|
||||
/**
|
||||
* Normalizes the given option.
|
||||
*
|
||||
* The evaluated value is written into the options array.
|
||||
*
|
||||
* @param string $option The option to normalizer.
|
||||
*
|
||||
* @throws OptionDefinitionException If the option has a cyclic dependency
|
||||
* on another option.
|
||||
*/
|
||||
private function normalize($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.');
|
||||
}
|
||||
|
||||
/** @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]);
|
||||
}
|
||||
}
|
||||
|
||||
@ -53,12 +53,6 @@ class OptionsResolver implements OptionsResolverInterface
|
||||
*/
|
||||
private $allowedTypes = array();
|
||||
|
||||
/**
|
||||
* A list of normalizers transforming each resolved options.
|
||||
* @var array
|
||||
*/
|
||||
private $normalizers = array();
|
||||
|
||||
/**
|
||||
* Creates a new instance.
|
||||
*/
|
||||
@ -194,7 +188,9 @@ class OptionsResolver implements OptionsResolverInterface
|
||||
{
|
||||
$this->validateOptionsExistence($normalizers);
|
||||
|
||||
$this->normalizers = array_replace($this->normalizers, $normalizers);
|
||||
foreach ($normalizers as $option => $normalizer) {
|
||||
$this->defaultOptions->setNormalizer($option, $normalizer);
|
||||
}
|
||||
|
||||
return $this;
|
||||
}
|
||||
@ -231,11 +227,6 @@ class OptionsResolver implements OptionsResolverInterface
|
||||
$combinedOptions->set($option, $value);
|
||||
}
|
||||
|
||||
// Apply filters
|
||||
foreach ($this->normalizers as $option => $filter) {
|
||||
$combinedOptions->overload($option, $filter);
|
||||
}
|
||||
|
||||
// Resolve options
|
||||
$resolvedOptions = $combinedOptions->all();
|
||||
|
||||
|
||||
@ -79,6 +79,16 @@ class OptionsTest extends \PHPUnit_Framework_TestCase
|
||||
$this->options->remove('foo');
|
||||
}
|
||||
|
||||
/**
|
||||
* @expectedException Symfony\Component\OptionsResolver\Exception\OptionDefinitionException
|
||||
*/
|
||||
public function testSetNormalizerNotSupportedAfterGet()
|
||||
{
|
||||
$this->options->set('foo', 'bar');
|
||||
$this->options->get('foo');
|
||||
$this->options->setNormalizer('foo', function () {});
|
||||
}
|
||||
|
||||
public function testSetLazyOption()
|
||||
{
|
||||
$test = $this;
|
||||
@ -146,6 +156,23 @@ class OptionsTest extends \PHPUnit_Framework_TestCase
|
||||
$this->assertEquals('dynamic', $this->options->get('foo'));
|
||||
}
|
||||
|
||||
public function testPreviousValueIsNotEvaluatedIfNoSecondArgument()
|
||||
{
|
||||
$test = $this;
|
||||
|
||||
// defined by superclass
|
||||
$this->options->set('foo', function (Options $options) use ($test) {
|
||||
$test->fail('Should not be called');
|
||||
});
|
||||
|
||||
// defined by subclass, no $previousValue argument defined!
|
||||
$this->options->overload('foo', function (Options $options) use ($test) {
|
||||
return 'dynamic';
|
||||
});
|
||||
|
||||
$this->assertEquals('dynamic', $this->options->get('foo'));
|
||||
}
|
||||
|
||||
public function testLazyOptionCanAccessOtherOptions()
|
||||
{
|
||||
$test = $this;
|
||||
@ -182,6 +209,66 @@ class OptionsTest extends \PHPUnit_Framework_TestCase
|
||||
$this->assertEquals('dynamic', $this->options->get('bam'));
|
||||
}
|
||||
|
||||
public function testNormalizer()
|
||||
{
|
||||
$this->options->set('foo', 'bar');
|
||||
|
||||
$this->options->setNormalizer('foo', function () {
|
||||
return 'normalized';
|
||||
});
|
||||
|
||||
$this->assertEquals('normalized', $this->options->get('foo'));
|
||||
}
|
||||
|
||||
public function testNormalizerReceivesUnnormalizedValue()
|
||||
{
|
||||
$this->options->set('foo', 'bar');
|
||||
|
||||
$this->options->setNormalizer('foo', function (Options $options, $value) {
|
||||
return 'normalized[' . $value . ']';
|
||||
});
|
||||
|
||||
$this->assertEquals('normalized[bar]', $this->options->get('foo'));
|
||||
}
|
||||
|
||||
public function testNormalizerCanAccessOtherOptions()
|
||||
{
|
||||
$test = $this;
|
||||
|
||||
$this->options->set('foo', 'bar');
|
||||
$this->options->set('bam', 'baz');
|
||||
|
||||
$this->options->setNormalizer('bam', function (Options $options) use ($test) {
|
||||
/* @var \PHPUnit_Framework_TestCase $test */
|
||||
$test->assertEquals('bar', $options->get('foo'));
|
||||
|
||||
return 'normalized';
|
||||
});
|
||||
|
||||
$this->assertEquals('bar', $this->options->get('foo'));
|
||||
$this->assertEquals('normalized', $this->options->get('bam'));
|
||||
}
|
||||
|
||||
public function testNormalizerCanAccessOtherLazyOptions()
|
||||
{
|
||||
$test = $this;
|
||||
|
||||
$this->options->set('foo', function (Options $options) {
|
||||
return 'bar';
|
||||
});
|
||||
$this->options->set('bam', 'baz');
|
||||
|
||||
$this->options->setNormalizer('bam', function (Options $options) use ($test) {
|
||||
/* @var \PHPUnit_Framework_TestCase $test */
|
||||
$test->assertEquals('bar', $options->get('foo'));
|
||||
|
||||
return 'normalized';
|
||||
});
|
||||
|
||||
$this->assertEquals('bar', $this->options->get('foo'));
|
||||
$this->assertEquals('normalized', $this->options->get('bam'));
|
||||
}
|
||||
|
||||
/**
|
||||
* @expectedException Symfony\Component\OptionsResolver\Exception\OptionDefinitionException
|
||||
*/
|
||||
@ -198,6 +285,85 @@ class OptionsTest extends \PHPUnit_Framework_TestCase
|
||||
$this->options->get('foo');
|
||||
}
|
||||
|
||||
/**
|
||||
* @expectedException Symfony\Component\OptionsResolver\Exception\OptionDefinitionException
|
||||
*/
|
||||
public function testFailForCyclicDependenciesBetweenNormalizers()
|
||||
{
|
||||
$this->options->set('foo', 'bar');
|
||||
$this->options->set('bam', 'baz');
|
||||
|
||||
$this->options->setNormalizer('foo', function (Options $options) {
|
||||
$options->get('bam');
|
||||
});
|
||||
|
||||
$this->options->setNormalizer('bam', function (Options $options) {
|
||||
$options->get('foo');
|
||||
});
|
||||
|
||||
$this->options->get('foo');
|
||||
}
|
||||
|
||||
/**
|
||||
* @expectedException Symfony\Component\OptionsResolver\Exception\OptionDefinitionException
|
||||
*/
|
||||
public function testFailForCyclicDependenciesBetweenNormalizerAndLazyOption()
|
||||
{
|
||||
$this->options->set('foo', function (Options $options) {
|
||||
$options->get('bam');
|
||||
});
|
||||
$this->options->set('bam', 'baz');
|
||||
|
||||
$this->options->setNormalizer('bam', function (Options $options) {
|
||||
$options->get('foo');
|
||||
});
|
||||
|
||||
$this->options->get('foo');
|
||||
}
|
||||
|
||||
public function testAllInvokesEachLazyOptionOnlyOnce()
|
||||
{
|
||||
$test = $this;
|
||||
$i = 1;
|
||||
|
||||
$this->options->set('foo', function (Options $options) use ($test, &$i) {
|
||||
$test->assertSame(1, $i);
|
||||
++$i;
|
||||
|
||||
// Implicitly invoke lazy option for "bam"
|
||||
$options->get('bam');
|
||||
});
|
||||
$this->options->set('bam', function (Options $options) use ($test, &$i) {
|
||||
$test->assertSame(2, $i);
|
||||
++$i;
|
||||
});
|
||||
|
||||
$this->options->all();
|
||||
}
|
||||
|
||||
public function testAllInvokesEachNormalizerOnlyOnce()
|
||||
{
|
||||
$test = $this;
|
||||
$i = 1;
|
||||
|
||||
$this->options->set('foo', 'bar');
|
||||
$this->options->set('bam', 'baz');
|
||||
|
||||
$this->options->setNormalizer('foo', function (Options $options) use ($test, &$i) {
|
||||
$test->assertSame(1, $i);
|
||||
++$i;
|
||||
|
||||
// Implicitly invoke normalizer for "bam"
|
||||
$options->get('bam');
|
||||
});
|
||||
$this->options->setNormalizer('bam', function (Options $options) use ($test, &$i) {
|
||||
$test->assertSame(2, $i);
|
||||
++$i;
|
||||
});
|
||||
|
||||
$this->options->all();
|
||||
}
|
||||
|
||||
public function testReplaceClearsAndSets()
|
||||
{
|
||||
$this->options->set('one', '1');
|
||||
|
||||
Reference in New Issue
Block a user