Bug 575893 - avoid synchronization between UI and search workers
The search worker thread did synchronize with the UI Thread.
Now the search does not need to wait for updating the UI anymore.
The workers called postUpdate() while the UI was drawing in
runBatchedUpdates()/elementsChanged().
By using a concurrent Datastructure the synchronisation can be avoided.
Change-Id: I64248a05ef2f4d4d9960e90b7be3628e0360bfa0
Signed-off-by: Joerg Kubitz <jkubitz-eclipse@gmx.de>
Reviewed-on: https://git.eclipse.org/r/c/platform/eclipse.platform.text/+/185271
Tested-by: Platform Bot <platform-bot@eclipse.org>
Reviewed-by: Mickael Istria <mistria@redhat.com>
diff --git a/org.eclipse.search/new search/org/eclipse/search/ui/text/AbstractTextSearchViewPage.java b/org.eclipse.search/new search/org/eclipse/search/ui/text/AbstractTextSearchViewPage.java
index 65ba5c1..443ae3c 100644
--- a/org.eclipse.search/new search/org/eclipse/search/ui/text/AbstractTextSearchViewPage.java
+++ b/org.eclipse.search/new search/org/eclipse/search/ui/text/AbstractTextSearchViewPage.java
@@ -15,9 +15,12 @@
package org.eclipse.search.ui.text;
import java.util.ArrayList;
+import java.util.Collection;
import java.util.Collections;
import java.util.HashSet;
+import java.util.Objects;
import java.util.Set;
+import java.util.concurrent.LinkedBlockingDeque;
import org.osgi.framework.FrameworkUtil;
@@ -226,8 +229,8 @@
private PageBook fPagebook;
private boolean fIsBusyShown;
private ISearchResultViewPart fViewPart;
- private Set<Object> fBatchedUpdates;
- private boolean fBatchedClearAll;
+ private final LinkedBlockingDeque<Object> fBatchedUpdates = new LinkedBlockingDeque<>();
+ private volatile boolean fBatchedClearAll;
private ISearchResultListener fListener;
private IQueryListener fQueryListener;
@@ -299,8 +302,6 @@
fSelectAllAction= new SelectAllAction();
createLayoutActions();
- fBatchedUpdates = new HashSet<>();
- fBatchedClearAll= false;
fListener = this::handleSearchResultChanged;
fFilterActions= null;
@@ -1231,24 +1232,30 @@
}
}
- private synchronized void postUpdate(Match[] matches) {
- evaluateChangedElements(matches, fBatchedUpdates);
- scheduleUIUpdate();
+ private void postUpdate(Match[] matches) {
+ HashSet<Object> collect = new HashSet<>();
+ // for compatibility we do not pass the "fBatchedUpdates" directly:
+ evaluateChangedElements(matches, collect);
+ // nulls are forbidden in concurrent datastructures:
+ collect.removeIf(Objects::isNull);
+ fBatchedUpdates.addAll(collect);
+ scheduleUIUpdate(); // still synchronized
}
- private synchronized void runBatchedUpdates() {
- elementsChanged(fBatchedUpdates.toArray());
- fBatchedUpdates.clear();
+ private void runBatchedUpdates() {
+ Collection<Object> drain = new ArrayList<>();
+ fBatchedUpdates.drainTo(drain);
+ elementsChanged(drain.toArray());
updateBusyLabel();
}
- private synchronized void postClear() {
+ private void postClear() {
fBatchedClearAll= true;
fBatchedUpdates.clear();
- scheduleUIUpdate();
+ scheduleUIUpdate(); // still synchronized
}
- private synchronized boolean hasMoreUpdates() {
+ private boolean hasMoreUpdates() {
return fBatchedClearAll || !fBatchedUpdates.isEmpty();
}
diff --git a/org.eclipse.search/search/org/eclipse/search/internal/ui/text/FileSearchPage.java b/org.eclipse.search/search/org/eclipse/search/internal/ui/text/FileSearchPage.java
index 232b682..dbe5105 100644
--- a/org.eclipse.search/search/org/eclipse/search/internal/ui/text/FileSearchPage.java
+++ b/org.eclipse.search/search/org/eclipse/search/internal/ui/text/FileSearchPage.java
@@ -453,7 +453,10 @@
protected void evaluateChangedElements(Match[] matches, Set<Object> changedElements) {
if (showLineMatches()) {
for (Match match : matches) {
- changedElements.add(((FileMatch) match).getLineElement());
+ LineElement lineElement = ((FileMatch) match).getLineElement();
+ if (lineElement != null) {
+ changedElements.add(lineElement);
+ }
}
} else {
super.evaluateChangedElements(matches, changedElements);
