From f46fb351c038098fa50690dd1848acaddd87b355 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christoph=20L=C3=A4ubrich?= Date: Tue, 8 Jul 2025 09:38:00 +0200 Subject: [PATCH] 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. --- .../text/reconciler/AbstractReconciler.java | 176 +++++++++--------- .../text/reconciler/DirtyRegionQueue.java | 4 + .../reconciler/AbstractReconcilerTest.java | 2 +- 3 files changed, 96 insertions(+), 86 deletions(-) diff --git a/bundles/org.eclipse.jface.text/src/org/eclipse/jface/text/reconciler/AbstractReconciler.java b/bundles/org.eclipse.jface.text/src/org/eclipse/jface/text/reconciler/AbstractReconciler.java index e4ad7855bf6..19c8ab9ed26 100644 --- a/bundles/org.eclipse.jface.text/src/org/eclipse/jface/text/reconciler/AbstractReconciler.java +++ b/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. - *

- * Calls {@link AbstractReconciler#initialProcess()} on entrance. - *

*/ @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 true 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; } } diff --git a/bundles/org.eclipse.jface.text/src/org/eclipse/jface/text/reconciler/DirtyRegionQueue.java b/bundles/org.eclipse.jface.text/src/org/eclipse/jface/text/reconciler/DirtyRegionQueue.java index c342fc77627..e8a53dfd266 100644 --- a/bundles/org.eclipse.jface.text/src/org/eclipse/jface/text/reconciler/DirtyRegionQueue.java +++ b/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. */ diff --git a/tests/org.eclipse.jface.text.tests/src/org/eclipse/jface/text/tests/reconciler/AbstractReconcilerTest.java b/tests/org.eclipse.jface.text.tests/src/org/eclipse/jface/text/tests/reconciler/AbstractReconcilerTest.java index 9778c251510..7975edbfa6d 100644 --- a/tests/org.eclipse.jface.text.tests/src/org/eclipse/jface/text/tests/reconciler/AbstractReconcilerTest.java +++ b/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()); }