Skip to content

Commit

Permalink
Prevent memory leak in CandleStickChart & VolumeChart
Browse files Browse the repository at this point in the history 
Ensure that the superclass methods XYChart.removeDataItemFromDisplay &
XYChart.removeSeriesFromDisplay are always called from the implemented
dataItemRemoved & seriesRemoved methods respectively, as specified by
the API javadoc.

This prevents a leak of old Candle & VolumeBar objects every time the
trades charts view is updated. The former is quite substantial, as each
Candle object has a retained size of about 70kB and there are up to 90
candlesticks / volume bars leaked per chart update.
  • Loading branch information
@stejbac
stejbac committed Jan 22, 2020
1 parent d4e566f commit ab2ad33
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 12 deletions.
17 changes: 12 additions & 5 deletions desktop/src/main/java/bisq/desktop/main/market/trades/charts/price/CandleStickChart.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -162,8 +162,7 @@ protected void dataItemChanged(XYChart.Data<Number, Number> item) {
@Override
protected void dataItemAdded(XYChart.Series<Number, Number> series, int itemIndex, XYChart.Data<Number, Number> item) {
Node candle = createCandle(getData().indexOf(series), item, itemIndex);
if (getPlotChildren().contains(candle))
getPlotChildren().remove(candle);
getPlotChildren().remove(candle);

if (shouldAnimate()) {
candle.setOpacity(0);
Expand Down Expand Up @@ -195,18 +194,22 @@ protected void dataItemRemoved(XYChart.Data<Number, Number> item, XYChart.Series
// fade out old candle
FadeTransition ft = new FadeTransition(Duration.millis(500), node);
ft.setToValue(0);
ft.setOnFinished((ActionEvent actionEvent) -> getPlotChildren().remove(node));
ft.setOnFinished((ActionEvent actionEvent) -> {
getPlotChildren().remove(node);
removeDataItemFromDisplay(series, item);
});
ft.play();
} else {
getPlotChildren().remove(node);
removeDataItemFromDisplay(series, item);
}
}

@Override
protected void seriesAdded(XYChart.Series<Number, Number> series, int seriesIndex) {
// handle any data already in series
for (int j = 0; j < series.getData().size(); j++) {
XYChart.Data item = series.getData().get(j);
XYChart.Data<Number, Number> item = series.getData().get(j);
Node candle = createCandle(seriesIndex, item, j);

if (!getPlotChildren().contains(candle)) {
Expand Down Expand Up @@ -256,12 +259,16 @@ protected void seriesRemoved(XYChart.Series<Number, Number> series) {
ft.setOnFinished((ActionEvent actionEvent) -> {
getPlotChildren().remove(seriesPath);
seriesPath.getElements().clear();
removeSeriesFromDisplay(series);
});
ft.play();
} else {
getPlotChildren().remove(seriesPath);
seriesPath.getElements().clear();
removeSeriesFromDisplay(series);
}
} else {
removeSeriesFromDisplay(series);
}
}

Expand All @@ -273,7 +280,7 @@ protected void seriesRemoved(XYChart.Series<Number, Number> series) {
* @param itemIndex The index of the data item in the series
* @return New candle node to represent the give data item
*/
private Node createCandle(int seriesIndex, final XYChart.Data item, int itemIndex) {
private Node createCandle(int seriesIndex, final XYChart.Data<Number, Number> item, int itemIndex) {
Node candle = item.getNode();
// check if candle has already been created
if (candle instanceof Candle) {
Expand Down
23 changes: 16 additions & 7 deletions desktop/src/main/java/bisq/desktop/main/market/trades/charts/volume/VolumeChart.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,10 @@
package bisq.desktop.main.market.trades.charts.volume;

import bisq.desktop.main.market.trades.charts.CandleData;
import bisq.desktop.main.market.trades.charts.price.CandleStickChart;

import javafx.animation.FadeTransition;
import javafx.animation.KeyFrame;
import javafx.animation.Timeline;

import javafx.scene.Node;
import javafx.scene.chart.Axis;
Expand All @@ -40,7 +41,7 @@
import org.slf4j.LoggerFactory;

public class VolumeChart extends XYChart<Number, Number> {
private static final Logger log = LoggerFactory.getLogger(CandleStickChart.class);
private static final Logger log = LoggerFactory.getLogger(VolumeChart.class);

private final StringConverter<Number> toolTipStringConverter;

Expand Down Expand Up @@ -91,8 +92,7 @@ protected void dataItemChanged(XYChart.Data<Number, Number> item) {
@Override
protected void dataItemAdded(XYChart.Series<Number, Number> series, int itemIndex, XYChart.Data<Number, Number> item) {
Node volumeBar = createCandle(getData().indexOf(series), item, itemIndex);
if (getPlotChildren().contains(volumeBar))
getPlotChildren().remove(volumeBar);
getPlotChildren().remove(volumeBar);

if (shouldAnimate()) {
volumeBar.setOpacity(0);
Expand All @@ -111,17 +111,21 @@ protected void dataItemRemoved(XYChart.Data<Number, Number> item, XYChart.Series
if (shouldAnimate()) {
FadeTransition ft = new FadeTransition(Duration.millis(500), node);
ft.setToValue(0);
ft.setOnFinished((ActionEvent actionEvent) -> getPlotChildren().remove(node));
ft.setOnFinished((ActionEvent actionEvent) -> {
getPlotChildren().remove(node);
removeDataItemFromDisplay(series, item);
});
ft.play();
} else {
getPlotChildren().remove(node);
removeDataItemFromDisplay(series, item);
}
}

@Override
protected void seriesAdded(XYChart.Series<Number, Number> series, int seriesIndex) {
for (int j = 0; j < series.getData().size(); j++) {
XYChart.Data item = series.getData().get(j);
XYChart.Data<Number, Number> item = series.getData().get(j);
Node volumeBar = createCandle(seriesIndex, item, j);
if (shouldAnimate()) {
volumeBar.setOpacity(0);
Expand All @@ -148,9 +152,14 @@ protected void seriesRemoved(XYChart.Series<Number, Number> series) {
getPlotChildren().remove(volumeBar);
}
}
if (shouldAnimate()) {
new Timeline(new KeyFrame(Duration.millis(500), event -> removeSeriesFromDisplay(series))).play();
} else {
removeSeriesFromDisplay(series);
}
}

private Node createCandle(int seriesIndex, final XYChart.Data item, int itemIndex) {
private Node createCandle(int seriesIndex, final XYChart.Data<Number, Number> item, int itemIndex) {
Node volumeBar = item.getNode();
if (volumeBar instanceof VolumeBar) {
((VolumeBar) volumeBar).setSeriesAndDataStyleClasses("series" + seriesIndex, "data" + itemIndex);
Expand Down

0 comments on commit ab2ad33

Please sign in to comment.