8000 [Form] Precalculated the closure for deciding whether a choice is sel… · Lumbendil/symfony@5984b18 · GitHub
[go: up one dir, main page]
Skip to content

Commit 5984b18

Browse files
webmozartwebmozart
committed
[Form] Precalculated the closure for deciding whether a choice is selected (PHP +30ms, Twig +30ms)
1 parent 5dc3c39 commit 5984b18

File tree

5 files changed

+65
-82
lines changed

5 files changed

+65
-82
lines changed

‎src/Symfony/Bridge/Twig/Extension/FormExtension.php

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -88,12 +88,18 @@ public function getFilters()
8888
/**
8989
* Returns whether a choice is selected for a given form value.
9090
*
91-
* This method exists for the sole purpose that Twig performs (a lot) better
92-
* with filters than with methods of an object.
91+
* Unfortunately Twig does not support an efficient way to execute the
92+
* "is_selected" closure passed to the template by ChoiceType. It is faster
93+
* to implement the logic here (around 65ms for a specific form).
9394
*
94-
* To give this some perspective, I'm currently testing this on a form with
95-
* a large list of entity fields. Using the filter is around 220ms faster than
96-
* accessing the method directly on the object in the Twig template.
95+
* Directly implementing the logic here is also faster than doing so in
96+
* ChoiceView (around 30ms).
97+
*
98+
* The worst option tested so far is to implement the logic in ChoiceView
99+
* and access the ChoiceView method directly in the template. Doing so is
100+
* around 220ms slower than doing the method call here in the filter. Twig
101+
* seems to be much more efficient at executing filters than at executing
102+
* methods of an object.
97103
*
98104
* @param ChoiceView $choice The choice to check.
99105
* @param string|array $selectedValue The selected value to compare.
@@ -104,7 +110,11 @@ public function getFilters()
104110
*/
105111
public function isChoiceSelected(ChoiceView $choice, $selectedValue)
106112
{
107-
return $choice->isSelected($selectedValue);
113+
if (is_array($selectedValue)) {
114+
return false !== array_search($choice->value, $selectedValue, true);
115+
}
116+
117+
return $choice->value === $selectedValue;
108118
}
109119

110120
/**

‎src/Symfony/Bridge/Twig/Tests/Extension/FormExtensionDivLayoutTest.php

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
use Symfony\Bridge\Twig\Tests\Extension\Fixtures\StubTranslator;
1919
use Symfony\Bridge\Twig\Tests\Extension\Fixtures\StubFilesystemLoader;
2020
use Symfony\Component\Form\FormView;
21+
use Symfony\Component\Form\Extension\Core\View\ChoiceView;
2122
use Symfony\Component\Form\Tests\AbstractDivLayoutTest;
2223

2324
class FormExtensionDivLayoutTest extends AbstractDivLayoutTest
@@ -105,6 +106,39 @@ public function testThemeBlockInheritanceUsingExtend()
105106
);
106107
}
107108

109+
public function isChoiceSelectedProvider()
110+
{
111+
// The commented cases should not be necessary anymore, because the
112+
// choice lists should assure that both values passed here are always
113+
// strings
114+
return array(
115+
// array(true, 0, 0),
116+
array(true, '0', '0'),
117+
array(true, '1', '1'),
118+
// array(true, false, 0),
119+
// array(true, true, 1),
120+
array(true, '', ''),
121+
// array(true, null, ''),
122+
array(true, '1.23', '1.23'),
123+
array(true, 'foo', 'foo'),
124+
array(true, 'foo10', 'foo10'),
125+
array(true, 'foo', array(1, 'foo', 'foo10')),
126+
127+
array(false, 10, array(1, 'foo', 'foo10')),
128+
array(false, 0, array(1, 'foo', 'foo10')),
129+
);
130+
}
131+
132+
/**
133+
* @dataProvider isChoiceSelectedProvider
134+
*/
135+
public function testIsChoiceSelected($expected, $choice, $value)
136+
{
137+
$choice = new ChoiceView($choice, $choice . ' label');
138+
139+
$this->assertSame($expected, $this->extension->isChoiceSelected($choice, $value));
140+
}
141+
108142
protected function renderEnctype(FormView $view)
109143
{
110144
return (string) $this->extension->renderer->renderEnctype($view);

‎src/Symfony/Bundle/FrameworkBundle/Resources/views/Form/choice_widget_options.html.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,6 @@
66
<?php echo $formHelper->block('choice_widget_options', array('choices' => $choice)) ?>
77
</optgroup>
88
<?php else: ?>
9-
<option value="<?php echo $view->escape($choice->value) ?>"<?php if ($choice->isSelected($value)): ?> selected="selected"<?php endif?>><?php echo $view->escape($translatorHelper->trans($choice->label, array(), $translation_domain)) ?></option>
9+
<option value="<?php echo $view->escape($choice->value) ?>"<?php if ($is_selected($choice->value, $value)): ?> selected="selected"<?php endif?>><?php echo $view->escape($translatorHelper->trans($choice->label, array(), $translation_domain)) ?></option>
1010
<?php endif ?>
1111
<?php endforeach ?>

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

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,20 @@ public function buildView(FormView $view, FormInterface $form, array $options)
9090
'empty_value' => null,
9191
));
9292

93+
// The decision, whether a choice is selected, is potentially done
94+
// thousand of times during the rendering of a template. Provide a
95+
// closure here that is optimized for the value of the form, to
96+
// avoid making the type check inside the closure.
97+
if ($options['multiple']) {
98+
$view->vars['is_selected'] = function ($choice, array $values) {
99+
return false !== array_search($choice, $values, true);
100+
};
101+
} else {
102+
$view->vars['is_selected'] = function ($choice, $value) {
103+
return $choice === $value;
104+
};
105+
}
106+
93107
// Check if the choices already contain the empty value
94108
// Only add the empty value option if this is not the case
95109
if (0 === count($options['choice_list']->getIndicesForValues(array('')))) {

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

Lines changed: 0 additions & 75 deletions
This file was deleted.

0 commit comments

Comments
 (0)
0