Skip to content

Commit

Permalink
Reduce freeze times on updating search results on resource changes
Browse files Browse the repository at this point in the history
If the search view shows ~100000 matches, deleting resources that are
shown in the search view freezes Eclipse for many minutes. With this
change the freeze is only about 30 seconds.

See #2279
  • Loading branch information
iloveeclipse authored and elsazac committed Nov 8, 2024
1 parent 4f7c5d7 commit 9514d38
Show file tree
Hide file tree
Showing 6 changed files with 161 additions and 56 deletions.
1 change: 1 addition & 0 deletions bundles/org.eclipse.search/META-INF/MANIFEST.MF
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ Bundle-ManifestVersion: 2
Bundle-Name: %pluginName
Bundle-SymbolicName: org.eclipse.search; singleton:=true
Bundle-Version: 3.16.100.qualifier
Bundle-Version: 3.17.0.qualifier
Bundle-Activator: org.eclipse.search.internal.ui.SearchPlugin
Bundle-ActivationPolicy: lazy
Bundle-Vendor: %providerName
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,11 @@
import java.util.HashSet;
import java.util.Iterator;
import java.util.List;
import java.util.Map.Entry;
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 @@ -43,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 @@ -53,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 @@ -155,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 @@ -177,6 +181,7 @@ public void removeAll() {
fireChange(new RemoveAllEvent(this));
}
private void doRemoveAll() {
matchCount.set(0);
fElementsToMatches.clear();
}

Expand Down Expand Up @@ -214,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 @@ -301,11 +307,47 @@ 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;
}
// Changed once again, fetch again latest value
return getMatchCount();
}

/**
* @return {@code true} if the result is not empty
* @since 3.17
*/
public boolean hasMatches() {
for (Entry<Object, Set<Match>> entry : fElementsToMatches.entrySet()) {
if (!entry.getValue().isEmpty()) {
return true;
}
}
return false;
}

/**
* @return {@code true} if the result is not empty
* @since 3.17
*/
public boolean hasMatches() {
for (Entry<Object, Set<Match>> entry : fElementsToMatches.entrySet()) {
if (!entry.getValue().isEmpty()) {
return true;
}
}
return count;
return false;
}

/**
Expand Down Expand Up @@ -337,6 +379,18 @@ public Object[] getElements() {
return fElementsToMatches.keySet().toArray();
}

/**
* Returns a number of all elements that have matches. Note that count of
* all elements that contain matches are returned. The filter state of the
* matches is not relevant.
*
* @return the number of elements in this search result
* @since 3.17
*/
public int getElementsCount() {
return fElementsToMatches.size();
}

