iterative/vscode-dvc

Improve plots performance when there are many plots

Closed this issue · 5 comments

sroy3 commented

With 200+ plots, updating in real-time takes a long time and even feels very buggy. Any other action that applies to all plots (resizing, changing the order...) takes more time than it should and makes things confusing to the user.

In the case of resizing, the problem seems to affect the virtualized grid (lots of re-calculating and re-rendering), while updating blocks even before rendering.

  • Remove drag and drop when not dragging (#4934)
  • Set the default number of plots selected to 20 (#5008 )
  • Remove drag and drop container and replace it with a hook (no element creation on the fly) (#5062)
  • Remove drag and drop when not dragging for the comparison table(#5017 )
  • Remove drag and drop when not dragging for the comparison table rows
  • Do not render a section if it is closed (#5025)
sroy3 commented

The main bottleneck comes from DragDropContainer. We create a lot of functions in there and re-create all plots to add these functions. This causes a lot of re-rendering whenever we try to do something (update the plots, rearrange them, resize them...).

There is a good cleanup to be done in there. That and all the usual tricks to limit re-rendering (useCallback, useMemo, stop passing down complex props, stop re-creating functions...) should help a lot.

Cutting the performance lag completely while still using drag and drop will be challenging, though. One solution is to have a drag-and-drop mode. In this mode, the details of the plots will be hidden, showing only their title. It would be possible to move the plots freely in this mode. In normal mode, it would be impossible to change the order of the plots. It would be possible to toggle modes from the section header. This solution should increase the speed of actions on plots dramatically.

sroy3 commented

As @mattseddon mentioned in our last planning meeting, enabling the drag-and-drop when gripping the plot and disabling it on drop should work as a drag-and-drop toggle and enable us to render plots without drag and drops components most of the time for added performance.

sroy3 commented

While removing the drag-and-drop features while not in drag-and-drop mode helps with performance, it doesn't address the core of the problem. If someone has 200+ plots in their project, there is probably a need for them all, but there is no need to view them all simultaneously. Toggling single plots every time in a seemingly infinite list is painful. To improve the user experience, we could iterate on the plot section feature (currently only used to separate multi-view and single-view plots) and make it so we can group multiple plots. Once grouped and that section named, it should be possible to toggle the visibility of a whole section instead of individual plots. Reducing the number of plots rendered when updating the webview will not only make the experience smoother but also help the user arrange their plots in a way that makes sense to them and will make it easier to find the right one at the right time.

If everyone is okay with that, this could be the next feature to increase the performance of the plots webview when there are many plots.

There is still more that we can do. But this could be done on a second pass.

Btw, folks. I wonder if we can somehow put some safeguards agains performance degradation in the future? A lot of good work put into this it would be sad to break it later. A topic for discussion.