Hash tree paths incrementally when restoring the expansion state

bundles/org.eclipse.jface/src/org/eclipse/jface/viewers/AbstractTreeViewer.java

@@ -2202,27 +2202,54 @@ private void internalSetExpanded(CustomHashtable expandedElements,
 	 *            which are expanded
 	 * @param widget
 	 *            the widget
+	 * @param currentHash
+	 *            the hash {@link TreePath#hashCode(IElementComparer)} would return
+	 *            for <code>currentPath</code>
 	 */
 	private void internalSetExpandedTreePaths(
 			CustomHashtable expandedTreePaths, Widget widget,
-			TreePath currentPath) {
+			TreePath currentPath, int currentHash, IElementComparer comparer) {
 		Item[] items = getChildren(widget);
 		for (Item item : items) {
+			if (expandedTreePaths.size() == 0) {
+				// Every path that had to be expanded has been found, so the rest of the
+				// tree can only be collapsed, which needs neither a path nor its hash.
+				internalCollapseSubtree(item);
+				continue;
+			}
 			Object data = item.getData();
 			TreePath childPath = data == null ? null : currentPath
 					.createChildPath(data);
+			int childHash = currentHash;
 			if (data != null && childPath != null) {
+				// A tree path hashes as the sum of its segments, so the child's hash
+				// follows from the parent's. Computing it from the path instead would
+				// hash every segment again, which is what makes an element with an
+				// expensive hashCode cost the whole traversal.
+				childHash += comparer == null ? data.hashCode() : comparer.hashCode(data);
 				// remove the element to avoid an infinite loop
 				// if the same element appears on a child item
-				boolean expanded = expandedTreePaths.remove(childPath) != null;
+				boolean expanded = expandedTreePaths.remove(childPath, childHash) != null;
 				if (expanded != getExpanded(item)) {
 					if (expanded) {
 						createChildren(item);
 					}
 					setExpanded(item, expanded);
 				}
 			}
-			internalSetExpandedTreePaths(expandedTreePaths, item, childPath);
+			internalSetExpandedTreePaths(expandedTreePaths, item, childPath, childHash, comparer);
+		}
+	}
+
+	/**
+	 * Collapses the given item and everything below it.
+	 */
+	private void internalCollapseSubtree(Item item) {
+		if (item.getData() != null && getExpanded(item)) {
+			setExpanded(item, false);
+		}
+		for (Item child : getChildren(item)) {
+			internalCollapseSubtree(child);
 		}
 	}
 
@@ -2677,7 +2704,7 @@ public int hashCode(Object element) {
 		// equal elements, and those are in the set of elements to be expanded,
 		// only the first item found for each element will be expanded.
 		internalSetExpandedTreePaths(expandedTreePaths, getControl(),
-				new TreePath(new Object[0]));
+				new TreePath(new Object[0]), 0, comparer);
 	}
 
 	/**

bundles/org.eclipse.jface/src/org/eclipse/jface/viewers/CustomHashtable.java

@@ -36,11 +36,19 @@
 	private static class HashMapEntry {
 		Object key, value;
 
+		/**
+		 * The key's hash, kept so that growing the table does not hash the keys again
+		 * and so that a lookup can rule out an entry without comparing the keys. Both
+		 * matter for elements whose hashCode and equals are expensive.
+		 */
+		final int hash;
+
 		HashMapEntry next;
 
-		HashMapEntry(Object theKey, Object theValue) {
+		HashMapEntry(Object theKey, Object theValue, int theHash) {
 			key = theKey;
 			value = theValue;
+			hash = theHash;
 		}
 	}
 
@@ -239,29 +247,29 @@ public Enumeration elements() {
 	 *				does not exist
 	 */
 	public Object get(Object key) {
-		int index = (hashCode(key) & 0x7FFFFFFF) % elementData.length;
-		HashMapEntry entry = elementData[index];
-		while (entry != null) {
-			if (keyEquals(key, entry.key)) {
-				return entry.value;
-			}
-			entry = entry.next;
-		}
-		return null;
+		HashMapEntry entry = getEntry(key);
+		return entry == null ? null : entry.value;
 	}
 
 	private HashMapEntry getEntry(Object key) {
-		int index = (hashCode(key) & 0x7FFFFFFF) % elementData.length;
-		HashMapEntry entry = elementData[index];
+		int hash = hashCode(key);
+		HashMapEntry entry = elementData[indexFor(hash)];
 		while (entry != null) {
-			if (keyEquals(key, entry.key)) {
+			if (entry.hash == hash && keyEquals(key, entry.key)) {
 				return entry;
 			}
 			entry = entry.next;
 		}
 		return null;
 	}
 
+	/**
+	 * Answers the slot the given hash belongs into.
+	 */
+	private int indexFor(int hash) {
+		return (hash & 0x7FFFFFFF) % elementData.length;
+	}
+
 	/**
 	 * Answers the hash code for the given key.
 	 */
@@ -308,23 +316,24 @@ public Enumeration keys() {
 	 */
 	public Object put(Object key, Object value) {
 		if (key != null && value != null) {
-			int index = (hashCode(key) & 0x7FFFFFFF) % elementData.length;
+			int hash = hashCode(key);
+			int index = indexFor(hash);
 			HashMapEntry entry = elementData[index];
-			while (entry != null && !keyEquals(key, entry.key)) {
+			while (entry != null && !(entry.hash == hash && keyEquals(key, entry.key))) {
 				entry = entry.next;
 			}
 			if (entry == null) {
 				if (++elementCount > threshold) {
 					rehash();
-					index = (hashCode(key) & 0x7FFFFFFF) % elementData.length;
+					index = indexFor(hash);
 				}
 				if (index < firstSlot) {
 					firstSlot = index;
 				}
 				if (index > lastSlot) {
 					lastSlot = index;
 				}
-				entry = new HashMapEntry(key, value);
+				entry = new HashMapEntry(key, value, hash);
 				entry.next = elementData[index];
 				elementData[index] = entry;
 				return null;
@@ -352,7 +361,7 @@ private void rehash() {
 		for (int i = elementData.length; --i >= 0;) {
 			HashMapEntry entry = elementData[i];
 			while (entry != null) {
-				int index = (hashCode(entry.key) & 0x7FFFFFFF) % length;
+				int index = (entry.hash & 0x7FFFFFFF) % length;
 				if (index < firstSlot) {
 					firstSlot = index;
 				}
@@ -377,10 +386,30 @@ private void rehash() {
 	 *				did not exist
 	 */
 	public Object remove(Object key) {
+		if (elementCount == 0) {
+			return null;
+		}
+		return remove(key, hashCode(key));
+	}
+
+	/**
+	 * Removes the key/value pair for the given key, whose hash the caller has
+	 * already computed. The hash must be the one this table's comparer would
+	 * produce for the key.
+	 *
+	 * @param key  the key to remove
+	 * @param hash the key's hash
+	 * @return the value associated with the key, or <code>null</code> if the key
+	 *         did not exist
+	 */
+	public Object remove(Object key, int hash) {
+		if (elementCount == 0) {
+			return null;
+		}
 		HashMapEntry last = null;
-		int index = (hashCode(key) & 0x7FFFFFFF) % elementData.length;
+		int index = indexFor(hash);
 		HashMapEntry entry = elementData[index];
-		while (entry != null && !keyEquals(key, entry.key)) {
+		while (entry != null && !(entry.hash == hash && keyEquals(key, entry.key))) {
 			last = entry;
 			entry = entry.next;
 		}

tests/org.eclipse.jface.tests/src/org/eclipse/jface/tests/viewers/AllViewersTests.java

@@ -32,7 +32,8 @@
 		Bug256889TableViewerTest.class, Bug287765Test.class, Bug242231Test.class,
 		EditingSupportValueMismatchTest.class, StyledStringBuilderTest.class,
 		TreeViewerWithLimitTest.class, TreeViewerWithLimitCompatibilityTest.class, TableViewerWithLimitTest.class,
-		TableViewerWithLimitCompatibilityTest.class })
+		TableViewerWithLimitCompatibilityTest.class, TreeViewerExpansionStateTest.class,
+		StructuredViewerElementMapTest.class })
 public class AllViewersTests {
 
 }