changeset 656:3befd8ddad2f

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
author Omair Majid <omajid@redhat.com>
date Thu, 04 Oct 2012 17:35:18 -0400
parents ec2d7827cfae
children be076fc26f78
files client/heapdumper/src/main/java/com/redhat/thermostat/client/heap/HeapDumpController.java client/heapdumper/src/main/java/com/redhat/thermostat/client/heap/HeapView.java client/heapdumper/src/main/java/com/redhat/thermostat/client/heap/chart/OverviewChart.java client/heapdumper/src/main/java/com/redhat/thermostat/client/heap/swing/HeapSwingView.java
diffstat 4 files changed, 37 insertions(+), 15 deletions(-) [+]
line wrap: on
line diff
--- 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);
             }
         }
     }
--- 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<HeapDump> heapDumps);
+
 }
--- 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(
--- 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);
             }