[STCON-85] Inventory, Users etc Slow and Glitchy on View and Edit Created: 09/Jul/19 Updated: 19/Jul/19 Resolved: 19/Jul/19 |
|
| Status: | Closed |
| Project: | stripes-connect |
| Components: | None |
| Affects versions: | None |
| Fix versions: | 5.3.0 |
| Type: | Bug | Priority: | P1 |
| Reporter: | Cate Boerema (Inactive) | Assignee: | Michal Kuklis |
| Resolution: | Done | Votes: | 0 |
| Labels: | performance, q3.1-2019 | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original estimate: | Not Specified | ||
| Attachments: |
|
||||||||||||||||
| Issue links: |
|
||||||||||||||||
| Sprint: | Core: F - Sprint 68, Core: F - Sprint 67 | ||||||||||||||||
| Story Points: | 13 | ||||||||||||||||
| Development Team: | Prokopovych | ||||||||||||||||
| Tester Assignee: | Magda Zacharska | ||||||||||||||||
| Description |
|
Steps to repro:
Expected: Entered text displays right away Actual: Field is empty for 10+ seconds before the entered text displays Additional info: the above is just one example of generaly slowness and glitchyness in Inventory lately. Ann-Marie passed on this feedback: Screencast: https://www.screencast.com/t/if7u3iEgSj Other performance issues seen recently:
|
| Comments |
| Comment by Zak Burke [ 10/Jul/19 ] |
|
In this screenie, all that happens is a little bit of keyboard interaction in a form field: typing new characters and then tabbing among fields. The borders that show up indicate which components are being re-rendered. You can see it's basically the entire form every time. I don't know if this is an issue with redux-form itself or merely the way we are using redux-form, but it's clearly not helping us any. |
| Comment by Zak Burke [ 11/Jul/19 ] |
|
The are many differences between the yarn.lock files on folio-release-core (where performance is OK) and folio-snapshot-core (where things are glitchy). That's gives us a lot of potential options to investigate, but it ... gives us a lot of potential options to investigate. I'm working backwards with the snapshot branch's yarn.lock file (it's updated automatically every hour) to see if I can find a performance tipping point, (think git bisect) but it's a slow process. |
| Comment by Zak Burke [ 11/Jul/19 ] |
|
Michal Kuklis narrowed this down to a bug fix we recently implemented. Essentially we were not correctly filtering a stream of events for changes and therefore our components did not always update as expected. Unfortunately, correctly filtering that stream is costly in terms of performance. We are trying to figure out if we can have our cake and eat it too. |
| Comment by Cate Boerema (Inactive) [ 11/Jul/19 ] |
|
Yay Michal Kuklis! What a relief to at least have a diagnosis. Which bug fix was it? Is this the same root cause as for
|
| Comment by Zak Burke [ 11/Jul/19 ] |
|
Yes, closed
|
| Comment by Cate Boerema (Inactive) [ 11/Jul/19 ] |
|
You have a PR in already for a fix for this? Amazing. Thank you so much. I have moved this to STCON |
| Comment by Zak Burke [ 11/Jul/19 ] |
|
We have a patch but we are still testing. The last time we patched a bug in stripes-connect it didn't go so well |
| Comment by Viktor Soroka [ 12/Jul/19 ] |
|
Hi guys, I found that after the merge the tests in ui-circulation started to fail. I reverted locally the change and they started to work. As I found it happens because the arePropsEqual function may happen to deal with react elements which are pretty deep objects. As I tried locally omitting such may resolve the issue. Please check PR. |
| Comment by Cate Boerema (Inactive) [ 18/Jul/19 ] |
|
Hi Michal Kuklis I don't have a great environment to test this in since I've broken both testing and snapshot, but I tried anyway. Users feels like it is working much better but I am still seeing the issue in Inventory where you type in a field and the entered text takes a long time to display. Reopening. |
| Comment by Michal Kuklis [ 18/Jul/19 ] |
|
Cate Boerema unfortunately there is nothing else we can do about this. I feel like we did everything possible given our current architecture (stripes-connect, redux-form). In order to improve it we would have to restructure things significantly (replace redux-form and switch to GraphQL). But I would suggest reviewing this in folio-snapshot. The fix is already there and it should perform a bit better. The reason for it is that the snapshot env is built in production mode which improves the react performance. I'm hoping it will be good enough for the time being. |
| Comment by Michal Kuklis [ 18/Jul/19 ] |
|
It also looks like folio-testing is misconfigured again which probably affects the performance too. I see these errors in folio-testing: You are currently using minified code outside of NODE_ENV === 'production'. This means that you are running a slower development build of Redux. You can use loose-envify (https://github.com/zertosh/loose-envify) for browserify or DefinePlugin for webpack (http://stackoverflow.com/questions/30030031) to ensure you have the correct code for your production build. This was already fixed so I'm not sure why we see it again. I think after this is fixed again folio-testing should also perform a bit better too. |
| Comment by Cate Boerema (Inactive) [ 19/Jul/19 ] |
|
Thanks for the context Michal Kuklis. I do think we will need to restructure things, as the performance is still too slow. But I understand that that's a bigger issue and out of scope for this release. Things have improved significantly from when I first filed this ticket. I don't think the performance is a release blocker at this point so I will close this as done. Thanks for all your help with this. We'll come back to the architecture restructure question later. |
| Comment by Michal Kuklis [ 19/Jul/19 ] |
|
Perfect. Thank you Cate Boerema! Yes there are couple steps we can take to improve the overall performance. The big one will be to move away from redux-form. |