Puncture the bloat balloon #807
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The widget gallery is removed with no replacement. Live network table value previews are removed. Data must now be viewed only in a widget. This removes unnecessary subscriptions to unused fields (saving network bandwidth) and removes a high-frequency updating pane that is often hidden, which resulted in unnecessarily high CPU usage
NT sources do not provide their current values
|
How coupled are these changes? I think it'd be better to split it into different PRs. |
The live NT preview and widget gallery changes are both resulting from profiling the JavaFX scene graph. The NT subscribe-to-all change naturally falls out of that, since there's no need to have subscriptions that are never used |
Widgets were getting destroyed sources added, but weren't replacing the backing NT source. These widgets would be permanently disabled and would need to be deleted and recreated, or a dashboard application restart, to be usable again Topics written to by the dashboard would not receive "unpublish" events from the server, so they'd always be active and available. The `allUris` observable list was incorrectly adding/removing entries so URIs for those topics could still be removed from the list (disabling connected widgets with a destroyed source) if those URIs happened to be at the same index in the `allUris` list as a removed item in the plugin's URI list Topics written to by the dashboard would not receive a "publish" event from the server on reconnect, so no data sources would be created for them. We now manually trigger the data source creation logic for every topic that still exists in the NT client upon a server reconnect to ensure those sources are recreated
|
Live NT preview is pretty popular. Should Shuffleboard get a pinned message at the top of the tree view pane saying to use AdvantageScope for this? |
PeterJohnson
approved these changes
Oct 12, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Overview
Removes various sources of high CPU usage:
And some bug fixes: