8000 feature #16747 [Form] Improved performance of ChoiceType and its subt… · symfony/symfony@140ed98 · GitHub
[go: up one dir, main page]
Skip to content

Commit 140ed98

Browse files
TobionTobion
committed
feature #16747 [Form] Improved performance of ChoiceType and its subtypes (webmozart)
This PR was merged into the 2.7 branch. Discussion ---------- [Form] Improved performance of ChoiceType and its subtypes | Q | A | ------------- | --- | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | - I found out today that, although CachingFactoryDecorator is part of Symfony 2.7, it is not configured to be used in the DI configuration. This simple in-memory cache improved the page load by 50% for one considerably large form with many (~600) choice/entity fields that I was working on today. Also, caching of query builders with parameters was broken, since the parameters are represented by objects. PHP's object hashes were used to calculate the cache keys, hence the cache always missed. I converted parameters to arrays for calculating the cache keys to fix this problem. Commits ------- a0ef101 [Form] Improved performance of ChoiceType and its subtypes
2 parents 2c2daf1 + a0ef101 commit 140ed98

File tree

5 files changed

+110
-10
lines changed

5 files changed

+110
-10
lines changed

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

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -94,12 +94,12 @@ public static function createChoiceName($choice, $key, $value)
9494
* Gets important parts from QueryBuilder that will allow to cache its results.
9595
* For instance in ORM two query builders with an equal SQL string and
9696
* equal parameters are considered to be equal.
97-
*
97+
*
9898
* @param object $queryBuilder
99-
*
99+
*
100100
* @return array|false Array with important QueryBuilder parts or false if
101101
* they can't be determined
102-
*
102+
*
103103
* @internal This method is public to be usable as callback. It should not
104104
* be used in user code.
105105
*/
@@ -111,7 +111,12 @@ public function getQueryBuilderPartsForCachingHash($queryBuilder)
111111
public function __construct(ManagerRegistry $registry, PropertyAccessorInterface $propertyAccessor = null, ChoiceListFactoryInterface $choiceListFactory = null)
112112
{
113113
$this->registry = $registry;
114-
$this->choiceListFactory = $choiceListFactory ?: new PropertyAccessDecorator(new DefaultChoiceListFactory(), $propertyAccessor);
114+
$this->choiceListFactory = $choiceListFactory ?: new CachingFactoryDecorator(
115+
new PropertyAccessDecorator(
116+
new DefaultChoiceListFactory(),
117+
$propertyAccessor
118+
)
119+
);
115120
}
116121

117122
public function buildForm(FormBuilderInterface $builder, array $options)

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

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
namespace Symfony\Bridge\Doctrine\Form\Type;
1313

1414
use Doctrine\Common\Persistence\ObjectManager;
15+
use Doctrine\ORM\Query\Parameter;
1516
use Doctrine\ORM\QueryBuilder;
1617
use Symfony\Bridge\Doctrine\Form\ChoiceList\ORMQueryBuilderLoader;
1718
use Symfony\Component\Form\Exception\UnexpectedTypeException;
@@ -64,19 +65,31 @@ public function getName()
6465
/**
6566
* We consider two query builders with an equal SQL string and
6667
* equal parameters to be equal.
67-
*
68+
*
6869
* @param QueryBuilder $queryBuilder
69-
*
70+
*
7071
* @return array
71-
*
72+
*
7273
* @internal This method is public to be usable as callback. It should not
7374
* be used in user code.
7475
*/
7576
public function getQueryBuilderPartsForCachingHash($queryBuilder)
7677
{
7778
return array(
78-
$queryBuilder->getQuery()->getSQL(),
79-
$queryBuilder->getParameters()->toArray(),
79+
$queryBuilder->getQuery()->getSQL(),
80+
array_map(array($this, 'parameterToArray'), $queryBuilder->getParameters()->toArray()),
8081
);
8182
}
83+
84+
/**
85+
* Converts a query parameter to an array.
86+
*
87+
* @param Parameter $parameter The query parameter
88+
*
89+
* @return array The array representation of the parameter
90+
*/
91+
private function parameterToArray(Parameter $parameter)
92+
{
93+
return array($parameter->getName(), $parameter->getType(), $parameter->getValue());
94+
}
8295
}

‎src/Symfony/Bridge/Doctrine/Tests/Form/Type/EntityTypeTest.php

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1114,6 +1114,69 @@ public function testLoaderCaching()
11141114
$this->assertSame($choiceList1, $choiceList3);
11151115
}
11161116

1117+
public function testLoaderCachingWithParameters()
1118+
{
1119+
$entity1 = new SingleIntIdEntity(1, 'Foo');
1120+
$entity2 = new SingleIntIdEntity(2, 'Bar');
1121+
$entity3 = new SingleIntIdEntity(3, 'Baz');
1122+
1123+
$this->persist(array($entity1, $entity2, $entity3));
1124+
1125+
$repo = $this->em->getRepository(self::SINGLE_IDENT_CLASS);
1126+
1127+
$entityType = new EntityType(
1128+
$this->emRegistry,
1129+
PropertyAccess::createPropertyAccessor()
1130+
);
1131+
1132+
$entityTypeGuesser = new DoctrineOrmTypeGuesser($this->emRegistry);
1133+
1134+
$factory = Forms::createFormFactoryBuilder()
1135+
->addType($entityType)
1136+
->addTypeGuesser($entityTypeGuesser)
1137+
->getFormFactory();
1138+
1139+
$formBuilder = $factory->createNamedBuilder('form', 'form');
1140+
1141+
$formBuilder->add('property1', 'entity', array(
F438 1142+
'em' => 'default',
1143+
'class' => self::SINGLE_IDENT_CLASS,
1144+
'query_builder' => $repo->createQueryBuilder('e')->where('e.id = :id')->setParameter('id', 1),
1145+
));
1146+
1147+
$formBuilder->add('property2', 'entity', array(
1148+
'em' => 'default',
1149+
'class' => self::SINGLE_IDENT_CLASS,
1150+
'query_builder' => function (EntityRepository $repo) {
1151+
return $repo->createQueryBuilder('e')->where('e.id = :id')->setParameter('id', 1);
1152+
},
1153+
));
1154+
1155+
$formBuilder->add('property3', 'entity', array(
1156+
'em' => 'default',
1157+
'class' => self::SINGLE_IDENT_CLASS,
1158+
'query_builder' => function (EntityRepository $repo) {
1159+
return $repo->createQueryBuilder('e')->where('e.id = :id')->setParameter('id', 1);
1160+
},
1161+
));
1162+
1163+
$form = $formBuilder->getForm();
1164+
1165+
$form->submit(array(
1166+
'property1' => 1,
1167+
'property2' => 1,
1168+
'property3' => 2,
1169+
));
1170+
1171+
$choiceList1 = $form->get('property1')->getConfig()->getOption('choice_list');
1172+
$choiceList2 = $form->get('property2')->getConfig()->getOption('choice_list');
1173+
$choiceList3 = $form->get('property3')->getConfig()->getOption('choice_list');
1174+
1175+
$this->assertInstanceOf('Symfony\Component\Form\ChoiceList\ChoiceListInterface', $choiceList1);
1176+
$this->assertSame($choiceList1, $choiceList2);
1177+
$this->assertSame($choiceList1, $choiceList3);
1178+
}
1179+
11171180
public function testCacheChoiceLists()
11181181
{
11191182
$entity1 = new SingleIntIdEntity(1, 'Foo');

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

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,19 @@
5757
<!-- CoreExtension -->
5858
<service id="form.property_accessor" alias="property_accessor" public="false" />
5959

60+
<service id="form.choice_list_factory.default" class="Symfony\Component\Form\ChoiceList\Factory\DefaultChoiceListFactory" public="false"/>
61+
62+
<service id="form.choice_list_factory.property_access" class="Symfony\Component\Form\ChoiceList\Factory\PropertyAccessDecorator" public="false">
63+
<argument type="service" id="form.choice_list_factory.default"/>
64+
<argument type="service" id="form.property_accessor"/>
65+
</service>
66+
67+
<service id="form.choice_list_factory.cached" class="Symfony\Component\Form\ChoiceList\Factory\CachingFactoryDecorator" public="false">
68+
<argument type="service" id="form.choice_list_factory.property_access"/>
69+
</service>
70+
71+
<service id="form.choice_list_factory" alias="form.choice_list_factory.cached" public="false"/>
72+
6073
<service id="form.type.form" class="Symfony\Component\Form\Extension\Core\Type\FormType">
6174
<argument type="service" id="form.property_accessor" />
6275
<tag name="form.type" alias="form" />
@@ -69,6 +82,7 @@
6982
</service>
7083
<service id="form.type.choice" class="Symfony\Component\Form\Extension\Core\Type\ChoiceType">
7184
<tag name="form.type" alias="choice" />
85+
<argument type="service" id="form.choice_list_factory"/>
7286
</service>
7387
<service id="form.type.collection" class="Symfony\Component\Form\Extension\Core\Type\CollectionType">
7488
<tag name="form.type" alias="collection" />

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

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
namespace Symfony\Component\Form\Extension\Core\Type;
1313

1414
use Symfony\Component\Form\AbstractType;
15+
use Symfony\Component\Form\ChoiceList\Factory\CachingFactoryDecorator;
1516
use Symfony\Component\Form\ChoiceList\Factory\PropertyAccessDecorator;
1617
use Symfony\Component\Form\ChoiceList\LegacyChoiceListAdapter;
1718
use Symfony\Component\Form\ChoiceList\View\ChoiceGroupView;
@@ -46,7 +47,11 @@ class ChoiceType extends AbstractType
4647

4748
public function __construct(ChoiceListFactoryInterface $choiceListFactory = null)
4849
{
49-
$this->choiceListFactory = $choiceListFactory ?: new PropertyAccessDecorator(new DefaultChoiceListFactory());
50+
$this->choiceListFactory = $choiceListFactory ?: new CachingFactoryDecorator(
51+
new PropertyAccessDecorator(
52+
new DefaultChoiceListFactory()
53+
)
54+
);
5055
}
5156

5257
/**

0 commit comments

Comments
 (0)
0