Skip to content

Commit

Permalink
File search: Reliable and fast cancel (eclipse-platform#83)
Browse files Browse the repository at this point in the history 
File search did not cancel it's worker jobs if the user canceled before
the monitorUpdateJob was running (scheduling it is no guarantee that is
already running).
* monitor job is now incorporated into the search thread.
* fixed expected number of files to scan
* do not exit search thread before all workers finished.
* cancel search if any of the workers canceled
* forward cancel immediately (do not wait 100ms)
  • Loading branch information
@jukzi
jukzi authored Sep 5, 2022
1 parent 3d4df84 commit c7d94e3
Show file tree
Hide file tree
Showing 4 changed files with 67 additions and 67 deletions.
2 changes: 1 addition & 1 deletion org.eclipse.search/META-INF/MANIFEST.MF
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ Manifest-Version: 1.0
Bundle-ManifestVersion: 2
Bundle-Name: %pluginName
Bundle-SymbolicName: org.eclipse.search; singleton:=true
Bundle-Version: 3.14.200.qualifier
Bundle-Version: 3.14.300.qualifier
Bundle-Activator: org.eclipse.search.internal.ui.SearchPlugin
Bundle-ActivationPolicy: lazy
Bundle-Vendor: %providerName
Expand Down
128 changes: 65 additions & 63 deletions org.eclipse.search/search/org/eclipse/search/internal/core/text/TextSearchVisitor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@ protected IStatus run(IProgressMonitor inner) {
SubMonitor subMonitor = SubMonitor.convert(inner, fileBatches.size() / jobCount); // approximate
this.fileCharSequenceProvider= new FileCharSequenceProvider();
List<IFile> sameFiles;
while (((sameFiles = fileBatches.poll()) != null) && !fFatalError) {
while (((sameFiles = fileBatches.poll()) != null) && !fFatalError && !fProgressMonitor.isCanceled()) {
IStatus status = processFile(sameFiles, subMonitor.split(1));
// Only accumulate interesting status
if (!status.isOK())
Expand All @@ -190,6 +190,9 @@ protected IStatus run(IProgressMonitor inner) {
// Stop processing and return the status for the completed jobs.
}
fileCharSequenceProvider= null;
synchronized (fLock) {
fLock.notify();
}
return multiStatus;
}

Expand Down Expand Up @@ -291,17 +294,16 @@ public Map<IFile, IDocument> getDocumentsInEditors() {
private final TextSearchRequestor fCollector;
private final Pattern fSearchPattern;

private IProgressMonitor fProgressMonitor;
private volatile IProgressMonitor fProgressMonitor;

private int fNumberOfFilesToScan;
private int fNumberOfScannedFiles; // Protected by fLock
private IFile fCurrentFile; // Protected by fLock
private Object fLock= new Object();
private final Object fLock = new Object();

private final MultiStatus fStatus;
private volatile boolean fFatalError; // If true, terminates the search.

private boolean fIsLightweightAutoRefresh;
private volatile boolean fIsLightweightAutoRefresh;

public TextSearchVisitor(TextSearchRequestor collector, Pattern searchPattern) {
fCollector= collector;
Expand All @@ -317,102 +319,101 @@ public IStatus search(IFile[] files, IProgressMonitor monitor) {
if (files.length == 0) {
return fStatus;
}
fProgressMonitor= monitor == null ? new NullProgressMonitor() : monitor;
fNumberOfScannedFiles= 0;
fNumberOfFilesToScan= files.length;
fCurrentFile= null;

fProgressMonitor = monitor == null ? new NullProgressMonitor() : monitor;
synchronized (fLock) {
fNumberOfScannedFiles = 0;
fCurrentFile = null;
}
int threadsNeeded = Math.min(files.length, NUMBER_OF_LOGICAL_THREADS);
// All but 1 threads should search. 1 thread does the UI updates:
int jobCount = fCollector.canRunInParallel() && threadsNeeded > 1 ? threadsNeeded - 1 : 1;

// Seed count over 1 can cause endless waits, see bug 543629 comment 2
// TODO use seed = jobCount after the bug 543660 in JobGroup is fixed
final int seed = 1;
final JobGroup jobGroup = new TextSearchJobGroup("Text Search", jobCount, seed); //$NON-NLS-1$
long startTime= TRACING ? System.currentTimeMillis() : 0;

Job monitorUpdateJob= new Job(SearchMessages.TextSearchVisitor_progress_updating_job) {
private int fLastNumberOfScannedFiles= 0;

@Override
public IStatus run(IProgressMonitor inner) {
while (!inner.isCanceled()) {
// Propagate user cancellation to the JobGroup.
if (fProgressMonitor.isCanceled()) {
jobGroup.cancel();
break;
}

IFile file;
int numberOfScannedFiles;
synchronized (fLock) {
file= fCurrentFile;
numberOfScannedFiles= fNumberOfScannedFiles;
}
if (file != null) {
String fileName= file.getName();
Object[] args= { fileName, Integer.valueOf(numberOfScannedFiles), Integer.valueOf(fNumberOfFilesToScan)};
fProgressMonitor.subTask(Messages.format(SearchMessages.TextSearchVisitor_scanning, args));
int steps= numberOfScannedFiles - fLastNumberOfScannedFiles;
fProgressMonitor.worked(steps);
fLastNumberOfScannedFiles += steps;
}
try {
Thread.sleep(100);
} catch (InterruptedException e) {
return Status.OK_STATUS;
}
}
return Status.OK_STATUS;
}
};

try {
String taskName= fSearchPattern.pattern().isEmpty()
? SearchMessages.TextSearchVisitor_filesearch_task_label
: ""; //$NON-NLS-1$
fProgressMonitor.beginTask(taskName, fNumberOfFilesToScan);
monitorUpdateJob.setSystem(true);
monitorUpdateJob.schedule();
try {
fCollector.beginReporting();
if (fProgressMonitor.isCanceled()) {
throw new OperationCanceledException(SearchMessages.TextSearchVisitor_canceled);
}

Map<IFile, IDocument> documentsInEditors= PlatformUI.isWorkbenchRunning() ? evalNonFileBufferDocuments() : Collections.emptyMap();

// group files with same content together:

Map<String, List<IFile>> localFilesByLocation = new LinkedHashMap<>();
Map<String, List<IFile>> remotFilesByLocation = new LinkedHashMap<>();
Map<String, List<IFile>> remoteFilesByLocation = new LinkedHashMap<>();

for (IFile file : files) {
IPath path = file.getLocation();
String key = path == null ? file.getLocationURI().toString() : path.toString();
Map<String, List<IFile>> filesByLocation = (path != null) ? localFilesByLocation
: remotFilesByLocation;
: remoteFilesByLocation;
filesByLocation.computeIfAbsent(key, k -> new ArrayList<>()).add(file);

}
localFilesByLocation.values().forEach(fileBatches::offer);
remotFilesByLocation.values().forEach(fileBatches::offer);
remoteFilesByLocation.values().forEach(fileBatches::offer);
int numberOfFilesToScan = fileBatches.size();
fProgressMonitor.beginTask(taskName, numberOfFilesToScan);

// Seed count over 1 can cause endless waits, see bug 543629
// comment 2
// TODO use seed = jobCount after the bug 543660 in JobGroup is
// fixed

final int seed = 1;
final JobGroup jobGroup = new TextSearchJobGroup("Text Search", jobCount, seed); //$NON-NLS-1$
for (int i = 0; i < jobCount; i++) {
Job job = new TextSearchJob(documentsInEditors, jobCount);
job.setJobGroup(jobGroup);
job.schedule();
}

// The monitorUpdateJob is managing progress and cancellation,
// so it is ok to pass a null monitor into the job group.
// update progress until finished or canceled:
int numberOfScannedFiles = 0;
int lastNumberOfScannedFiles = 0;
while (!fProgressMonitor.isCanceled() && !jobGroup.getActiveJobs().isEmpty()
&& numberOfScannedFiles != numberOfFilesToScan) {
IFile file;
synchronized (fLock) {
try {
// time only relevant on how often progress is
// updated, but cancel is notified immediately:
fLock.wait(100);
} catch (InterruptedException e) {
fProgressMonitor.setCanceled(true);
break;
}
file = fCurrentFile;
numberOfScannedFiles = fNumberOfScannedFiles;
}
if (file != null) {
String fileName = file.getName();
Object[] args = { fileName, Integer.valueOf(numberOfScannedFiles),
Integer.valueOf(numberOfFilesToScan) };
fProgressMonitor.subTask(Messages.format(SearchMessages.TextSearchVisitor_scanning, args));
int steps = numberOfScannedFiles - lastNumberOfScannedFiles;
fProgressMonitor.worked(steps);
lastNumberOfScannedFiles += steps;
}
}
if (fProgressMonitor.isCanceled()) {
jobGroup.cancel();
}
// no need to pass progressMonitor (which would show wrong
// progress) but null because jobGroup was already finished /
// canceled anyway:
jobGroup.join(0, null);
if (fProgressMonitor.isCanceled())
if (fProgressMonitor.isCanceled()) {
throw new OperationCanceledException(SearchMessages.TextSearchVisitor_canceled);
}

fStatus.addAll(jobGroup.getResult());
return fStatus;
} catch (InterruptedException e) {
throw new OperationCanceledException(SearchMessages.TextSearchVisitor_canceled);
} finally {
monitorUpdateJob.cancel();
fileBatches.clear();
}
} finally {
Expand Down Expand Up @@ -501,6 +502,7 @@ private boolean hasBinaryContent(CharSequence seq, IFile file) throws CoreExcept
}
}
} catch (IndexOutOfBoundsException e) {
// ignored
} catch (FileCharSequenceException ex) {
if (ex.getCause() instanceof CharConversionException)
return true;
Expand Down
1 change: 0 additions & 1 deletion org.eclipse.search/search/org/eclipse/search/internal/ui/SearchMessages.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,6 @@ private SearchMessages() {
public static String TextSearchPage_searchBinary_label;
public static String TextSearchVisitor_filesearch_task_label;
public static String TextSearchVisitor_patterntoocomplex0;
public static String TextSearchVisitor_progress_updating_job;
public static String TextSearchVisitor_scanning;
public static String TextSearchVisitor_error;
public static String TextSearchVisitor_canceled;
Expand Down
3 changes: 1 addition & 2 deletions org.eclipse.search/search/org/eclipse/search/internal/ui/SearchMessages.properties
View file
Original file line number Diff line number Diff line change
Expand Up @@ -106,9 +106,8 @@ TextSearchVisitor_error= File ''{1}'' has been skipped, problem while reading: (
TextSearchVisitor_canceled= Operation Canceled
TextSearchVisitor_unsupportedcharset=File ''{1}'' has been skipped: Unsupported encoding ''{0}''.
TextSearchVisitor_patterntoocomplex0=Search pattern is too complex. Search canceled.
TextSearchVisitor_progress_updating_job=Search progress polling
TextSearchVisitor_filesearch_task_label=Searching for files...
TextSearchVisitor_textsearch_task_label=Searching for pattern ''{0}''...
TextSearchVisitor_textsearch_task_label=Searching ''{0}''
TextSearchVisitor_illegalcharset=File ''{1}'' has been skipped: Illegal encoding ''{0}''.

SortDropDownAction_label= S&ort By
Expand Down

0 comments on commit c7d94e3

Please sign in to comment.