Search View: Performance issues on remove and sort

bundles/org.eclipse.search/newsearch/org/eclipse/search/ui/text/AbstractTextSearchResult.java

@@ -14,7 +14,6 @@
 package org.eclipse.search.ui.text;
 
 import java.util.ArrayList;
-import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.Enumeration;
@@ -23,8 +22,10 @@
 import java.util.Set;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.ConcurrentMap;
+import java.util.concurrent.ConcurrentSkipListSet;
 import java.util.concurrent.CopyOnWriteArrayList;
 import java.util.concurrent.atomic.AtomicInteger;
+import java.util.concurrent.atomic.AtomicLong;
 
 import org.eclipse.search.ui.ISearchResult;
 import org.eclipse.search.ui.ISearchResultListener;
@@ -44,6 +45,8 @@ public abstract class AbstractTextSearchResult implements ISearchResult {
 	private final ConcurrentMap<Object, Set<Match>> fElementsToMatches;
 	private final List<ISearchResultListener> fListeners;
 	private final AtomicInteger matchCount;
+	private final ConcurrentHashMap<Match, Long> collisionOrder;
+	private final AtomicLong collisionCounter;
 
 	private MatchFilter[] fMatchFilters;
 
@@ -55,6 +58,8 @@ protected AbstractTextSearchResult() {
 		fListeners = new CopyOnWriteArrayList<>();
 		matchCount = new AtomicInteger(0);
 		fMatchFilters= null; // filtering disabled by default
+		collisionOrder = new ConcurrentHashMap<>();
+		collisionCounter = new AtomicLong(0);
 	}
 
 	/**
@@ -78,9 +83,7 @@ public Match[] getMatches(Object element) {
 		}
 		Set<Match> matches = fElementsToMatches.get(element);
 		if (matches != null) {
-			Match[] sortingCopy = matches.toArray(new Match[matches.size()]);
-			Arrays.sort(sortingCopy, AbstractTextSearchResult::compare);
-			return sortingCopy;
+			return matches.toArray(new Match[0]);
 		}
 		return EMPTY_ARRAY;
 	}
@@ -161,15 +164,36 @@ private MatchEvent getSearchResultEvent(Collection<Match> matches, int eventKind
 	private boolean didAddMatch(Match match) {
 		matchCount.set(0);
 		updateFilterState(match);
-		return fElementsToMatches.computeIfAbsent(match.getElement(), k -> ConcurrentHashMap.newKeySet()).add(match);
+		return fElementsToMatches.computeIfAbsent(match.getElement(),
+				k -> new ConcurrentSkipListSet<>(this::compare)).add(match);
 	}
 
-	private static int compare(Match match2, Match match1) {
-		int diff= match2.getOffset()-match1.getOffset();
+	private int compare(Match match2, Match match1) {
+		if (match1 == match2) {
+			return 0;
+		}
+		int diff = match2.getOffset() - match1.getOffset();
+		if (diff != 0) {
+			return diff;
+		}
+		diff = match2.getLength() - match1.getLength();
 		if (diff != 0) {
 			return diff;
 		}
-		return match2.getLength()-match1.getLength();
+		diff = Integer.compare(System.identityHashCode(match2), System.identityHashCode(match1));
+		if (diff != 0) {
+			return diff;
+		}
+		// Identity hash collision for two distinct objects: use a stable
+		// tiebreaker so the set does not treat them as duplicates.
+		return resolveCollision(match2, match1);
+	}
+
+	@SuppressWarnings("boxing")
+	private int resolveCollision(Match a, Match b) {
+		long orderA = collisionOrder.computeIfAbsent(a, k -> collisionCounter.incrementAndGet());
+		long orderB = collisionOrder.computeIfAbsent(b, k -> collisionCounter.incrementAndGet());
+		return Long.compare(orderA, orderB);
 	}
 
 	/**
@@ -185,6 +209,7 @@ public void removeAll() {
 	private void doRemoveAll() {
 		matchCount.set(0);
 		fElementsToMatches.clear();
+		collisionOrder.clear();
 	}
 
 	/**
@@ -233,6 +258,9 @@ private boolean didRemoveMatch(Match match) {
 			}
 			return matches;
 		});
+		if (existed[0]) {
+			collisionOrder.remove(match);
+		}
 		return existed[0];
 	}
 
@@ -333,6 +361,25 @@ public boolean hasMatches() {
 		return false;
 	}
 
+	/**
+	 * Returns whether the given element has any matches in this search result.
+	 *
+	 * @param element
+	 *            the element to test for matches
+	 * @return {@code true} if the given element has at least one match
+	 * @since 3.18
+	 */
+	public boolean hasMatches(Object element) {
+		if (element == null) {
+			return false;
+		}
+		Set<Match> matches = fElementsToMatches.get(element);
+		if (matches != null) {
+			return !matches.isEmpty();
+		}
+		return false;
+	}
+
 	/**
 	 * Returns the number of matches reported against a given element. This is
 	 * equivalent to calling <code>getMatches(element).length</code>
@@ -442,4 +489,28 @@ public MatchFilter[] getAllMatchFilters() {
 	 * @see IFileMatchAdapter
 	 */
 	public abstract IFileMatchAdapter getFileMatchAdapter();
+
+	/**
+	 * Batch removing of matches by using their enclosing elements.
+	 *
+	 * @param elements
+	 *            the elements of which matches should be removed.
+	 * @since 3.18
+	 */
+	public void removeElements(Collection<?> elements) {
+		matchCount.set(0);
+		List<Match> removedMatches = new ArrayList<>();
+		for (Object object : elements) {
+			Set<Match> matches = fElementsToMatches.remove(object);
+			if (matches != null) {
+				removedMatches.addAll(matches);
+			}
+		}
+		if (!removedMatches.isEmpty()) {
+			if (!collisionOrder.isEmpty()) {
+				removedMatches.forEach(collisionOrder::remove);
+			}
+			fireChange(getSearchResultEvent(removedMatches, MatchEvent.REMOVED));
+		}
+	}
 }

bundles/org.eclipse.search/newsearch/org/eclipse/search/ui/text/AbstractTextSearchViewPage.java

@@ -18,6 +18,7 @@
 import java.util.Collection;
 import java.util.Collections;
 import java.util.HashSet;
+import java.util.List;
 import java.util.Objects;
 import java.util.Set;
 import java.util.concurrent.LinkedBlockingDeque;
@@ -41,7 +42,9 @@
 import org.eclipse.core.runtime.SafeRunner;
 import org.eclipse.core.runtime.Status;
 
+import org.eclipse.core.resources.IContainer;
 import org.eclipse.core.resources.IFile;
+import org.eclipse.core.resources.IResource;
 
 import org.eclipse.jface.action.Action;
 import org.eclipse.jface.action.IAction;
@@ -61,6 +64,7 @@
 import org.eclipse.jface.viewers.ITreeContentProvider;
 import org.eclipse.jface.viewers.OpenEvent;
 import org.eclipse.jface.viewers.SelectionChangedEvent;
+import org.eclipse.jface.viewers.StructuredSelection;
 import org.eclipse.jface.viewers.StructuredViewer;
 import org.eclipse.jface.viewers.TableViewer;
 import org.eclipse.jface.viewers.TreeViewer;
@@ -645,7 +649,7 @@ protected void postEnsureSelection() {
 
 	private void updateBusyLabel() {
 		AbstractTextSearchResult result = getInput();
-		boolean shouldShowBusy = result != null && NewSearchUI.isQueryRunning(result.getQuery()) && result.getMatchCount() == 0;
+		boolean shouldShowBusy = result != null && NewSearchUI.isQueryRunning(result.getQuery()) && !result.hasMatches();
 		if (shouldShowBusy == fIsBusyShown) {
 			return;
 		}
@@ -1387,6 +1391,10 @@ public void internalRemoveSelected() {
 		StructuredViewer viewer = getViewer();
 		IStructuredSelection selection = viewer.getStructuredSelection();
 
+		selection = handleRemovalOfResources(result, selection);
+		if (selection.isEmpty()) {
+			return;
+		}
 		HashSet<Match> set = new HashSet<>();
 		if (viewer instanceof TreeViewer) {
 			ITreeContentProvider cp = (ITreeContentProvider) viewer.getContentProvider();
@@ -1401,6 +1409,35 @@ public void internalRemoveSelected() {
 		result.removeMatches(matches);
 	}
 
+	private IStructuredSelection handleRemovalOfResources(AbstractTextSearchResult result,
+			IStructuredSelection selection) {
+		Set<IResource> elementsToRemove = new HashSet<>();
+		List<Object> others = new ArrayList<>();
+		Object[] resultElements = result.getElements();
+		for (Object selectedObj : selection) {
+			if (selectedObj instanceof IFile resource) {
+				elementsToRemove.add(resource);
+			} else if (selectedObj instanceof IContainer container) {
+				for (Object resElement : resultElements) {
+					if (resElement instanceof IResource res && container.getFullPath().isPrefixOf(res.getFullPath())) {
+						elementsToRemove.add(res);
+					}
+				}
+			} else {
+				others.add(selectedObj);
+			}
+		}
+		if (!elementsToRemove.isEmpty()) {
+			result.removeElements(elementsToRemove);
+			if (others.isEmpty()) {
+				navigateNext(true);
+				return StructuredSelection.EMPTY;
+			}
+			selection = new StructuredSelection(others);
+		}
+		return selection;
+	}
+
 	private void collectAllMatches(HashSet<Match> set, Object[] elements) {
 		for (Object element : elements) {
 			Match[] matches = getDisplayedMatches(element);