Reduce runtime memory use (#699)

* Allow JVM args to be specified as gradle properties

* Check for external-only contours in watershed

* Release temp Mats and use preallocated objects in WatershedOperation

* Add Cleaner class to run GC periodically

* Limit eden size in deployed and native apps

* Move args and jvmArgs to common configuration block

Author: SamCarlberg

build.gradle

@@ -276,12 +276,6 @@ project(":core") {
 
     mainClassName = 'edu.wpi.grip.core.Main'
 
-    if (project.hasProperty('args')) {
-        run {
-            args = (project.args.split("\\s+") as List)
-        }
-    }
-
     jar {
         manifest {
             attributes 'Implementation-Version': version, 'Main-Class': mainClassName
@@ -445,12 +439,6 @@ project(":ui") {
         }
     }
 
-    if (project.hasProperty('args')) {
-        run {
-            args = (project.args.split("\\s+") as List)
-        }
-    }
-
     task testSharedLib() {
         description 'Compiles the shared library used by c++ generation testing.'
         doLast {
@@ -548,12 +536,17 @@ project(":ui") {
 
         jfxMainAppJarName = "${jfx.appName}-${jfx.nativeReleaseVersion}.jar"
 
-        // This prevents the JIT from eating stack traces that get thrown a lot
+        // -XX:-OmitStackTraceInFastThrow prevents the JIT from eating stack traces that get thrown a lot
         // This is slower but means we actually get the stack traces instead of
         // having them become one line like `java.lang.ArrayIndexOutOfBoundsException`
         // and as such, would be useless.
         // See: https://plumbr.eu/blog/java/on-a-quest-for-missing-stacktraces
-        jvmArgs = ["-XX:-OmitStackTraceInFastThrow"]
+        // -Xmx limits the heap size. This prevents memory use from ballooning with a lot
+        // of JavaCV native objects being allocated hanging around waiting to get GC'd.
+        // -XX:MaxNewSize limits the size of the eden space to force minor GCs to run more often.
+        // This causes old mats (which take up little space on the heap but a lot of native memory) to get deallocated
+        // and free up native memory quicker, limiting the memory the app takes up.
+        jvmArgs = ["-XX:-OmitStackTraceInFastThrow", "-Xmx200m", "-XX:MaxNewSize=32m"]
 
         bundleArguments = [
                 "linux.launcher.url": file('linuxLauncher/build/exe/linuxLauncher/linuxLauncher').toURI().toURL()
@@ -562,6 +555,24 @@ project(":ui") {
     mainClassName = jfx.mainClass
 }
 
+
+configure([
+        project(":core"),
+        project(":ui"),
+        project(":ui:preloader")
+]) {
+    if (project.hasProperty('jvmArgs')) {
+        run {
+            jvmArgs = (project.jvmArgs.split("\\s+") as List)
+        }
+    }
+    if (project.hasProperty('args')) {
+        run {
+            args = (project.args.split("\\s+") as List)
+        }
+    }
+}
+
 /*
  * This is roughly based upon this post:
  * https://discuss.gradle.org/t/merge-jacoco-coverage-reports-for-multiproject-setups/12100/6

core/src/main/java/edu/wpi/grip/core/Cleaner.java

@@ -0,0 +1,70 @@
+package edu.wpi.grip.core;
+
+import edu.wpi.grip.core.events.RunStoppedEvent;
+
+import com.google.common.base.Stopwatch;
+import com.google.common.eventbus.Subscribe;
+import com.google.inject.Singleton;
+
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
+
+import javax.annotation.Nullable;
+
+import static java.util.concurrent.TimeUnit.MILLISECONDS;
+
+/**
+ * Cleans up unused objects by periodically calling {@link System#gc()} to nudge the
+ * garbage collector to clean up dead native (JavaCV) objects. This is required because JavaCV
+ * objects only free their native memory when they're garbage collected, so if they accumulate in
+ * the heap, the app will use about 40x the memory as used heap (i.e. 230MB of used heap results in
+ * about 9.8GB of used memory for the process). This is because {@code Mats} and anything
+ * else extending {@link org.bytedeco.javacpp.Pointer} use native memory that greatly exceeds the
+ * Java objects size on the heap.
+ *
+ * <p>JavaCV has a system property {@code org.bytedeco.javacpp.maxphysicalbytes} that it uses to
+ * determine when to start deallocating native memory. However, this only results in calls to
+ * {@code System.gc()} and imposes a hard upper limit on native memory use, limiting large images
+ * or long pipelines. It's also not very portable: running from source needs it to be passed
+ * as a JVM argument with gradle, and it can't be adjusted based on the amount of memory on the
+ * system it's installed on. For us, manually running System.gc() periodically is a better solution.
+ * </p>
+ */
+@Singleton
+public class Cleaner {
+
+  /**
+   * The minimum time delay before running System.gc(). This is in milliseconds. A gc call will
+   * never happen less than this amount of time after the previous call.
+   */
+  private static final long MIN_DELAY = 1000;
+
+  /**
+   * Stopwatch to keep track of the elapsed time since the last gc call.
+   */
+  private final Stopwatch stopwatch = Stopwatch.createUnstarted();
+
+  /**
+   * The minimum number of runs allowed before calling System.gc().
+   */
+  private static final int MIN_RUNS_BEFORE_GC = 5;
+
+  /**
+   * The number of runs since the last gc call.
+   */
+  private int runsSinceLastGc = 0;
+
+  @Subscribe
+  @SuppressFBWarnings(value = "DM_GC", justification = "GC is called infrequently")
+  public void onRunFinished(@Nullable RunStoppedEvent e) {
+    runsSinceLastGc++;
+    if (!stopwatch.isRunning()) {
+      stopwatch.start();
+    }
+    if (runsSinceLastGc >= MIN_RUNS_BEFORE_GC && stopwatch.elapsed(MILLISECONDS) >= MIN_DELAY) {
+      runsSinceLastGc = 0;
+      stopwatch.reset();
+      System.gc();
+    }
+  }
+
+}

core/src/main/java/edu/wpi/grip/core/GripCoreModule.java

@@ -160,6 +160,8 @@ public <I> void hear(TypeLiteral<I> type, TypeEncounter<I> encounter) {
     install(new FactoryModuleBuilder().build(Timer.Factory.class));
 
     bind(BenchmarkRunner.class).asEagerSingleton();
+
+    bind(Cleaner.class).asEagerSingleton();
   }
 
   protected void onSubscriberException(Throwable exception, @Nullable SubscriberExceptionContext

core/src/main/java/edu/wpi/grip/core/operations/composite/ContoursReport.java

@@ -150,6 +150,7 @@ public synchronized double[] getSolidity() {
       convexHull(contours.get(i), hull);
       solidities[i] = contourArea(contours.get(i)) / contourArea(hull);
     }
+    hull.release();
     return solidities;
   }
 

core/src/main/java/edu/wpi/grip/core/operations/composite/FilterContoursOperation.java

@@ -183,6 +183,7 @@ public void perform() {
 
       convexHull(contour, hull);
       final double solidity = 100 * area / contourArea(hull);
+      hull.release();
       if (solidity < minSolidity || solidity > maxSolidity) {
         continue;
       }

core/src/main/java/edu/wpi/grip/core/operations/composite/FindContoursOperation.java

@@ -94,6 +94,7 @@ public void perform() {
     MatVector contours = new MatVector();
     findContours(tmp, contours, externalOnly ? CV_RETR_EXTERNAL : CV_RETR_LIST,
         CV_CHAIN_APPROX_TC89_KCOS);
+    tmp.release();
 
     contoursSocket.setValue(new ContoursReport(contours, input.rows(), input.cols()));
   }

core/src/main/java/edu/wpi/grip/core/operations/composite/FindLinesOperation.java

@@ -73,11 +73,11 @@ public void perform() {
       lsd.detect(input, lines);
     } else {
       // The line detector works on a single channel.  If the input is a color image, we can just
-      // give the line
-      // detector a grayscale version of it
+      // give the line  detector a grayscale version of it
       final Mat tmp = new Mat();
       cvtColor(input, tmp, COLOR_BGR2GRAY);
       lsd.detect(tmp, lines);
+      tmp.release();
     }
 
     // Store the lines in the LinesReport object
@@ -90,6 +90,7 @@ public void perform() {
         lineList.add(new LinesReport.Line(tmp[0], tmp[1], tmp[2], tmp[3]));
       }
     }
+    lines.release();
 
     linesReportSocket.setValue(new LinesReport(lsd, input, lineList));
   }

core/src/main/java/edu/wpi/grip/core/operations/composite/WatershedOperation.java

@@ -14,6 +14,8 @@
 
 import java.util.ArrayList;
 import java.util.List;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
 
 import static org.bytedeco.javacpp.opencv_core.CV_32SC1;
 import static org.bytedeco.javacpp.opencv_core.CV_8UC1;
@@ -24,6 +26,7 @@
 import static org.bytedeco.javacpp.opencv_core.Point;
 import static org.bytedeco.javacpp.opencv_core.Point2f;
 import static org.bytedeco.javacpp.opencv_core.Scalar;
+import static org.bytedeco.javacpp.opencv_core.bitwise_xor;
 import static org.bytedeco.javacpp.opencv_imgproc.CV_CHAIN_APPROX_TC89_KCOS;
 import static org.bytedeco.javacpp.opencv_imgproc.CV_FILLED;
 import static org.bytedeco.javacpp.opencv_imgproc.CV_RETR_EXTERNAL;
@@ -64,12 +67,22 @@ public class WatershedOperation implements Operation {
   private final InputSocket<ContoursReport> contoursSocket;
   private final OutputSocket<ContoursReport> outputSocket;
 
+  private static final int MAX_MARKERS = 253;
+  private final List<Mat> markerPool;
+  private final MatVector contour = new MatVector(); // vector with a single element
+  private final Mat markers = new Mat();
+  private final Mat output = new Mat();
+  private final Point backgroundLabel = new Point();
+
   @SuppressWarnings("JavadocMethod")
   public WatershedOperation(InputSocket.Factory inputSocketFactory,
                             OutputSocket.Factory outputSocketFactory) {
     srcSocket = inputSocketFactory.create(srcHint);
     contoursSocket = inputSocketFactory.create(contoursHint);
     outputSocket = outputSocketFactory.create(outputHint);
+    markerPool = ImmutableList.copyOf(
+        Stream.generate(Mat::new).limit(MAX_MARKERS).collect(Collectors.toList())
+    );
   }
 
   @Override
@@ -97,54 +110,50 @@ public void perform() {
     final ContoursReport contourReport = contoursSocket.getValue().get();
     final MatVector contours = contourReport.getContours();
 
-    final int maxMarkers = 253;
-    if (contours.size() > maxMarkers) {
+    if (contours.size() > MAX_MARKERS) {
       throw new IllegalArgumentException(
-          "A maximum of " + maxMarkers + " contours can be used as markers."
+          "A maximum of " + MAX_MARKERS + " contours can be used as markers."
               + " Filter contours before connecting them to this operation if this keeps happening."
               + " The contours must also all be external; nested contours will not work");
     }
 
-    final Mat markers = new Mat(input.size(), CV_32SC1, new Scalar(0.0));
-    final Mat output = new Mat(markers.size(), CV_8UC1, new Scalar(0.0));
+    markers.create(input.size(), CV_32SC1);
+    output.create(input.size(), CV_8UC1);
+    bitwise_xor(markers, markers, markers);
+    bitwise_xor(output, output, output);
 
-    try {
-      // draw foreground markers (these have to be different colors)
-      for (int i = 0; i < contours.size(); i++) {
-        drawContours(markers, contours, i, Scalar.all(i + 1), CV_FILLED, LINE_8, null, 2, null);
-      }
+    // draw foreground markers (these have to be different colors)
+    for (int i = 0; i < contours.size(); i++) {
+      drawContours(markers, contours, i, Scalar.all(i + 1), CV_FILLED, LINE_8, null, 2, null);
+    }
 
-      // draw background marker a different color from the foreground markers
-      Point backgroundLabel = fromPoint2f(findBackgroundMarker(markers, contours));
-      circle(markers, backgroundLabel, 1, Scalar.WHITE, -1, LINE_8, 0);
-
-      // Perform watershed
-      watershed(input, markers);
-      markers.convertTo(output, CV_8UC1);
-
-      List<Mat> contourList = new ArrayList<>();
-      for (int i = 1; i < contours.size(); i++) {
-        Mat dst = new Mat();
-        output.copyTo(dst, opencv_core.equals(markers, i).asMat());
-        MatVector contour = new MatVector(); // vector with a single element
-        findContours(dst, contour, CV_RETR_EXTERNAL, CV_CHAIN_APPROX_TC89_KCOS);
-        assert contour.size() == 1;
-        contourList.add(contour.get(0).clone());
-        contour.get(0).deallocate();
-        contour.deallocate();
+    // draw background marker a different color from the foreground markers
+    findBackgroundMarker(markers, contours);
+    circle(markers, backgroundLabel, 1, Scalar.WHITE, -1, LINE_8, 0);
+
+    // Perform watershed
+    watershed(input, markers);
+    markers.convertTo(output, CV_8UC1);
+
+    List<Mat> contourList = new ArrayList<>((int) contours.size());
+    for (int i = 1; i < contours.size(); i++) {
+      Mat dst = markerPool.get(i - 1);
+      bitwise_xor(dst, dst, dst);
+      output.copyTo(dst, opencv_core.equals(markers, i).asMat());
+      findContours(dst, contour, CV_RETR_EXTERNAL, CV_CHAIN_APPROX_TC89_KCOS);
+      if (contour.size() < 1) {
+        throw new IllegalArgumentException("No contours for marker");
       }
-      MatVector foundContours = new MatVector(contourList.toArray(new Mat[contourList.size()]));
-      outputSocket.setValue(new ContoursReport(foundContours, output.rows(), output.cols()));
-    } finally {
-      // make sure that the working mat is freed to avoid a memory leak
-      markers.release();
+      contourList.add(contour.get(0).clone());
     }
+    MatVector foundContours = new MatVector(contourList.toArray(new Mat[contourList.size()]));
+    outputSocket.setValue(new ContoursReport(foundContours, output.rows(), output.cols()));
   }
 
   /**
    * Finds the first available point to place a background marker for the watershed operation.
    */
-  private static Point2f findBackgroundMarker(Mat markers, MatVector contours) {
+  private void findBackgroundMarker(Mat markers, MatVector contours) {
     final int cols = markers.cols();
     final int rows = markers.rows();
     final int minDist = 5;
@@ -169,13 +178,16 @@ private static Point2f findBackgroundMarker(Mat markers, MatVector contours) {
     }
     if (!found) {
       // Should only happen if the image is clogged with contours
+      backgroundLabel.deallocate();
       throw new IllegalStateException("Could not find a point for the background label");
     }
-    return backgroundLabel;
+    setBackgroundLabel(backgroundLabel);
+    backgroundLabel.deallocate();
   }
 
-  private static Point fromPoint2f(Point2f p) {
-    return new Point((int) p.x(), (int) p.y());
+  private void setBackgroundLabel(Point2f p) {
+    this.backgroundLabel.x((int) p.x());
+    this.backgroundLabel.y((int) p.y());
   }
 
 }

core/src/main/java/edu/wpi/grip/core/settings/ProjectSettings.java

@@ -41,7 +41,7 @@ public class ProjectSettings implements Cloneable {
   @Setting(label = "Deploy JVM options", description = "Command line options passed to the "
       + "roboRIO JVM")
   private String deployJvmOptions = "-Xmx50m -XX:-OmitStackTraceInFastThrow "
-      + "-XX:+HeapDumpOnOutOfMemoryError";
+      + "-XX:+HeapDumpOnOutOfMemoryError -XX:MaxNewSize=16m";
 
   @Setting(label = "Internal server port",
       description = "The port that the internal server should run on.")