/**
* Sets the active match filters for this result. If set to non-null, the match filters will be used to update the filter
* state ({@link Match#isFiltered()} of matches and the {@link AbstractTextSearchViewPage} will only
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,14 +25,24 @@
import java.util.Objects;
import java.util.Set;

import org.eclipse.swt.dnd.DND;
import org.eclipse.swt.dnd.Transfer;
import org.eclipse.swt.widgets.Display;
import org.eclipse.swt.widgets.Item;
import org.eclipse.swt.widgets.Table;
import org.eclipse.swt.widgets.Tree;

import org.eclipse.core.runtime.IAdaptable;

import org.eclipse.core.resources.IContainer;
import org.eclipse.core.resources.IFile;
import org.eclipse.core.resources.IResource;

import org.eclipse.core.filebuffers.FileBuffers;
import org.eclipse.core.filebuffers.ITextFileBuffer;
import org.eclipse.core.filebuffers.ITextFileBufferManager;
import org.eclipse.core.filebuffers.LocationKind;
import org.eclipse.core.resources.IContainer;
import org.eclipse.core.resources.IFile;
import org.eclipse.core.resources.IResource;
import org.eclipse.core.runtime.IAdaptable;

import org.eclipse.jface.action.IMenuManager;
import org.eclipse.jface.action.MenuManager;
import org.eclipse.jface.dialogs.ErrorDialog;
Expand All @@ -50,21 +60,7 @@
import org.eclipse.jface.viewers.TreeViewer;
import org.eclipse.jface.viewers.Viewer;
import org.eclipse.jface.viewers.ViewerComparator;
import org.eclipse.search.internal.ui.Messages;
import org.eclipse.search.internal.ui.SearchMessages;
import org.eclipse.search.ui.IContextMenuConstants;
import org.eclipse.search.ui.ISearchResultViewPart;
import org.eclipse.search.ui.NewSearchUI;
import org.eclipse.search.ui.text.AbstractTextSearchResult;
import org.eclipse.search.ui.text.AbstractTextSearchViewPage;
import org.eclipse.search.ui.text.Match;
import org.eclipse.search2.internal.ui.OpenSearchPreferencesAction;
import org.eclipse.swt.dnd.DND;
import org.eclipse.swt.dnd.Transfer;
import org.eclipse.swt.widgets.Display;
import org.eclipse.swt.widgets.Item;
import org.eclipse.swt.widgets.Table;
import org.eclipse.swt.widgets.Tree;

import org.eclipse.ui.IMemento;
import org.eclipse.ui.IPageLayout;
import org.eclipse.ui.IWorkbenchPage;
Expand All @@ -77,6 +73,17 @@
import org.eclipse.ui.part.ResourceTransfer;
import org.eclipse.ui.part.ShowInContext;

import org.eclipse.search.internal.ui.Messages;
import org.eclipse.search.internal.ui.SearchMessages;
import org.eclipse.search.ui.IContextMenuConstants;
import org.eclipse.search.ui.ISearchResultViewPart;
import org.eclipse.search.ui.NewSearchUI;
import org.eclipse.search.ui.text.AbstractTextSearchResult;
import org.eclipse.search.ui.text.AbstractTextSearchViewPage;
import org.eclipse.search.ui.text.Match;

import org.eclipse.search2.internal.ui.OpenSearchPreferencesAction;


public class FileSearchPage extends AbstractTextSearchViewPage implements IAdaptable {

Expand Down Expand Up @@ -432,29 +439,22 @@ private boolean isQueryRunning() {
@Override
public String getLabel() {
String label= super.getLabel();
StructuredViewer viewer= getViewer();
AbstractTextSearchResult result = getInput();
String msg = label;
if (result != null) {
if (viewer instanceof TableViewer) {
TableViewer tv = (TableViewer) viewer;

int itemCount = ((FileTableContentProvider) tv.getContentProvider())
.getUnfilteredElements(getInput()).length;
if (showLineMatches()) {
int matchCount= getInput().getMatchCount();
if (itemCount < matchCount) {
msg = Messages.format(SearchMessages.FileSearchPage_limited_format_matches,
new Object[] { label, Integer.valueOf(itemCount), Integer.valueOf(matchCount) });
}
} else {
int fileCount= getInput().getElements().length;
if (itemCount < fileCount) {
msg = Messages.format(SearchMessages.FileSearchPage_limited_format_files,
new Object[] { label, Integer.valueOf(itemCount), Integer.valueOf(fileCount) });
}
int itemCount = fContentProvider.getLeafCount(result);
if (showLineMatches()) {
int matchCount = result.getMatchCount();
if (itemCount < matchCount) {
msg = Messages.format(SearchMessages.FileSearchPage_limited_format_matches,
new Object[] { label, Integer.valueOf(itemCount), Integer.valueOf(matchCount) });
}
} else {
int fileCount = result.getElementsCount();
if (itemCount < fileCount) {
msg = Messages.format(SearchMessages.FileSearchPage_limited_format_files,
new Object[] { label, Integer.valueOf(itemCount), Integer.valueOf(fileCount) });
}

}
if (result.getActiveMatchFilters() != null && result.getActiveMatchFilters().length > 0) {
if (isQueryRunning()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import org.eclipse.jface.viewers.IStructuredContentProvider;
import org.eclipse.jface.viewers.TableViewer;
import org.eclipse.jface.viewers.Viewer;

import org.eclipse.search.ui.text.AbstractTextSearchResult;

public class FileTableContentProvider implements IStructuredContentProvider, IFileSearchContentProvider {
Expand All @@ -37,15 +38,17 @@ public void dispose() {
// nothing to do
}

public Object[] getUnfilteredElements(AbstractTextSearchResult searchResult) {
@Override
public int getLeafCount(Object parentElement) {
if (!(parentElement instanceof AbstractTextSearchResult searchResult)) {
return 0;
}
int elementLimit = getElementLimit();
Object[] elements = searchResult.getElements();
if (elementLimit != -1 && elements.length > elementLimit) {
Object[] shownElements = new Object[elementLimit];
System.arraycopy(elements, 0, shownElements, 0, elementLimit);
return shownElements;
int elementsCount = searchResult.getElementsCount();
if (elementLimit != -1 && elementsCount > elementLimit) {
return elementLimit;
}
return elements;
return elementsCount;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,11 @@

import org.eclipse.core.resources.IProject;
import org.eclipse.core.resources.IResource;

import org.eclipse.jface.viewers.AbstractTreeViewer;
import org.eclipse.jface.viewers.ITreeContentProvider;
import org.eclipse.jface.viewers.Viewer;

import org.eclipse.search.ui.text.AbstractTextSearchResult;
import org.eclipse.search.ui.text.Match;
import org.eclipse.search.ui.text.MatchFilter;
Expand All @@ -52,6 +54,7 @@ public class FileTreeContentProvider implements ITreeContentProvider, IFileSearc
FileTreeContentProvider(FileSearchPage page, AbstractTreeViewer viewer) {
fPage= page;
fTreeViewer= viewer;
fChildrenMap = new HashMap<>();
}

@Override
Expand Down Expand Up @@ -177,17 +180,27 @@ private boolean hasMatches(Object element) {
if (element instanceof LineElement) {
LineElement lineElement= (LineElement) element;
IResource resource = lineElement.getParent();
if (getMatchCount(resource) > 0) {
if (hasMatches(resource)) {
return lineElement.hasMatches(fResult);
}
}
return fPage.getDisplayedMatchCount(element) > 0;
}

private boolean hasMatches(IResource element) {
if (hasActiveMatchFilters()) {
return fPage.getDisplayedMatchCount(element) > 0;
} else {
return fResult.hasMatches();
}
}

private int getMatchCount(Object element) {
return fResult.getActiveMatchFilters() != null && fResult.getActiveMatchFilters().length > 0
? fPage.getDisplayedMatchCount(element)
: fResult.getMatchCount();
if (hasActiveMatchFilters()) {
return fPage.getDisplayedMatchCount(element);
} else {
return fResult.getMatchCount();
}
}

private void removeFromSiblings(Object element, Object parent) {
Expand All @@ -205,9 +218,31 @@ public Object[] getChildren(Object parentElement) {
return children.toArray();
}

@Override
public int getLeafCount(Object parentElement) {
Object[] children = getChildren(parentElement);
if (children.length == 0) {
return 0;
}
int count = 0;
for (Object object : children) {
boolean leaf = !hasChildren(object);
if (leaf) {
count++;
} else {
count += getLeafCount(object);
}
}
return count;
}

@Override
public boolean hasChildren(Object element) {
return getChildren(element).length > 0;
Set<Object> children = fChildrenMap.get(element);
if (children == null) {
return false;
}
return !children.isEmpty();
}

static <T> Stream<T> toStream(Enumeration<T> e) {
Expand Down Expand Up @@ -243,7 +278,7 @@ public synchronized void elementsChanged(Object[] updatedElements) {
Set<LineElement> lineMatches = Collections.emptySet();
// if we have active match filters, we should only use non-filtered FileMatch
// objects to collect LineElements to update
if (fResult.getActiveMatchFilters() != null && fResult.getActiveMatchFilters().length > 0) {
if (hasActiveMatchFilters()) {
lineMatches = Arrays.stream(updatedElements).filter(LineElement.class::isInstance)
// only for distinct files:
.map(u -> ((LineElement) u).getParent()).distinct()
Expand Down Expand Up @@ -292,6 +327,11 @@ public synchronized void elementsChanged(Object[] updatedElements) {
}
}

private boolean hasActiveMatchFilters() {
MatchFilter[] activeMatchFilters = fResult.getActiveMatchFilters();
return activeMatchFilters != null && activeMatchFilters.length > 0;
}

@Override
public void clear() {
initialize(fResult);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,4 +19,11 @@ public interface IFileSearchContentProvider {

public abstract void clear();

}
/**
* @param parentElement
* parent element or input
* @return number of leaf elements in the tree maintained by the provider
*/
public abstract int getLeafCount(Object parentElement);

}

0 comments on commit 9514d38

Please sign in to comment.