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

Vue makes telemetry collections reactive in Telemetry Tables #5037

Closed
1 of 5 tasks
akhenry opened this issue Apr 6, 2022 · 2 comments · Fixed by #5046
Closed
1 of 5 tasks

Vue makes telemetry collections reactive in Telemetry Tables #5037

akhenry opened this issue Apr 6, 2022 · 2 comments · Fixed by #5046
Assignees
Labels
performance impacts or improves performance type:bug

Comments

@akhenry
Copy link
Contributor

akhenry commented Apr 6, 2022

Summary

In our telemetry tables Vue is making telemetry collections reactive which comes with a pretty heavy performance penalty. Some investigation is required to understand exactly why this is happening.

Expected vs Current Behavior

The expected behavior is that any collections of telemetry (including the tableRows and telemetryCollections) should remain non-reactive after Vue has mounted the telemetry table component. Ideally the entire TelemetryTable object, and all of its direct properties should remain non-reactive.

Pausing while the Telemetry Table component is running and inspecting the TelemetryTable object currently reveals that all of the properties have been replaced with reactive getters and setters:

Screen Shot 2022-04-05 at 6 18 41 PM

If non-reactive, it would look like so:
Screen Shot 2022-04-05 at 6 24 24 PM

Impact Check List

  • Data loss or misrepresented data?
  • Regression? Did this used to work or has it always been broken?
  • Is there a workaround available?
  • Does this impact a critical component?
  • Is this just a visual bug with no functional impact?

Additional Information

Vue is getting a reference to the TelemetryTable object via the TelemetryTable view, which itself is being made reactive by the ActionsCollection in Layout.vue which holds a reference to the view that the actions apply to. This means that all view objects are being reactified, so we need to be careful not to hold any references from them to objects that we do not want to become reactive.

@akhenry akhenry self-assigned this Apr 6, 2022
@unlikelyzero unlikelyzero added the performance impacts or improves performance label Apr 6, 2022
@akhenry
Copy link
Contributor Author

akhenry commented Apr 8, 2022

Testing Notes

This is a non-functional change, but it may introduce regressions in existing table functionality.

  • Verify that tables display telemetry data
  • Verify that filtering and sorting still work as expected
  • Edit a table and verify that columns can be reordered, hidden, and shown as expected.

@michaelrogers
Copy link
Contributor

There do no appear to be any regressions related to the telemetry table changes and so the fix is verified in testathon 04/25/2022.

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:bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants