search

day8 / re-frame-10x

A debugging dashboard for re-frame. X-ray vision as tooling.
MIT License
632 stars 68 forks source link

[Bug]: 1.2.0 app-db inspector major performance slowdown #330

Closed hipitihop closed 2 years ago

hipitihop commented 2 years ago

What happened?

The updated app-db inspector is now too slow. In my example, a full app-db path takes 15-20 sec to open the root node. It is slightly faster when subsequently closing the node. Any significant sub-node of the tree takes considerable time also to expand or collapse it. In my case out site-config map which is not extensive, so others are likely to be worse.

10x Version

1.2.0

Reagent Version

1.1.0

React Version

17.0.2

re-frame Version

1.2.0

What browsers are you seeing the problem on?

Chrome

Relevant console output

No response

superstructor commented 2 years ago

@MawiraIke Please urgently take a look at this issue.

MawiraIke commented 2 years ago

Okay, Let me try to reproduce the issue now.

MawiraIke commented 2 years ago

@hipitihop could you please share the data used to produce this issue? I have tried rendering more data to the app-db but my perfomance doesn't seem to be impacted a lot, (max about a second more) which is understandable due to extra functionality on devtools to keep track of the object history, aiding in generating the path. I suspect this issue could be related to the type of data that is passed to devtools instead. If you could please share the data that caused this issue, or the structure of one of the maps that takes long to load in the form {:key [int {:another-key #{int char}}] , I think that would assist me in reproducing the issue. Thank you.

hipitihop commented 2 years ago

@hipitihop could you please share the data used to produce this issue? ...

@MawiraIke , I asked @superstructor to pass on a zip of my entire app-db to test.

hipitihop commented 2 years ago

@MawiraIke Just thought I would note that I am seeing similar lag in version 1.1.13 when large maps/list are expanded in the inspector. Although it is worse now in 1.2.0

MawiraIke commented 2 years ago

@hipitihop I have been able to reproduce this issue in version 1.2.0 with the data you sent. The issue seems to happen in body rendering of json-ml while header rendering still has fair speed. According to my investigation it seems apparent that the speed is impacted from devtools. When the data is sent to devtools, the time to get the json-ml is slower in version 1.2.0. Using a section of the data for instance, I get ~50ms in version 1.1.12 and 1.1.13. In version 1.2.0 it however takes more time, around 700ms to receive the response jsonml. Including the time to render by re-frame-10x, I average around 120ms in version 1.1.12 and 1.1.13, and around 1192ms in 1.2.0. This might be due to generation of paths in extremely large maps within devtools. Proof to this is the size of jsonml returned by devtools which seems to be around 600kb in version 1.1.12 and 1.1.13 while its around 3mb in version 1.2.0. This is using a section of the data sent. The reason the generation of paths might take a lot of time is because every item within the map has a different path which is currently found by looping through all the items in the parent object to determine the position/key and also convert the added annotations for every item to html elements I believe there could be ways of improving the speed in devtools path generation and I am trying them out. If I'm successful, I will make a PR in devtools to fix this. Thank you for spotting this.

MawiraIke commented 2 years ago

I have been able to improve the speed of path generation substantively and made these PRs 1, 2. I'm not sure we can go back to speeds of pre path-generation days while rendering extremely large maps due to the method of path generation used. Using your data, I was getting crashes most of the time on my website and the data could not even render. However, I am now able to render the large map in ~7seconds. This would be better faster and therefore, PRs to improve the speed are welcome.

Another improvement that could be done on this is maybe adding a toggle to turn on/off path generation. For bigger maps than this ~(>3mb), that could be a saver.

hipitihop commented 2 years ago

@MawiraIke thanks for the thorough analysis and improvements.

mike-thompson-day8 commented 2 years ago

Another improvement that could be done on this is maybe adding a toggle to turn on/off path generation. For bigger maps than this ~(>3mb), that could be a saver.

@MawiraIke could you create an issue for this, please? It seems like a good idea. Might need to be a configuration option?

I fear a 7-second pause would make re-frame-10x largely unusable. Users would need an "out".

MawiraIke commented 2 years ago

@mike-thompson-day8 I have created a new issue #341 with the details. I will also proceed to make the fix.