# HG changeset patch # User Omair Majid # Date 1349386518 14400 # Node ID 3befd8ddad2f345d01518fd21111693fc47930a2 # Parent ec2d7827cfae644637c168df2e1c93a2a5e1fb28 Fix UI hangs caused by Memory Analyzer chart Certain operations in the "Memory Analyzer" tab can cause the entire GUI to free and the client application to hang. This behaviour can be seen if you leave thermostat (agent and client) running for a little while looking at a large java application (I used eclipse in my tests) and trigger heap dumps occasionally. It takes about 10 minutes for this bug to become fully visible. I noticed that the memory chart (that displays used/free memory) is recreated in HeapSwingView whenever HeapDumpController fetches some new data from storage. This creation also causes a clone of the TimeSeries objects containing the _entire_ dataset for the target application. A JFreeChart object is constructed from this but then thrown away after being used once. All this is done on the EDT too, which completely freezes the GUI. The client can only be killed by using kill -9 at this point. This commit changes the ChartPanel from a c.r.t.swing.ChartPanel to a org.jfree.chart.ChartPanel which automatically redraws itself whenever the underlying model changes. It also connects the ChartPanel directly to the model used by the controller so values are automatically updated in the chart without additional copying of memory. It is still possible to for the client to run out of memory and trigger OutOfMemoryError when triggering heap dumps, but at least the UI stays responsive enough and can be killed using ctrl-c on the console. Reviewed-by: jerboaa, neugens, vanaltj Review-thread: http://icedtea.classpath.org/pipermail/thermostat/2012-October/003585.html diff -r ec2d7827cfae -r 3befd8ddad2f client/heapdumper/src/main/java/com/redhat/thermostat/client/heap/HeapDumpController.java --- a/client/heapdumper/src/main/java/com/redhat/thermostat/client/heap/HeapDumpController.java Wed Oct 03 16:21:59 2012 -0400 +++ b/client/heapdumper/src/main/java/com/redhat/thermostat/client/heap/HeapDumpController.java Thu Oct 04 17:35:18 2012 -0400 @@ -107,6 +107,7 @@ timer.setSchedulingType(SchedulingType.FIXED_RATE); view = ApplicationContext.getInstance().getViewFactory().getView(HeapView.class); + view.setModel(model); HeapDump dump = null; view.clearHeapDumpList(); @@ -227,6 +228,7 @@ max =+ space.maxCapacity; } } + // model will automatically update view model.addData(memoryStats.getTimeStamp(), used, capacity); NumberFormat formatter = DecimalFormat.getInstance(); @@ -237,7 +239,7 @@ res = Scale.convertTo(Scale.B, capacity); String _capacity= formatter.format(capacity) + " " + Scale.B; - view.updateOverview(model, _used, _capacity); + view.updateUsedAndCapacity(_used, _capacity); } } } diff -r ec2d7827cfae -r 3befd8ddad2f client/heapdumper/src/main/java/com/redhat/thermostat/client/heap/HeapView.java --- a/client/heapdumper/src/main/java/com/redhat/thermostat/client/heap/HeapView.java Wed Oct 03 16:21:59 2012 -0400 +++ b/client/heapdumper/src/main/java/com/redhat/thermostat/client/heap/HeapView.java Thu Oct 04 17:35:18 2012 -0400 @@ -65,7 +65,9 @@ heapDumperNotifier.removeActionListener(listener); } - abstract public void updateOverview(OverviewChart model, String used, String capacity); + abstract public void updateUsedAndCapacity(String used, String capacity); + /** View updates automatically based on the model */ + abstract public void setModel(OverviewChart model); abstract public void addHeapDump(HeapDump dump); abstract public void clearHeapDumpList(); @@ -74,4 +76,5 @@ public abstract void notifyHeapDumpComplete(); public abstract void updateHeapDumpList(List heapDumps); + } diff -r ec2d7827cfae -r 3befd8ddad2f client/heapdumper/src/main/java/com/redhat/thermostat/client/heap/chart/OverviewChart.java --- a/client/heapdumper/src/main/java/com/redhat/thermostat/client/heap/chart/OverviewChart.java Wed Oct 03 16:21:59 2012 -0400 +++ b/client/heapdumper/src/main/java/com/redhat/thermostat/client/heap/chart/OverviewChart.java Thu Oct 04 17:35:18 2012 -0400 @@ -88,12 +88,8 @@ TimeSeriesCollection dataset = new TimeSeriesCollection(); synchronized (lock) { - try { - dataset.addSeries(total.createCopy(0, total.getItemCount() - 1)); - dataset.addSeries(used.createCopy(0, used.getItemCount() - 1)); - } catch (CloneNotSupportedException e) { - e.printStackTrace(); - } + dataset.addSeries(total); + dataset.addSeries(used); } JFreeChart chart = ChartFactory.createTimeSeriesChart( diff -r ec2d7827cfae -r 3befd8ddad2f client/heapdumper/src/main/java/com/redhat/thermostat/client/heap/swing/HeapSwingView.java --- a/client/heapdumper/src/main/java/com/redhat/thermostat/client/heap/swing/HeapSwingView.java Wed Oct 03 16:21:59 2012 -0400 +++ b/client/heapdumper/src/main/java/com/redhat/thermostat/client/heap/swing/HeapSwingView.java Thu Oct 04 17:35:18 2012 -0400 @@ -45,10 +45,11 @@ import javax.swing.BoxLayout; import javax.swing.JPanel; import javax.swing.SwingUtilities; -import javax.swing.SwingWorker; import javax.swing.event.ListSelectionEvent; import javax.swing.event.ListSelectionListener; +import org.jfree.chart.ChartPanel; + import com.redhat.thermostat.client.heap.HeapView; import com.redhat.thermostat.client.heap.LocaleResources; import com.redhat.thermostat.client.heap.Translate; @@ -57,7 +58,6 @@ import com.redhat.thermostat.client.ui.ComponentVisibleListener; import com.redhat.thermostat.client.ui.SwingComponent; import com.redhat.thermostat.common.heap.HeapDump; -import com.redhat.thermostat.swing.ChartPanel; import com.redhat.thermostat.swing.HeaderPanel; public class HeapSwingView extends HeapView implements SwingComponent { @@ -115,16 +115,37 @@ } @Override - public void updateOverview(final OverviewChart model, final String used, final String capacity) { + public void setModel(final OverviewChart model) { + SwingUtilities.invokeLater(new Runnable() { + @Override + public void run() { + ChartPanel charts = new ChartPanel(model.createChart(stats.getWidth(), stats.getBackground())); + /* + * By default, ChartPanel scales itself instead of redrawing things when + * it's resized. To have it resize automatically, we need to set minimum + * and maximum sizes. Lets constrain the minimum, but not the maximum + * size. + */ + final int MINIMUM_DRAW_SIZE = 100; + + charts.setMinimumDrawHeight(MINIMUM_DRAW_SIZE); + charts.setMinimumDrawWidth(MINIMUM_DRAW_SIZE); + + charts.setMaximumDrawHeight(Integer.MAX_VALUE); + charts.setMaximumDrawWidth(Integer.MAX_VALUE); + + stats.setChartPanel(charts); + } + }); + } + + @Override + public void updateUsedAndCapacity(final String used, final String capacity) { SwingUtilities.invokeLater(new Runnable() { @Override public void run() { - - ChartPanel charts = new ChartPanel(model.createChart(stats.getWidth(), stats.getBackground())); - stats.setChartPanel(charts); - stats.setMax(capacity); stats.setUsed(used); }