Skip to content

Commit a27aeda

Browse files
fabpotfabpot
committed
merged branch bschussek/performance (PR #4882)
Commits ------- cd7835d [Form] Cached the form type hierarchy in order to improve performance 2ca753b [Form] Fixed choice list hashing in DoctrineType 2bf4d6c [Form] Fixed FormFactory not to set "data" option if not explicitely given 7149d26 [Form] Removed invalid PHPDoc text Discussion ---------- [Form] WIP Improved performance of form building Bug fix: no Feature addition: no Backwards compatibility break: no Symfony2 tests pass: yes Fixes the following tickets: - Todo: **Update the Silex extension** This PR is work in progress and up for discussion. It increases the performance of FormFactory::createForm() on a specific, heavy-weight form from **0.848** to **0.580** seconds. Before, the FormFactory had to traverse the hierarchy and calculate the default options of each FormType everytime a form was created of that type. Now, FormTypes are wrapped within instances of a new class `ResolvedFormType`, which caches the parent type, the type's extensions and its default options. The updated responsibilities: `FormFactory` is a registry and proxy for `ResolvedFormType` objects, `FormType` specifies how a form can be built on a specific layer of the type hierarchy (e.g. "form", or "date", etc.) and `ResolvedFormType` *does the actual building* across all layers of the hierarchy (by delegating to the parent type, which delegates to its parent type etc.). --------------------------------------------------------------------------- by schmittjoh at 2012-07-12T18:25:40Z Maybe ResolvedFormType --------------------------------------------------------------------------- by jmather at 2012-07-13T02:56:38Z I really like ResolvedFormType. That's the naming method I took for my tag parser that handes the same conceptual issue. --------------------------------------------------------------------------- by axelarge at 2012-07-13T05:25:00Z ResolvedFormType sounds very clear. This change is great and I desperately hope to see more of this kind --------------------------------------------------------------------------- by Baachi at 2012-07-13T06:41:26Z Yes `ResolvedFormType` sounds good :) :+1: --------------------------------------------------------------------------- by fabpot at 2012-07-13T07:11:33Z I like `ResolvedFormType` as well. --------------------------------------------------------------------------- by henrikbjorn at 2012-07-13T07:46:48Z :+1: `ResolvedFormType` :shipit: --------------------------------------------------------------------------- by stof at 2012-07-13T18:01:51Z This looks good to me
2 parents 2dcc448 + cd7835d commit a27aeda

33 files changed

+1545
-756
lines changed

‎UPGRADE-2.1.md

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -249,6 +249,16 @@
249249
public function finishView(FormViewInterface $view, FormInterface $form, array $options)
250250
```
251251
252+
* The method `createBuilder` was removed from `FormTypeInterface` for performance
253+
reasons. It is now not possible anymore to use custom implementations of
254+
`FormBuilderInterface` for specific form types.
255+
256+
If you are in such a situation, you can subclass `FormRegistry` instead and override
257+
`resolveType` to return a custom `ResolvedFormTypeInterface` implementation, within
258+
which you can create your own `FormBuilderInterface` implementation. You should
259+
register this custom registry class under the service name "form.registry" in order
260+
to replace the default implementation.
261+
252262
* If you previously inherited from `FieldType`, you should now inherit from
253263
`FormType`. You should also set the option `compound` to `false` if your field
254264
is not supposed to contain child fields.
@@ -1001,6 +1011,24 @@
10011011
));
10021012
```
10031013
1014+
* The methods `addType`, `hasType` and `getType` in `FormFactory` are deprecated
1015+
and will be removed in Symfony 2.3. You should use the methods with the same
1016+
name on the `FormRegistry` instead.
1017+
1018+
Before:
1019+
1020+
```
1021+
$this->get('form.factory')->addType(new MyFormType());
1022+
```
1023+
1024+
After:
1025+
1026+
```
1027+
$registry = $this->get('form.registry');
1028+
1029+
$registry->addType($registry->resolveType(new MyFormType()));
1030+
```
1031+
10041032
### Validator
10051033
10061034
* The methods `setMessage()`, `getMessageTemplate()` and

‎src/Symfony/Bridge/Doctrine/CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,3 +7,4 @@ CHANGELOG
77
* added a default implementation of the ManagerRegistry
88
* added a session storage for Doctrine DBAL
99
* DoctrineOrmTypeGuesser now guesses "collection" for array Doctrine type
10+
* DoctrineType now caches its choice lists in order to improve performance

