Bug 506562 - Progress view shows too many finished jobs
Listeners to be notified should not be filtered asynchronously. Thus we
must keep the list of them to be notified later in the UI thread.
Also, all jobs notifications now goes through throttler.
Change-Id: I1367029ea81fe5b506c89f7d571753d22531cd86
Signed-off-by: Mikael Barbero <mikael@eclipse.org>
diff --git a/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/progress/ProgressManager.java b/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/progress/ProgressManager.java
index a00de14..a072e4c 100644
--- a/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/progress/ProgressManager.java
+++ b/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/progress/ProgressManager.java
@@ -23,15 +23,16 @@
import java.util.Collection;
import java.util.Collections;
import java.util.Enumeration;
-import java.util.HashMap;
-import java.util.HashSet;
import java.util.Hashtable;
import java.util.Iterator;
+import java.util.LinkedHashMap;
import java.util.LinkedHashSet;
import java.util.Map;
+import java.util.Objects;
import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ConcurrentMap;
+import java.util.stream.StreamSupport;
import org.eclipse.core.runtime.Assert;
import org.eclipse.core.runtime.IProgressMonitor;
import org.eclipse.core.runtime.IProgressMonitorWithBlocking;
@@ -98,7 +99,7 @@
final private ConcurrentMap<Job, JobInfo> jobs = new ConcurrentHashMap<>();
final private Map<Object, Collection<IJobBusyListener>> familyListeners = Collections
- .synchronizedMap(new HashMap<>());
+ .synchronizedMap(new LinkedHashMap<>());
// list of IJobProgressManagerListener
private ListenerList<IJobProgressManagerListener> listeners = new ListenerList<>();
@@ -142,22 +143,36 @@
private final INotificationListener notificationListener;
/**
- * Lock object for synchronizing updates of {@code pendingJobUpdates} and
- * {@code pendingGroupUpdates}
+ * Lock object for synchronizing updates of {@code pendingJobUpdates},
+ * {@code pendingGroupUpdates}, {@code pendingJobRemoval},
+ * {@code pendingGroupRemoval} and {@code pendingJobAddition}.
*/
private final Object pendingUpdatesMutex = new Object();
/**
* Modification guarded by {@link #pendingUpdatesMutex}.
*/
- private Set<JobInfo> pendingJobUpdates = new HashSet<>();
+ private Map<JobInfo, Set<IJobProgressManagerListener>> pendingJobUpdates = new LinkedHashMap<>();
/**
* Modification guarded by {@link #pendingUpdatesMutex}.
*/
- private Set<GroupInfo> pendingGroupUpdates = new HashSet<>();
+ private Set<GroupInfo> pendingGroupUpdates = new LinkedHashSet<>();
- private final Display display;
+ /**
+ * Modification guarded by {@link #pendingUpdatesMutex}.
+ */
+ private Map<JobInfo, Set<IJobProgressManagerListener>> pendingJobRemoval = new LinkedHashMap<>();
+
+ /**
+ * Modification guarded by {@link #pendingUpdatesMutex}.
+ */
+ private Set<GroupInfo> pendingGroupRemoval = new LinkedHashSet<>();
+
+ /**
+ * Modification guarded by {@link #pendingUpdatesMutex}.
+ */
+ private Map<JobInfo, Set<IJobProgressManagerListener>> pendingJobAddition = new LinkedHashMap<>();
private static final String IMAGE_KEY = "org.eclipse.ui.progress.images"; //$NON-NLS-1$
@@ -342,42 +357,44 @@
Job.getJobManager().addJobChangeListener(this.changeListener);
StatusManager.getManager().addListener(notificationListener);
- display = Display.getDefault();
-
- uiRefreshThrottler = new Throttler(display, Duration.ofMillis(100), () -> {
- Set<JobInfo> localPendingJobUpdates;
- Set<GroupInfo> localPendingGroupUpdates;
+ uiRefreshThrottler = new Throttler(Display.getDefault(), Duration.ofMillis(100), () -> {
+ Set<GroupInfo> localPendingGroupUpdates, localPendingGroupRemoval;
+ Map<JobInfo, Set<IJobProgressManagerListener>> localPendingJobUpdates, localPendingJobAddition,
+ localPendingJobRemoval;
synchronized (pendingUpdatesMutex) {
localPendingJobUpdates = pendingJobUpdates;
- pendingJobUpdates = new HashSet<>();
+ pendingJobUpdates = new LinkedHashMap<>();
localPendingGroupUpdates = pendingGroupUpdates;
- pendingGroupUpdates = new HashSet<>();
+ pendingGroupUpdates = new LinkedHashSet<>();
+ localPendingJobRemoval = pendingJobRemoval;
+ pendingJobRemoval = new LinkedHashMap<>();
+ localPendingGroupRemoval = pendingGroupRemoval;
+ pendingGroupRemoval = new LinkedHashSet<>();
+ localPendingJobAddition = pendingJobAddition;
+ pendingJobAddition = new LinkedHashMap<>();
}
- Iterator<JobInfo> jobUpdatesIterator = localPendingJobUpdates.iterator();
- while (jobUpdatesIterator.hasNext()) {
- JobInfo info = jobUpdatesIterator.next();
- GroupInfo group = info.getGroupInfo();
- if (group != null) {
- localPendingGroupUpdates.remove(group);
- doRefreshGroup(group);
- }
+ localPendingJobAddition.entrySet()
+ .forEach(e -> e.getValue().forEach(listener -> listener.addJob(e.getKey())));
- Object[] listenersArray = listeners.getListeners();
- for (int i = 0; i < listenersArray.length; i++) {
- IJobProgressManagerListener listener = (IJobProgressManagerListener) listenersArray[i];
- if (!isCurrentDisplaying(info.getJob(), listener.showsDebug())) {
- listener.refreshJobInfo(info);
- }
- }
- }
+ // Adds all non null JobInfo#getGroupInfo to the list of groups to
+ // be refreshed
+ localPendingJobUpdates.entrySet().stream().map(e -> e.getKey().getGroupInfo()).filter(Objects::nonNull)
+ .forEach(localPendingGroupUpdates::add);
+
+ localPendingJobUpdates.entrySet()
+ .forEach(e -> e.getValue().forEach(listener -> listener.refreshJobInfo(e.getKey())));
// refresh groups
- Iterator<GroupInfo> groupUpdatesIterator = localPendingGroupUpdates.iterator();
- while (groupUpdatesIterator.hasNext()) {
- GroupInfo groupInfo = groupUpdatesIterator.next();
- doRefreshGroup(groupInfo);
- }
+ localPendingGroupUpdates
+ .forEach(groupInfo -> listeners.forEach(listener -> listener.refreshGroup(groupInfo)));
+
+ localPendingJobRemoval.entrySet()
+ .forEach(e -> e.getValue().forEach(listener -> listener.removeJob(e.getKey())));
+
+ localPendingGroupRemoval.forEach(group -> {
+ listeners.forEach(listener -> listener.removeGroup(group));
+ });
});
}
@@ -651,17 +668,11 @@
*/
public void refreshJobInfo(JobInfo info) {
synchronized (pendingUpdatesMutex) {
- pendingJobUpdates.add(info);
+ rememberListenersForJob(info, pendingJobUpdates);
}
uiRefreshThrottler.throttledExec();
}
- private void safeAsyncExec(Runnable runnable) {
- if (!display.isDisposed()) {
- display.asyncExec(runnable);
- }
- }
-
/**
* Refreshes the IJobProgressManagerListeners as a result of a change in
* info.
@@ -675,12 +686,6 @@
uiRefreshThrottler.throttledExec();
}
- private void doRefreshGroup(GroupInfo info) {
- for (IJobProgressManagerListener listener : listeners) {
- listener.refreshGroup(info);
- }
- }
-
/**
* Refreshes the content providers as a result of a deletion of info.
*
@@ -691,17 +696,10 @@
Job job = info.getJob();
jobs.remove(job);
synchronized (pendingUpdatesMutex) {
- pendingJobUpdates.remove(info);
+ rememberListenersForJob(info, pendingJobRemoval);
}
runnableMonitors.remove(job);
-
- safeAsyncExec(() -> {
- for (IJobProgressManagerListener listener : listeners) {
- if (!isCurrentDisplaying(info.getJob(), listener.showsDebug())) {
- listener.removeJob(info);
- }
- }
- });
+ uiRefreshThrottler.throttledExec();
}
/**
@@ -712,14 +710,9 @@
*/
public void removeGroup(GroupInfo group) {
synchronized (pendingUpdatesMutex) {
- pendingGroupUpdates.remove(group);
+ pendingGroupRemoval.add(group);
}
-
- safeAsyncExec(() -> {
- for (IJobProgressManagerListener listener : listeners) {
- listener.removeGroup(group);
- }
- });
+ uiRefreshThrottler.throttledExec();
}
/**
@@ -734,13 +727,18 @@
}
jobs.put(info.getJob(), info);
- safeAsyncExec(() -> {
- for (IJobProgressManagerListener listener : listeners) {
- if (!isCurrentDisplaying(info.getJob(), listener.showsDebug())) {
- listener.addJob(info);
- }
- }
- });
+ synchronized (pendingUpdatesMutex) {
+ rememberListenersForJob(info, pendingJobAddition);
+ }
+ uiRefreshThrottler.throttledExec();
+ }
+
+ private void rememberListenersForJob(JobInfo info, Map<JobInfo, Set<IJobProgressManagerListener>> listenersMap) {
+ Set<IJobProgressManagerListener> localListeners = listenersMap.computeIfAbsent(info,
+ k -> new LinkedHashSet<>());
+ StreamSupport.stream(listeners.spliterator(), false)
+ .filter(listener -> !isCurrentDisplaying(info.getJob(), listener.showsDebug()))
+ .forEach(localListeners::add);
}
/**
@@ -950,7 +948,7 @@
synchronized (familyListeners) {
Collection<IJobBusyListener> currentListeners = familyListeners.get(family);
if (currentListeners == null) {
- currentListeners = new HashSet<>();
+ currentListeners = new LinkedHashSet<>();
familyListeners.put(family, currentListeners);
}
currentListeners.add(listener);
@@ -994,7 +992,7 @@
}
Iterator<Object> families = familyListeners.keySet().iterator();
- Collection<IJobBusyListener> returnValue = new HashSet<>();
+ Collection<IJobBusyListener> returnValue = new LinkedHashSet<>();
while (families.hasNext()) {
Object next = families.next();
if (job.belongsTo(next)) {
