From: gmungoc Date: Tue, 14 Jul 2020 08:43:50 +0000 (+0100) Subject: JAL-3597 additional references nulled, bug in test corrected X-Git-Tag: Develop-2_11_2_0-d20201215~24^2~15^2~6^2~4 X-Git-Url: http://source.jalview.org/gitweb/?a=commitdiff_plain;h=9a20ebe6844fd13b704e5b73fe752bd00230aba6;p=jalview.git JAL-3597 additional references nulled, bug in test corrected --- diff --git a/src/jalview/gui/PCAPanel.java b/src/jalview/gui/PCAPanel.java index 96a8b0d..6c76c3e 100644 --- a/src/jalview/gui/PCAPanel.java +++ b/src/jalview/gui/PCAPanel.java @@ -140,6 +140,12 @@ public class PCAPanel extends GPCAPanel protected void close_actionPerformed() { setPcaModel(null); + if (this.rc != null) + { + this.rc.sequencePoints = null; + this.rc.setAxisEndPoints(null); + this.rc = null; + } } @Override diff --git a/src/jalview/gui/TreePanel.java b/src/jalview/gui/TreePanel.java index 37b4dac..2d8e729 100755 --- a/src/jalview/gui/TreePanel.java +++ b/src/jalview/gui/TreePanel.java @@ -159,6 +159,7 @@ public class TreePanel extends GTreePanel { av.removePropertyChangeListener(listener); } + releaseReferences(); } }); @@ -168,6 +169,17 @@ public class TreePanel extends GTreePanel } /** + * Ensure any potentially large object references are nulled + */ + public void releaseReferences() + { + this.tree = null; + this.treeCanvas.tree = null; + this.treeCanvas.nodeHash = null; + this.treeCanvas.nameHash = null; + } + + /** * @return */ protected PropertyChangeListener addAlignmentListener() diff --git a/src/jalview/renderer/AnnotationRenderer.java b/src/jalview/renderer/AnnotationRenderer.java index c61606e..fa2900d 100644 --- a/src/jalview/renderer/AnnotationRenderer.java +++ b/src/jalview/renderer/AnnotationRenderer.java @@ -149,6 +149,7 @@ public class AnnotationRenderer */ public void dispose() { + hiddenColumns = null; hconsensus = null; complementConsensus = null; hStrucConsensus = null; diff --git a/src/jalview/viewmodel/AlignmentViewport.java b/src/jalview/viewmodel/AlignmentViewport.java index 4f399f0..0d26db1 100644 --- a/src/jalview/viewmodel/AlignmentViewport.java +++ b/src/jalview/viewmodel/AlignmentViewport.java @@ -963,6 +963,7 @@ public abstract class AlignmentViewport ranges = null; currentTree = null; selectionGroup = null; + colSel = null; setAlignment(null); } diff --git a/test/jalview/gui/FreeUpMemoryTest.java b/test/jalview/gui/FreeUpMemoryTest.java index 24697c0..277c27e 100644 --- a/test/jalview/gui/FreeUpMemoryTest.java +++ b/test/jalview/gui/FreeUpMemoryTest.java @@ -22,6 +22,69 @@ import org.testng.annotations.Test; import junit.extensions.PA; +/** + * Provides a simple test that memory is released when all windows are closed. + * + * If the test fails, this means that reference(s) to large object(s) have + * failed to be garbage collected. In this case: + * + *

+ *

Fixing memory leaks

+ *

+ * Experience shows that often a reference is retained (directly or indirectly) + * by a Swing (or related) component (for example a {@code MouseListener} or + * {@code ActionListener}). There are two possible approaches to fixing: + *

+ * Add code to 'null unused large object references' until the test passes. For + * a final sanity check, capture the heap dump for a passing test, and satisfy + * yourself that only 'small' or 'harmless' {@code jalview} object instances + * (such as enums or singletons) are left in the heap. + */ public class FreeUpMemoryTest { private static final int ONE_MB = 1000 * 1000; @@ -32,8 +95,9 @@ public class FreeUpMemoryTest @BeforeClass(alwaysRun = true) public void setUp() { - Jalview.main(new String[] { "-nonews", "-props", - "test/jalview/testProps.jvprops" }); + Jalview.main( + new String[] + { "-nonews", "-props", "test/jalview/testProps.jvprops" }); String True = Boolean.TRUE.toString(); Cache.applicationProperties.setProperty("SHOW_ANNOTATIONS", True); Cache.applicationProperties.setProperty("SHOW_QUALITY", True); @@ -42,39 +106,14 @@ public class FreeUpMemoryTest Cache.applicationProperties.setProperty("SHOW_IDENTITY", True); } - /** - * A simple test that memory is released when all windows are closed. - * - * If the test fails, this suggests that a reference to some large object - * (perhaps the alignment data, or some annotation / Tree / PCA data) has - * failed to be garbage collected. If this is the case, the heap will need to - * be inspected manually (suggest using jvisualvm) in order to track down - * where large objects are still referenced. The code (for example - * AlignmentViewport.dispose()) should then be updated to ensure references to - * large objects are set to null when they are no longer required. - * - * @throws IOException - */ @Test(groups = "Memory") public void testFreeMemoryOnClose() throws IOException { + long expectedMin = 37L; + File f = generateAlignment(); f.deleteOnExit(); - long expectedMin = 35L; - long usedMemoryAtStart=getUsedMemory(); - if (usedMemoryAtStart>expectedMin) - { - System.err.println("used memory before test is "+usedMemoryAtStart+" > "+expectedMin+"MB .. adjusting minimum."); - expectedMin = usedMemoryAtStart; - } doStuffInJalview(f); Desktop.instance.closeAll_actionPerformed(null); @@ -89,6 +128,7 @@ public class FreeUpMemoryTest long usedMemory = availableMemory - freeMemory; return usedMemory; } + /** * Requests garbage collection and then checks whether remaining memory in use * is less than the expected value (in Megabytes) @@ -129,11 +169,11 @@ public class FreeUpMemoryTest * - identify large objects in the heap and their referers * - fix code as necessary to null the references on close */ - System.out.println("Used memory after gc = " + usedMemory + "MB"); + System.out.println("Used memory after gc = " + usedMemory + + "MB (should be <" + expectedMax + ")"); assertTrue(usedMemory < expectedMax, String.format( "Used memory %d should be less than %d (Recommend running test manually to verify)", - usedMemory, - expectedMax)); + usedMemory, expectedMax)); } /** @@ -193,7 +233,7 @@ public class FreeUpMemoryTest * wait until Tree and PCA have been computed */ while (af.viewport.getCurrentTree() == null - && dialog.getPcaPanel().isWorking()) + || dialog.getPcaPanel().isWorking()) { waitFor(10); } @@ -237,6 +277,7 @@ public class FreeUpMemoryTest int width = 100000; int height = 100; ag.generate(width, height, 0, 10, 15); + ps.close(); return f; } }