Perform initial work in the job

Currently the first thing a reconciler after startup does is to sleep
for a while then call initialProcess then start the processing loop
(that sleeps again...).

This now moves this initial work into the start job so we have a plain
processing loop in the thread to make further improvements easier.

Author: laeubi

bundles/org.eclipse.jface.text/src/org/eclipse/jface/text/reconciler/AbstractReconciler.java

@@ -55,29 +55,27 @@ abstract public class AbstractReconciler implements IReconciler {
 	/**
 	 * Background thread for the reconciling activity.
 	 */
-	class BackgroundThread extends Thread {
+	class BackgroundWorker implements Runnable {
 
 		/** Has the reconciler been canceled. */
-		private boolean fCanceled= false;
+		private boolean fCanceled;
 		/** Has the reconciler been reset. */
-		private boolean fReset= false;
+		private boolean fReset;
 		/** Some changes need to be processed. */
-		private boolean fIsDirty= false;
+		private boolean fIsDirty;
 		/** Is a reconciling strategy active. */
-		private boolean fIsActive= false;
+		private boolean fIsActive;
 
 		private boolean fStarted;
 
-		/**
-		 * Creates a new background thread. The thread
-		 * runs with minimal priority.
-		 *
-		 * @param name the thread's name
-		 */
-		public BackgroundThread(String name) {
-			super(name);
-			setPriority(Thread.MIN_PRIORITY);
-			setDaemon(true);
+		private String fName;
+
+		private boolean fIsAlive;
+
+		private volatile Thread fThread;
+
+		public BackgroundWorker(String name) {
+			fName= name;
 		}
 
 		/**
@@ -166,80 +164,86 @@ public void reset() {
 		 * The background activity. Waits until there is something in the
 		 * queue managing the changes that have been applied to the text viewer.
 		 * Removes the first change from the queue and process it.
-		 * <p>
-		 * Calls {@link AbstractReconciler#initialProcess()} on entrance.
-		 * </p>
 		 */
 		@Override
 		public void run() {
+			try {
+				while (!fCanceled) {
 
-			delay();
-
-			if (fCanceled)
-				return;
-
-			initialProcess();
-
-			while (!fCanceled) {
-
-				delay();
-
-				if (fCanceled)
-					break;
+					delay();
 
-				if (!isDirty()) {
-					waitFinish= false; //signalWaitForFinish() was called but nothing todo
-					continue;
-				}
+					if (fCanceled)
+						break;
 
-				synchronized (this) {
-					if (fReset) {
-						fReset= false;
+					if (!isDirty()) {
+						waitFinish= false; //signalWaitForFinish() was called but nothing todo
 						continue;
 					}
-				}
 
-				DirtyRegion r= null;
-				synchronized (fDirtyRegionQueue) {
-					r= fDirtyRegionQueue.removeNextDirtyRegion();
-				}
+					synchronized (this) {
+						if (fReset) {
+							fReset= false;
+							continue;
+						}
+					}
+
+					DirtyRegion r= null;
+					synchronized (fDirtyRegionQueue) {
+						r= fDirtyRegionQueue.removeNextDirtyRegion();
+					}
 
-				fIsActive= true;
+					fIsActive= true;
 
-				fProgressMonitor.setCanceled(false);
+					fProgressMonitor.setCanceled(false);
 
-				process(r);
+					process(r);
 
-				synchronized (fDirtyRegionQueue) {
-					if (0 == fDirtyRegionQueue.getSize()) {
-						synchronized (this) {
-							fIsDirty= fProgressMonitor.isCanceled();
+					synchronized (fDirtyRegionQueue) {
+						if (fDirtyRegionQueue.isEmpty()) {
+							synchronized (this) {
+								fIsDirty= fProgressMonitor.isCanceled();
+							}
+							fDirtyRegionQueue.notifyAll();
 						}
-						fDirtyRegionQueue.notifyAll();
 					}
+					fIsActive= false;
 				}
-
-				fIsActive= false;
+			} finally {
+				fIsAlive= false;
 			}
 		}
 
+		boolean isAlive() {
+			return fIsAlive;
+		}
+
+		/**
+		 * Star the reconciling if not running (and calls
+		 * {@link AbstractReconciler#initialProcess()}) or {@link #reset()} otherwise.
+		 */
 		public void startReconciling() {
-			if (!isAlive()) {
-				if (fStarted) {
-					return;
-				}
+			if (!fStarted) {
+				fIsAlive= true;
 				fStarted= true;
-				Job.createSystem("Delayed Reconciler startup", m -> { //$NON-NLS-1$
-					try {
-						start();
-						return Status.OK_STATUS;
-					} catch (IllegalThreadStateException e) {
-						// see https://bugs.eclipse.org/bugs/show_bug.cgi?id=40549
-						// This is the only instance where the thread is started; since
-						// we checked that it is not alive, it must be dead already due
-						// to a run-time exception or error. Exit.
+				Job.createSystem("Delayed Reconciler startup for " + fName, m -> { //$NON-NLS-1$
+					//Until we process some code from the job, the reconciler thread is the current thread
+					fThread= Thread.currentThread();
+					delay();
+					if (fCanceled) {
+						return Status.CANCEL_STATUS;
+					}
+					initialProcess();
+					if (fCanceled) {
 						return Status.CANCEL_STATUS;
 					}
+					Thread thread= new Thread(this);
+					thread.setName(fName);
+					thread.setPriority(Thread.MIN_PRIORITY);
+					thread.setDaemon(true);
+					//we will no longer process any code here, so hand over to the worker thread.
+					fThread= thread;
+					thread.start();
+					return Status.OK_STATUS;
 				}).schedule();
 			} else {
 				reset();
@@ -260,8 +264,8 @@ public void documentAboutToBeChanged(DocumentEvent e) {
 		@Override
 		public void documentChanged(DocumentEvent e) {
 
-			if (fThread.isActive() || !fThread.isDirty() && fThread.isAlive()) {
-				if (!fIsAllowedToModifyDocument && Thread.currentThread() == fThread)
+			if (fWorker.isActive() || !fWorker.isDirty() && fWorker.isAlive()) {
+				if (!fIsAllowedToModifyDocument && isRunningInReconcilerThread())
 					throw new UnsupportedOperationException("The reconciler thread is not allowed to modify the document"); //$NON-NLS-1$
 				aboutToBeReconciledInternal();
 			}
@@ -270,13 +274,13 @@ public void documentChanged(DocumentEvent e) {
 			 * The second OR condition handles the case when the document
 			 * gets changed while still inside initialProcess().
 			 */
-			if (fThread.isActive() || fThread.isDirty() && fThread.isAlive())
+			if (fWorker.isActive() || fWorker.isDirty() && fWorker.isAlive())
 				fProgressMonitor.setCanceled(true);
 
 			if (fIsIncrementalReconciler)
 				createDirtyRegion(e);
 
-			fThread.reset();
+			fWorker.reset();
 
 		}
 
@@ -292,11 +296,11 @@ public void inputDocumentAboutToBeChanged(IDocument oldInput, IDocument newInput
 					synchronized (fDirtyRegionQueue) {
 						fDirtyRegionQueue.purgeQueue();
 					}
-					if (fDocument != null && fDocument.getLength() > 0 && fThread.isDirty() && fThread.isAlive()) {
+					if (fDocument != null && fDocument.getLength() > 0 && fWorker.isDirty() && fWorker.isAlive()) {
 						DocumentEvent e= new DocumentEvent(fDocument, 0, fDocument.getLength(), ""); //$NON-NLS-1$
 						createDirtyRegion(e);
-						fThread.reset();
-						fThread.suspendCallerWhileDirty();
+						fWorker.reset();
+						fWorker.suspendCallerWhileDirty();
 					}
 				}
 
@@ -316,7 +320,7 @@ public void inputDocumentChanged(IDocument oldInput, IDocument newInput) {
 
 			fDocument.addDocumentListener(this);
 
-			if (!fThread.isDirty())
+			if (!fWorker.isDirty())
 				aboutToBeReconciledInternal();
 
 			startReconciling();
@@ -326,7 +330,7 @@ public void inputDocumentChanged(IDocument oldInput, IDocument newInput) {
 	/** Queue to manage the changes applied to the text viewer. */
 	private DirtyRegionQueue fDirtyRegionQueue;
 	/** The background thread. */
-	private BackgroundThread fThread;
+	private BackgroundWorker fWorker;
 	/** Internal document and text input listener. */
 	private Listener fListener;
 	/** The background thread delay. */
@@ -475,9 +479,9 @@ public void install(ITextViewer textViewer) {
 		fViewer= textViewer;
 
 		synchronized (this) {
-			if (fThread != null)
+			if (fWorker != null)
 				return;
-			fThread= new BackgroundThread(getClass().getName());
+			fWorker= new BackgroundWorker(getClass().getName());
 		}
 
 		fDirtyRegionQueue= new DirtyRegionQueue();
@@ -511,8 +515,8 @@ public void uninstall() {
 
 			synchronized (this) {
 				// http://dev.eclipse.org/bugs/show_bug.cgi?id=19135
-				BackgroundThread bt= fThread;
-				fThread= null;
+				BackgroundWorker bt= fWorker;
+				fWorker= null;
 				bt.cancel();
 			}
 		}
@@ -618,10 +622,10 @@ protected void forceReconciling() {
 
 		if (fDocument != null) {
 
-			if (!fThread.isDirty()&& fThread.isAlive())
+			if (!fWorker.isDirty()&& fWorker.isAlive())
 				aboutToBeReconciledInternal();
 
-			if (fThread.isActive())
+			if (fWorker.isActive())
 				fProgressMonitor.setCanceled(true);
 
 			if (fIsIncrementalReconciler) {
@@ -638,10 +642,10 @@ protected void forceReconciling() {
 	 * Clients may extend this method.
 	 */
 	protected synchronized void startReconciling() {
-		if (fThread == null)
+		if (fWorker == null)
 			return;
 
-		fThread.startReconciling();
+		fWorker.startReconciling();
 	}
 
 	/**
@@ -657,7 +661,9 @@ protected void reconcilerReset() {
 	 * @return <code>true</code> if running in this reconciler's background thread
 	 * @since 3.4
 	 */
-	protected boolean isRunningInReconcilerThread() {
-		return Thread.currentThread() == fThread;
+	protected synchronized boolean isRunningInReconcilerThread() {
+		if (fWorker == null)
+			return false;
+		return Thread.currentThread() == fWorker.fThread;
 	}
 }

bundles/org.eclipse.jface.text/src/org/eclipse/jface/text/reconciler/DirtyRegionQueue.java

@@ -85,6 +85,10 @@ public int getSize() {
 		return fDirtyRegions.size();
 	}
 
+	public boolean isEmpty() {
+		return fDirtyRegions.isEmpty();
+	}
+
 	/**
 	 * Throws away all entries in the queue.
 	 */

tests/org.eclipse.jface.text.tests/src/org/eclipse/jface/text/tests/reconciler/AbstractReconcilerTest.java

@@ -190,7 +190,7 @@ public IReconcilingStrategy getReconcilingStrategy(String contentType) {
 		fReconciler.install(fViewer);
 
 		fAccessor= new Accessor(fReconciler, AbstractReconciler.class);
-		Object object= fAccessor.get("fThread");
+		Object object= fAccessor.get("fWorker");
 		fAccessor= new Accessor(object, object.getClass());
 	}