Skip to content

Commit

Permalink
File search: cache getMatchCount() result to avoid UI hangs
Browse files Browse the repository at this point in the history
If the search view has matches for 100.000 files, it would hang forever
on updating the view via F5 because it asks getMatchCount() for every
element and that traverses entire result again and again.

This change remembers last computed result and returns that value if
there were no further changes on the search result itself.

See eclipse-platform#2279
  • Loading branch information
iloveeclipse authored and elsazac committed Nov 8, 2024
1 parent 7eb62b8 commit b05951e
Showing 1 changed file with 19 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ConcurrentMap;
import java.util.concurrent.atomic.AtomicInteger;

import org.eclipse.search.ui.ISearchResult;
import org.eclipse.search.ui.ISearchResultListener;
Expand All @@ -44,6 +45,7 @@ public abstract class AbstractTextSearchResult implements ISearchResult {
private final ConcurrentMap<Object, Set<Match>> fElementsToMatches;
private final List<ISearchResultListener> fListeners;
private final MatchEvent fMatchEvent;
private final AtomicInteger matchCount;

private MatchFilter[] fMatchFilters;

Expand All @@ -54,7 +56,7 @@ protected AbstractTextSearchResult() {
fElementsToMatches= new ConcurrentHashMap<>();
fListeners= new ArrayList<>();
fMatchEvent= new MatchEvent(this);

matchCount = new AtomicInteger(0);
fMatchFilters= null; // filtering disabled by default
}

Expand Down Expand Up @@ -156,6 +158,7 @@ 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);
}
Expand All @@ -178,6 +181,7 @@ public void removeAll() {
fireChange(new RemoveAllEvent(this));
}
private void doRemoveAll() {
matchCount.set(0);
fElementsToMatches.clear();
}

Expand Down Expand Up @@ -215,6 +219,7 @@ public void removeMatches(Match[] matches) {


private boolean didRemoveMatch(Match match) {
matchCount.set(0);
boolean[] existed = new boolean[1];
fElementsToMatches.computeIfPresent(match.getElement(), (f, matches) -> {
existed[0] = matches.remove(match);
Expand Down Expand Up @@ -302,11 +307,21 @@ private boolean updateFilterState(Match match) {
* @return total number of matches
*/
public int getMatchCount() {
int count = 0;
final int oldCount = matchCount.get();
if (oldCount != 0) {
return oldCount;
}
// The oldCount is zero here => we have to calculate again
int newCount = 0;
for (Set<Match> element : fElementsToMatches.values()) {
count += element.size();
newCount += element.size();
}
if (matchCount.compareAndSet(0, newCount)) {
// Only return if not changed meanwhile
return newCount;
}
return count;
// Changed once again, fetch again latest value
return getMatchCount();
}

/**
Expand Down

0 comments on commit b05951e

Please sign in to comment.