‎src/Symfony/Bridge/Doctrine/Form/Type/DoctrineType.php

Lines changed: 29 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -68,17 +68,40 @@ public function setDefaultOptions(OptionsResolverInterface $resolver)
6868
$choiceList = function (Options $options) use ($registry, &$choiceListCache, &$time) {
6969
$manager = $registry->getManager($options['em']);
7070

71-
$choiceHashes = is_array($options['choices'])
72-
? array_map('spl_object_hash', $options['choices'])
73-
: $options['choices'];
71+
// Support for closures
72+
$propertyHash = is_object($options['property'])
73+
? spl_object_hash($options['property'])
74+
: $options['property'];
75+
76+
$choiceHashes = $options['choices'];
77+
78+
// Support for recursive arrays
79+
if (is_array($choiceHashes)) {
80+
// A second parameter ($key) is passed, so we cannot use
81+
// spl_object_hash() directly (which strictly requires
82+
// one parameter)
83+
array_walk_recursive($choiceHashes, function ($value) {
84+
return spl_object_hash($value);
85+
});
86+
}
87+
88+
// Support for custom loaders (with query builders)
89+
$loaderHash = is_object($options['loader'])
90+
? spl_object_hash($options['loader'])
91+
: $options['loader'];
92+
93+
// Support for closures
94+
$groupByHash = is_object($options['group_by'])
95+
? spl_object_hash($options['group_by'])
96+
: $options['group_by'];
7497

7598
$hash = md5(json_encode(array(
7699
spl_object_hash($manager),
77100
$options['class'],
78-
$options['property'],
79-
$options['loader'],
101+
$propertyHash,
102+
$loaderHash,
80103
$choiceHashes,
81-
$options['group_by']
104+
$groupByHash
82105
)));
83106

84107
if (!isset($choiceListCache[$hash])) {

‎src/Symfony/Bundle/FrameworkBundle/Resources/config/form.xml

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,13 +6,14 @@
66

77
<parameters>
88
<parameter key="form.extension.class">Symfony\Component\Form\Extension\DependencyInjection\DependencyInjectionExtension</parameter>
9+
<parameter key="form.registry.class">Symfony\Component\Form\FormRegistry</parameter>
910
<parameter key="form.factory.class">Symfony\Component\Form\FormFactory</parameter>
1011
<parameter key="form.type_guesser.validator.class">Symfony\Component\Form\Extension\Validator\ValidatorTypeGuesser</parameter>
1112
</parameters>
1213

1314
<services>
14-
<!-- FormFactory -->
15-
<service id="form.factory" class="%form.factory.class%">
15+
<!-- FormRegistry -->
16+
<service id="form.registry" class="%form.registry.class%">
1617
<argument type="collection">
1718
<!--
1819
We don't need to be able to add more extensions.
@@ -23,6 +24,11 @@
2324
</argument>
2425
</service>
2526

27+
<!-- FormFactory -->
28+
<service id="form.factory" class="%form.factory.class%">
29+
<argument type="service" id="form.registry" />
30+
</service>
31+
2632
<!-- DependencyInjectionExtension -->
2733
<service id="form.extension" class="%form.extension.class%" public="false">
2834
<argument type="service" id="service_container" />

‎src/Symfony/Component/Form/AbstractType.php

Lines changed: 16 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,9 @@
2020
abstract class AbstractType implements FormTypeInterface
2121
{
2222
/**
23-
* The extensions for this type
24-
* @var array An array of FormTypeExtensionInterface instances
23+
* @var array
24+
*
25+
* @deprecated Deprecated since version 2.1, to be removed in 2.3.
2526
*/
2627
private $extensions = array();
2728

@@ -46,14 +47,6 @@ public function finishView(FormViewInterface $view, FormInterface $form, array $
4647
{
4748
}
4849

49-
/**
50-
* {@inheritdoc}
51-
*/
52-
public function createBuilder($name, FormFactoryInterface $factory, array $options)
53-
{
54-
return null;
55-
}
56-
5750
/**
5851
* {@inheritdoc}
5952
*/
@@ -98,21 +91,26 @@ public function getParent()
9891
}
9992

10093
/**
101-
* {@inheritdoc}
94+
* Sets the extensions for this type.
95+
*
96+
* @param array $extensions An array of FormTypeExtensionInterface
97+
*
98+
* @throws Exception\UnexpectedTypeException if any extension does not implement FormTypeExtensionInterface
99+
*
100+
* @deprecated Deprecated since version 2.1, to be removed in 2.3.
102101
*/
103102
public function setExtensions(array $extensions)
104103
{
105-
foreach ($extensions as $extension) {
106-
if (!$extension instanceof FormTypeExtensionInterface) {
107-
throw new UnexpectedTypeException($extension, 'Symfony\Component\Form\FormTypeExtensionInterface');
108-
}
109-
}
110-
111104
$this->extensions = $extensions;
112105
}
113106

114107
/**
115-
* {@inheritdoc}
108+
* Returns the extensions associated with this type.
109+
*
110+
* @return array An array of FormTypeExtensionInterface
111+
*
112+
* @deprecated Deprecated since version 2.1, to be removed in 2.3. Use
113+
* {@link ResolvedFormTypeInterface::getTypeExtensions()} instead.
116114
*/
117115
public function getExtensions()
118116
{

‎src/Symfony/Component/Form/CHANGELOG.md

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,7 @@ CHANGELOG
8686
* `hasAttribute`
8787
* `getClientData`
8888
* added FormBuilder methods
89+
* `getTypes`
8990
* `addViewTransformer`
9091
* `getViewTransformers`
9192
* `resetViewTransformers`
@@ -157,3 +158,15 @@ CHANGELOG
157158
* deprecated the options "data_timezone" and "user_timezone" in DateType, DateTimeType and TimeType
158159
and renamed them to "model_timezone" and "view_timezone"
159160
* fixed: TransformationFailedExceptions thrown in the model transformer are now caught by the form
161+
* added FormRegistry and ResolvedFormTypeInterface
162+
* deprecated FormFactory methods
163+
* `addType`
164+
* `hasType`
165+
* `getType`
166+
* [BC BREAK] FormFactory now expects a FormRegistryInterface as constructor argument
167+
* [BC BREAK] The method `createBuilder` in FormTypeInterface is not supported anymore for performance reasons
168+
* [BC BREAK] Removed `setTypes` from FormBuilder
169+
* deprecated AbstractType methods
170+
* `getExtensions`
171+
* `setExtensions`
172+
* ChoiceType now caches its created choice lists to improve performance

‎src/Symfony/Component/Form/Extension/Core/Type/FormType.php

Lines changed: 3 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@
2020
use Symfony\Component\Form\Extension\Core\EventListener\BindRequestListener;
2121
use Symfony\Component\Form\Extension\Core\EventListener\TrimListener;
2222
use Symfony\Component\Form\Extension\Core\DataMapper\PropertyPathMapper;
23-
use Symfony\Component\EventDispatcher\EventDispatcher;
2423
use Symfony\Component\Form\Exception\FormException;
2524
use Symfony\Component\OptionsResolver\Options;
2625
use Symfony\Component\OptionsResolver\OptionsResolverInterface;
@@ -93,8 +92,8 @@ public function buildView(FormViewInterface $view, FormInterface $form, array $o
9392
}
9493

9594
$types = array();
96-
foreach ($form->getConfig()->getTypes() as $type) {
97-
$types[] = $type->getName();
95+
for ($type = $form->getConfig()->getType(); null !== $type; $type = $type->getParent()) {
96+
array_unshift($types, $type->getName());
9897
}
9998

10099
if (!$translationDomain) {
@@ -149,7 +148,7 @@ public function setDefaultOptions(OptionsResolverInterface $resolver)
149148
{
150149
// Derive "data_class" option from passed "data" object
151150
$dataClass = function (Options $options) {
152-
return is_object($options['data']) ? get_class($options['data']) : null;
151+
return isset($options['data']) && is_object($options['data']) ? get_class($options['data']) : null;
153152
};
154153

155154
// Derive "empty_data" closure from "data_class" option
@@ -211,14 +210,6 @@ public function setDefaultOptions(OptionsResolverInterface $resolver)
211210
));
212211
}
213212

214-
/**
215-
* {@inheritdoc}
216-
*/
217-
public function createBuilder($name, FormFactoryInterface $factory, array $options)
218-
{
219-
return new FormBuilder($name, $options['data_class'], new EventDispatcher(), $factory, $options);
220-
}
221-
222213
/**
223214
* {@inheritdoc}
224215
*/

‎src/Symfony/Component/Form/Form.php

Lines changed: 9 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -195,11 +195,17 @@ public function getPropertyPath()
195195
* @return array An array of FormTypeInterface
196196
*
197197
* @deprecated Deprecated since version 2.1, to be removed in 2.3. Use
198-
* {@link getConfig()} and {@link FormConfigInterface::getTypes()} instead.
198+
* {@link getConfig()} and {@link FormConfigInterface::getType()} instead.
199199
*/
200200
public function getTypes()
201201
{
202-
return $this->config->getTypes();
202+
$types = array();
203+
204+
for ($type = $this->config->getType(); null !== $type; $type = $type->getParent()) {
205+
array_unshift($types, $type->getInnerType());
206+
}
207+
208+
return $types;
203209
}
204210

205211
/**
@@ -948,34 +954,7 @@ public function createView(FormViewInterface $parent = null)
948954
$parent = $this->parent->createView();
949955
}
950956

951-
$view = new FormView($this->config->getName());
952-
953-
$view->setParent($parent);
954-
955-
$types = (array) $this->config->getTypes();
956-
$options = $this->config->getOptions();
957-
958-
foreach ($types as $type) {
959-
$type->buildView($view, $this, $options);
960-
961-
foreach ($type->getExtensions() as $typeExtension) {
962-
$typeExtension->buildView($view, $this, $options);
963-
}
964-
}
965-
966-
foreach ($this->children as $child) {
967-
$view->add($child->createView($view));
968-
}
969-
970-
foreach ($types as $type) {
971-
$type->finishView($view, $this, $options);
972-
973-
foreach ($type->getExtensions() as $typeExtension) {
974-
$typeExtension->finishView($view, $this, $options);
975-
}
976-
}
977-
978-
return $view;
957+
return $this->config->getType()->createView($this, $parent);
979958
}
980959

981960
/**

‎src/Symfony/Component/Form/FormBuilder.php

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -115,10 +115,10 @@ public function create($name, $type = null, array $options = array())
115115
}
116116

117117
if (null !== $type) {
118-
return $this->getFormFactory()->createNamedBuilder($name, $type, null, $options, $this);
118+
return $this->factory->createNamedBuilder($name, $type, null, $options, $this);
119119
}
120120

121-
return $this->getFormFactory()->createBuilderForProperty($this->getDataClass(), $name, null, $options, $this);
121+
return $this->factory->createBuilderForProperty($this->getDataClass(), $name, null, $options, $this);
122122
}
123123

124124
/**
@@ -266,4 +266,23 @@ public function getIterator()
266266
{
267267
return new \ArrayIterator($this->children);
268268
}
269+
270+
/**
271+
* Returns the types used by this builder.
272+
*
273+
* @return array An array of FormTypeInterface
274+
*
275+
* @deprecated Deprecated since version 2.1, to be removed in 2.3. Use
276+
* {@link FormConfigInterface::getType()} instead.
277+
*/
278+
public function getTypes()
279+
{
280+
$types = array();
281+
282+
for ($type = $this->getType(); null !== $type; $type = $type->getParent()) {
283+
array_unshift($types, $type->getInnerType());
284+
}
285+
286+
return $types;
287+
}
269288
}

‎src/Symfony/Component/Form/FormConfig.php

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -59,9 +59,9 @@ class FormConfig implements FormConfigEditorInterface
5959
private $compound = false;
6060

6161
/**
62-
* @var array
62+
* @var ResolvedFormTypeInterface
6363
*/
64-
private $types = array();
64+
private $type;
6565

6666
/**
6767
* @var array
@@ -377,9 +377,9 @@ public function getCompound()
377377
/**
378378
* {@inheritdoc}
379379
*/
380-
public function getTypes()
380+
public function getType()
381381
{
382-
return $this->types;
382+
return $this->type;
383383
}
384384

385385
/**
@@ -671,9 +671,9 @@ public function setCompound($compound)
671671
/**
672672
* {@inheritdoc}
673673
*/
674-
public function setTypes(array $types)
674+
public function setType(ResolvedFormTypeInterface $type)
675675
{
676-
$this->types = $types;
676+
$this->type = $type;
677677

678678
return $this;
679679
}

0 commit comments

Comments
 (0)