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

Remove large series models from reactive data in Plots #6879

Closed
scottbell opened this issue Aug 2, 2023 · 8 comments · Fixed by #6961
Closed

Remove large series models from reactive data in Plots #6879

scottbell opened this issue Aug 2, 2023 · 8 comments · Fixed by #6961
Labels
performance impacts or improves performance type:feature Feature. Required intentional design
Milestone

Comments

@scottbell
Copy link
Contributor

Summary

Plot configurations, highlights, and annotations have series models in them, and are reactive all the way down. We should instead being passing around series IDs instead of the full series model, and then have the individual components look up the series models internally. The trick here is they’ll need to then follow updates to add/removals of both series, and in the case of stacked plots, adding/removal of telemetry from the composition. I think this logic would be common to Legend components, MctPlot, and MctChart, and could exist in a separate helper class. The helper class could also return a series given an ID, which is what all these components want in the first place.

@scottbell scottbell added the type:maintenance tests, chores, or project maintenance label Aug 2, 2023
@ozyx ozyx added this to the Target:3.0.1 milestone Aug 2, 2023
@scottbell scottbell self-assigned this Aug 4, 2023
@scottbell
Copy link
Contributor Author

scottbell commented Aug 7, 2023

With legends, was able to get a 6x speedup by removing series from highlights object. Before:

Screenshot 2023-08-07 at 3 30 42 PM

After:
Screenshot 2023-08-07 at 3 29 52 PM

@akhenry
Copy link
Contributor

akhenry commented Aug 7, 2023

This must have been an issue pre Vue3 as well, no? Does it improve steady state rendering of plots as well?

@scottbell
Copy link
Contributor Author

This must have been an issue pre Vue3 as well, no? Does it improve steady state rendering of plots as well?

Steady state looks about the same (surprisingly to me). It's anything interactive (zoom/rectangles/highlighting points) that drags the performance down with traversals of the big reactive objects.

@akhenry
Copy link
Contributor

akhenry commented Aug 7, 2023

Got it, thanks, this helps me to prioritize the fix.

@scottbell
Copy link
Contributor Author

To test:

  • Create a high rate Sine Wave Generator (3Hz)
  • Move mouse around and look at legend, should be much faster than previous incarnations.
  • Create some annotations in real time mode and move mouse around. Should feel a lot faster.

@scottbell
Copy link
Contributor Author

Before (note long mouseover time and annotation draw):
before

After:
after

@ozyx
Copy link
Contributor

ozyx commented Sep 12, 2023

Verified Testathon 9/12/23 -- Plots are much more responsive, with or without annotations

@rukmini-bose
Copy link
Contributor

Verified Testathon 9/12/2023

@unlikelyzero unlikelyzero added type:feature Feature. Required intentional design performance impacts or improves performance and removed type:maintenance tests, chores, or project maintenance labels Sep 12, 2023
@ozyx ozyx removed the unverified label Oct 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
performance impacts or improves performance type:feature Feature. Required intentional design
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants