merged branch bschussek/setdata_performance (PR #5003)
Commits
-------
d4f4038
[Form] Reduced the number of setData() calls by deferring a Form's initialization (+40ms)
Discussion
----------
[Form] Reduced the number of setData() calls
Bug fix: no
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: -
Todo: -
This PR decreases the number of expensive `setData()` calls on `Form` instances by deferring the form's initialization with default data to the first call to a `get*Data()` method. If `setData()` is called manually before invoking `get*Data()`, the initialization with the default data will not take place.
Before:
```
$form = new Form($config); // implicit setData($config->getData());
$form->setData($object); // setData() is now called twice
```
After:
```
$form = new Form($config); // no implicit setData()
$form->getData(); // implicit setData($config->getData())
// or
$form = new Form($config);
$form->setData($object);
$form->getData(); // setData() was called only once
```
This commit is contained in:
commit
deb98d9163
@ -119,6 +119,26 @@ class Form implements \IteratorAggregate, FormInterface
|
|||||||
*/
|
*/
|
||||||
private $synchronized = true;
|
private $synchronized = true;
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Whether the form's data has been initialized.
|
||||||
|
*
|
||||||
|
* When the data is initialized with its default value, that default value
|
||||||
|
* is passed through the transformer chain in order to synchronize the
|
||||||
|
* model, normalized and view format for the first time. This is done
|
||||||
|
* lazily in order to save performance when {@link setData()} is called
|
||||||
|
* manually, making the initialization with the configured default value
|
||||||
|
* superfluous.
|
||||||
|
*
|
||||||
|
* @var Boolean
|
||||||
|
*/
|
||||||
|
private $initialized = false;
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Whether setData() is currently being called.
|
||||||
|
* @var Boolean
|
||||||
|
*/
|
||||||
|
private $lockSetData = false;
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Creates a new form based on the given configuration.
|
* Creates a new form based on the given configuration.
|
||||||
*
|
*
|
||||||
@ -138,8 +158,6 @@ class Form implements \IteratorAggregate, FormInterface
|
|||||||
}
|
}
|
||||||
|
|
||||||
$this->config = $config;
|
$this->config = $config;
|
||||||
|
|
||||||
$this->setData($config->getData());
|
|
||||||
}
|
}
|
||||||
|
|
||||||
public function __clone()
|
public function __clone()
|
||||||
@ -327,13 +345,16 @@ class Form implements \IteratorAggregate, FormInterface
|
|||||||
/**
|
/**
|
||||||
* Updates the form with default data.
|
* Updates the form with default data.
|
||||||
*
|
*
|
||||||
* @param array $modelData The data formatted as expected for the underlying object
|
* @param mixed $modelData The data formatted as expected for the underlying object
|
||||||
*
|
*
|
||||||
* @return Form The current form
|
* @return Form The current form
|
||||||
*/
|
*/
|
||||||
public function setData($modelData)
|
public function setData($modelData)
|
||||||
{
|
{
|
||||||
if ($this->bound) {
|
// If the form is bound while disabled, it is set to bound, but the data is not
|
||||||
|
// changed. In such cases (i.e. when the form is not initialized yet) don't
|
||||||
|
// abort this method.
|
||||||
|
if ($this->bound && $this->initialized) {
|
||||||
throw new AlreadyBoundException('You cannot change the data of a bound form');
|
throw new AlreadyBoundException('You cannot change the data of a bound form');
|
||||||
}
|
}
|
||||||
|
|
||||||
@ -346,6 +367,12 @@ class Form implements \IteratorAggregate, FormInterface
|
|||||||
$modelData = clone $modelData;
|
$modelData = clone $modelData;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if ($this->lockSetData) {
|
||||||
|
throw new FormException('A cycle was detected. Listeners to the PRE_SET_DATA event must not call setData(). You should call setData() on the FormEvent object instead.');
|
||||||
|
}
|
||||||
|
|
||||||
|
$this->lockSetData = true;
|
||||||
|
|
||||||
// Hook to change content of the data
|
// Hook to change content of the data
|
||||||
$event = new FormEvent($this, $modelData);
|
$event = new FormEvent($this, $modelData);
|
||||||
$this->config->getEventDispatcher()->dispatch(FormEvents::PRE_SET_DATA, $event);
|
$this->config->getEventDispatcher()->dispatch(FormEvents::PRE_SET_DATA, $event);
|
||||||
@ -395,8 +422,12 @@ class Form implements \IteratorAggregate, FormInterface
|
|||||||
$this->normData = $normData;
|
$this->normData = $normData;
|
||||||
$this->viewData = $viewData;
|
$this->viewData = $viewData;
|
||||||
$this->synchronized = true;
|
$this->synchronized = true;
|
||||||
|
$this->initialized = true;
|
||||||
|
$this->lockSetData = false;
|
||||||
|
|
||||||
if ($this->config->getCompound()) {
|
// It is not necessary to invoke this method if the form doesn't have children,
|
||||||
|
// even if the form is compound.
|
||||||
|
if (count($this->children) > 0) {
|
||||||
// Update child forms from the data
|
// Update child forms from the data
|
||||||
$this->config->getDataMapper()->mapDataToForms($viewData, $this->children);
|
$this->config->getDataMapper()->mapDataToForms($viewData, $this->children);
|
||||||
}
|
}
|
||||||
@ -414,9 +445,29 @@ class Form implements \IteratorAggregate, FormInterface
|
|||||||
*/
|
*/
|
||||||
public function getData()
|
public function getData()
|
||||||
{
|
{
|
||||||
|
if (!$this->initialized) {
|
||||||
|
$this->setData($this->config->getData());
|
||||||
|
}
|
||||||
|
|
||||||
return $this->modelData;
|
return $this->modelData;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Returns the normalized data of the form.
|
||||||
|
*
|
||||||
|
* @return mixed When the form is not bound, the default data is returned.
|
||||||
|
* When the form is bound, the normalized bound data is
|
||||||
|
* returned if the form is valid, null otherwise.
|
||||||
|
*/
|
||||||
|
public function getNormData()
|
||||||
|
{
|
||||||
|
if (!$this->initialized) {
|
||||||
|
$this->setData($this->config->getData());
|
||||||
|
}
|
||||||
|
|
||||||
|
return $this->normData;
|
||||||
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Returns the data transformed by the value transformer.
|
* Returns the data transformed by the value transformer.
|
||||||
*
|
*
|
||||||
@ -424,6 +475,10 @@ class Form implements \IteratorAggregate, FormInterface
|
|||||||
*/
|
*/
|
||||||
public function getViewData()
|
public function getViewData()
|
||||||
{
|
{
|
||||||
|
if (!$this->initialized) {
|
||||||
|
$this->setData($this->config->getData());
|
||||||
|
}
|
||||||
|
|
||||||
return $this->viewData;
|
return $this->viewData;
|
||||||
}
|
}
|
||||||
|
|
||||||
@ -543,7 +598,9 @@ class Form implements \IteratorAggregate, FormInterface
|
|||||||
}
|
}
|
||||||
|
|
||||||
// Merge form data from children into existing view data
|
// Merge form data from children into existing view data
|
||||||
if ($this->config->getCompound()) {
|
// It is not necessary to invoke this method if the form has no children,
|
||||||
|
// even if it is compound.
|
||||||
|
if (count($this->children) > 0) {
|
||||||
$this->config->getDataMapper()->mapFormsToData($this->children, $viewData);
|
$this->config->getDataMapper()->mapFormsToData($this->children, $viewData);
|
||||||
}
|
}
|
||||||
|
|
||||||
@ -573,6 +630,7 @@ class Form implements \IteratorAggregate, FormInterface
|
|||||||
$this->viewData = $viewData;
|
$this->viewData = $viewData;
|
||||||
$this->extraData = $extraData;
|
$this->extraData = $extraData;
|
||||||
$this->synchronized = $synchronized;
|
$this->synchronized = $synchronized;
|
||||||
|
$this->initialized = true;
|
||||||
|
|
||||||
$event = new FormEvent($this, $viewData);
|
$event = new FormEvent($this, $viewData);
|
||||||
$this->config->getEventDispatcher()->dispatch(FormEvents::POST_BIND, $event);
|
$this->config->getEventDispatcher()->dispatch(FormEvents::POST_BIND, $event);
|
||||||
@ -604,18 +662,6 @@ class Form implements \IteratorAggregate, FormInterface
|
|||||||
return $this->bind($request);
|
return $this->bind($request);
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
|
||||||
* Returns the normalized data of the form.
|
|
||||||
*
|
|
||||||
* @return mixed When the form is not bound, the default data is returned.
|
|
||||||
* When the form is bound, the normalized bound data is
|
|
||||||
* returned if the form is valid, null otherwise.
|
|
||||||
*/
|
|
||||||
public function getNormData()
|
|
||||||
{
|
|
||||||
return $this->normData;
|
|
||||||
}
|
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Adds an error to this form.
|
* Adds an error to this form.
|
||||||
*
|
*
|
||||||
@ -833,11 +879,32 @@ class Form implements \IteratorAggregate, FormInterface
|
|||||||
throw new FormException('You cannot add children to a simple form. Maybe you should set the option "compound" to true?');
|
throw new FormException('You cannot add children to a simple form. Maybe you should set the option "compound" to true?');
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Obtain the view data
|
||||||
|
$viewData = null;
|
||||||
|
|
||||||
|
// If setData() is currently being called, there is no need to call
|
||||||
|
// mapDataToForms() here, as mapDataToForms() is called at the end
|
||||||
|
// of setData() anyway. Not doing this check leads to an endless
|
||||||
|
// recursion when initializing the form lazily and an event listener
|
||||||
|
// (such as ResizeFormListener) adds fields depending on the data:
|
||||||
|
//
|
||||||
|
// * setData() is called, the form is not initialized yet
|
||||||
|
// * add() is called by the listener (setData() is not complete, so
|
||||||
|
// the form is still not initialized)
|
||||||
|
// * getViewData() is called
|
||||||
|
// * setData() is called since the form is not initialized yet
|
||||||
|
// * ... endless recursion ...
|
||||||
|
if (!$this->lockSetData) {
|
||||||
|
$viewData = $this->getViewData();
|
||||||
|
}
|
||||||
|
|
||||||
$this->children[$child->getName()] = $child;
|
$this->children[$child->getName()] = $child;
|
||||||
|
|
||||||
$child->setParent($this);
|
$child->setParent($this);
|
||||||
|
|
||||||
$this->config->getDataMapper()->mapDataToForms($this->getViewData(), array($child));
|
if (!$this->lockSetData) {
|
||||||
|
$this->config->getDataMapper()->mapDataToForms($viewData, array($child));
|
||||||
|
}
|
||||||
|
|
||||||
return $this;
|
return $this;
|
||||||
}
|
}
|
||||||
|
@ -20,7 +20,7 @@ use Symfony\Component\HttpFoundation\File\UploadedFile;
|
|||||||
use Symfony\Component\EventDispatcher\EventDispatcher;
|
use Symfony\Component\EventDispatcher\EventDispatcher;
|
||||||
use Symfony\Component\Form\Tests\Fixtures\FixedDataTransformer;
|
use Symfony\Component\Form\Tests\Fixtures\FixedDataTransformer;
|
||||||
|
|
||||||
class FormTest extends AbstractFormTest
|
class CompoundFormTest extends AbstractFormTest
|
||||||
{
|
{
|
||||||
public function testValidIfAllChildrenAreValid()
|
public function testValidIfAllChildrenAreValid()
|
||||||
{
|
{
|
||||||
|
@ -12,6 +12,8 @@
|
|||||||
namespace Symfony\Component\Form\Tests;
|
namespace Symfony\Component\Form\Tests;
|
||||||
|
|
||||||
use Symfony\Component\Form\Form;
|
use Symfony\Component\Form\Form;
|
||||||
|
use Symfony\Component\Form\FormEvent;
|
||||||
|
use Symfony\Component\Form\FormEvents;
|
||||||
use Symfony\Component\Form\Util\PropertyPath;
|
use Symfony\Component\Form\Util\PropertyPath;
|
||||||
use Symfony\Component\Form\FormConfig;
|
use Symfony\Component\Form\FormConfig;
|
||||||
use Symfony\Component\Form\FormView;
|
use Symfony\Component\Form\FormView;
|
||||||
@ -732,6 +734,21 @@ class SimpleFormTest extends AbstractFormTest
|
|||||||
$form->setData('foo');
|
$form->setData('foo');
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* @expectedException Symfony\Component\Form\Exception\FormException
|
||||||
|
*/
|
||||||
|
public function testSetDataCannotInvokeItself()
|
||||||
|
{
|
||||||
|
// Cycle detection to prevent endless loops
|
||||||
|
$config = new FormConfig('name', 'stdClass', $this->dispatcher);
|
||||||
|
$config->addEventListener(FormEvents::PRE_SET_DATA, function (FormEvent $event) {
|
||||||
|
$event->getForm()->setData('bar');
|
||||||
|
});
|
||||||
|
$form = new Form($config);
|
||||||
|
|
||||||
|
$form->setData('foo');
|
||||||
|
}
|
||||||
|
|
||||||
protected function createForm()
|
protected function createForm()
|
||||||
{
|
{
|
||||||
return $this->getBuilder()->getForm();
|
return $this->getBuilder()->getForm();
|
||||||
|
Reference in New Issue
Block a user