Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

[Form] added the newChoiceList class to configureChoiceType options#13182

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged

Conversation

HeahDude
Copy link
Contributor

Documentation forsymfony/symfony#30994.

@HeahDudeHeahDudeforce-pushed thecached-choice_list-options branch 2 times, most recently from7a2a769 tof5a4169CompareFebruary 18, 2020 21:00
Copy link
ContributorAuthor

@HeahDudeHeahDude left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Thanks. I've also missed thechoice_attr option, now added. And I've also added some doc about the$vary argument.

OskarStark reacted with thumbs up emoji
@HeahDudeHeahDudeforce-pushed thecached-choice_list-options branch 3 times, most recently from28a7c66 to4748837CompareFebruary 18, 2020 21:07
fabpot added a commit to symfony/symfony that referenced this pull requestFeb 21, 2020
… options (HeahDude)This PR was merged into the 5.1-dev branch.Discussion----------[Form] Added support for caching choice lists based on options| Q             | A| ------------- | ---| Branch?       | master| Bug fix?      | no| New feature?  | yes| BC breaks?    | no| Deprecations? | no| Tests pass?   | yes| Fixed tickets | ~| License       | MIT| Doc PR        |symfony/symfony-docs#13182Currently, the `CachingFactoryDecorator` is responsible for unnecessary memory usage, anytime a choice option is set with a callback option defined as an anonymous function or a loader, then a new hash is generated for the choice list, while we may expect the list to be reused once "finally" configured in a form type or choice type extension.A simple case is when using one of the core intl choice types in a collection:```php// ...class SomeFormType extends AbstractType{    public function buildForm(FormBuilderInterface $builder, array $options)    {        $builder            ->add('some_choices', ChoiceType::class, [                // before: cached choice list (unnecessary overhead)                // after: no cache (better perf)                'choices' => $someObjects,                'choice_value' => function (?object $choice) { /* return some string */ },            ])            // see below the nested effects            ->add('nested_fields', CollectionType::class, [                'entry_type' => NestedFormType::class,            ])    // ...}// ...class NestedFormType extends AbstractType{    public function buildForm(FormBuilderInterface $builder, array $options)    {        $builder            // ...            ->add('some_other_choices', ChoiceType::class, [                // before: cached choice list for every entry because we define a new closure instance for each field                // after: no cache, a bit better for the same result, but much better if it were not nested in a collection                'choices' => $someOtherObjects,                'choice_value' => function (?object $otherChoice) { /* return some string */ },            ])            ->add('some_loaded_choices', ChoiceType::class, [                // before: cached but for every entry since every field will have its                //         own instance of loader, generating a new hash                // after: no cache, same pro/cons as above                'choice_loader' => new CallbackChoiceLoader(function() { /* return some choices */}),                // or                'choice_loader' => new SomeLoader(),            ])            ->add('person', EntityType::class, [                // before: cached but for every entry, because we define extra `choice_*` option                // after: no cache, same pro/cons as above                'class' => SomeEntity::class,                'choice_label' => function (?SomeEntity $choice) { /* return some label */},            ])            // before: cached for every entry, because the type define some "choice_*" option            // after: cached only once, better perf since the same loader is used for every entry            ->add('country', CountryType::class)            // before: cached for every entry, because the type define some "choice_*" option            // after: no cache, same pro/cons as above            ->add('locale', LocaleType::class, [                'preferred_choices' => [ /* some preferred locales */ ],                'group_by' => function (?string $locale, $label) { /* return some group */ },            ])// ...```In such cases, we would expect every entries to use the same cached intl choice list, but not, as many list and views as entries will be kept in cache. This is even worse if some callback options like `choice_label`, `choice_value`, `choice_attr`, `choice_name`, `preferred_choices` or `group_by` are used.This PR helps making cache explicit when needed and ~deprecate~ drop unexpected implicit caching of choice list for most simple cases responsible of unnecessary overhead.The result is better performance just by upgrading to 5.1 \o/.But to solve the cases above when cache is needed per options, one should now use the new `ChoiceList` static methods to wrap option values, which is already done internally in this PR.```phpuse Symfony\Component\Form\ChoiceList\ChoiceList;// ...class NestedFormType extends AbstractType{    public function buildForm(FormBuilderInterface $builder, array $options)    {        $builder            // explicitly shared cached choice lists between entries            ->add('some_other_choices', ChoiceType::class, [                'choices' => $someOtherObjects,                'choice_value' => ChoiceList::value($this, function (?object $otherChoice) {                    /* return some string */                }),            ]),            ->add('some_loaded_choices', ChoiceType::class, [                'choice_loader' => ChoiceList::lazy($this, function() {                    /* return some choices */                })),                // or                'choice_loader' => ChoiceList::loader($this, new SomeLoader()),            ]),            ->add('person', EntityType::class, [                'class' => SomeEntity::class,                'choice_label' => ChoiceList::label($this, function (?SomeEntity $choice) {                    /* return some label */                },            ])            // nothing to do :)            ->add('country', CountryType::class)            ->add('locale', LocaleType::class, [                'preferred_choices' => ChoiceList::preferred($this, [ /* some preferred locales */ ]),                'group_by' => ChoiceList::groupBy($this, function (?string $locale, $label) {                    /* return some group */                }),            ])// ...```I've done some nice profiling with Blackfire and the simple example above in a fresh website skeleton and only two empty entries as initial data, then submitting an empty form. That gives the following results: * Rendering the form - Before vs After  <img width="714" alt="Screenshot 2020-02-16 at 9 24 58 PM" src="https://user-images.githubusercontent.com/10107633/74612132-de533180-5102-11ea-9cc4-296a16949d90.png"> * Rendering the form - Before vs After with `ChoiceList` helpers  <img width="670" alt="Screenshot 2020-02-16 at 9 26 51 PM" src="https://user-images.githubusercontent.com/10107633/74612155-122e5700-5103-11ea-9c16-5d80a7541f4b.png"> * Submitting the form - Before vs After  <img width="670" alt="Screenshot 2020-02-16 at 9 28 01 PM" src="https://user-images.githubusercontent.com/10107633/74612172-3be77e00-5103-11ea-9a18-4294e05402d2.png"> * Submitting the form - Before vs After with `ChoiceList` helpers  <img width="670" alt="Screenshot 2020-02-16 at 9 29 10 PM" src="https://user-images.githubusercontent.com/10107633/74612193-689b9580-5103-11ea-86b9-5b4906200021.png">_________TODO:- [x] Docs- [x] More profiling- [x] Add some tests#EUFOSSACommits-------b25973c [Form] Added support for caching choice lists based on options
@OskarStarkOskarStark removed the Waiting Code MergeDocs for features pending to be merged labelFeb 21, 2020
@OskarStarkOskarStark modified the milestones:next,5.1Feb 21, 2020
@javiereguiluzjaviereguiluzforce-pushed thecached-choice_list-options branch from596ce8e to9fc18e7CompareMarch 24, 2020 11:56
@javiereguiluzjaviereguiluz merged commit4fad298 intosymfony:masterMar 24, 2020
@javiereguiluz
Copy link
Member

Thanks a lot Jules. This is now merged!

HeahDude reacted with hooray emoji

@HeahDudeHeahDude deleted the cached-choice_list-options branchMarch 25, 2020 10:23
HeahDude added a commit that referenced this pull requestApr 12, 2020
This PR was merged into the master branch.Discussion----------[Form] added the "choice_filter" optionDocumentation forsymfony/symfony#35733.Based on#13182 for now, so better be reviewed by commit here until it's merged and rebased, thanks!Commits-------8b0c09e [Form] added the "choice_filter" option
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@javiereguiluzjaviereguiluzjaviereguiluz left review comments

@bigfoot90bigfoot90bigfoot90 left review comments

@fabpotfabpotfabpot approved these changes

@OskarStarkOskarStarkOskarStark approved these changes

@xabbuhxabbuhxabbuh approved these changes

Assignees
No one assigned
Projects
None yet
Milestone
5.1
Development

Successfully merging this pull request may close these issues.

7 participants
@HeahDude@javiereguiluz@fabpot@OskarStark@xabbuh@bigfoot90@carsonbot

[8]ページ先頭

©2009-2025 Movatter.jp