Skip to content

Commit

Permalink
Puncture the bloat balloon (#807)
Browse files Browse the repository at this point in the history
Removes various sources of high CPU usage:

- The live network table preview has been removed due to the high frequency of node redraws. Spot checking data will now need to be done by dragging a data widget into a tab. Generic table data should still be displayable with the generic tree table widget. The NT source preview now displays the data type of a topic instead of its current value.
- The subscribe-to-all-NT-data behavior has been changed to only subscribe to topic information. This should cut down on bandwidth and some CPU usage
- The widget gallery has been removed with no replacement. Drawing the large number of dummy widgets took a surprising CPU load and occasionally caused problems at startup when third party widgets from plugins would crash

And some bug fixes:

- Fixes widgets getting permanently disabled after network disconnects
  - This was caused by the placeholder destroyed sources not always being removed when restored, causing the widget to think it still had a disconnected data source
  • Loading branch information
SamCarlberg authored Oct 12, 2024
1 parent 45eb356 commit cdd5e7a
Show file tree
Hide file tree
Showing 23 changed files with 246 additions and 369 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -44,23 +44,23 @@ public class SourceTreeTable<S extends SourceEntry, V> extends TreeTableView<S>
private final ObjectProperty<SourceType> sourceType = new SimpleObjectProperty<>(this, "sourceType", null);

private final TreeTableColumn<S, String> keyColumn = new TreeTableColumn<>("Name");
private final TreeTableColumn<S, V> valueColumn = new TreeTableColumn<>("Value");
private final TreeTableColumn<S, V> infoColumn = new TreeTableColumn<>("Info");

/**
* Creates a new source tree table. It comes pre-populated with a key and a value column.
*/
public SourceTreeTable() {
keyColumn.prefWidthProperty().bind(widthProperty().divide(2).subtract(2));
valueColumn.prefWidthProperty().bind(widthProperty().divide(2).subtract(2));
infoColumn.prefWidthProperty().bind(widthProperty().divide(2).subtract(2));

keyColumn.setCellValueFactory(
f -> new ReadOnlyStringWrapper(getEntryForCellData(f).getViewName()));
valueColumn.setCellValueFactory(
f -> new ReadOnlyObjectWrapper(getEntryForCellData(f).getValueView()));
infoColumn.setCellValueFactory(
f -> new ReadOnlyObjectWrapper(getEntryForCellData(f).getInfo()));
Label placeholder = new Label("No data available");
setPlaceholder(placeholder);

getColumns().addAll(keyColumn, valueColumn);
getColumns().addAll(keyColumn, infoColumn);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,6 @@ public final class DataFormats {
*/
public static final DataFormat source = new DataFormat(APP_PREFIX + "/data-source");

/**
* The data format for widget type names (string).
*/
public static final DataFormat widgetType = new DataFormat(APP_PREFIX + "/widget-type");

/**
* The data format for components that do not exist inside a tile.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,9 @@ default String getViewName() {
Object getValue();

/**
* Gets an object used to display the value of the source this entry represents. Implementers are encouraged to
* Gets an object used to display information about the source. Implementers are encouraged to
* sharpen the return type
*/
Object getValueView();
Object getInfo();

}
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,13 @@
import edu.wpi.first.shuffleboard.api.data.DataType;
import edu.wpi.first.shuffleboard.api.data.DataTypes;
import edu.wpi.first.shuffleboard.api.sources.recording.TimestampedData;
import edu.wpi.first.shuffleboard.api.util.PropertyUtils;
import edu.wpi.first.shuffleboard.api.util.Registry;

import org.fxmisc.easybind.EasyBind;
import javafx.collections.ListChangeListener;

import java.util.HashMap;
import java.util.Map;
import java.util.Objects;
import java.util.Optional;

import javafx.beans.InvalidationListener;
import javafx.collections.FXCollections;
Expand Down Expand Up @@ -49,11 +47,25 @@ public SourceTypes() {
register(Static);

typeNames.addListener((InvalidationListener) __ -> {
Optional<ObservableList<String>> names = typeNames.stream()
typeNames
.stream()
.map(this::forName)
.map(SourceType::getAvailableSourceUris)
.reduce(PropertyUtils::combineLists);
names.ifPresent(l -> EasyBind.listBind(allUris, l));
.forEach(observableUriList -> {
observableUriList.addListener((ListChangeListener<? super String>) c -> {
while (c.next()) {
if (c.wasAdded()) {
for (String uri : c.getAddedSubList()) {
if (!allUris.contains(uri)) {
allUris.add(uri);
}
}
} else if (c.wasRemoved()) {
allUris.removeAll(c.getRemoved());
}
}
});
});
});
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,8 @@

import edu.wpi.first.shuffleboard.api.DashboardMode;
import edu.wpi.first.shuffleboard.api.data.DataType;
import edu.wpi.first.shuffleboard.api.data.DataTypes;
import edu.wpi.first.shuffleboard.api.properties.AtomicBooleanProperty;
import edu.wpi.first.shuffleboard.api.sources.DataSource;
import edu.wpi.first.shuffleboard.api.sources.SourceType;
import edu.wpi.first.shuffleboard.api.sources.SourceTypes;
import edu.wpi.first.shuffleboard.api.sources.recording.serialization.Serializer;
import edu.wpi.first.shuffleboard.api.sources.recording.serialization.Serializers;
import edu.wpi.first.shuffleboard.api.util.ShutdownHooks;
Expand Down Expand Up @@ -135,14 +132,6 @@ public void start() {
startTime = Instant.now();
firstSave = true;
recording = new Recording();
// Record initial conditions
SourceTypes.getDefault().getItems().stream()
.map(SourceType::getAvailableSources)
.forEach(sources -> sources.forEach((id, value) -> {
DataTypes.getDefault().forJavaType(value.getClass())
.map(t -> new TimestampedData(id, t, value, 0L))
.ifPresent(recording::append);
}));
}
setRunning(true);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,10 @@

import edu.wpi.first.shuffleboard.api.data.IncompatibleSourceException;
import edu.wpi.first.shuffleboard.api.sources.DataSource;

import javafx.beans.property.ObjectProperty;
import javafx.beans.property.Property;
import javafx.beans.property.SimpleObjectProperty;
import javafx.collections.ListChangeListener;

/**
* A partial implementation of {@code Widget} that only has a single source.
Expand All @@ -14,6 +14,36 @@ public abstract class SingleSourceWidget extends AbstractWidget {

protected final ObjectProperty<DataSource> source = new SimpleObjectProperty<>(this, "source", DataSource.none());

/**
* Instantiates a new single source widget. This automatically registers listeners to make the
* {@link #source} and {@link #sources} properties stay in sync such that both properties will
* have the same, single data source object.
*/
public SingleSourceWidget() {
// Bidirectional binding to make the sources list act like a single-element wrapper around
// the source property
source.addListener((__, oldSource, newSource) -> sources.setAll(newSource));

sources.addListener(new ListChangeListener<DataSource>() {
@Override
public void onChanged(Change<? extends DataSource> c) {
while (c.next()) {
if (c.wasAdded()) {
var added = c.getAddedSubList();
if (!added.isEmpty()) {
var addedSource = added.get(0);
if (addedSource != source.get()) {
source.set(addedSource);
}
}
} else if (c.wasRemoved()) {
source.set(DataSource.none());
}
}
}
});
}

@Override
public final void addSource(DataSource source) throws IncompatibleSourceException {
if (getDataTypes().contains(source.getDataType())) {
Expand Down
14 changes: 0 additions & 14 deletions api/src/main/resources/edu/wpi/first/shuffleboard/api/base.css
Original file line number Diff line number Diff line change
Expand Up @@ -261,20 +261,6 @@
-fx-stroke-width: 0.25em;
}

/*******************************************************************************
* *
* Widget Gallery *
* *
******************************************************************************/
.widget-gallery .item {
-fx-border-width: 2;
-fx-border-color: -swatch-200;
-fx-border-insets: 5;
-fx-background-insets: 5;
-fx-alignment: center;
-fx-cursor: hand;
}

/*******************************************************************************
* *
* Property sheet *
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ public Object getValue() {
}

@Override
public Object getValueView() {
public Object getInfo() {
return uri;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,13 @@
import edu.wpi.first.shuffleboard.api.util.FxUtils;
import edu.wpi.first.shuffleboard.api.util.StringUtils;
import edu.wpi.first.shuffleboard.api.widget.Component;
import edu.wpi.first.shuffleboard.api.widget.Components;
import edu.wpi.first.shuffleboard.app.components.InteractiveSourceTree;
import edu.wpi.first.shuffleboard.app.components.WidgetGallery;
import edu.wpi.first.shuffleboard.app.plugin.PluginLoader;

import com.google.common.collect.ArrayListMultimap;
import com.google.common.collect.Multimap;

import javafx.scene.control.ScrollPane;
import org.controlsfx.glyphfont.FontAwesome;
import org.controlsfx.glyphfont.Glyph;
import org.controlsfx.glyphfont.GlyphFont;
Expand All @@ -22,7 +21,6 @@

import java.util.Comparator;
import java.util.function.Consumer;
import java.util.stream.Collectors;

import javafx.animation.KeyFrame;
import javafx.animation.KeyValue;
Expand All @@ -36,7 +34,6 @@
import javafx.geometry.Pos;
import javafx.scene.control.Accordion;
import javafx.scene.control.Labeled;
import javafx.scene.control.TabPane;
import javafx.scene.control.TextField;
import javafx.scene.control.TitledPane;
import javafx.scene.control.Tooltip;
Expand All @@ -58,12 +55,10 @@ public final class LeftDrawerController {
@FXML
private Pane root;
@FXML
private TabPane tabs;
private ScrollPane sourceContainer;
@FXML
private Accordion sourcesAccordion;
@FXML
private WidgetGallery widgetGallery;
@FXML
private Pane handle;
@FXML
private Labeled expandContractButton;
Expand All @@ -81,7 +76,8 @@ private void initialize() {
listenToPluginChanges(plugin);
setup(plugin);
});
tabs.maxWidthProperty().bind(root.widthProperty().subtract(handle.widthProperty()));
sourceContainer.maxWidthProperty().bind(root.widthProperty().subtract(handle.widthProperty()));
sourceContainer.minWidthProperty().bind(root.widthProperty().subtract(handle.widthProperty()));
sourcesAccordion.getPanes().sort(Comparator.comparing(TitledPane::getText));
PluginLoader.getDefault().getKnownPlugins().addListener((ListChangeListener<Plugin>) c -> {
while (c.next()) {
Expand Down Expand Up @@ -173,23 +169,16 @@ private void setup(Plugin plugin) {
sourcesAccordion.setExpandedPane(titledPane);
}
});

// Add widgets to the gallery as well
widgetGallery.setWidgets(Components.getDefault().allWidgets().collect(Collectors.toList()));
});
}

/**
* Removes all traces from a plugin from the left drawer. Source trees will be removed and all widgets
* defined by the plugin will be removed from the gallery.
* Removes all traces from a plugin from the left drawer.
*/
private void tearDown(Plugin plugin) {
// Remove the source panes
sourcesAccordion.getPanes().removeAll(sourcePanes.removeAll(plugin));
FXCollections.sort(sourcesAccordion.getPanes(), Comparator.comparing(TitledPane::getText));

// Remove widgets from the gallery
widgetGallery.setWidgets(Components.getDefault().allWidgets().collect(Collectors.toList()));
}

@FXML
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package edu.wpi.first.shuffleboard.app;

import com.google.common.base.Stopwatch;
import edu.wpi.first.shuffleboard.api.plugin.Plugin;
import edu.wpi.first.shuffleboard.api.prefs.Category;
import edu.wpi.first.shuffleboard.api.sources.recording.Recorder;
Expand All @@ -21,6 +22,7 @@
import edu.wpi.first.shuffleboard.app.sources.recording.Playback;
import edu.wpi.first.shuffleboard.app.tab.TabInfoRegistry;

import java.util.concurrent.TimeUnit;
import org.fxmisc.easybind.EasyBind;

import java.awt.Desktop;
Expand Down Expand Up @@ -251,7 +253,15 @@ public void load() throws IOException {
* @throws IOException if the file could not be read from
*/
public void load(File saveFile) throws IOException {
var timer = Stopwatch.createStarted();
setDashboard(saveFileHandler.load(saveFile));
log.info(
"Loaded save file "
+ saveFile.getAbsolutePath()
+ " in "
+ timer.elapsed(TimeUnit.MILLISECONDS)
+ " milliseconds"
);
}

@FXML
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,16 +71,6 @@ public void handle(DragEvent event) {
event.consume();
}

// Dragging a widget from the gallery
if (dragboard.hasContent(DataFormats.widgetType) && tile instanceof LayoutTile) {
String widgetType = (String) dragboard.getContent(DataFormats.widgetType);

dropGalleryWidgetOntoLayout(widgetType, eventPos);
event.consume();

return;
}

// Dragging a source from the sources tree
if (dragboard.hasContent(DataFormats.source) && tile instanceof LayoutTile) {
SourceEntry entry = DeserializationHelper.sourceFromDrag(dragboard.getContent(DataFormats.source));
Expand Down Expand Up @@ -136,18 +126,6 @@ private void dropSourceOntoLayout(Layout layout, SourceEntry entry, Point2D scre
.ifPresent(w -> add(layout, w, screenPos));
}

/**
* Drops a widget from the gallery onto the tile, if it contains a layout.
*
* @param widgetType the type of the widget that is being dragged
* @param screenPos the screen coordinates where the widget was dropped
*/
private void dropGalleryWidgetOntoLayout(String widgetType, Point2D screenPos) {
Components.getDefault().createWidget(widgetType).ifPresent(widget -> {
add((Layout) tile.getContent(), widget, screenPos);
});
}

/**
* Drops multiple tiles onto the tile, if it contains a layout.
*
Expand Down
Loading

0 comments on commit cdd5e7a

Please sign in to comment.