From f61b2748a32ef03e687abc37b97640873c2e44aa Mon Sep 17 00:00:00 2001 From: jprocter Date: Tue, 12 Jun 2012 11:57:52 +0100 Subject: [PATCH] JAL-961, JAL-1115 - pending test to ensure only one worker waits around for another one for the same type to complete. --- src/jalview/api/AlignCalcManagerI.java | 10 ++ src/jalview/workers/AlignCalcManager.java | 221 +++++++++++++++++++------ src/jalview/workers/ConsensusThread.java | 31 +++- src/jalview/workers/ConservationThread.java | 2 +- src/jalview/workers/StrucConsensusThread.java | 26 ++- 5 files changed, 221 insertions(+), 69 deletions(-) diff --git a/src/jalview/api/AlignCalcManagerI.java b/src/jalview/api/AlignCalcManagerI.java index a7b17c6..1eedc74 100644 --- a/src/jalview/api/AlignCalcManagerI.java +++ b/src/jalview/api/AlignCalcManagerI.java @@ -105,4 +105,14 @@ public interface AlignCalcManagerI */ boolean startRegisteredWorkersOfClass(Class workerClass); + /** + * work out if there is an instance of a worker that is *waiting* to start + * calculating + * + * @param workingClass + * @return true if workingClass is already waiting to calculate. false if it + * is calculating, or not queued. + */ + boolean isPending(AlignCalcWorkerI workingClass); + } diff --git a/src/jalview/workers/AlignCalcManager.java b/src/jalview/workers/AlignCalcManager.java index 784c64e..c42e424 100644 --- a/src/jalview/workers/AlignCalcManager.java +++ b/src/jalview/workers/AlignCalcManager.java @@ -1,6 +1,7 @@ package jalview.workers; import java.util.ArrayList; +import java.util.Collections; import java.util.HashSet; import java.util.Hashtable; import java.util.List; @@ -12,42 +13,99 @@ import jalview.datamodel.AlignmentAnnotation; public class AlignCalcManager implements AlignCalcManagerI { - private volatile List restartable = new ArrayList(); + private volatile List restartable = Collections + .synchronizedList(new ArrayList()); - private final List blackList = new ArrayList(); + private volatile List blackList = Collections + .synchronizedList(new ArrayList()); /** * global record of calculations in progress */ - private final Hashtable inProgress = new Hashtable(); + private volatile Map inProgress = Collections + .synchronizedMap(new Hashtable()); /** * record of calculations pending or in progress in the current context */ - private final Map> updating = new Hashtable>(); + private volatile Map> updating = Collections + .synchronizedMap(new Hashtable>()); @Override public void notifyStart(AlignCalcWorkerI worker) { - List upd = updating.get(worker.getClass()); - if (upd == null) + synchronized (updating) { - updating.put(worker.getClass(), - upd = new ArrayList()); + List upd = updating.get(worker.getClass()); + if (upd == null) + { + updating.put( + worker.getClass(), + upd = Collections + .synchronizedList(new ArrayList())); + } + synchronized (upd) + { + upd.add(worker); + } } - upd.add(worker); } @Override - public synchronized boolean alreadyDoing(AlignCalcWorkerI worker) + public boolean alreadyDoing(AlignCalcWorkerI worker) + { + synchronized (inProgress) + { + return inProgress.containsKey(worker.getClass()); + } + } + + /* + * (non-Javadoc) + * + * @see jalview.api.AlignCalcManagerI#isPending(jalview.api.AlignCalcWorkerI) + */ + @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; + } + } + + // TODO make into api method if needed ? + public int numberLive(AlignCalcWorkerI worker) { - return inProgress.containsKey(worker.getClass()); + synchronized (updating) + { + List upd = updating.get(worker.getClass()); + if (upd == null) + { + return 0; + } + ; + return upd.size(); + } } @Override - public synchronized boolean notifyWorking(AlignCalcWorkerI worker) + public boolean notifyWorking(AlignCalcWorkerI worker) { - // synchronized (inProgress) + synchronized (inProgress) { // TODO: decide if we should throw exceptions here if multiple workers // start to work @@ -66,92 +124,133 @@ public class AlignCalcManager implements AlignCalcManagerI return true; } - private final HashSet canUpdate=new HashSet(); + private final HashSet canUpdate = new HashSet(); + @Override - public synchronized void workerComplete(AlignCalcWorkerI worker) + public void workerComplete(AlignCalcWorkerI worker) { - inProgress.remove(worker.getClass()); - List upd = updating.get(worker.getClass()); - if (upd != null) + synchronized (inProgress) { - upd.remove(worker); - canUpdate.add(worker); + inProgress.remove(worker.getClass()); + List upd = updating.get(worker.getClass()); + if (upd != null) + { + synchronized (upd) + { + upd.remove(worker); + } + canUpdate.add(worker); + } } - } @Override public void workerCannotRun(AlignCalcWorkerI worker) { - blackList.add(worker.getClass()); + synchronized (blackList) + { + blackList.add(worker.getClass()); + } } public boolean isBlackListed(Class workerType) { - return blackList.contains(workerType); + synchronized (blackList) + { + return blackList.contains(workerType); + } } @Override public void startWorker(AlignCalcWorkerI worker) { - new Thread(worker).start(); + Thread tw = new Thread(worker); + tw.setName(worker.getClass().toString()); + tw.start(); } @Override - public synchronized boolean isWorking(AlignCalcWorkerI worker) + public boolean isWorking(AlignCalcWorkerI worker) { - // System.err.println("isWorking : worker "+(worker!=null ? - // worker.getClass():"null")+ " "+hashCode()); - return worker != null && inProgress.get(worker.getClass()) == worker; + synchronized (inProgress) + {// System.err.println("isWorking : worker "+(worker!=null ? + // worker.getClass():"null")+ " "+hashCode()); + return worker != null && inProgress.get(worker.getClass()) == worker; + } } @Override public boolean isWorking() { - // System.err.println("isWorking "+hashCode()); - return inProgress.size() > 0; + synchronized (inProgress) + { + // System.err.println("isWorking "+hashCode()); + return inProgress.size() > 0; + } } @Override public void registerWorker(AlignCalcWorkerI worker) { - if (!restartable.contains(worker)) + synchronized (restartable) { - restartable.add(worker); + if (!restartable.contains(worker)) + { + restartable.add(worker); + } + startWorker(worker); } - startWorker(worker); } @Override public void restartWorkers() { - for (AlignCalcWorkerI worker : restartable) + synchronized (restartable) { - startWorker(worker); + for (AlignCalcWorkerI worker : restartable) + { + startWorker(worker); + } } } @Override public boolean workingInvolvedWith(AlignmentAnnotation alignmentAnnotation) { - if (isWorking()) + synchronized (inProgress) { - for (List workers: updating.values()) + for (AlignCalcWorkerI worker : inProgress.values()) { - for (AlignCalcWorkerI worker:workers) if (worker.involves(alignmentAnnotation)) { return true; } } } + synchronized (updating) + { + for (List workers : updating.values()) + { + for (AlignCalcWorkerI worker : workers) + if (worker.involves(alignmentAnnotation)) + { + return true; + } + } + } return false; } @Override public void updateAnnotationFor(Class workerClass) { - for (AlignCalcWorkerI worker:canUpdate.toArray(new AlignCalcWorkerI[0])) + + AlignCalcWorkerI[] workers; + synchronized (canUpdate) + { + workers = canUpdate.toArray(new AlignCalcWorkerI[0]); + } + for (AlignCalcWorkerI worker : workers) { if (workerClass.equals(worker.getClass())) { @@ -164,37 +263,53 @@ public class AlignCalcManager implements AlignCalcManagerI public List getRegisteredWorkersOfClass( Class workerClass) { - List workingClass=new ArrayList(); - for (AlignCalcWorkerI worker:canUpdate.toArray(new AlignCalcWorkerI[0])) + List workingClass = new ArrayList(); + AlignCalcWorkerI[] workers; + synchronized (canUpdate) + { + workers = canUpdate.toArray(new AlignCalcWorkerI[0]); + } + for (AlignCalcWorkerI worker : workers) { if (workerClass.equals(worker.getClass())) { workingClass.add(worker); } } - return (workingClass.size()==0) ? null : workingClass; + return (workingClass.size() == 0) ? null : workingClass; } @Override public boolean startRegisteredWorkersOfClass(Class workerClass) { - List workers=getRegisteredWorkersOfClass(workerClass); - if (workers==null) - { - return false; - } - for (AlignCalcWorkerI worker: workers) { - startWorker(worker); - } - return true; + List workers = getRegisteredWorkersOfClass(workerClass); + if (workers == null) + { + return false; + } + for (AlignCalcWorkerI worker : workers) + { + if (!isPending(worker)) + { + startWorker(worker); + } + else + { + System.err.println("Pending exists for " + workerClass); + } + } + return true; } @Override public void workerMayRun(AlignCalcWorkerI worker) { - if (blackList.contains(worker.getClass())) + synchronized (blackList) { - blackList.remove(worker.getClass()); + if (blackList.contains(worker.getClass())) + { + blackList.remove(worker.getClass()); + } } } } diff --git a/src/jalview/workers/ConsensusThread.java b/src/jalview/workers/ConsensusThread.java index 7dd25f8..9bdd714 100644 --- a/src/jalview/workers/ConsensusThread.java +++ b/src/jalview/workers/ConsensusThread.java @@ -19,30 +19,37 @@ public class ConsensusThread extends AlignCalcWorker implements AlignCalcWorkerI super(alignViewport, alignPanel); } + @Override public void run() { + if (calcMan.isPending(this)) + { + return; + } + calcMan.notifyStart(this); + long started=System.currentTimeMillis(); try { AlignmentAnnotation consensus = alignViewport.getAlignmentConsensusAnnotation(); - if (consensus==null) { return; + if (consensus==null || calcMan.isPending(this)) { + calcMan.workerComplete(this); + return; } - calcMan.notifyStart(this); - while (!calcMan.notifyWorking(this)) + while (!calcMan.notifyWorking(this)) { + // System.err.println("Thread (Consensus"+Thread.currentThread().getName()+") Waiting around."); try { if (ap != null) { - ap.paintAlignment(false); + ap.paintAlignment(false); } - Thread.sleep(200); } catch (Exception ex) { ex.printStackTrace(); } } - calcMan.notifyWorking(this); if (alignViewport.isClosed()) { abortAndDestroy(); @@ -66,8 +73,17 @@ public class ConsensusThread extends AlignCalcWorker implements AlignCalcWorkerI consensus.annotations = new Annotation[aWidth]; Hashtable[] hconsensus = alignViewport.getSequenceConsensusHash(); hconsensus = new Hashtable[aWidth]; - AAFrequency.calculate(alignment.getSequencesArray(), 0, + try { + AAFrequency.calculate(alignment.getSequencesArray(), 0, alignment.getWidth(), hconsensus, true); + } catch (ArrayIndexOutOfBoundsException x){ + // this happens due to a race condition - + // alignment was edited at same time as calculation was running + // +// calcMan.workerCannotRun(this); + calcMan.workerComplete(this); + return; + } alignViewport.setSequenceConsensusHash(hconsensus); updateResultAnnotation(true); ColourSchemeI globalColourScheme = alignViewport @@ -97,6 +113,7 @@ public class ConsensusThread extends AlignCalcWorker implements AlignCalcWorkerI * update the consensus annotation from the sequence profile data using * current visualization settings. */ + @Override public void updateAnnotation() { updateResultAnnotation(false); diff --git a/src/jalview/workers/ConservationThread.java b/src/jalview/workers/ConservationThread.java index 47ce6eb..23d1773 100644 --- a/src/jalview/workers/ConservationThread.java +++ b/src/jalview/workers/ConservationThread.java @@ -54,7 +54,7 @@ public class ConservationThread extends AlignCalcWorker implements AlignCalcWork { if (ap != null) { - ap.paintAlignment(false); + // ap.paintAlignment(false); } Thread.sleep(200); } catch (Exception ex) diff --git a/src/jalview/workers/StrucConsensusThread.java b/src/jalview/workers/StrucConsensusThread.java index ea900dc..a7f919b 100644 --- a/src/jalview/workers/StrucConsensusThread.java +++ b/src/jalview/workers/StrucConsensusThread.java @@ -2,7 +2,6 @@ package jalview.workers; import java.util.Hashtable; -import jalview.analysis.AAFrequency; import jalview.analysis.StructureFrequency; import jalview.api.AlignCalcWorkerI; import jalview.api.AlignViewportI; @@ -29,6 +28,10 @@ public class StrucConsensusThread extends AlignCalcWorker implements { try { + if (calcMan.isPending(this)) + { + return; + } calcMan.notifyStart(this); while (!calcMan.notifyWorking(this)) { @@ -36,7 +39,7 @@ public class StrucConsensusThread extends AlignCalcWorker implements { if (ap != null) { - ap.paintAlignment(false); + // ap.paintAlignment(false); } Thread.sleep(200); @@ -85,9 +88,15 @@ public class StrucConsensusThread extends AlignCalcWorker implements return; } - jalview.analysis.StructureFrequency.calculate( + try { + jalview.analysis.StructureFrequency.calculate( alignment.getSequencesArray(), 0, alignment.getWidth(), hStrucConsensus, true, rnaStruc); + } catch (ArrayIndexOutOfBoundsException x) + { + calcMan.workerComplete(this); + return; + } alignViewport.setRnaStructureConsensusHash(hStrucConsensus); // TODO AlignmentAnnotation rnaStruc!!! updateResultAnnotation(true); @@ -103,12 +112,13 @@ public class StrucConsensusThread extends AlignCalcWorker implements // consensus = null; // hconsensus = null; ap.raiseOOMWarning("calculating RNA structure consensus", error); - } - - calcMan.workerComplete(this); - if (ap != null) + } finally { - ap.paintAlignment(true); + calcMan.workerComplete(this); + if (ap != null) + { + ap.paintAlignment(true); + } } } -- 1.7.10.2