From d7ce5ab37a70cf207e3bfe0045c0df3f1fb98415 Mon Sep 17 00:00:00 2001
From: Marko Luukkainen <marko.luukkainen@semantum.fi>
Date: Tue, 13 Apr 2021 09:23:47 +0300
Subject: [PATCH] Performance optimizations for NatTableGraphExplorer

Fine tuned dispose logic to avoid single item removals when whole GE is
disposed.

Node expand was causing unnecessary events, when it tried to expand
non-expandable node.

#692

Change-Id: I06938693af2b28dc51b5d2d5a0269cfde7121fc7
---
 .../ui/nattable/NatTableGraphExplorer.java    | 21 ++++++++++++++-----
 .../browsing/ui/nattable/TreeNode.java        | 19 +++++++++++++++++
 .../ui/nattable/override/TreeLayer2.java      |  2 ++
 3 files changed, 37 insertions(+), 5 deletions(-)

diff --git a/bundles/org.simantics.browsing.ui.nattable/src/org/simantics/browsing/ui/nattable/NatTableGraphExplorer.java b/bundles/org.simantics.browsing.ui.nattable/src/org/simantics/browsing/ui/nattable/NatTableGraphExplorer.java
index 37eafd4eb..9d12fd9b5 100644
--- a/bundles/org.simantics.browsing.ui.nattable/src/org/simantics/browsing/ui/nattable/NatTableGraphExplorer.java
+++ b/bundles/org.simantics.browsing.ui.nattable/src/org/simantics/browsing/ui/nattable/NatTableGraphExplorer.java
@@ -993,11 +993,15 @@ public class NatTableGraphExplorer extends GraphExplorerImplBase implements Grap
        //       we have to remove all references here to reduce memory consumption.
        //	
        //       Proper fix would be to remove references between QueryCache and GENodeQueryManagers.
+    	
+    	// Clearing explorerContext replaces GECache with dummy implementation, which makes node disposal much faster.
+    	explorerContext.close();
     	if (rootNode != null) {
-     		rootNode.dispose();
+    		// Using fastDispose bypasses item removal from nodeMap, which is cleared later. 
+     		rootNode.fastDispose();
          	rootNode = null;	
         }	
-        explorerContext.dispose();
+    	explorerContext.dispose();
         explorerContext = null;
         processors.clear();
         detachPrimitiveProcessors();
@@ -1026,8 +1030,7 @@ public class NatTableGraphExplorer extends GraphExplorerImplBase implements Grap
         }
         resourceManager.dispose();
         resourceManager = null;
-       		
-        contextToNodeMap.clear(); // should be empty at this point.
+        contextToNodeMap.clear();	
         contextToNodeMap = null;
         if (postSelectionProvider != null) {
         	postSelectionProvider.dispose();
@@ -2394,6 +2397,13 @@ public class NatTableGraphExplorer extends GraphExplorerImplBase implements Grap
             }
         };
         
+        public void close() {
+        	cache.dispose();
+        	cache = new DummyCache();
+        	scheduleList.clear();
+        	autoExpanded.clear();
+        }
+        
         @Override
         public void dispose() {
         	cache.dispose();
@@ -2605,6 +2615,7 @@ public class NatTableGraphExplorer extends GraphExplorerImplBase implements Grap
 		final HashMap<GECacheKey, IGECacheEntry> entries = new HashMap<GECacheKey, IGECacheEntry>();
 		final HashMap<GECacheKey, Set<UIElementReference>> treeReferences = new HashMap<GECacheKey, Set<UIElementReference>>();
 		final HashMap<NodeContext, Set<GECacheKey>> keyRefs = new HashMap<NodeContext, Set<GECacheKey>>();
+		private TObjectIntHashMap<NodeContext> references = new TObjectIntHashMap<NodeContext>();
 		
 		 /**
 	     * This single instance is used for all get operations from the cache. This
@@ -2731,7 +2742,7 @@ public class NatTableGraphExplorer extends GraphExplorerImplBase implements Grap
 	    	return references.get(context) > 0;
 	    }
 
-	    private TObjectIntHashMap<NodeContext> references = new TObjectIntHashMap<NodeContext>();
+	    
 	    
 	    @Override
 	    public void incRef(NodeContext context) {
diff --git a/bundles/org.simantics.browsing.ui.nattable/src/org/simantics/browsing/ui/nattable/TreeNode.java b/bundles/org.simantics.browsing.ui.nattable/src/org/simantics/browsing/ui/nattable/TreeNode.java
index 4f9273d91..529d02f49 100644
--- a/bundles/org.simantics.browsing.ui.nattable/src/org/simantics/browsing/ui/nattable/TreeNode.java
+++ b/bundles/org.simantics.browsing.ui.nattable/src/org/simantics/browsing/ui/nattable/TreeNode.java
@@ -295,6 +295,25 @@ public class TreeNode implements IAdaptable {
 		manager = null;	
 	}
 	
+	/**
+	 * Fast dispose is used to wipe the whole tree.
+	 * 
+	 * ContextToNodeMap is cleared with one command, so we do not need to remove nodes one by one from the map.
+	 */
+	public void fastDispose() {
+		if (DEBUG)	System.out.println("dispose " + this);
+		parent = null;
+		for (TreeNode n : children) {
+			n.fastDispose();
+		}
+		clearCache();
+		children.clear();
+		context = null;
+		explorerContext = null;
+		manager.dispose();
+		manager = null;	
+	}
+	
 	private void clearCache() {
 		if (explorerContext != null) {
 			GECache2 cache = explorerContext.cache;
diff --git a/bundles/org.simantics.browsing.ui.nattable/src/org/simantics/browsing/ui/nattable/override/TreeLayer2.java b/bundles/org.simantics.browsing.ui.nattable/src/org/simantics/browsing/ui/nattable/override/TreeLayer2.java
index a44944ce6..ef09936c4 100644
--- a/bundles/org.simantics.browsing.ui.nattable/src/org/simantics/browsing/ui/nattable/override/TreeLayer2.java
+++ b/bundles/org.simantics.browsing.ui.nattable/src/org/simantics/browsing/ui/nattable/override/TreeLayer2.java
@@ -383,6 +383,8 @@ public class TreeLayer2 extends AbstractRowHideShowLayer2 {
         List<Integer> rowIndexes = this.treeRowModel.expand(parentIndex);
         // Bug 432865: iterating and removing every single item is faster than
         // removeAll()
+        if (rowIndexes.isEmpty())
+        	return;
         for (final Integer expandedChildRowIndex : rowIndexes) {
             this.hiddenRowIndexes.remove(expandedChildRowIndex);
         }
-- 
2.47.1