Revert "JAL-629 Add a --threads argument to allow a limited multiple number of alignf...
authorBen Soares <b.soares@dundee.ac.uk>
Mon, 15 May 2023 13:49:00 +0000 (14:49 +0100)
committerBen Soares <b.soares@dundee.ac.uk>
Mon, 15 May 2023 13:49:00 +0000 (14:49 +0100)
Threaded processing is failing tests.  Have put this on a separate branch features/JAL-629_with_threads_NOT_PASSING_TESTS

This reverts commit c59abae32801dce40b900ad3129567e5ef5fac4f.

src/jalview/bin/Commands.java
src/jalview/bin/argparser/Arg.java

index 33bb55e..50ff7c3 100644 (file)
@@ -12,14 +12,6 @@ import java.util.Iterator;
 import java.util.List;
 import java.util.Locale;
 import java.util.Map;
-import java.util.concurrent.ArrayBlockingQueue;
-import java.util.concurrent.BlockingQueue;
-import java.util.concurrent.Callable;
-import java.util.concurrent.RejectedExecutionException;
-import java.util.concurrent.RejectedExecutionHandler;
-import java.util.concurrent.ThreadPoolExecutor;
-import java.util.concurrent.TimeUnit;
-import java.util.concurrent.atomic.AtomicBoolean;
 
 import jalview.analysis.AlignmentUtils;
 import jalview.bin.argparser.Arg;
@@ -71,11 +63,6 @@ public class Commands
 
   private boolean argsWereParsed = false;
 
