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

Potential performance issue #302

Open
tylerlong opened this issue Jul 24, 2017 · 3 comments
Open

Potential performance issue #302

tylerlong opened this issue Jul 24, 2017 · 3 comments

Comments

@tylerlong
Copy link
Contributor

tylerlong commented Jul 24, 2017

I tried to create a jest snapshot, and the snapshot turned out to be extremely large.

A typical snapshot size ranges from 100 to 1000 lines, while this widget's snapshot is as large as 700,000 lines. That is 1000 times typical size. I am afraid that it will cause serious performance issues.

How to reproduce: get latest code of this PR: #296 and run yarn test. Then check the snapshot in test/__snapshots__/.

Update: I have updated the code to remove the snapshot generation because it caused the tests stuck. If you have problem reproduce this issue please let me know.

@embbnux
Copy link
Collaborator

embbnux commented Jul 24, 2017

I found that you create a snapshot from App container. App include a lot of container, and every container depend on multiple commons's module. A module has properties of store and modules that it depend on.
So your snapshot file include a lot of same properties. But in memory, the module object and store object is unique. So actually, usage of memory is much different to JSON tree file of snapshot.
If you create snapshot from independent container, it will be much smaller.

@tylerlong
Copy link
Contributor Author

What your explained makes sense but it doesn't convince me that it would take 1000 times typical size. I have other demo apps which only take 500 lines even I snapshot the whole app.

I also tried to snapshot a containers, the size is still very large. For example: DialerPage container has 250K lines. The only way that I can generate small snapshot is to snapshot a pure component.

@u9520107
Copy link
Contributor

u9520107 commented Aug 3, 2017

By our current design containers accepts modules as props. That's how we currently define what modules are used in each UI feature. The modules has reference to state and can have cyclic structure, which if the snapshot tool attempts to walk through the structure you will end up with a lot of duplicated data.
I understand that this prevent the use of snapshots for tests. But how is this a performance issue for the app? Furthermore, why are snapshots necessary for these tests?

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

No branches or pull requests

3 participants