Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Form] ResizeFormListener performance issues #42350

Closed
raing3 opened this issue Aug 3, 2021 · 4 comments
Closed

[Form] ResizeFormListener performance issues #42350

raing3 opened this issue Aug 3, 2021 · 4 comments

Comments

@raing3
Copy link

raing3 commented Aug 3, 2021

Description
Performance with Symfony's CollectionType has not matched what we want for processing large JSON payloads. As a result I am exploring options for a comparable replacement (without a lot of success) or ways in which we can improve this.

In particular we have identified "ResizeFormListener" (in conjunction with a large collection) as contributing a lot to the memory usage and duration of a request. This seems to be in large part due to the complex process Symfony goes through to create each child in a collection.

I'm openining this issue to discuss the potential of improving this and hear any thoughts on desired implementation.

Some discussion points:

  1. Is this even viable or is will my suggested changes below cause too many issues that I'm not aware of.

  2. Would there be any possiblity of exposing a clone-like method on FormInterface and FormConfigBuilderInterface (or another interface/s which ResizeFormListener could look at to opt-in to cloning a prototype vs. going through the form factory)?

  3. Do we need the option for form types (at the collection and/or child type level) to be able to opt-in/out of this proposed changed behaviour? How would we want this to look? I could see the potential where this may be useful in cases where a form type (or some of the objects it encompasses, eg: event listeners) are stateful.

Once I've got some feedback on viability and any desired implementation requirements from others I'm happy to look at contributing a PR. Just don't want to waste too much time if it is likely to be rejected.

Example
I've created a simple and very crude example comparing the performance difference between using the form builder/factory to create each entry instance vs. exposing a cloning method on the form and form config objects.

https://github.com/raing3/symfony-form-clone-test

More info showing some example benchmarks/usage is in the README.

Changes made to symfony/form can be viewed here: symfony/form@5.3...raing3:topic/resize_collection_clone_entry

I'm not at all suggesting this as the implementation, it was the simplest possible option to get something functional for evaluating the performance difference.

An additional before and after performance comparison using form types of our real application which prompted the investigation show a much bigger memory difference (4K items, PHP8 in a Docker container running on the same Mac):

Before (using form factory/builder):
^ "Start memory usage (MB): 12"
^ "Time (sec): 17.295336961746"
^ "After memory usage (MB): 1031"
^ "Memory diff (MB): 1019"

After (using clone):
^ "Start memory usage (MB): 12"
^ "Time (sec): 9.1904339790344"
^ "After memory usage (MB): 143"
^ "Memory diff (MB): 131"
@notana
Copy link

notana commented Dec 8, 2021

Hey guys,

Just wondering if anyone has looked into this yet? Looks like quite a substantial performance improvement in form processing for larger forms. Would be awesome to see those improvements in a tagged version :).

I would love to help, but as OP said, would prefer not to waste time if it is likely to be rejected.

@carsonbot
Copy link

Thank you for this issue.
There has not been a lot of activity here for a while. Has this been resolved?

@carsonbot
Copy link

Friendly reminder that this issue exists. If I don't hear anything I'll close this.

@carsonbot
Copy link

Hey,

I didn't hear anything so I'm going to close it. Feel free to comment if this is still relevant, I can always reopen!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants