[Form] Individual rows of CollectionType cannot be styled anymore #4909
Conversation
… performance reasons
|
@bschussek Can you also create a PR on symfony docs to update the documentation? |
Commits ------- 69e5e58 [Form] Individual rows of CollectionType cannot be styled anymore for performance reasons Discussion ---------- [Form] Individual rows of CollectionType cannot be styled anymore Bug fix: no Feature addition: no Backwards compatibility break: **yes** Symfony2 tests pass: yes Fixes the following tickets: #2806, #4733 Todo: - Individual theming of blocks in a row of a collection form is now unsupported: ``` {% block _author_tags_0_label %} {# ... #} {% endblock %} {% block _author_tags_1_label %} {# ... #} {% endblock %} ``` Instead, it is now possible to define a generic template for all rows, where the word "entry" replaces the previous occurence of the row index: ``` {% block _author_tags_entry_label %} {# ... #} {% endblock %} ``` The main motivation for this change is performance. Looking up whether individual styles exist for each single block within each row costs a lot of time. Because the row index is included in the block names, caching is virtually impossible. For [this specific, heavy form](http://advancedform.gpserver.dk/app_dev.php/taxclasses/1), this PR decreases rendering time from **7.7** to **2.5 seconds** on my machine. --------------------------------------------------------------------------- by fabpot at 2012-07-14T10:55:29Z @bschussek Can you also create a PR on symfony docs to update the documentation?
|
Nice work Bernhard, thank you! Is final 2.5 sec fixable, or it'll require too much of refactoring? |
|
@denis-chmel the form rendering refactoring in the next PR still improves it a bit |
|
@denis-chmel As @stof said, #4918 improves the performance for yet another 0.1 seconds. I'll see if we can improve it some more, but I have the feeling that we're soon hitting the limits of the templating engine. After all, forms consist of many small templates due to their theming mechanism, and evaluating these templates takes time. |
|
I see, @bschussek, anyway great increase so far. Right now lookupTemplate() is still called 3703 on the form (and this is only a 1st level calls. There are ~8800 if count with all the recursive calls). That is imho too much for the number of fields. However I can't offer anything exact now, have to learn the symfony code more. Maybe it's hard to cache anything more because symfony creates form/html based on the data. Maybe there is a way to extract raw form/template to cache. And create final form/html based on that cached raw form/template + data. |
|
@denis-chmel are you talking about the current code or the refactored one ? |
|
@denis-chmel #4918 reduces the number of template lookups to the possible minimum. The only possibilities that is left now is to remove features, such as
|
|
Then may be it make sense to made some setting that will disable some features and thus will increase performance? |
|
great! :) then let's wait for Fabien's response |
Bug fix: no
Feature addition: no
Backwards compatibility break: yes
Symfony2 tests pass: yes
Fixes the following tickets: #2806, #4733
Todo: -
Individual theming of blocks in a row of a collection form is now unsupported:
Instead, it is now possible to define a generic template for all rows, where the word "entry" replaces the previous occurence of the row index:
The main motivation for this change is performance. Looking up whether individual styles exist for each single block within each row costs a lot of time. Because the row index is included in the block names, caching is virtually impossible.
For this specific, heavy form, this PR decreases rendering time from 7.7 to 2.5 seconds on my machine.