Attachment 120102 Details for Bug 258352 – proposed patch

Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

[patch] proposed patch

ProgressManager.patch.txt (text/plain), 8.98 KB, created by Randall Theobald

on 2008-12-10 15:15:42 EST

(hide)

Description:

Filename:

MIME Type:

Creator: Randall Theobald

Created: 2008-12-10 15:15:42 EST

Size: 8.98 KB

patch

obsolete

>### Eclipse Workspace Patch 1.0
>#P org.eclipse.ui.workbench
>Index: Eclipse UI/org/eclipse/ui/internal/progress/ProgressViewUpdater.java
>===================================================================
>RCS file: /cvsroot/eclipse/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/progress/ProgressViewUpdater.java,v
>retrieving revision 1.26
>diff -u -r1.26 ProgressViewUpdater.java
>--- Eclipse UI/org/eclipse/ui/internal/progress/ProgressViewUpdater.java	24 Mar 2008 19:21:58 -0000	1.26
>+++ Eclipse UI/org/eclipse/ui/internal/progress/ProgressViewUpdater.java	10 Dec 2008 19:55:44 -0000
>@@ -217,13 +217,21 @@
> 		}
>     }
> 
>+    /** keep track of how often we schedule the job to avoid overloading the JobManager */
>+    private long lastUpdateJobScheduleRequest = 0;
>+    
>     /**
>      * Schedule an update.
>      */
>     void scheduleUpdate() {
>         if (PlatformUI.isWorkbenchRunning()) {
>-            //Add in a 100ms delay so as to keep priority low
>-            updateJob.schedule(100);
>+            // make sure we don't schedule too often
>+        	long now = System.currentTimeMillis();
>+        	if (now - lastUpdateJobScheduleRequest >= 100) {
>+        		//Add in a 100ms delay so as to keep priority low
>+        		updateJob.schedule(100);
>+        		lastUpdateJobScheduleRequest = now;
>+        	}
>         }
>     }
> 
>Index: Eclipse UI/org/eclipse/ui/internal/progress/ProgressManager.java
>===================================================================
>RCS file: /cvsroot/eclipse/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/progress/ProgressManager.java,v
>retrieving revision 1.139
>diff -u -r1.139 ProgressManager.java
>--- Eclipse UI/org/eclipse/ui/internal/progress/ProgressManager.java	29 Apr 2008 07:09:16 -0000	1.139
>+++ Eclipse UI/org/eclipse/ui/internal/progress/ProgressManager.java	10 Dec 2008 19:55:44 -0000
>@@ -96,6 +96,11 @@
> 
> 	final Object familyKey = new Object();
> 
>+	/** 
>+	 * Access to the listeners array is only protected by the listenersKey during modifications.
>+	 * As such, when iterating through the listeners array, a temporary variable should be used to
>+	 * hold the reference to the array, see #sleepJobInfo(JobInfo) for an example.
>+	 */
> 	private IJobProgressManagerListener[] listeners = new IJobProgressManagerListener[0];
> 
> 	final Object listenersKey = new Object();
>@@ -534,19 +539,16 @@
> 		if (group != null) {
> 			sleepGroup(group,info);
> 		}
>-
>-		synchronized (listenersKey) {
>-			for (int i = 0; i < listeners.length; i++) {
>-				IJobProgressManagerListener listener = listeners[i];
>-				// Is this one the user never sees?
>-				if (isNeverDisplaying(info.getJob(), listener.showsDebug()))
>-					continue;
>-				if (listener.showsDebug())
>-					listener.refreshJobInfo(info);
>-				else
>-					listener.removeJob(info);
>-
>-			}
>+		IJobProgressManagerListener[] listenersTemp = listeners;
>+		for (int i = 0; i < listenersTemp.length; i++) {
>+			IJobProgressManagerListener listener = listenersTemp[i];
>+			// Is this one the user never sees?
>+			if (isNeverDisplaying(info.getJob(), listener.showsDebug()))
>+				continue;
>+			if (listener.showsDebug())
>+				listener.refreshJobInfo(info);
>+			else
>+				listener.removeJob(info);
> 		}
> 
> 	}
>@@ -556,18 +558,16 @@
> 	 * @param group
> 	 */
> 	private void sleepGroup(GroupInfo group, JobInfo info) {
>-		synchronized (listenersKey) {
>-			for (int i = 0; i < listeners.length; i++) {
>-				
>-				IJobProgressManagerListener listener = listeners[i];
>-				if (isNeverDisplaying(info.getJob(), listener.showsDebug()))
>-					continue;
>+		IJobProgressManagerListener[] listenersTemp = listeners;
>+		for (int i = 0; i < listenersTemp.length; i++) {
>+			IJobProgressManagerListener listener = listenersTemp[i];
>+			if (isNeverDisplaying(info.getJob(), listener.showsDebug()))
>+				continue;
> 		
>-				if (listener.showsDebug() || group.isActive())
>-					listener.refreshGroup(group);
>-				else
>-					listener.removeGroup(group);
>-			}
>+			if (listener.showsDebug() || group.isActive())
>+				listener.refreshGroup(group);
>+			else
>+				listener.removeGroup(group);
> 		}
> 		
> 	}
>@@ -641,17 +641,20 @@
> 	 * @param listener
> 	 */
> 	void addListener(IJobProgressManagerListener listener) {
>-
>+		// if the listener is already held, ignore
>+		IJobProgressManagerListener[] listenersTemp = listeners;
>+		for (int i = 0; i < listenersTemp.length; i++) {
>+			if (listenersTemp[i].equals(listener)) return;
>+		}
>+		// add the new listener
> 		synchronized (listenersKey) {
>-			ArrayList newListeners = new ArrayList(listeners.length + 1);
>+			IJobProgressManagerListener[] newListeners = new IJobProgressManagerListener[listeners.length+1];
> 			for (int i = 0; i < listeners.length; i++) {
>-				newListeners.add(listeners[i]);
>+				newListeners[i] = listeners[i];
> 			}
>-			newListeners.add(listener);
>-			listeners = new IJobProgressManagerListener[newListeners.size()];
>-			newListeners.toArray(listeners);
>+			newListeners[listeners.length] = listener;
>+			listeners = newListeners;
> 		}
>-
> 	}
> 
> 	/**
>@@ -661,16 +664,26 @@
> 	 * @param listener
> 	 */
> 	void removeListener(IJobProgressManagerListener listener) {
>+		// if the listener is not held, ignore
>+		boolean listenerHeld = false;
>+		IJobProgressManagerListener[] listenersTemp = listeners;
>+		for (int i = 0; i < listenersTemp.length; i++) {
>+			if (listenersTemp[i].equals(listener)) {
>+				listenerHeld = true;
>+				break;
>+			}
>+		}
>+		if (!listenerHeld) return;
>+		// remove the listener
> 		synchronized (listenersKey) {
>-			ArrayList newListeners = new ArrayList();
>-			for (int i = 0; i < listeners.length; i++) {
>+			IJobProgressManagerListener[] newListeners = new IJobProgressManagerListener[listeners.length-1];
>+			for (int i=0,j=0; i < listeners.length; i++) {
> 				if (listeners[i].equals(listener)) {
> 					continue;
> 				}
>-				newListeners.add(listeners[i]);
>+				newListeners[j++] = listeners[i];
> 			}
>-			listeners = new IJobProgressManagerListener[newListeners.size()];
>-			newListeners.toArray(listeners);
>+			listeners = listenersTemp;
> 		}
> 	}
> 
>@@ -711,12 +724,11 @@
> 			refreshGroup(group);
> 		}
> 
>-		synchronized (listenersKey) {
>-			for (int i = 0; i < listeners.length; i++) {
>-				IJobProgressManagerListener listener = listeners[i];
>-				if (!isCurrentDisplaying(info.getJob(), listener.showsDebug())) {
>-					listener.refreshJobInfo(info);
>-				}
>+		IJobProgressManagerListener[] listenersTemp = listeners;
>+		for (int i = 0; i < listenersTemp.length; i++) {
>+			IJobProgressManagerListener listener = listenersTemp[i];
>+			if (!isCurrentDisplaying(info.getJob(), listener.showsDebug())) {
>+				listener.refreshJobInfo(info);
> 			}
> 		}
> 	}
>@@ -727,11 +739,9 @@
> 	 * @param info
> 	 */
> 	public void refreshGroup(GroupInfo info) {
>-
>-		synchronized (listenersKey) {
>-			for (int i = 0; i < listeners.length; i++) {
>-				listeners[i].refreshGroup(info);
>-			}
>+		IJobProgressManagerListener[] listenersTemp = listeners;
>+		for (int i = 0; i < listenersTemp.length; i++) {
>+			listenersTemp[i].refreshGroup(info);
> 		}
> 	}
> 
>@@ -742,10 +752,9 @@
> 	public void refreshAll() {
> 
> 		pruneStaleJobs();
>-		synchronized (listenersKey) {
>-			for (int i = 0; i < listeners.length; i++) {
>-				listeners[i].refreshAll();
>-			}
>+		IJobProgressManagerListener[] listenersTemp = listeners;
>+		for (int i = 0; i < listenersTemp.length; i++) {
>+			listenersTemp[i].refreshAll();
> 		}
> 
> 	}
>@@ -766,12 +775,11 @@
> 			}
> 		}
> 
>-		synchronized (listenersKey) {
>-			for (int i = 0; i < listeners.length; i++) {
>-				IJobProgressManagerListener listener = listeners[i];
>-				if (!isCurrentDisplaying(info.getJob(), listener.showsDebug())) {
>-					listener.removeJob(info);
>-				}
>+		IJobProgressManagerListener[] listenersTemp = listeners;
>+		for (int i = 0; i < listenersTemp.length; i++) {
>+			IJobProgressManagerListener listener = listenersTemp[i];
>+			if (!isCurrentDisplaying(info.getJob(), listener.showsDebug())) {
>+				listener.removeJob(info);
> 			}
> 		}
> 
>@@ -784,11 +792,9 @@
> 	 *            GroupInfo
> 	 */
> 	public void removeGroup(GroupInfo group) {
>-
>-		synchronized (listenersKey) {
>-			for (int i = 0; i < listeners.length; i++) {
>-				listeners[i].removeGroup(group);
>-			}
>+		IJobProgressManagerListener[] listenersTemp = listeners;
>+		for (int i = 0; i < listenersTemp.length; i++) {
>+			listenersTemp[i].removeGroup(group);
> 		}
> 	}
> 
>@@ -804,12 +810,11 @@
> 		}
> 
> 		jobs.put(info.getJob(), info);
>-		synchronized (listenersKey) {
>-			for (int i = 0; i < listeners.length; i++) {
>-				IJobProgressManagerListener listener = listeners[i];
>-				if (!isCurrentDisplaying(info.getJob(), listener.showsDebug())) {
>-					listener.addJob(info);
>-				}
>+		IJobProgressManagerListener[] listenersTemp = listeners;
>+		for (int i = 0; i < listenersTemp.length; i++) {
>+			IJobProgressManagerListener listener = listenersTemp[i];
>+			if (!isCurrentDisplaying(info.getJob(), listener.showsDebug())) {
>+				listener.addJob(info);
> 			}
> 		}
> 	}

Flags:

rstheo: review?

Actions: View | Diff

Attachments on bug 258352: 120102 | 120665 | 123174