-  private ThreadPoolExecutor executor = null;
-
-  // have we opened a file?
-  boolean opened = false;
-
   public Commands(ArgParser argparser, boolean headless)
   {
     this(Desktop.instance, argparser, headless);
@@ -87,61 +74,6 @@ public class Commands
     headless = h;
     desktop = d;
     afMap = new HashMap<String, AlignFrame>();
-
-    int threads = 3;
-    if (argParser.getBootstrapArgs().contains(Arg.THREADS))
-    {
-      String threadsString = argParser.getBootstrapArgs().get(Arg.THREADS);
-      try
-      {
-        threads = Integer.parseInt(threadsString);
-      } catch (NumberFormatException e)
-      {
-        Console.debug("Could not parse number of threads from '"
-                + Arg.THREADS.argString() + "=" + threadsString
-                + "', fallback to 1.");
-        threads = 1;
-      }
-    }
-
-    BlockingQueue<Runnable> bq = new ArrayBlockingQueue<>(1);
-    if (threads > 0)
-    {
-      // executor = Executors.newFixedThreadPool(threads);
-      executor = new ThreadPoolExecutor(threads, threads, 600,
-              TimeUnit.SECONDS, bq);
-    }
-    else
-    {
-      // executor = Executors.newCachedThreadPool();
-      executor = new ThreadPoolExecutor(threads, Integer.MAX_VALUE, 600,
-              TimeUnit.SECONDS, null);
-    }
-
-    // set a rejectedExecution to block and resubmit.
-    executor.setRejectedExecutionHandler(new RejectedExecutionHandler()
-    {
-      @Override
-      public void rejectedExecution(Runnable r, ThreadPoolExecutor tpe)
-      {
-        try
-        {
-          // block until there's room
-          tpe.getQueue().put(r);
-          // check afterwards and throw if pool shutdown
-          if (tpe.isShutdown())
-          {
-            throw new RejectedExecutionException(
-                    "Task " + r + " rejected from " + tpe);
-          }
-        } catch (InterruptedException e)
-        {
-          Thread.currentThread().interrupt();
-          throw new RejectedExecutionException("Producer interrupted", e);
-        }
-      }
-    });
-
     if (argparser != null)
     {
       processArgs(argparser, headless);
@@ -152,76 +84,41 @@ public class Commands
   {
     argParser = argparser;
     headless = h;
-    AtomicBoolean theseArgsWereParsed = new AtomicBoolean(false);
+    boolean theseArgsWereParsed = false;
 
     if (argParser != null && argParser.getLinkedIds() != null)
     {
-      long progress = -1;
-      boolean progressBarSet = false;
-      opened = false;
-      if (!headless && desktop != null)
-      {
-        desktop.setProgressBar(
-                MessageManager
-                        .getString("status.processing_commandline_args"),
-                progress = System.currentTimeMillis());
-        progressBarSet = true;
-      }
       for (String id : argParser.getLinkedIds())
       {
-
-        Callable<Void> process = () -> {
-          ArgValuesMap avm = argParser.getLinkedArgs(id);
-          theseArgsWereParsed.set(true);
-          theseArgsWereParsed.compareAndSet(true, processLinked(id)); // &=
-          processGroovyScript(id);
-          boolean processLinkedOkay = theseArgsWereParsed.get();
-          theseArgsWereParsed.compareAndSet(true, processImages(id)); // &=
-          if (processLinkedOkay)
-            theseArgsWereParsed.compareAndSet(true, processOutput(id)); // &=
-
-          // close ap
-          if (avm.getBoolean(Arg.CLOSE))
-          {
-            AlignFrame af = afMap.get(id);
-            if (af != null)
-            {
-              af.closeMenuItem_actionPerformed(true);
-            }
-            afMap.remove(id);
-          }
-          return null;
-        };
-
-        executor.submit(process);
-        Console.debug(
-                "Running " + executor.getActiveCount() + " processes.");
-      }
-
-      if (!opened) // first=true means nothing opened
-      {
-        if (headless)
-        {
-          Jalview.exit("Did not open any files in headless mode", 1);
-        }
-        else
+        ArgValuesMap avm = argParser.getLinkedArgs(id);
+        theseArgsWereParsed = true;
+        theseArgsWereParsed &= processLinked(id);
+        processGroovyScript(id);
+        boolean processLinkedOkay = theseArgsWereParsed;
+        theseArgsWereParsed &= processImages(id);
+        if (processLinkedOkay)
+          theseArgsWereParsed &= processOutput(id);
+
+        // close ap
+        if (avm.getBoolean(Arg.CLOSE))
         {
-          Console.warn("No more files to open");
+          AlignFrame af = afMap.get(id);
+          if (af != null)
+          {
+            af.closeMenuItem_actionPerformed(true);
+          }
         }
-      }
-      if (progressBarSet && desktop != null)
-      {
-        desktop.setProgressBar(null, progress);
+
       }
 
     }
-    if (argParser.getBootstrapArgs().getBoolean(Arg.QUIT))
+    if (argParser.getBoolean(Arg.QUIT))
     {
       Jalview.getInstance().quit();
       return true;
     }
     // carry on with jalview.bin.Jalview
-    argsWereParsed |= theseArgsWereParsed.get();
+    argsWereParsed = theseArgsWereParsed;
     return argsWereParsed;
   }
 
@@ -235,6 +132,11 @@ public class Commands
     return argsWereParsed;
   }
 
+  protected boolean processUnlinked(String id)
+  {
+    return processLinked(id);
+  }
+
   protected boolean processLinked(String id)
   {
     boolean theseArgsWereParsed = false;
@@ -252,6 +154,10 @@ public class Commands
     if (avm.containsArg(Arg.APPEND) || avm.containsArg(Arg.OPEN))
     {
       commandArgsProvided = true;
+      long progress = -1;
+
+      boolean first = true;
+      boolean progressBarSet = false;
       AlignFrame af;
       // Combine the APPEND and OPEN files into one list, along with whether it
       // was APPEND or OPEN
@@ -269,6 +175,18 @@ public class Commands
           continue;
 
         theseArgsWereParsed = true;
+        if (first)
+        {
+          first = false;
+          if (!headless && desktop != null)
+          {
+            desktop.setProgressBar(
+                    MessageManager.getString(
+                            "status.processing_commandline_args"),
+                    progress = System.currentTimeMillis());
+            progressBarSet = true;
+          }
+        }
 
         if (!Platform.isJS())
         /**
@@ -303,8 +221,6 @@ public class Commands
         if (af == null || "true".equals(av.getSubVal("new"))
                 || a == Arg.OPEN || format == FileFormat.Jalview)
         {
-          opened = true;
-
           if (a == Arg.OPEN)
           {
             Jalview.testoutput(argParser, Arg.OPEN, "examples/uniref50.fa",
@@ -443,6 +359,19 @@ public class Commands
         Console.debug("Command " + Arg.APPEND + " executed successfully!");
 
       }
+      if (first) // first=true means nothing opened
+      {
+        if (headless)
+        {
+          Jalview.exit("Could not open any files in headless mode", 1);
+        }
+        else
+        {
+          Console.warn("No more files to open");
+        }
+      }
+      if (progressBarSet && desktop != null)
+        desktop.setProgressBar(null, progress);
 
     }
 
index 73dec4e..a18057c 100644 (file)
@@ -179,7 +179,7 @@ public enum Arg
   OPENED("Apply the following output arguments to all of the last --open'ed set of linked arguments.",
           Opt.BOOLEAN, Opt.MULTI, Opt.NOACTION),
   QUIT("After all files have been opened, appended and output, quit Jalview. In ‑‑headless mode this already happens.",
-          Opt.UNARY, Opt.BOOTSTRAP),
+          Opt.UNARY),
 
   // secret options
   TESTOUTPUT(
@@ -198,8 +198,6 @@ public enum Arg
   UNSETARGFILE(
           "Unsets the current value of the argfilename.  Inserted after argfile contents.",
           Opt.UNARY, Opt.LINKED, Opt.MULTI, Opt.PRIVATE, Opt.NOACTION),
-  THREADS("When opening multiple alignment windows, set a limit to alignments being processed at one time.",
-          Opt.BOOTSTRAP, Opt.STRING, Opt.NODUPLICATEVALUES, Opt.NOACTION),
 
   // these last two have no purpose in the normal Jalview application but are
   // used by jalview.bin.Launcher to set memory settings. They are not used by