From: Jim Procter Date: Mon, 17 Aug 2020 09:03:47 +0000 (+0100) Subject: Merge branch 'patch/JAL-3597_for_Release_2_11_1_Branch' into releases/Release_2_11_1_... X-Git-Tag: Release_2_11_1_1~13^2~12 X-Git-Url: http://source.jalview.org/gitweb/?a=commitdiff_plain;h=f7c18c26f79036d16740e3077f172f8e9a9c2d32;hp=05a051ebe9afb8e7e778bb9270572b47e84ce4c7;p=jalview.git Merge branch 'patch/JAL-3597_for_Release_2_11_1_Branch' into releases/Release_2_11_1_Branch --- diff --git a/src/jalview/gui/PCAPanel.java b/src/jalview/gui/PCAPanel.java index 4f660a2..3512e42 100644 --- a/src/jalview/gui/PCAPanel.java +++ b/src/jalview/gui/PCAPanel.java @@ -141,6 +141,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 ca4f84e..6c9c892 100755 --- a/src/jalview/gui/TreePanel.java +++ b/src/jalview/gui/TreePanel.java @@ -159,6 +159,7 @@ public class TreePanel extends GTreePanel { getViewport().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 adca17e..671bf6b 100644 --- a/src/jalview/renderer/AnnotationRenderer.java +++ b/src/jalview/renderer/AnnotationRenderer.java @@ -150,6 +150,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 8dcd1b3..063d4ab 100644 --- a/src/jalview/viewmodel/AlignmentViewport.java +++ b/src/jalview/viewmodel/AlignmentViewport.java @@ -959,6 +959,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..3a5a9e4 100644 --- a/test/jalview/gui/FreeUpMemoryTest.java +++ b/test/jalview/gui/FreeUpMemoryTest.java @@ -1,17 +1,8 @@ package jalview.gui; -import static org.testng.Assert.assertEquals; import static org.testng.Assert.assertNotNull; import static org.testng.Assert.assertTrue; -import jalview.analysis.AlignmentGenerator; -import jalview.bin.Cache; -import jalview.bin.Jalview; -import jalview.datamodel.AlignmentI; -import jalview.datamodel.SequenceGroup; -import jalview.io.DataSourceType; -import jalview.io.FileLoader; - import java.awt.event.MouseEvent; import java.io.File; import java.io.IOException; @@ -20,20 +11,96 @@ import java.io.PrintStream; import org.testng.annotations.BeforeClass; import org.testng.annotations.Test; +import jalview.analysis.AlignmentGenerator; +import jalview.bin.Cache; +import jalview.bin.Jalview; +import jalview.datamodel.AlignmentI; +import jalview.datamodel.SequenceGroup; +import jalview.io.DataSourceType; +import jalview.io.FileLoader; 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; + /* + * maximum retained heap usage (in MB) for a passing test + */ + private static int MAX_RESIDUAL_HEAP = 45; + /** * Configure (read-only) Jalview property settings for test */ @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,98 +109,79 @@ 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 { 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); - checkUsedMemory(expectedMin); + checkUsedMemory(MAX_RESIDUAL_HEAP); } - private static long getUsedMemory() + /** + * Returns the current total used memory (available memory - free memory), + * rounded down to the nearest MB + * + * @return + */ + private static int getUsedMemory() { - long availableMemory = Runtime.getRuntime().totalMemory() / ONE_MB; - long freeMemory = Runtime.getRuntime().freeMemory() / ONE_MB; + long availableMemory = Runtime.getRuntime().totalMemory(); + long freeMemory = Runtime.getRuntime().freeMemory(); long usedMemory = availableMemory - freeMemory; - return usedMemory; + + return (int) (usedMemory / ONE_MB); } + /** * Requests garbage collection and then checks whether remaining memory in use * is less than the expected value (in Megabytes) * * @param expectedMax */ - protected void checkUsedMemory(long expectedMax) + protected void checkUsedMemory(int expectedMax) { /* - * request garbage collection and wait for it to run; + * request garbage collection and wait for it to run (up to 3 times); * NB there is no guarantee when, or whether, it will do so - * wait time depends on JRE/processor, generous allowance here */ - System.gc(); - waitFor(1500); - - /* - * a second gc() call should not be necessary - but it is! - * the test passes with it, and fails without it - */ - System.gc(); - waitFor(1500); - - /* - * check used memory is 'reasonably low' - */ - long usedMemory = getUsedMemory(); - /* - * sanity check - fails if any frame was added after - * closeAll_actionPerformed - */ - assertEquals(Desktop.instance.getAllFrames().length, 0); + long usedMemory = 0L; + Long minUsedMemory = null; + int gcCount = 0; + while (gcCount < 3) + { + gcCount++; + System.gc(); + waitFor(1500); + usedMemory = getUsedMemory(); + if (minUsedMemory == null || usedMemory < minUsedMemory) + { + minUsedMemory = usedMemory; + } + if (usedMemory < expectedMax) + { + break; + } + } /* - * if this assertion fails - * - set a breakpoint here - * - run jvisualvm to inspect a heap dump of Jalview - * - identify large objects in the heap and their referers + * if this assertion fails (reproducibly!) + * - set a breakpoint here, conditional on (usedMemory > expectedMax) + * - run VisualVM to inspect the heap usage, and run GC from VisualVM to check + * it is not simply delayed garbage collection causing the test failure + * - take a heap dump and 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"); - assertTrue(usedMemory < expectedMax, String.format( + System.out.println("(Minimum) Used memory after " + gcCount + + " call(s) to gc() = " + minUsedMemory + "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 +241,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 +285,7 @@ public class FreeUpMemoryTest int width = 100000; int height = 100; ag.generate(width, height, 0, 10, 15); + ps.close(); return f; } }