Add support for timing steps and pipelines (#613)

* Add support for timing steps and pipelines

* Suppress PMD warnings

* Add documentation to StepStarted and StepFinishedEvents

* Add analysis of step timings

* Updated UI test

* Add benchmarking to analysis window

* Undo accidental formatting

* Fix and suppress issues from Codacy

* Add tests for Timer

* Improve UI and increase tolerance for timer tests

* Increase timer test tolerance to 5ms (5%)

* Add tests for BenchmarkRunner

* Change analysis averager to a moving average, improve naming and documentation

Should fix the StackOverflowErrors @AustinShalit is running into on OS X

* Fix flickering, make Analysis immutable

* Reset timers after running a benchmark.

Fix an occasional NPE
Increase default number of runs to 100 (was 5)

* Add tests for MovingAverage

* Update TimerTest for reset()

* Fix tests (oops)

* Freeze sources while benchmarking

Remove useless suppressions

* Improve Timer API, documentation, and tests

* Fix accidental formatting (again)

* Remove redundant (ms)

* Fix PMD issues

* Fix PMD in UI

* Appease Codacy

* Don't make analysis window block main window. Disable inputs and outputs while benchmarking.

* Remove hacky data aggregation from Analysis class.

* Replace custom FixedSizeQueue with guava's EvictingQueue

* Add option to export results to CSV.

Fixed an issue with steps not being in their proper order in the analysis

* Fix tests

* Add tests for metrics

* Move CSV to it's own class

* Fix some checkstyle stuff I missed

* Rename Timer.stopped() to stop()

* Delete redudant Analysis class and improve variable names in Statistics

* Improve Javadoc for statistics hotness function

* Fix import order

* Move analysis window to fx:include

Author: SamCarlberg

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

@@ -1,6 +1,8 @@
 package edu.wpi.grip.core;
 
 import edu.wpi.grip.core.events.UnexpectedThrowableEvent;
+import edu.wpi.grip.core.metrics.BenchmarkRunner;
+import edu.wpi.grip.core.metrics.Timer;
 import edu.wpi.grip.core.serialization.Project;
 import edu.wpi.grip.core.settings.SettingsProvider;
 import edu.wpi.grip.core.sockets.InputSocket;
@@ -128,6 +130,7 @@ public <I> void hear(TypeLiteral<I> type, TypeEncounter<I> encounter) {
     install(new FactoryModuleBuilder().build(new TypeLiteral<Connection.Factory<Object>>() {
     }));
 
+    bind(StepIndexer.class).to(Pipeline.class);
     bind(ConnectionValidator.class).to(Pipeline.class);
     bind(Source.SourceFactory.class).to(Source.SourceFactoryImpl.class);
 
@@ -150,6 +153,9 @@ public <I> void hear(TypeLiteral<I> type, TypeEncounter<I> encounter) {
         .build(NetworkTableEntrySource.Factory.class));
 
     install(new FactoryModuleBuilder().build(ExceptionWitness.Factory.class));
+    install(new FactoryModuleBuilder().build(Timer.Factory.class));
+
+    bind(BenchmarkRunner.class).asEagerSingleton();
   }
 
   protected void onSubscriberException(Throwable exception, @Nullable SubscriberExceptionContext

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

@@ -26,6 +26,7 @@
 import java.util.Collections;
 import java.util.HashSet;
 import java.util.List;
+import java.util.NoSuchElementException;
 import java.util.Set;
 import java.util.concurrent.locks.Lock;
 import java.util.concurrent.locks.ReadWriteLock;
@@ -47,7 +48,7 @@
  */
 @Singleton
 @XStreamAlias(value = "grip:Pipeline")
-public class Pipeline implements ConnectionValidator, SettingsProvider {
+public class Pipeline implements ConnectionValidator, SettingsProvider, StepIndexer {
 
   private final transient ReadWriteLock sourceLock = new ReentrantReadWriteLock();
 
@@ -359,7 +360,18 @@ public synchronized void moveStep(Step step, int delta) {
 
     // Do not lock while posting the event
     eventBus.post(new StepMovedEvent(step, delta));
+  }
 
+  @Override
+  public int indexOf(Step step) {
+    checkNotNull(step, "step");
+    return readStepsSafely(steps -> {
+      int index = steps.indexOf(step);
+      if (index == -1) {
+        throw new NoSuchElementException("Step " + step + " is not in the pipeline");
+      }
+      return index;
+    });
   }
 
   @Subscribe

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

@@ -1,11 +1,13 @@
 package edu.wpi.grip.core;
 
 
+import edu.wpi.grip.core.events.BenchmarkEvent;
 import edu.wpi.grip.core.events.RenderEvent;
 import edu.wpi.grip.core.events.RunPipelineEvent;
 import edu.wpi.grip.core.events.RunStartedEvent;
 import edu.wpi.grip.core.events.RunStoppedEvent;
 import edu.wpi.grip.core.events.StopPipelineEvent;
+import edu.wpi.grip.core.metrics.Timer;
 import edu.wpi.grip.core.util.SinglePermitSemaphore;
 import edu.wpi.grip.core.util.service.AutoRestartingService;
 import edu.wpi.grip.core.util.service.LoggingListener;
@@ -24,6 +26,7 @@
 import java.util.concurrent.Executor;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.TimeoutException;
+import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.function.Supplier;
 import java.util.logging.Logger;
 
@@ -47,17 +50,25 @@ public class PipelineRunner implements RestartableService {
   private final Supplier<ImmutableList<Step>> stepSupplier;
   private final AutoRestartingService pipelineService;
 
+  private final AtomicBoolean benchmarking = new AtomicBoolean(false);
 
   @Inject
-  PipelineRunner(EventBus eventBus, Provider<Pipeline> pipelineProvider) {
-    this(eventBus, () -> pipelineProvider.get().getSources(), () -> pipelineProvider.get()
-        .getSteps());
-  }
-
-  PipelineRunner(EventBus eventBus, Supplier<ImmutableList<Source>> sourceSupplier,
-                 Supplier<ImmutableList<Step>> stepSupplier) {
+  PipelineRunner(EventBus eventBus,
+                 Provider<Pipeline> pipelineProvider,
+                 Timer.Factory timerFactory) {
+    this(eventBus,
+        () -> pipelineProvider.get().getSources(),
+        () -> pipelineProvider.get().getSteps(),
+        timerFactory);
+  }
+
+  PipelineRunner(EventBus eventBus,
+                 Supplier<ImmutableList<Source>> sourceSupplier,
+                 Supplier<ImmutableList<Step>> stepSupplier,
+                 Timer.Factory timerFactory) {
     this.sourceSupplier = sourceSupplier;
     this.stepSupplier = stepSupplier;
+    Timer timer = timerFactory.create(this);
     this.pipelineService = new AutoRestartingService<>(
         () -> new AbstractScheduledService() {
 
@@ -77,12 +88,12 @@ protected void runOneIteration() throws InterruptedException {
             if (!super.isRunning()) {
               return;
             }
-            runPipeline(super::isRunning);
+            timer.time(() -> runPipeline(super::isRunning));
             // This should not block access to the steps array
+            eventBus.post(new RunStoppedEvent());
             if (super.isRunning()) {
               eventBus.post(new RenderEvent());
             }
-            eventBus.post(new RunStoppedEvent());
           }
 
           @Override
@@ -182,20 +193,23 @@ private void runPipeline(Supplier<Boolean> isRunning) {
     final ImmutableList<Step> steps = stepSupplier.get();
     // Now that we have a snapshot we can run the pipeline with our copy.
 
-    for (Source source : sources) {
-      // if we have been stopped then we need to exit as soon as possible.
-      // then don't continue to run the pipeline.
-      if (!isRunning.get()) {
-        break;
+    if (!benchmarking.get()) {
+      // Don't update sources if this run is being benchmarked
+      for (Source source : sources) {
+        // if we have been stopped then we need to exit as soon as possible.
+        // then don't continue to run the pipeline.
+        if (!isRunning.get()) {
+          break;
+        }
+        source.updateOutputSockets();
       }
-      source.updateOutputSockets();
     }
 
     for (Step step : steps) {
       if (!isRunning.get()) {
         break;
       }
-      step.runPerformIfPossible();
+      step.runPerform(benchmarking.get());
     }
   }
 
@@ -213,4 +227,9 @@ public void onStopPipeline(@Nullable StopPipelineEvent event) {
     stopAsync();
   }
 
+  @Subscribe
+  public void onBenchmarkEvent(BenchmarkEvent event) {
+    benchmarking.set(event.isStart());
+  }
+
 }

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

@@ -1,5 +1,6 @@
 package edu.wpi.grip.core;
 
+import edu.wpi.grip.core.metrics.Timer;
 import edu.wpi.grip.core.sockets.InputSocket;
 import edu.wpi.grip.core.sockets.OutputSocket;
 import edu.wpi.grip.core.sockets.Socket;
@@ -27,6 +28,7 @@ public class Step {
   private static final String MISSING_SOCKET_MESSAGE_END = " must have a value to run this step.";
 
   private final ExceptionWitness witness;
+  private final Timer timer;
 
   private final Operation operation;
   private final OperationDescription description;
@@ -41,17 +43,20 @@ public class Step {
    * @param inputSockets            The input sockets from the operation.
    * @param outputSockets           The output sockets provided by the operation.
    * @param exceptionWitnessFactory A factory used to create an {@link ExceptionWitness}.
+   * @param timerFactory            A factory used to create a {@link Timer}.
    */
   Step(Operation operation,
        OperationDescription description,
        List<InputSocket> inputSockets,
        List<OutputSocket> outputSockets,
-       ExceptionWitness.Factory exceptionWitnessFactory) {
+       ExceptionWitness.Factory exceptionWitnessFactory,
+       Timer.Factory timerFactory) {
     this.operation = operation;
     this.description = description;
     this.inputSockets = inputSockets;
     this.outputSockets = outputSockets;
     this.witness = exceptionWitnessFactory.create(this);
+    this.timer = timerFactory.create(this);
   }
 
   /**
@@ -88,9 +93,23 @@ private void resetOutputSockets() {
   /**
    * The {@link Operation#perform} method should only be called if all {@link
    * InputSocket#getValue()} are not empty. If one input is invalid then the perform method will not
-   * run and all output sockets will be assigned to their default values.
+   * run and all output sockets will be assigned to their default values. If no input sockets have
+   * changed values, the perform method will not run.
    */
   protected final void runPerformIfPossible() {
+    runPerform(false);
+  }
+
+
+  /**
+   * The {@link Operation#perform} method should only be called if all {@link
+   * InputSocket#getValue()} are not empty. If one input is invalid then the perform method will not
+   * run and all output sockets will be assigned to their default values.
+   *
+   * @param force if this step should be forced to run. If {@code true}, the operation's perform
+   *              method will be called if every input is valid regardless of 'dirtiness'.
+   */
+  protected final void runPerform(boolean force) {
     boolean anyDirty = false; // Keeps track of if there are sockets that are dirty
 
     for (InputSocket<?> inputSocket : inputSockets) {
@@ -104,26 +123,26 @@ protected final void runPerformIfPossible() {
       }
       // If one value is true then this will stay true
       anyDirty |= inputSocket.dirtied();
-
     }
-    if (!anyDirty) { // If there aren't any dirty inputs Don't clear the exceptions just return
+    if (!force && !anyDirty) {
+      // If there aren't any dirty inputs don't clear the exceptions, just return
       return;
     }
 
     try {
       // We need to ensure that if perform disabled is switching states that we don't run the
-      // perform method
-      // while that is happening.
+      // perform method while that is happening.
       synchronized (removedLock) {
         if (!removed) {
-          this.operation.perform();
+          timer.time(this.operation::perform);
         }
       }
     } catch (RuntimeException e) {
       // We do not want to catch all exceptions, only runtime exceptions.
       // This is especially important when it comes to InterruptedExceptions
-      final String operationFailedMessage = String.format("The %s operation did not perform "
-          + "correctly.", getOperationDescription().name());
+      final String operationFailedMessage =
+          String.format("The %s operation did not perform correctly.",
+              getOperationDescription().name());
       logger.log(Level.WARNING, operationFailedMessage, e);
       witness.flagException(e, operationFailedMessage);
       resetOutputSockets();
@@ -158,10 +177,13 @@ protected boolean removed() {
   @Singleton
   public static class Factory {
     private final ExceptionWitness.Factory exceptionWitnessFactory;
+    private final Timer.Factory timerFactory;
 
     @Inject
-    public Factory(ExceptionWitness.Factory exceptionWitnessFactory) {
+    public Factory(ExceptionWitness.Factory exceptionWitnessFactory,
+                   Timer.Factory timerFactory) {
       this.exceptionWitnessFactory = exceptionWitnessFactory;
+      this.timerFactory = timerFactory;
     }
 
     /**
@@ -180,7 +202,8 @@ public Step create(OperationMetaData operationData) {
           operationData.getDescription(),
           inputSockets,
           outputSockets,
-          exceptionWitnessFactory
+          exceptionWitnessFactory,
+          timerFactory
       );
 
       for (Socket<?> socket : inputSockets) {

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

@@ -0,0 +1,29 @@
+package edu.wpi.grip.core;
+
+import java.util.Comparator;
+
+/**
+ * An interface for getting the indices of steps.
+ */
+public interface StepIndexer extends Comparator<Step> {
+
+  /**
+   * Gets the index of the given step.
+   *
+   * @param step the step to get the index of
+   * @return the index of the given step, or -1 if this object does not contain that step
+   */
+  int indexOf(Step step);
+
+  /**
+   * Compares two steps based on their indexes. <i>This is not consistent with {@code equals()}</i>.
+   *
+   * @param o1 the first step to compare
+   * @param o2 the second step to compare
+   */
+  @Override
+  default int compare(Step o1, Step o2) {
+    return indexOf(o1) - indexOf(o2);
+  }
+
+}

core/src/main/java/edu/wpi/grip/core/events/BenchmarkEvent.java

@@ -0,0 +1,25 @@
+package edu.wpi.grip.core.events;
+
+/**
+ * An event posted before and after a pipeline is benchmarked.
+ */
+public final class BenchmarkEvent {
+
+  private final boolean isStart;
+
+  private BenchmarkEvent(boolean isStart) {
+    this.isStart = isStart;
+  }
+
+  public static BenchmarkEvent started() {
+    return new BenchmarkEvent(true);
+  }
+
+  public static BenchmarkEvent finished() {
+    return new BenchmarkEvent(false);
+  }
+
+  public boolean isStart() {
+    return isStart;
+  }
+}

core/src/main/java/edu/wpi/grip/core/events/StartSingleBenchmarkRunEvent.java

@@ -0,0 +1,12 @@
+package edu.wpi.grip.core.events;
+
+/**
+ * An event representing the start of a single benchmarked pipeline run.
+ */
+public class StartSingleBenchmarkRunEvent implements RunPipelineEvent {
+
+  @Override
+  public boolean pipelineShouldRun() {
+    return true;
+  }
+}