Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for timing steps and pipelines #613

Merged
merged 43 commits into from
Oct 31, 2016
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
43 commits
Select commit Hold shift + click to select a range
3470404
Add support for timing steps and pipelines
SamCarlberg Jul 7, 2016
72a02ee
Suppress PMD warnings
SamCarlberg Jul 7, 2016
0a9302c
Add documentation to StepStarted and StepFinishedEvents
SamCarlberg Jul 7, 2016
bbf2379
Add analysis of step timings
SamCarlberg Jul 9, 2016
14911e5
Updated UI test
SamCarlberg Jul 9, 2016
0a53a78
Add benchmarking to analysis window
SamCarlberg Jul 9, 2016
b71d79a
Undo accidental formatting
SamCarlberg Jul 9, 2016
493560f
Fix and suppress issues from Codacy
SamCarlberg Jul 10, 2016
eed8a26
Add tests for Timer
SamCarlberg Jul 10, 2016
0d8a8a8
Improve UI and increase tolerance for timer tests
SamCarlberg Jul 11, 2016
849ad71
Increase timer test tolerance to 5ms (5%)
SamCarlberg Jul 11, 2016
b0dca11
Add tests for BenchmarkRunner
SamCarlberg Jul 11, 2016
ca33126
Change analysis averager to a moving average, improve naming and docu…
SamCarlberg Jul 12, 2016
7b8d888
Fix flickering, make Analysis immutable
SamCarlberg Jul 12, 2016
f84ae37
Reset timers after running a benchmark.
SamCarlberg Jul 12, 2016
16f3a38
Add tests for MovingAverage
SamCarlberg Jul 13, 2016
c79f8c3
Update TimerTest for reset()
SamCarlberg Jul 13, 2016
764cba8
Fix tests (oops)
SamCarlberg Jul 13, 2016
23260d2
Freeze sources while benchmarking
SamCarlberg Jul 13, 2016
9ab9953
Improve Timer API, documentation, and tests
SamCarlberg Jul 13, 2016
ba91b1c
Fix accidental formatting (again)
SamCarlberg Jul 13, 2016
39880c0
Merge branch 'master' into feat/timing
SamCarlberg Jul 15, 2016
a1ddd49
Remove redundant (ms)
SamCarlberg Jul 15, 2016
9ca431b
Fix PMD issues
SamCarlberg Jul 15, 2016
5d2dce9
Fix PMD in UI
SamCarlberg Jul 16, 2016
7265ddf
Appease Codacy
SamCarlberg Jul 18, 2016
2239269
Merge branch 'master' into feat/timing
SamCarlberg Jul 28, 2016
9be25db
Don't make analysis window block main window. Disable inputs and outp…
SamCarlberg Jul 28, 2016
b2db03d
Remove hacky data aggregation from Analysis class.
SamCarlberg Aug 4, 2016
785ff04
Replace custom FixedSizeQueue with guava's EvictingQueue
SamCarlberg Aug 4, 2016
8b7218d
Add option to export results to CSV.
SamCarlberg Aug 30, 2016
5e525ca
Merge branch master into feat/timing
SamCarlberg Aug 30, 2016
a513c84
Fix tests
SamCarlberg Aug 30, 2016
9dd68f4
Add tests for metrics
SamCarlberg Aug 31, 2016
b2f9d4e
Move CSV to it's own class
SamCarlberg Oct 4, 2016
a0aaaeb
Merge master
SamCarlberg Oct 4, 2016
6e06b3b
Fix some checkstyle stuff I missed
SamCarlberg Oct 4, 2016
7c3b79d
Rename Timer.stopped() to stop()
SamCarlberg Oct 6, 2016
3d9f2b6
Delete redudant Analysis class and improve variable names in Statistics
SamCarlberg Oct 11, 2016
b01b2b5
Improve Javadoc for statistics hotness function
SamCarlberg Oct 11, 2016
b61da85
Fix import order
SamCarlberg Oct 11, 2016
0c3b769
Merge remote-tracking branch 'refs/remotes/WPIRoboicsProjects/master'
SamCarlberg Oct 24, 2016
3d7481c
Move analysis window to fx:include
SamCarlberg Oct 24, 2016
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions core/src/main/java/edu/wpi/grip/core/GripCoreModule.java
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -143,6 +145,9 @@ public <I> void hear(TypeLiteral<I> type, TypeEncounter<I> encounter) {
.build(HttpSource.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
Expand Down
41 changes: 31 additions & 10 deletions core/src/main/java/edu/wpi/grip/core/PipelineRunner.java
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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;

Expand All @@ -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() {

Expand All @@ -77,12 +88,17 @@ protected void runOneIteration() throws InterruptedException {
if (!super.isRunning()) {
return;
}
runPipeline(super::isRunning);
try {
timer.started();
runPipeline(super::isRunning);
} finally {
timer.stopped();
}
// 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
Expand Down Expand Up @@ -195,7 +211,7 @@ private void runPipeline(Supplier<Boolean> isRunning) {
if (!isRunning.get()) {
break;
}
step.runPerformIfPossible();
step.runPerform(benchmarking.get());
}
}

Expand All @@ -213,4 +229,9 @@ public void onStopPipeline(@Nullable StopPipelineEvent event) {
stopAsync();
}

@Subscribe
public void onBenchmarkEvent(BenchmarkEvent event) {
benchmarking.set(event.isStart());
}

}
47 changes: 37 additions & 10 deletions core/src/main/java/edu/wpi/grip/core/Step.java
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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;
Expand All @@ -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);
}

