search

ringcentral / ringcentral-js-widgets

RingCentral Integration Widget Library
MIT License
39 stars 44 forks source link

Potential performance issue #302

Open tylerlong opened 7 years ago

tylerlong commented 7 years ago

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: https://github.com/ringcentral/ringcentral-js-widget/pull/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 commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

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?