clean up workers (bazel-contrib#1005)

* clean up workers

* whoops!

* git grep --name-only 'String args\[\]' | xargs sed -i 's|String args\[\]|String[] args|'

* subclass ByteArrayOutputStream to allow shrinking of internal buffer

* gc after each work request

* split workerMain implementation into two private methods for readability

* add comment about stdin

* add a test for the buffer

* improve tests

* wrap gc and friends in the finally clause

* comments

* make the worker public

* cleanup

* comments

* add stdin test

* clean up test

Author: andyscott

scala/scalafmt/BUILD

@@ -14,8 +14,8 @@ filegroup(
 
 scala_binary(
     name = "scalafmt",
-    srcs = ["scalafmt/ScalafmtRunner.scala"],
-    main_class = "io.bazel.rules_scala.scalafmt.ScalafmtRunner",
+    srcs = ["scalafmt/ScalafmtWorker.scala"],
+    main_class = "io.bazel.rules_scala.scalafmt.ScalafmtWorker",
     visibility = ["//visibility:public"],
     deps = [
         "//src/java/io/bazel/rulesscala/worker",

scala/scalafmt/scalafmt/ScalafmtRunner.scala renamed to scala/scalafmt/scalafmt/ScalafmtWorker.scala

@@ -1,6 +1,6 @@
 package io.bazel.rules_scala.scalafmt
 
-import io.bazel.rulesscala.worker.{GenericWorker, Processor};
+import io.bazel.rulesscala.worker.Worker
 import java.io.File
 import java.nio.file.Files
 import org.scalafmt.Scalafmt
@@ -10,21 +10,13 @@ import scala.annotation.tailrec
 import scala.collection.JavaConverters._
 import scala.io.Codec
 
-object ScalafmtRunner extends GenericWorker(new ScalafmtProcessor) {
-  def main(args: Array[String]) {
-    try run(args)
-    catch {
-      case x: Exception =>
-        x.printStackTrace()
-        System.exit(1)
-    }
-  }
-}
+object ScalafmtWorker extends Worker.Interface {
+
+  def main(args: Array[String]): Unit = Worker.workerMain(args, ScalafmtWorker)
 
-class ScalafmtProcessor extends Processor {
-  def processRequest(args: java.util.List[String]) {
+  def work(args: Array[String]) {
     val argName = List("config", "input", "output")
-    val argFile = args.asScala.map{x => new File(x)}
+    val argFile = args.map{x => new File(x)}
     val namespace = argName.zip(argFile).toMap
 
     val source = FileOps.readFile(namespace.getOrElse("input", new File("")))(Codec.UTF8)

scala_proto/scala_proto_toolchain.bzl

@@ -29,7 +29,7 @@ scala_proto_toolchain = rule(
         "code_generator": attr.label(
             executable = True,
             cfg = "host",
-            default = Label("@io_bazel_rules_scala//src/scala/scripts:scalapb_generator"),
+            default = Label("@io_bazel_rules_scala//src/scala/scripts:scalapb_worker"),
             allow_files = True,
         ),
         "named_generators": attr.string_dict(),

src/java/io/bazel/rulesscala/coverage/instrumenter/JacocoInstrumenter.java

@@ -1,8 +1,7 @@
 package io.bazel.rulesscala.coverage.instrumenter;
 
 import io.bazel.rulesscala.jar.JarCreator;
-import io.bazel.rulesscala.worker.GenericWorker;
-import io.bazel.rulesscala.worker.Processor;
+import io.bazel.rulesscala.worker.Worker;
 
 import java.io.BufferedInputStream;
 import java.io.BufferedOutputStream;
@@ -26,28 +25,18 @@
 import org.jacoco.core.instr.Instrumenter;
 import org.jacoco.core.runtime.OfflineInstrumentationAccessGenerator;
 
-public final class JacocoInstrumenter implements Processor {
+public final class JacocoInstrumenter implements Worker.Interface {
 
     public static void main(String[] args) throws Exception {
-        (new Worker()).run(args);
-    }
-
-    private static final class Worker extends GenericWorker {
-        public Worker() {
-            super(new JacocoInstrumenter());
-        }
+	Worker.workerMain(args, new JacocoInstrumenter());
     }
 
     @Override
-    public void processRequest(List < String > args) {
+    public void work(String[] args) throws Exception {
         Instrumenter jacoco = new Instrumenter(new OfflineInstrumentationAccessGenerator());
-        args.forEach(arg -> {
-            try {
-                processArg(jacoco, arg);
-            } catch (final Exception e) {
-                throw new RuntimeException(e);
-            }
-        });
+        for (String arg : args) {
+            processArg(jacoco, arg);
+	}
     }
 
     private void processArg(Instrumenter jacoco, String arg) throws Exception {

src/java/io/bazel/rulesscala/scalac/BUILD

@@ -17,7 +17,7 @@ java_binary(
         "-source 1.8",
         "-target 1.8",
     ],
-    main_class = "io.bazel.rulesscala.scalac.ScalaCInvoker",
+    main_class = "io.bazel.rulesscala.scalac.ScalacWorker",
     visibility = ["//visibility:public"],
     deps = [
         ":exported_scalac_repositories_from_toolchain_to_jvm",
@@ -38,8 +38,7 @@ filegroup(
         "CompileOptions.java",
         "ProtoReporter.java",
         "Resource.java",
-        "ScalaCInvoker.java",
-        "ScalacProcessor.java",
+        "ScalacWorker.java",
     ],
     visibility = ["//visibility:public"],
 )

src/java/io/bazel/rulesscala/scalac/ScalaCInvoker.java

@@ -2,8 +2,7 @@
 
 import com.illicitonion.scalac.reporters.CompositeReporter;
 import io.bazel.rulesscala.jar.JarCreator;
-import io.bazel.rulesscala.worker.GenericWorker;
-import io.bazel.rulesscala.worker.Processor;
+import io.bazel.rulesscala.worker.Worker;
 import java.io.File;
 import java.io.FileNotFoundException;
 import java.io.FileOutputStream;
@@ -32,7 +31,7 @@
 import scala.tools.nsc.reporters.ConsoleReporter;
 import scala.tools.nsc.reporters.Reporter;
 
-class ScalacProcessor implements Processor {
+class ScalacWorker implements Worker.Interface {
   private static boolean isWindows = System.getProperty("os.name").toLowerCase().contains("windows");
 
   /** This is the reporter field for scalac, which we want to access */
@@ -47,11 +46,15 @@ class ScalacProcessor implements Processor {
     }
   }
 
+  public static void main(String[] args) throws Exception {
+    Worker.workerMain(args, new ScalacWorker());
+  }
+
   @Override
-  public void processRequest(List<String> args) throws Exception {
+  public void work(String[] args) throws Exception {
     Path tmpPath = null;
     try {
-      CompileOptions ops = new CompileOptions(args);
+      CompileOptions ops = new CompileOptions(Arrays.asList(args));
 
       Path outputPath = FileSystems.getDefault().getPath(ops.outputName);
       tmpPath = Files.createTempDirectory(outputPath.getParent(), "tmp");
@@ -67,7 +70,7 @@ public void processRequest(List<String> args) throws Exception {
 
       String[] scalaSources = collectSrcJarSources(ops.files, scalaJarFiles, javaJarFiles);
 
-      String[] javaSources = GenericWorker.appendToString(ops.javaFiles, javaJarFiles);
+      String[] javaSources = appendToString(ops.javaFiles, javaJarFiles);
       if (scalaSources.length == 0 && javaSources.length == 0) {
         throw new RuntimeException("Must have input files from either source jars or local files.");
       }
@@ -99,8 +102,8 @@ public void processRequest(List<String> args) throws Exception {
 
   private static String[] collectSrcJarSources(
       String[] files, List<File> scalaJarFiles, List<File> javaJarFiles) {
-    String[] scalaSources = GenericWorker.appendToString(files, scalaJarFiles);
-    return GenericWorker.appendToString(scalaSources, javaJarFiles);
+    String[] scalaSources = appendToString(files, scalaJarFiles);
+    return appendToString(scalaSources, javaJarFiles);
   }
 
   private static List<File> filterFilesByExtension(List<File> files, String extension) {
@@ -170,7 +173,7 @@ private static boolean matchesFileExtensions(String fileName, String[] extension
   }
 
   private static String[] encodeBazelTargets(String[] targets) {
-    return Arrays.stream(targets).map(ScalacProcessor::encodeBazelTarget).toArray(String[]::new);
+    return Arrays.stream(targets).map(ScalacWorker::encodeBazelTarget).toArray(String[]::new);
   }
 
   private static String encodeBazelTarget(String target) {
@@ -227,7 +230,7 @@ private static void compileScalaSources(CompileOptions ops, String[] scalaSource
     String[] constParams = {"-classpath", ops.classpath, "-d", tmpPath.toString()};
 
     String[] compilerArgs =
-        GenericWorker.merge(ops.scalaOpts, ops.pluginArgs, constParams, pluginParams, scalaSources);
+        merge(ops.scalaOpts, ops.pluginArgs, constParams, pluginParams, scalaSources);
 
     MainClass comp = new MainClass() {
       private Global compiler;
@@ -357,4 +360,30 @@ private static void copyResourceJars(String[] resourceJars, Path dest) throws IO
       extractJar(jarPath, dest.toString(), null);
     }
   }
+
+  private static <T> String[] appendToString(String[] init, List<T> rest) {
+    String[] tmp = new String[init.length + rest.size()];
+    System.arraycopy(init, 0, tmp, 0, init.length);
+    int baseIdx = init.length;
+    for (T t : rest) {
+      tmp[baseIdx] = t.toString();
+      baseIdx += 1;
+    }
+    return tmp;
+  }
+
+  private static String[] merge(String[]... arrays) {
+    int totalLength = 0;
+    for (String[] arr : arrays) {
+      totalLength += arr.length;
+    }
+
+    String[] result = new String[totalLength];
+    int offset = 0;
+    for (String[] arr : arrays) {
+      System.arraycopy(arr, 0, result, offset, arr.length);
+      offset += arr.length;
+    }
+    return result;
+  }
 }

src/java/io/bazel/rulesscala/scalac/ScalacProcessor.java renamed to src/java/io/bazel/rulesscala/scalac/ScalacWorker.java

@@ -1,13 +1,23 @@
-load("@rules_java//java:defs.bzl", "java_library")
-
 java_library(
     name = "worker",
+    srcs = ["Worker.java"],
+    visibility = ["//visibility:public"],
+    deps = [
+        "//third_party/bazel/src/main/protobuf:worker_protocol_java_proto",
+    ],
+)
+
+java_test(
+    name = "worker_test",
     srcs = [
-        "GenericWorker.java",
-        "Processor.java",
+        "WorkerTest.java",
     ],
-    visibility = ["//visibility:public"],
+    jvm_flags = [
+        "-Dcom.google.testing.junit.runner.shouldInstallTestSecurityManager=false",
+    ],
+    test_class = "io.bazel.rulesscala.worker.WorkerTest",
     deps = [
+        ":worker",
         "//third_party/bazel/src/main/protobuf:worker_protocol_java_proto",
     ],
 )