/**
Expand Down Expand Up @@ -88,9 +93,24 @@ 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 benchmarking if this step is being benchmarked. The operation's perform method will be
* called if every input is valid regardless of 'dirtiness' if it's being
* benchmarked.
*/
protected final void runPerform(boolean benchmarking) {
boolean anyDirty = false; // Keeps track of if there are sockets that are dirty

for (InputSocket<?> inputSocket : inputSockets) {
Expand All @@ -104,16 +124,16 @@ 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 (!benchmarking && !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.
timer.started();
synchronized (removedLock) {
if (!removed) {
this.operation.perform();
Expand All @@ -122,12 +142,15 @@ protected final void runPerformIfPossible() {
} 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();
return;
} finally {
timer.stopped();
}
witness.clearException();
}
Expand Down Expand Up @@ -158,10 +181,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;
}

/**
Expand All @@ -180,7 +206,8 @@ public Step create(OperationMetaData operationData) {
operationData.getDescription(),
inputSockets,
outputSockets,
exceptionWitnessFactory
exceptionWitnessFactory,
timerFactory
);

for (Socket<?> socket : inputSockets) {
Expand Down
25 changes: 25 additions & 0 deletions core/src/main/java/edu/wpi/grip/core/events/BenchmarkEvent.java
Original file line number Diff line number Diff line change
@@ -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;
}
}
12 changes: 12 additions & 0 deletions core/src/main/java/edu/wpi/grip/core/events/BenchmarkRunEvent.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
package edu.wpi.grip.core.events;

/**
* An event representing the start of benchmarked pipeline run.
*/
public class BenchmarkRunEvent implements RunPipelineEvent {

@Override
public boolean pipelineShouldRun() {
return true;
}
}
59 changes: 59 additions & 0 deletions core/src/main/java/edu/wpi/grip/core/events/TimerEvent.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
package edu.wpi.grip.core.events;

import edu.wpi.grip.core.metrics.Analysis;

import static com.google.common.base.Preconditions.checkNotNull;

/**
* An event fired when a {@link edu.wpi.grip.core.metrics.Timer Timer} finishes timing something.
*
* <p>This contains:
* <ul>
* <li>The object being timed</li>
* <li>How long the timed action took, in microseconds</li>
* <li>Historical data about the action</li>
* </ul>
* </p>
*/
public class TimerEvent<T> {

private final T source;
private final long elapsedTime;
private final Analysis data;

/**
* Creates a new timer event.
*
* @param source the object that was timed
* @param elapsedTime the time elapsed in microseconds
* @param data the analysis data of the event
*/
public TimerEvent(T source, long elapsedTime, Analysis data) {
checkNotNull(source, "source");
checkNotNull(data, "data");
this.source = source;
this.elapsedTime = elapsedTime;
this.data = data;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

checkNotNull(source, "source"); returns the source so you can just use it in the same line as the assignment.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is cleaner IMO

}

/**
* Gets the source object that had an action being timed.
*/
public T getSource() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems ambiguous with Source as the input to the pipeline.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe call it target?

return source;
}

/**
* Gets the elapsed time of the action, in microseconds.
*/
public long getElapsedTime() {
return elapsedTime;
}

/**
* Gets the analysis of the historical timing data for the action.
*/
public Analysis getData() {
return data;
}
}
Loading