From 6a6fb5010c2cd06b14a1e055eabaeb4848e65d2d Mon Sep 17 00:00:00 2001 From: Mateusz Warowny Date: Sun, 2 Aug 2020 18:12:26 +0100 Subject: [PATCH] JAL - 3690 AlignCalc rebuilt - FutureTask-based manager --- src/jalview/api/AlignCalcListener.java | 11 + src/jalview/api/AlignCalcManagerI.java | 4 +- src/jalview/api/AlignCalcManagerI2.java | 26 ++ src/jalview/api/AlignCalcWorkerI.java | 23 +- src/jalview/api/AlignViewportI.java | 2 +- src/jalview/appletgui/AlignViewport.java | 2 +- src/jalview/viewmodel/AlignmentViewport.java | 40 +-- src/jalview/workers/AlignCalcManager.java | 61 ++-- src/jalview/workers/AlignCalcManager2.java | 319 ++++++++++++++++++++ src/jalview/workers/AlignCalcWorker.java | 6 +- src/jalview/workers/AnnotationWorker.java | 74 ++--- src/jalview/workers/ColumnCounterSetWorker.java | 42 +-- src/jalview/workers/ConsensusThread.java | 74 ++--- src/jalview/workers/ConservationThread.java | 88 ++---- src/jalview/workers/InformationThread.java | 77 ++--- src/jalview/workers/StrucConsensusThread.java | 40 --- src/jalview/ws/jws2/Jws2ClientFactory.java | 12 +- .../ws/jws2/SeqAnnotationServiceCalcWorker.java | 76 +---- .../ws/jws2/SequenceAnnotationWSClient.java | 2 +- test/jalview/workers/AlignCalcManagerTest.java | 18 +- 20 files changed, 551 insertions(+), 446 deletions(-) create mode 100644 src/jalview/api/AlignCalcListener.java create mode 100644 src/jalview/api/AlignCalcManagerI2.java create mode 100644 src/jalview/workers/AlignCalcManager2.java diff --git a/src/jalview/api/AlignCalcListener.java b/src/jalview/api/AlignCalcListener.java new file mode 100644 index 0000000..96491fa --- /dev/null +++ b/src/jalview/api/AlignCalcListener.java @@ -0,0 +1,11 @@ +package jalview.api; + +import java.util.EventListener; + +public interface AlignCalcListener extends EventListener +{ + default void workerQueued(AlignCalcWorkerI worker) {} + default void workerStarted(AlignCalcWorkerI worker) {} + default void workerCompleted(AlignCalcWorkerI worker) {} + default void workerExceptional(AlignCalcWorkerI worker, Throwable throwable) {} +} diff --git a/src/jalview/api/AlignCalcManagerI.java b/src/jalview/api/AlignCalcManagerI.java index 18605b8..005ecd7 100644 --- a/src/jalview/api/AlignCalcManagerI.java +++ b/src/jalview/api/AlignCalcManagerI.java @@ -32,7 +32,7 @@ public interface AlignCalcManagerI * * @param worker */ - void notifyStart(AlignCalcWorkerI worker); + void notifyStarted(AlignCalcWorkerI worker); /** * tell manager that a thread running worker's run() loop is ready to start @@ -150,7 +150,7 @@ public interface AlignCalcManagerI * * @param typeToRemove */ - void removeRegisteredWorkersOfClass( + void removeWorkersOfClass( Class typeToRemove); /** diff --git a/src/jalview/api/AlignCalcManagerI2.java b/src/jalview/api/AlignCalcManagerI2.java new file mode 100644 index 0000000..c735c36 --- /dev/null +++ b/src/jalview/api/AlignCalcManagerI2.java @@ -0,0 +1,26 @@ +package jalview.api; + +import java.util.List; + +import jalview.datamodel.AlignmentAnnotation; + +public interface AlignCalcManagerI2 +{ + void registerWorker(AlignCalcWorkerI worker); + List getWorkers(); + List getWorkersOfClass(Class cls); + void removeWorker(AlignCalcWorkerI worker); + void removeWorkerForAnnotation(AlignmentAnnotation annot); + void removeWorkersOfClass(Class cls); + void disableWorker(AlignCalcWorkerI worker); + void enableWorker(AlignCalcWorkerI worker); + boolean isDisabled(AlignCalcWorkerI worker); + boolean isWorking(AlignCalcWorkerI worker); + boolean isWorkingWithAnnotation(AlignmentAnnotation annot); + boolean isWorking(); + void startWorker(AlignCalcWorkerI worker); + void restartWorkers(); + void cancelWorker(AlignCalcWorkerI worker); + void addAlignCalcListener(AlignCalcListener listener); + void removeAlignCalcListener(AlignCalcListener listener); +} diff --git a/src/jalview/api/AlignCalcWorkerI.java b/src/jalview/api/AlignCalcWorkerI.java index 1387cba..1184853 100644 --- a/src/jalview/api/AlignCalcWorkerI.java +++ b/src/jalview/api/AlignCalcWorkerI.java @@ -20,13 +20,15 @@ */ package jalview.api; +import java.util.concurrent.Callable; + import jalview.datamodel.AlignmentAnnotation; /** * Interface describing a worker that calculates alignment annotation(s). The * main (re-)calculation should be performed by the inherited run() method. */ -public interface AlignCalcWorkerI extends Runnable +public interface AlignCalcWorkerI extends Callable { /** * Answers true if this worker updates the given annotation (regardless of its @@ -48,7 +50,24 @@ public interface AlignCalcWorkerI extends Runnable * Removes any annotation(s) managed by this worker from the alignment */ void removeAnnotation(); - + + + /** + * Default implementation of call which calls run and propagates the + * exception. + */ + @Override + public default Void call() throws Exception + { + run(); + return null; + } + + /** + * The main calculation happens here + */ + public void run() throws Exception; + /** * Answers true if the worker should be deleted entirely when its annotation * is deleted from the display, or false if it should continue to run. Some diff --git a/src/jalview/api/AlignViewportI.java b/src/jalview/api/AlignViewportI.java index c487bdc..2f992a6 100644 --- a/src/jalview/api/AlignViewportI.java +++ b/src/jalview/api/AlignViewportI.java @@ -166,7 +166,7 @@ public interface AlignViewportI extends ViewStyleI * * @return */ - AlignCalcManagerI getCalcManager(); + AlignCalcManagerI2 getCalcManager(); /** * get the percentage gaps allowed in a conservation calculation diff --git a/src/jalview/appletgui/AlignViewport.java b/src/jalview/appletgui/AlignViewport.java index 0fa569e..97eb0c0 100644 --- a/src/jalview/appletgui/AlignViewport.java +++ b/src/jalview/appletgui/AlignViewport.java @@ -63,7 +63,7 @@ public class AlignViewport extends AlignmentViewport public AlignViewport(AlignmentI al, JalviewLite applet) { super(al); - calculator = new jalview.workers.AlignCalcManager(); + calculator = new jalview.workers.AlignCalcManager2(); this.applet = applet; // we always pad gaps diff --git a/src/jalview/viewmodel/AlignmentViewport.java b/src/jalview/viewmodel/AlignmentViewport.java index 8f1fc83..77210ee 100644 --- a/src/jalview/viewmodel/AlignmentViewport.java +++ b/src/jalview/viewmodel/AlignmentViewport.java @@ -24,6 +24,8 @@ import jalview.analysis.AnnotationSorter.SequenceAnnotationOrder; import jalview.analysis.Conservation; import jalview.analysis.TreeModel; import jalview.api.AlignCalcManagerI; +import jalview.api.AlignCalcManagerI2; +import jalview.api.AlignCalcWorkerI; import jalview.api.AlignExportSettingsI; import jalview.api.AlignViewportI; import jalview.api.AlignmentViewPanel; @@ -57,6 +59,7 @@ import jalview.util.MappingUtils; import jalview.util.MessageManager; import jalview.viewmodel.styles.ViewStyle; import jalview.workers.AlignCalcManager; +import jalview.workers.AlignCalcManager2; import jalview.workers.ComplementConsensusThread; import jalview.workers.ConsensusThread; import jalview.workers.InformationThread; @@ -848,7 +851,7 @@ public abstract class AlignmentViewport return strucConsensus; } - protected AlignCalcManagerI calculator = new AlignCalcManager(); + protected AlignCalcManagerI2 calculator = new AlignCalcManager2(); /** * trigger update of conservation annotation @@ -862,8 +865,8 @@ public abstract class AlignmentViewport { return; } - if (calculator.getRegisteredWorkersOfClass( - jalview.workers.ConservationThread.class) == null) + if (calculator.getWorkersOfClass( + jalview.workers.ConservationThread.class).isEmpty()) { calculator.registerWorker( new jalview.workers.ConservationThread(this, ap)); @@ -880,8 +883,7 @@ public abstract class AlignmentViewport { return; } - if (calculator - .getRegisteredWorkersOfClass(ConsensusThread.class) == null) + if (calculator.getWorkersOfClass(ConsensusThread.class).isEmpty()) { calculator.registerWorker(new ConsensusThread(this, ap)); } @@ -912,11 +914,9 @@ public abstract class AlignmentViewport } if (doConsensus) { - if (calculator.getRegisteredWorkersOfClass( - ComplementConsensusThread.class) == null) + if (calculator.getWorkersOfClass(ComplementConsensusThread.class).isEmpty()) { - calculator - .registerWorker(new ComplementConsensusThread(this, ap)); + calculator.registerWorker(new ComplementConsensusThread(this, ap)); } } } @@ -925,8 +925,7 @@ public abstract class AlignmentViewport @Override public void initInformationWorker(final AlignmentViewPanel ap) { - if (calculator - .getRegisteredWorkersOfClass(InformationThread.class) == null) + if (calculator.getWorkersOfClass(InformationThread.class).isEmpty()) { calculator.registerWorker(new InformationThread(this, ap)); } @@ -947,8 +946,7 @@ public abstract class AlignmentViewport { return; } - if (calculator.getRegisteredWorkersOfClass( - StrucConsensusThread.class) == null) + if (calculator.getWorkersOfClass(StrucConsensusThread.class).isEmpty()) { calculator.registerWorker(new StrucConsensusThread(this, ap)); } @@ -967,7 +965,7 @@ public abstract class AlignmentViewport { return false; } - if (calculator.workingInvolvedWith(alignmentAnnotation)) + if (calculator.isWorkingWithAnnotation(alignmentAnnotation)) { // System.err.println("grey out ("+alignmentAnnotation.label+")"); return true; @@ -1020,7 +1018,7 @@ public abstract class AlignmentViewport } @Override - public AlignCalcManagerI getCalcManager() + public AlignCalcManagerI2 getCalcManager() { return calculator; } @@ -1094,9 +1092,15 @@ public abstract class AlignmentViewport // TODO: decouple settings setting from calculation when refactoring // annotation update method from alignframe to viewport this.showSequenceLogo = showSequenceLogo; - calculator.updateAnnotationFor(ConsensusThread.class); - calculator.updateAnnotationFor(ComplementConsensusThread.class); - calculator.updateAnnotationFor(StrucConsensusThread.class); + for (AlignCalcWorkerI worker : calculator.getWorkers()) + { + if (worker.getClass().equals(ConsensusThread.class) || + worker.getClass().equals(ComplementConsensusThread.class) || + worker.getClass().equals(StrucConsensusThread.class)) + { + worker.updateAnnotation(); + } + } } this.showSequenceLogo = showSequenceLogo; } diff --git a/src/jalview/workers/AlignCalcManager.java b/src/jalview/workers/AlignCalcManager.java index 1967375..79ca863 100644 --- a/src/jalview/workers/AlignCalcManager.java +++ b/src/jalview/workers/AlignCalcManager.java @@ -27,6 +27,7 @@ import jalview.datamodel.AlignmentAnnotation; import java.util.ArrayList; import java.util.Collections; +import java.util.HashMap; import java.util.HashSet; import java.util.Hashtable; import java.util.List; @@ -38,45 +39,33 @@ public class AlignCalcManager implements AlignCalcManagerI /* * list of registered workers */ - private volatile List restartable; + private final List restartable = Collections + .synchronizedList(new ArrayList()); /* * types of worker _not_ to run (for example, because they have * previously thrown errors) */ - private volatile List> blackList; + private final List> blackList = Collections + .synchronizedList(new ArrayList>()); /* * global record of calculations in progress */ - private volatile List inProgress; + private final List inProgress = Collections + .synchronizedList(new ArrayList()); /* * record of calculations pending or in progress in the current context */ - private volatile Map, List> updating; + private final Map, List> updating = + new Hashtable, List>(); /* * workers that have run to completion so are candidates for visual-only * update of their results */ - private HashSet canUpdate; - - /** - * Constructor - */ - public AlignCalcManager() - { - restartable = Collections - .synchronizedList(new ArrayList()); - blackList = Collections.synchronizedList( - new ArrayList>()); - inProgress = Collections - .synchronizedList(new ArrayList()); - updating = Collections.synchronizedMap( - new Hashtable, List>()); - canUpdate = new HashSet<>(); - } + private HashSet canUpdate = new HashSet<>();; private static boolean listContains(List upd, AlignCalcWorkerI worker) @@ -92,7 +81,7 @@ public class AlignCalcManager implements AlignCalcManagerI return false; } @Override - public void notifyStart(AlignCalcWorkerI worker) + public void notifyStarted(AlignCalcWorkerI worker) { synchronized (updating) { @@ -126,22 +115,10 @@ public class AlignCalcManager implements AlignCalcManagerI @Override public boolean isPending(AlignCalcWorkerI workingClass) { - List upd; synchronized (updating) { - upd = updating.get(workingClass.getClass()); - if (upd == null) - { - return false; - } - synchronized (upd) - { - if (upd.size() > 1) - { - return true; - } - } - return false; + List upd = updating.get(workingClass.getClass()); + return upd != null && upd.size() > 1; } } @@ -204,7 +181,15 @@ public class AlignCalcManager implements AlignCalcManagerI { if (!isDisabled(worker)) { - Thread tw = new Thread(worker); + Thread tw = new Thread(() -> { + try + { + worker.run(); + } catch (Exception e) + { + e.printStackTrace(); + } + }); tw.setName(worker.getClass().toString()); tw.start(); } @@ -332,7 +317,7 @@ public class AlignCalcManager implements AlignCalcManagerI } @Override - public void removeRegisteredWorkersOfClass( + public void removeWorkersOfClass( Class typeToRemove) { List removable = new ArrayList<>(); diff --git a/src/jalview/workers/AlignCalcManager2.java b/src/jalview/workers/AlignCalcManager2.java new file mode 100644 index 0000000..7d3cca6 --- /dev/null +++ b/src/jalview/workers/AlignCalcManager2.java @@ -0,0 +1,319 @@ +package jalview.workers; + +import java.util.List; +import java.util.Set; +import java.util.concurrent.Callable; +import java.util.concurrent.CancellationException; +import java.util.concurrent.CopyOnWriteArrayList; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.FutureTask; +import java.util.concurrent.atomic.AtomicInteger; +import java.util.stream.Collectors; + +import jalview.api.AlignCalcListener; +import jalview.api.AlignCalcManagerI; +import jalview.api.AlignCalcManagerI2; +import jalview.api.AlignCalcWorkerI; +import jalview.datamodel.AlignmentAnnotation; + +import static java.util.Collections.synchronizedList; +import static java.util.Collections.synchronizedSet; +import static java.util.Collections.unmodifiableList; + +import java.util.ArrayList; +import java.util.HashSet; + +public class AlignCalcManager2 implements AlignCalcManagerI2 +{ + class AlignCalcTask extends FutureTask + { + final AlignCalcWorkerI worker; + + public AlignCalcTask(AlignCalcWorkerI worker) + { + super(new Callable() { + public Void call() throws Exception { + notifyStarted(worker); + worker.run(); + return null; + } + }); + this.worker = worker; + } + + public AlignCalcWorkerI getWorker() + { + return worker; + } + + @Override + protected void done() + { + boolean success = false; + Throwable exception = null; + try + { + get(); + success = true; + } catch (ExecutionException e) + { + exception = e.getCause(); + if (exception instanceof OutOfMemoryError) { + disableWorker(getWorker()); + } + } catch (Throwable e) + { + exception = e; + } + finally { + inProgress.remove(getWorker()); + tasks.remove(this); + } + if (success) + notifyCompleted(worker); + else + notifyExceptional(worker, exception); + } + } + + // main executor for running workers one-by-one + private final ExecutorService executor = Executors.newSingleThreadExecutor(); + + private final List listeners = new CopyOnWriteArrayList(); + + // list of all registered workers (other collections are subsets of this) + private final List registered = synchronizedList(new ArrayList<>()); + + // list of tasks holding queued and running workers + private final List tasks = synchronizedList(new ArrayList<>()); + + // the collection of currently running workers + private final Set inProgress = synchronizedSet(new HashSet<>()); + + // the collection of workers that will not be started + private final Set disabled = synchronizedSet(new HashSet<>()); + + /* + * Register the worker with this manager and scheduler for execution. + */ + @Override + public void registerWorker(AlignCalcWorkerI worker) + { + synchronized (registered) + { + if (!registered.contains(worker)) + registered.add(worker); + } + startWorker(worker); + } + + @Override + public List getWorkers() + { + return unmodifiableList(new ArrayList<>(registered)); + } + + @Override + public List getWorkersOfClass( + Class cls) + { + synchronized (registered) + { + return registered.stream() + .filter(worker -> worker.getClass().equals(cls)) + .collect(Collectors.toUnmodifiableList()); + } + } + + @Override + public void removeWorker(AlignCalcWorkerI worker) + { + registered.remove(worker); + disabled.remove(worker); + } + + @Override + public void removeWorkersOfClass(Class cls) + { + synchronized (registered) + { + for (var it = registered.iterator(); it.hasNext();) + { + var worker = it.next(); + if (worker.getClass().equals(cls)) + { + it.remove(); + disabled.remove(worker); + } + } + } + } + + @Override + public void removeWorkerForAnnotation(AlignmentAnnotation annot) + { + synchronized (registered) + { + for (var it = registered.iterator(); it.hasNext();) + { + var worker = it.next(); + if (worker.involves(annot) && worker.isDeletable()) + { + it.remove(); + disabled.remove(worker); + } + } + } + } + + @Override + public void disableWorker(AlignCalcWorkerI worker) + { + assert registered.contains(worker); + disabled.add(worker); + } + + @Override + public void enableWorker(AlignCalcWorkerI worker) + { + disabled.remove(worker); + } + + @Override + public void restartWorkers() + { + synchronized (registered) + { + for (AlignCalcWorkerI worker : registered) + { + if (!isDisabled(worker)) + startWorker(worker); + } + } + } + + @Override + public void startWorker(AlignCalcWorkerI worker) + { + assert registered.contains(worker); + synchronized (tasks) { + for (var task : tasks) + { + if (task.getWorker().equals(worker)) + task.cancel(true); + } + } + AlignCalcTask newTask = new AlignCalcTask(worker); + tasks.add(newTask); + notifyQueued(worker); + executor.execute(newTask); + } + + @Override + public void cancelWorker(AlignCalcWorkerI worker) + { + if (isWorking(worker)) + { + synchronized (tasks) + { + for (var task : tasks) + { + if (task.getWorker().equals(worker)) + task.cancel(true); + } + } + } + } + + @Override + public boolean isDisabled(AlignCalcWorkerI worker) + { + return disabled.contains(worker); + } + + @Override + public boolean isWorking(AlignCalcWorkerI worker) + { + return inProgress.contains(worker); + } + + @Override + public boolean isWorking() + { + return !inProgress.isEmpty(); + } + + @Override + public boolean isWorkingWithAnnotation(AlignmentAnnotation annot) + { + synchronized (inProgress) + { + for (AlignCalcWorkerI worker : inProgress) + { + if (worker.involves(annot)) + { + return true; + } + } + } + return false; + } + + private void notifyQueued(AlignCalcWorkerI worker) + { + for (AlignCalcListener listener : listeners) + { + listener.workerQueued(worker); + } + } + + private void notifyStarted(AlignCalcWorkerI worker) + { + for (AlignCalcListener listener : listeners) + { + listener.workerStarted(worker); + } + } + + private void notifyCompleted(AlignCalcWorkerI worker) + { + for (AlignCalcListener listener : listeners) + { + try { + listener.workerCompleted(worker); + } catch (RuntimeException e) + { + e.printStackTrace(); + } + } + } + + private void notifyExceptional(AlignCalcWorkerI worker, + Throwable throwable) + { + for (AlignCalcListener listener : listeners) + { + try { + listener.workerExceptional(worker, throwable); + } catch (RuntimeException e) + { + e.printStackTrace(); + } + } + } + + @Override + public void addAlignCalcListener(AlignCalcListener listener) + { + listeners.add(listener); + } + + @Override + public void removeAlignCalcListener(AlignCalcListener listener) + { + listeners.remove(listener); + } + +} diff --git a/src/jalview/workers/AlignCalcWorker.java b/src/jalview/workers/AlignCalcWorker.java index cf4805d..c94032f 100644 --- a/src/jalview/workers/AlignCalcWorker.java +++ b/src/jalview/workers/AlignCalcWorker.java @@ -21,6 +21,7 @@ package jalview.workers; import jalview.api.AlignCalcManagerI; +import jalview.api.AlignCalcManagerI2; import jalview.api.AlignCalcWorkerI; import jalview.api.AlignViewportI; import jalview.api.AlignmentViewPanel; @@ -43,7 +44,7 @@ public abstract class AlignCalcWorker implements AlignCalcWorkerI */ protected AlignViewportI alignViewport; - protected AlignCalcManagerI calcMan; + protected AlignCalcManagerI2 calcMan; protected AlignmentViewPanel ap; @@ -61,7 +62,8 @@ public abstract class AlignCalcWorker implements AlignCalcWorkerI { if (calcMan != null) { - calcMan.workerComplete(this); + calcMan.cancelWorker(this); + calcMan.removeWorker(this); } alignViewport = null; calcMan = null; diff --git a/src/jalview/workers/AnnotationWorker.java b/src/jalview/workers/AnnotationWorker.java index 8f37f15..98e26b3 100644 --- a/src/jalview/workers/AnnotationWorker.java +++ b/src/jalview/workers/AnnotationWorker.java @@ -59,63 +59,41 @@ class AnnotationWorker extends AlignCalcWorker @Override public void run() { - try + if (alignViewport.isClosed()) { - calcMan.notifyStart(this); - - while (!calcMan.notifyWorking(this)) - { - try - { - Thread.sleep(200); - } catch (InterruptedException ex) - { - ex.printStackTrace(); - } - } - if (alignViewport.isClosed()) - { - abortAndDestroy(); - return; - } + abortAndDestroy(); + return; + } - // removeAnnotation(); + // removeAnnotation(); AlignmentI alignment = alignViewport.getAlignment(); - if (alignment != null) + if (alignment != null) + { + try { - try + List anns = counter.calculateAnnotation( + alignment, new FeatureRenderer(alignViewport)); + for (AlignmentAnnotation ann : anns) { - List anns = counter.calculateAnnotation( - alignment, new FeatureRenderer(alignViewport)); - for (AlignmentAnnotation ann : anns) + AlignmentAnnotation theAnn = alignment.findOrCreateAnnotation( + ann.label, ann.description, false, null, null); + theAnn.showAllColLabels = true; + theAnn.graph = AlignmentAnnotation.BAR_GRAPH; + theAnn.scaleColLabel = true; + theAnn.annotations = ann.annotations; + setGraphMinMax(theAnn, theAnn.annotations); + theAnn.validateRangeAndDisplay(); + if (!ourAnnots.contains(theAnn)) { - AlignmentAnnotation theAnn = alignment.findOrCreateAnnotation( - ann.label, ann.description, false, null, null); - theAnn.showAllColLabels = true; - theAnn.graph = AlignmentAnnotation.BAR_GRAPH; - theAnn.scaleColLabel = true; - theAnn.annotations = ann.annotations; - setGraphMinMax(theAnn, theAnn.annotations); - theAnn.validateRangeAndDisplay(); - if (!ourAnnots.contains(theAnn)) - { - ourAnnots.add(theAnn); - } - // alignment.addAnnotation(ann); + ourAnnots.add(theAnn); } - } catch (IndexOutOfBoundsException x) - { - // probable race condition. just finish and return without any fuss. - return; + // alignment.addAnnotation(ann); } + } catch (IndexOutOfBoundsException x) + { + // probable race condition. just finish and return without any fuss. + return; } - } catch (OutOfMemoryError error) - { - ap.raiseOOMWarning("calculating annotations", error); - calcMan.disableWorker(this); - } finally - { - calcMan.workerComplete(this); } if (ap != null) diff --git a/src/jalview/workers/ColumnCounterSetWorker.java b/src/jalview/workers/ColumnCounterSetWorker.java index 74695fe..a711f2b 100644 --- a/src/jalview/workers/ColumnCounterSetWorker.java +++ b/src/jalview/workers/ColumnCounterSetWorker.java @@ -70,44 +70,22 @@ class ColumnCounterSetWorker extends AlignCalcWorker public void run() { boolean annotationAdded = false; - try + if (alignViewport.isClosed()) { - calcMan.notifyStart(this); + abortAndDestroy(); + return; + } - while (!calcMan.notifyWorking(this)) + if (alignViewport.getAlignment() != null) + { + try { - try - { - Thread.sleep(200); - } catch (InterruptedException ex) - { - ex.printStackTrace(); - } - } - if (alignViewport.isClosed()) + annotationAdded = computeAnnotations(); + } catch (IndexOutOfBoundsException x) { - abortAndDestroy(); + // probable race condition. just finish and return without any fuss. return; } - - if (alignViewport.getAlignment() != null) - { - try - { - annotationAdded = computeAnnotations(); - } catch (IndexOutOfBoundsException x) - { - // probable race condition. just finish and return without any fuss. - return; - } - } - } catch (OutOfMemoryError error) - { - ap.raiseOOMWarning("calculating feature counts", error); - calcMan.disableWorker(this); - } finally - { - calcMan.workerComplete(this); } if (ap != null) diff --git a/src/jalview/workers/ConsensusThread.java b/src/jalview/workers/ConsensusThread.java index 32204b9..b97c4c5 100644 --- a/src/jalview/workers/ConsensusThread.java +++ b/src/jalview/workers/ConsensusThread.java @@ -41,71 +41,33 @@ public class ConsensusThread extends AlignCalcWorker @Override public void run() { - if (calcMan.isPending(this)) + AlignmentAnnotation consensus = getConsensusAnnotation(); + AlignmentAnnotation gap = getGapAnnotation(); + if ((consensus == null && gap == null)) { return; } - calcMan.notifyStart(this); - // long started = System.currentTimeMillis(); - try + if (alignViewport.isClosed()) { - AlignmentAnnotation consensus = getConsensusAnnotation(); - AlignmentAnnotation gap = getGapAnnotation(); - if ((consensus == null && gap == null) || calcMan.isPending(this)) - { - calcMan.workerComplete(this); - return; - } - while (!calcMan.notifyWorking(this)) - { - // System.err.println("Thread - // (Consensus"+Thread.currentThread().getName()+") Waiting around."); - try - { - if (ap != null) - { - ap.paintAlignment(false, false); - } - Thread.sleep(200); - } catch (Exception ex) - { - ex.printStackTrace(); - } - } - if (alignViewport.isClosed()) - { - abortAndDestroy(); - return; - } - AlignmentI alignment = alignViewport.getAlignment(); + abortAndDestroy(); + return; + } + AlignmentI alignment = alignViewport.getAlignment(); - int aWidth = -1; + int aWidth = -1; - if (alignment == null || (aWidth = alignment.getWidth()) < 0) - { - calcMan.workerComplete(this); - return; - } + if (alignment == null || (aWidth = alignment.getWidth()) < 0) + { + return; + } - eraseConsensus(aWidth); - computeConsensus(alignment); - updateResultAnnotation(true); + eraseConsensus(aWidth); + computeConsensus(alignment); + updateResultAnnotation(true); - if (ap != null) - { - ap.paintAlignment(true, true); - } - } catch (OutOfMemoryError error) - { - calcMan.disableWorker(this); - ap.raiseOOMWarning("calculating consensus", error); - } finally + if (ap != null) { - /* - * e.g. ArrayIndexOutOfBoundsException can happen due to a race condition - * - alignment was edited at same time as calculation was running - */ - calcMan.workerComplete(this); + ap.paintAlignment(true, true); } } diff --git a/src/jalview/workers/ConservationThread.java b/src/jalview/workers/ConservationThread.java index 54b0191..3752841 100644 --- a/src/jalview/workers/ConservationThread.java +++ b/src/jalview/workers/ConservationThread.java @@ -51,69 +51,39 @@ public class ConservationThread extends AlignCalcWorker @Override public void run() { - try + if ((alignViewport == null) || (calcMan == null) + || (alignViewport.isClosed())) { - calcMan.notifyStart(this); // updatingConservation = true; - - while ((calcMan != null) && (!calcMan.notifyWorking(this))) - { - try - { - if (ap != null) - { - // ap.paintAlignment(false); - } - Thread.sleep(200); - } catch (Exception ex) - { - ex.printStackTrace(); - } - } - if ((alignViewport == null) || (calcMan == null) - || (alignViewport.isClosed())) - { - abortAndDestroy(); - return; - } - List ourAnnot = new ArrayList<>(); - AlignmentI alignment = alignViewport.getAlignment(); - conservation = alignViewport.getAlignmentConservationAnnotation(); - quality = alignViewport.getAlignmentQualityAnnot(); - ourAnnot.add(conservation); - ourAnnot.add(quality); - ourAnnots = ourAnnot; - ConsPercGaps = alignViewport.getConsPercGaps(); - // AlignViewport.UPDATING_CONSERVATION = true; - - if (alignment == null || (alWidth = alignment.getWidth()) < 0) - { - calcMan.workerComplete(this); - // .updatingConservation = false; - // AlignViewport.UPDATING_CONSERVATION = false; + abortAndDestroy(); + return; + } + List ourAnnot = new ArrayList<>(); + AlignmentI alignment = alignViewport.getAlignment(); + conservation = alignViewport.getAlignmentConservationAnnotation(); + quality = alignViewport.getAlignmentQualityAnnot(); + ourAnnot.add(conservation); + ourAnnot.add(quality); + ourAnnots = ourAnnot; + ConsPercGaps = alignViewport.getConsPercGaps(); + // AlignViewport.UPDATING_CONSERVATION = true; - return; - } - try - { - cons = Conservation.calculateConservation("All", - alignment.getSequences(), 0, alWidth - 1, false, - ConsPercGaps, quality != null); - } catch (IndexOutOfBoundsException x) - { - // probable race condition. just finish and return without any fuss. - calcMan.workerComplete(this); - return; - } - updateResultAnnotation(true); - } catch (OutOfMemoryError error) + if (alignment == null || (alWidth = alignment.getWidth()) < 0) { - ap.raiseOOMWarning("calculating conservation", error); - calcMan.disableWorker(this); - // alignViewport.conservation = null; - // this.alignViewport.quality = null; - + // .updatingConservation = false; + // AlignViewport.UPDATING_CONSERVATION = false; + return; + } + try + { + cons = Conservation.calculateConservation("All", + alignment.getSequences(), 0, alWidth - 1, false, + ConsPercGaps, quality != null); + } catch (IndexOutOfBoundsException x) + { + // probable race condition. just finish and return without any fuss. + return; } - calcMan.workerComplete(this); + updateResultAnnotation(true); if ((alignViewport == null) || (calcMan == null) || (alignViewport.isClosed())) diff --git a/src/jalview/workers/InformationThread.java b/src/jalview/workers/InformationThread.java index 85cd92f..c8084b9 100644 --- a/src/jalview/workers/InformationThread.java +++ b/src/jalview/workers/InformationThread.java @@ -48,73 +48,34 @@ public class InformationThread extends AlignCalcWorker { return; } - if (calcMan.isPending(this)) + if (alignViewport.isClosed()) { + abortAndDestroy(); return; } - calcMan.notifyStart(this); - // long started = System.currentTimeMillis(); - try + AlignmentI alignment = alignViewport.getAlignment(); + int aWidth = alignment == null ? -1 : alignment.getWidth(); + if (aWidth < 0) { - if (calcMan.isPending(this)) - { - // another instance of this is waiting to run - calcMan.workerComplete(this); - return; - } - while (!calcMan.notifyWorking(this)) - { - // another thread in progress, wait my turn - try - { - if (ap != null) - { - ap.paintAlignment(false, false); - } - Thread.sleep(200); - } catch (Exception ex) - { - ex.printStackTrace(); - } - } - if (alignViewport.isClosed()) - { - abortAndDestroy(); - return; - } - - AlignmentI alignment = alignViewport.getAlignment(); - int aWidth = alignment == null ? -1 : alignment.getWidth(); - if (aWidth < 0) - { - calcMan.workerComplete(this); - return; - } + return; + } - /* - * compute information profiles for any HMM consensus sequences - * for the alignment or sub-groups - */ - computeProfiles(alignment); + /* + * compute information profiles for any HMM consensus sequences + * for the alignment or sub-groups + */ + computeProfiles(alignment); - /* - * construct the corresponding annotations - */ - updateAnnotation(); + /* + * construct the corresponding annotations + */ + updateAnnotation(); - if (ap != null) - { - ap.adjustAnnotationHeight(); - ap.paintAlignment(true, true); - } - } catch (OutOfMemoryError error) - { - calcMan.disableWorker(this); - ap.raiseOOMWarning("calculating information", error); - } finally + if (ap != null) { - calcMan.workerComplete(this); + ap.adjustAnnotationHeight(); + ap.paintAlignment(true, true); } } diff --git a/src/jalview/workers/StrucConsensusThread.java b/src/jalview/workers/StrucConsensusThread.java index 61ec3d0..3cb6114 100644 --- a/src/jalview/workers/StrucConsensusThread.java +++ b/src/jalview/workers/StrucConsensusThread.java @@ -47,28 +47,6 @@ public class StrucConsensusThread extends AlignCalcWorker @Override public void run() { - try - { - if (calcMan.isPending(this)) - { - return; - } - calcMan.notifyStart(this); - while (!calcMan.notifyWorking(this)) - { - try - { - if (ap != null) - { - // ap.paintAlignment(false); - } - - Thread.sleep(200); - } catch (Exception ex) - { - ex.printStackTrace(); - } - } if (alignViewport.isClosed()) { abortAndDestroy(); @@ -80,7 +58,6 @@ public class StrucConsensusThread extends AlignCalcWorker if (alignment == null || (aWidth = alignment.getWidth()) < 0) { - calcMan.workerComplete(this); return; } strucConsensus = alignViewport.getAlignmentStrucConsensusAnnotation(); @@ -109,7 +86,6 @@ public class StrucConsensusThread extends AlignCalcWorker if (rnaStruc == null || !rnaStruc.isValidStruc()) { - calcMan.workerComplete(this); return; } @@ -121,27 +97,11 @@ public class StrucConsensusThread extends AlignCalcWorker alignment.getWidth(), hStrucConsensus, true, rnaStruc); } catch (ArrayIndexOutOfBoundsException x) { - calcMan.workerComplete(this); return; } alignViewport.setRnaStructureConsensusHash(hStrucConsensus); // TODO AlignmentAnnotation rnaStruc!!! updateResultAnnotation(true); - } catch (OutOfMemoryError error) - { - calcMan.disableWorker(this); - - // consensus = null; - // hconsensus = null; - ap.raiseOOMWarning("calculating RNA structure consensus", error); - } finally - { - calcMan.workerComplete(this); - if (ap != null) - { - ap.paintAlignment(true, true); - } - } } diff --git a/src/jalview/ws/jws2/Jws2ClientFactory.java b/src/jalview/ws/jws2/Jws2ClientFactory.java index 6445cd5..2456528 100644 --- a/src/jalview/ws/jws2/Jws2ClientFactory.java +++ b/src/jalview/ws/jws2/Jws2ClientFactory.java @@ -52,7 +52,7 @@ public class Jws2ClientFactory { List aaconClient = alignFrame.getViewport() .getCalcManager() - .getRegisteredWorkersOfClass(aaui.getClient()); + .getWorkersOfClass(aaui.getClient()); if (aaconClient != null && aaconClient.size() > 0) { SeqAnnotationServiceCalcWorker worker = (SeqAnnotationServiceCalcWorker) aaconClient @@ -88,8 +88,8 @@ public class Jws2ClientFactory { List aaconClient = alignFrame.getViewport() - .getCalcManager().getRegisteredWorkersOfClass( - SeqAnnotationServiceCalcWorker.class); + .getCalcManager() + .getWorkersOfClass(SeqAnnotationServiceCalcWorker.class); if (aaconClient != null) { for (AlignCalcWorkerI worker : aaconClient) @@ -142,8 +142,8 @@ public class Jws2ClientFactory return; } List aaconClient = alignFrame.getViewport() - .getCalcManager().getRegisteredWorkersOfClass( - SeqAnnotationServiceCalcWorker.class); + .getCalcManager() + .getWorkersOfClass(SeqAnnotationServiceCalcWorker.class); boolean serviceEnabled = false; if (aaconClient != null) @@ -287,6 +287,6 @@ public class Jws2ClientFactory AlignFrame alignFrame) { alignFrame.getViewport().getCalcManager() - .removeRegisteredWorkersOfClass(aaui.getClient()); + .removeWorkersOfClass(aaui.getClient()); } } \ No newline at end of file diff --git a/src/jalview/ws/jws2/SeqAnnotationServiceCalcWorker.java b/src/jalview/ws/jws2/SeqAnnotationServiceCalcWorker.java index 132408b..ae03dfb 100644 --- a/src/jalview/ws/jws2/SeqAnnotationServiceCalcWorker.java +++ b/src/jalview/ws/jws2/SeqAnnotationServiceCalcWorker.java @@ -227,15 +227,15 @@ public class SeqAnnotationServiceCalcWorker extends AlignCalcWorker private int min_valid_seqs; @Override - public void run() + public void run() throws Exception { - if (checkDone()) + if (alignViewport.isClosed()) { + abortAndDestroy(); return; } if (!hasService()) { - calcMan.workerComplete(this); return; } @@ -257,7 +257,6 @@ public class SeqAnnotationServiceCalcWorker extends AlignCalcWorker { jalview.bin.Cache.log.debug( "Sequences for analysis service were null or not valid"); - calcMan.workerComplete(this); return; } @@ -329,41 +328,13 @@ public class SeqAnnotationServiceCalcWorker extends AlignCalcWorker Cache.log.debug("Status now " + running.getState()); } - if (calcMan.isPending(this) && isInteractiveUpdate()) - { - Cache.log.debug("Analysis service job is stale. aborting."); - // job has become stale. - if (!finished) { - finished = true; - // cancel this job and yield to the new job - try - { - if (cancellable - && ((CancellableI) annotService).cancel(running)) - { - System.err.println("Cancelled job: " + rslt); - } - else - { - System.err.println("FAILED TO CANCEL job: " + rslt); - } - - } catch (Exception x) - { - - } - } - rslt = running.getJobHandle(); - return; - } - // pull any stats - some services need to flush log output before // results are available Cache.log.debug("Updating progress log for annotation service."); try { - annotService.updateJobProgress(running); + annotService.updateJobProgress(running); } catch (Throwable thr) { Cache.log.debug("Ignoring exception during progress update.", @@ -523,10 +494,10 @@ public class SeqAnnotationServiceCalcWorker extends AlignCalcWorker System.err .println("Blacklisting worker due to unexpected exception:"); x.printStackTrace(); + throw new Exception(x); } finally { - calcMan.workerComplete(this); if (ap != null) { if (guiProgress != null && progressId != -1) @@ -889,43 +860,6 @@ public class SeqAnnotationServiceCalcWorker extends AlignCalcWorker } } - /** - * notify manager that we have started, and wait for a free calculation slot - * - * @return true if slot is obtained and work still valid, false if another - * thread has done our work for us. - */ - protected boolean checkDone() - { - calcMan.notifyStart(this); - ap.paintAlignment(false, false); - while (!calcMan.notifyWorking(this)) - { - if (calcMan.isWorking(this)) - { - return true; - } - try - { - if (ap != null) - { - ap.paintAlignment(false, false); - } - - Thread.sleep(200); - } catch (Exception ex) - { - ex.printStackTrace(); - } - } - if (alignViewport.isClosed()) - { - abortAndDestroy(); - return true; - } - return false; - } - protected void updateOurAnnots(List ourAnnot) { List our = ourAnnots; diff --git a/src/jalview/ws/jws2/SequenceAnnotationWSClient.java b/src/jalview/ws/jws2/SequenceAnnotationWSClient.java index 11ce798..16d8220 100644 --- a/src/jalview/ws/jws2/SequenceAnnotationWSClient.java +++ b/src/jalview/ws/jws2/SequenceAnnotationWSClient.java @@ -83,7 +83,7 @@ public class SequenceAnnotationWSClient extends Jws2Client List clnts = alignFrame.getViewport() .getCalcManager() - .getRegisteredWorkersOfClass(SeqAnnotationServiceCalcWorker.class); + .getWorkersOfClass(SeqAnnotationServiceCalcWorker.class); SeqAnnotationServiceCalcWorker worker = null; if (clnts != null) diff --git a/test/jalview/workers/AlignCalcManagerTest.java b/test/jalview/workers/AlignCalcManagerTest.java index 9cc4fc2..f0fcdba 100644 --- a/test/jalview/workers/AlignCalcManagerTest.java +++ b/test/jalview/workers/AlignCalcManagerTest.java @@ -25,6 +25,7 @@ import static org.testng.AssertJUnit.assertFalse; import static org.testng.AssertJUnit.assertTrue; import jalview.api.AlignCalcManagerI; +import jalview.api.AlignCalcManagerI2; import jalview.api.AlignCalcWorkerI; import jalview.api.FeatureRenderer; import jalview.datamodel.Alignment; @@ -63,7 +64,7 @@ public class AlignCalcManagerTest @Test(groups = "Functional") public void testRemoveWorkerForAnnotation() { - AlignCalcManagerI acm = alignFrame.getViewport().getCalcManager(); + AlignCalcManagerI2 acm = alignFrame.getViewport().getCalcManager(); final AlignmentAnnotation ann1 = new AlignmentAnnotation("Ann1", "desc", new Annotation[] {}); final AlignmentAnnotation ann2 = new AlignmentAnnotation("Ann2", @@ -96,8 +97,7 @@ public class AlignCalcManagerTest } } - List workers = acm - .getRegisteredWorkersOfClass(worker1.getClass()); + List workers = acm.getWorkersOfClass(worker1.getClass()); assertEquals(2, workers.size()); assertTrue(workers.contains(worker1)); assertTrue(workers.contains(worker2)); @@ -108,10 +108,8 @@ public class AlignCalcManagerTest * remove workers for ann2 (there aren't any) */ acm.removeWorkerForAnnotation(ann2); - assertTrue(acm.getRegisteredWorkersOfClass(worker1.getClass()) - .contains(worker1)); - assertTrue(acm.getRegisteredWorkersOfClass(worker1.getClass()) - .contains(worker2)); + assertTrue(acm.getWorkersOfClass(worker1.getClass()).contains(worker1)); + assertTrue(acm.getWorkersOfClass(worker1.getClass()).contains(worker2)); assertFalse(acm.isDisabled(worker1)); assertFalse(acm.isDisabled(worker2)); @@ -120,10 +118,8 @@ public class AlignCalcManagerTest * - should delete worker1 but not worker2 */ acm.removeWorkerForAnnotation(ann1); - assertEquals(1, acm.getRegisteredWorkersOfClass(worker1.getClass()) - .size()); - assertTrue(acm.getRegisteredWorkersOfClass(worker1.getClass()) - .contains(worker2)); + assertEquals(1, acm.getWorkersOfClass(worker1.getClass()).size()); + assertTrue(acm.getWorkersOfClass(worker1.getClass()).contains(worker2)); assertFalse(acm.isDisabled(worker1)); assertFalse(acm.isDisabled(worker2)); } -- 1.7.10.2