Cleanup timing state machine

BUG=skia:

Review URL: https://codereview.chromium.org/1386933002

bench/Benchmark.h

@@ -111,6 +111,19 @@ public:
      */
     virtual bool isVisual() { return false; }
 
+    /*
+     * VisualBench frequently resets the canvas.  As a result we need to bulk call all of the hooks
+     */
+    void preTimingHooks(SkCanvas* canvas) {
+        this->perCanvasPreDraw(canvas);
+        this->preDraw(canvas);
+    }
+
+    void postTimingHooks(SkCanvas* canvas)  {
+        this->postDraw(canvas);
+        this->perCanvasPostDraw(canvas);
+    }
+
 protected:
     virtual void setupPaint(SkPaint* paint);
 

tools/VisualBench/TimingStateMachine.cpp

@@ -18,53 +18,63 @@ TimingStateMachine::TimingStateMachine()
     : fCurrentFrame(0)
     , fLoops(1)
     , fLastMeasurement(0.)
-    , fState(kPreWarmLoopsPerCanvasPreDraw_State) {
+    , fState(kPreWarm_State)
+    , fInnerState(kTuning_InnerState) {
 }
 
-TimingStateMachine::ParentEvents TimingStateMachine::nextFrame(SkCanvas* canvas,
+TimingStateMachine::ParentEvents TimingStateMachine::nextFrame(bool preWarmBetweenSamples) {
-                                                               Benchmark* benchmark) {
+    ParentEvents parentEvent = kTiming_ParentEvents;
     switch (fState) {
-        case kPreWarmLoopsPerCanvasPreDraw_State:
+        case kPreWarm_State: {
-            return this->perCanvasPreDraw(canvas, benchmark, kPreWarmLoops_State);
-        case kPreWarmLoops_State:
-            return this->preWarm(kTuneLoops_State);
-        case kTuneLoops_State:
-            return this->tuneLoops();
-        case kPreWarmTimingPerCanvasPreDraw_State:
-            return this->perCanvasPreDraw(canvas, benchmark, kPreWarmTiming_State);
-        case kPreWarmTiming_State:
-            return this->preWarm(kTiming_State);
-        case kTiming_State:
-            return this->timing(canvas, benchmark);
-    }
-    SkFAIL("Incomplete switch\n");
-    return kTiming_ParentEvents;
-}
-
-inline void TimingStateMachine::nextState(State nextState) {
-    fState = nextState;
-}
-
-TimingStateMachine::ParentEvents TimingStateMachine::perCanvasPreDraw(SkCanvas* canvas,
-                                                                      Benchmark* benchmark,
-                                                                      State nextState) {
-    benchmark->perCanvasPreDraw(canvas);
-    benchmark->preDraw(canvas);
-    fCurrentFrame = 0;
-    this->nextState(nextState);
-    return kTiming_ParentEvents;
-}
-
-TimingStateMachine::ParentEvents TimingStateMachine::preWarm(State nextState) {
             if (fCurrentFrame >= FLAGS_gpuFrameLag) {
-        // we currently time across all frames to make sure we capture all GPU work
-        this->nextState(nextState);
                 fCurrentFrame = 0;
                 fTimer.start();
+                fState = kTiming_State;
             } else {
                 fCurrentFrame++;
             }
-    return kTiming_ParentEvents;
+            break;
+        }
+        case kTiming_State: {
+            switch (fInnerState) {
+                case kTuning_InnerState: {
+                    if (1 << 30 == fLoops) {
+                        // We're about to wrap.  Something's wrong with the bench.
+                        SkDebugf("InnerLoops wrapped\n");
+                        fLoops = 1;
+                    } else {
+                        double elapsedMs = this->elapsed();
+                        if (elapsedMs < FLAGS_loopMs) {
+                            fLoops *= 2;
+                        } else {
+                            fInnerState = kTiming_InnerState;
+                            fState = kPreWarm_State;
+                        }
+                        this->resetTimingState();
+                        parentEvent = kReset_ParentEvents;
+                    }
+                    break;
+                }
+                case kTiming_InnerState: {
+                    if (fCurrentFrame >= FLAGS_frames) {
+                        this->recordMeasurement();
+                        this->resetTimingState();
+                        parentEvent = kTimingFinished_ParentEvents;
+                        if (preWarmBetweenSamples) {
+                            fState = kPreWarm_State;
+                        } else {
+                            fTimer.start(); // start timing again, don't change state
+                        }
+                    } else {
+                        fCurrentFrame++;
+                    }
+                    break;
+                }
+            }
+        }
+        break;
+    }
+    return parentEvent;
 }
 
 inline double TimingStateMachine::elapsed() {
@@ -77,52 +87,14 @@ void TimingStateMachine::resetTimingState() {
     fTimer = WallTimer();
 }
 
-inline TimingStateMachine::ParentEvents TimingStateMachine::tuneLoops() {
-    if (1 << 30 == fLoops) {
-        // We're about to wrap.  Something's wrong with the bench.
-        SkDebugf("InnerLoops wrapped\n");
-        fLoops = 1;
-        return kTiming_ParentEvents;
-    } else {
-        double elapsedMs = this->elapsed();
-        if (elapsedMs > FLAGS_loopMs) {
-            this->nextState(kPreWarmTimingPerCanvasPreDraw_State);
-        } else {
-            fLoops *= 2;
-            this->nextState(kPreWarmLoops_State);
-        }
-        this->resetTimingState();
-        return kReset_ParentEvents;
-    }
-}
-
 void TimingStateMachine::recordMeasurement() {
     fLastMeasurement = this->elapsed() / (FLAGS_frames * fLoops);
 }
 
-inline TimingStateMachine::ParentEvents TimingStateMachine::timing(SkCanvas* canvas,
-                                                                   Benchmark* benchmark) {
-    if (fCurrentFrame >= FLAGS_frames) {
-        this->recordMeasurement();
-        this->resetTimingState();
-        return kTimingFinished_ParentEvents;
-    } else {
-        fCurrentFrame++;
-        return kTiming_ParentEvents;
-    }
-}
-
 void TimingStateMachine::nextBenchmark(SkCanvas* canvas, Benchmark* benchmark) {
     benchmark->postDraw(canvas);
     benchmark->perCanvasPostDraw(canvas);
     fLoops = 1;
-    this->nextState(kPreWarmLoopsPerCanvasPreDraw_State);
+    fInnerState = kTuning_InnerState;
-}
+    fState = kPreWarm_State;
-
-void TimingStateMachine::nextSampleWithPrewarm() {
-    this->nextState(kPreWarmTimingPerCanvasPreDraw_State);
-}
-
-void TimingStateMachine::nextSample() {
-    fTimer.start();
 }

tools/VisualBench/TimingStateMachine.h

@@ -36,13 +36,7 @@ public:
                                      // reset
     };
 
-    ParentEvents nextFrame(SkCanvas* canvas, Benchmark* benchmark);
+    ParentEvents nextFrame(bool preWarmBetweenSamples);
-
-    /*
-     * Before taking another sample, the owner can choose to prewarm or not
-     */
-    void nextSampleWithPrewarm();
-    void nextSample();
 
     /*
      * The caller should call this when they are ready to move to the next benchmark.  The caller
@@ -50,7 +44,6 @@ public:
      */
     void nextBenchmark(SkCanvas*, Benchmark*);
 
-
     /*
      * When TimingStateMachine returns kTimingFinished_ParentEvents, then the owner can call
      * lastMeasurement() to get the time
@@ -60,43 +53,17 @@ public:
     int loops() const { return fLoops; }
 
 private:
-    /*
-     * The heart of the timing state machine is an event driven timing loop.
-     * kPreWarmLoopsPerCanvasPreDraw_State:  Before we begin timing, Benchmarks have a hook to
-     *                                       access the canvas.  Then we prewarm before the autotune
-     *                                       loops step.
-     * kPreWarmLoops_State:                  We prewarm the gpu before auto tuning to enter a steady
-     *                                       work state
-     * kTuneLoops_State:                     Then we tune the loops of the benchmark to ensure we
-     *                                       are doing a measurable amount of work
-     * kPreWarmTimingPerCanvasPreDraw_State: Because reset the context after tuning loops to ensure
-     *                                       coherent state, we need to give the benchmark
-     *                                       another hook
-     * kPreWarmTiming_State:                 We prewarm the gpu again to enter a steady state
-     * kTiming_State:                        Finally we time the benchmark.  When finished timing
-     *                                       if we have enough samples then we'll start the next
-     *                                       benchmark in the kPreWarmLoopsPerCanvasPreDraw_State.
-     *                                       otherwise, we enter the
-     *                                       kPreWarmTimingPerCanvasPreDraw_State for another sample
-     *                                       In either case we reset the context.
-     */
     enum State {
-        kPreWarmLoopsPerCanvasPreDraw_State,
+        kPreWarm_State,
-        kPreWarmLoops_State,
-        kTuneLoops_State,
-        kPreWarmTimingPerCanvasPreDraw_State,
-        kPreWarmTiming_State,
         kTiming_State,
     };
+    enum InnerState {
+        kTuning_InnerState,
+        kTiming_InnerState,
+    };
 
-    inline void nextState(State);
-    ParentEvents perCanvasPreDraw(SkCanvas*, Benchmark*, State);
-    ParentEvents preWarm(State nextState);
-    inline ParentEvents tuneLoops();
-    inline ParentEvents timing(SkCanvas*, Benchmark*);
     inline double elapsed();
     void resetTimingState();
-    void postDraw(SkCanvas*, Benchmark*);
     void recordMeasurement();
 
     int fCurrentFrame;
@@ -104,6 +71,7 @@ private:
     double fLastMeasurement;
     WallTimer fTimer;
     State fState;
+    InnerState fInnerState;
 };
 
 #endif

tools/VisualBench/VisualInteractiveModule.cpp

@@ -27,6 +27,7 @@ VisualInteractiveModule::VisualInteractiveModule(VisualBench* owner)
     : fCurrentMeasurement(0)
     , fBenchmark(nullptr)
     , fAdvance(false)
+    , fHasBeenReset(false)
     , fOwner(SkRef(owner)) {
     fBenchmarkStream.reset(new VisualBenchmarkStream);
 
@@ -93,7 +94,7 @@ bool VisualInteractiveModule::advanceRecordIfNecessary(SkCanvas* canvas) {
     fOwner->clear(canvas, SK_ColorWHITE, 2);
 
     fBenchmark->delayedSetup();
-
+    fBenchmark->preTimingHooks(canvas);
     return true;
 }
 #include "GrGpu.h"
@@ -104,10 +105,18 @@ void VisualInteractiveModule::draw(SkCanvas* canvas) {
         fOwner->closeWindow();
         return;
     }
+
+    if (fHasBeenReset) {
+        fHasBeenReset = false;
+        fBenchmark->preTimingHooks(canvas);
+    }
+
     this->renderFrame(canvas);
-    TimingStateMachine::ParentEvents event = fTSM.nextFrame(canvas, fBenchmark);
+    TimingStateMachine::ParentEvents event = fTSM.nextFrame(false);
     switch (event) {
         case TimingStateMachine::kReset_ParentEvents:
+            fBenchmark->postTimingHooks(canvas);
+            fHasBeenReset = true;
             fOwner->reset();
             break;
         case TimingStateMachine::kTiming_ParentEvents:
@@ -121,10 +130,10 @@ void VisualInteractiveModule::draw(SkCanvas* canvas) {
             if (fAdvance) {
                 fAdvance = false;
                 fTSM.nextBenchmark(canvas, fBenchmark);
+                fBenchmark->postTimingHooks(canvas);
                 fBenchmark.reset(nullptr);
                 fOwner->reset();
-            } else {
+                fHasBeenReset = true;
-                fTSM.nextSample();
             }
             break;
     }

tools/VisualBench/VisualInteractiveModule.h

@@ -47,6 +47,7 @@ private:
     SkAutoTUnref<Benchmark> fBenchmark;
     TimingStateMachine fTSM;
     bool fAdvance;
+    bool fHasBeenReset;
 
     // support framework
     SkAutoTUnref<VisualBench> fOwner;

tools/VisualBench/VisualLightweightBenchModule.cpp

@@ -44,6 +44,7 @@ static SkString humanize(double ms) {
 
 VisualLightweightBenchModule::VisualLightweightBenchModule(VisualBench* owner)
     : fCurrentSample(0)
+    , fHasBeenReset(false)
     , fOwner(SkRef(owner))
     , fResults(new ResultsWriter) {
     fBenchmarkStream.reset(new VisualBenchmarkStream);
@@ -132,12 +133,14 @@ bool VisualLightweightBenchModule::advanceRecordIfNecessary(SkCanvas* canvas) {
 
     fOwner->clear(canvas, SK_ColorWHITE, 2);
 
-    fBenchmark->delayedSetup();
     fRecords.push_back();
 
     // Log bench name
     fResults->bench(fBenchmark->getUniqueName(), fBenchmark->getSize().fX,
                     fBenchmark->getSize().fY);
+
+    fBenchmark->delayedSetup();
+    fBenchmark->preTimingHooks(canvas);
     return true;
 }
 
@@ -147,15 +150,24 @@ void VisualLightweightBenchModule::draw(SkCanvas* canvas) {
         fOwner->closeWindow();
         return;
     }
+
+    if (fHasBeenReset) {
+        fHasBeenReset = false;
+        fBenchmark->preTimingHooks(canvas);
+    }
+
     this->renderFrame(canvas);
-    TimingStateMachine::ParentEvents event = fTSM.nextFrame(canvas, fBenchmark);
+    TimingStateMachine::ParentEvents event = fTSM.nextFrame(true);
     switch (event) {
         case TimingStateMachine::kReset_ParentEvents:
+            fBenchmark->postTimingHooks(canvas);
             fOwner->reset();
+            fHasBeenReset = true;
             break;
         case TimingStateMachine::kTiming_ParentEvents:
             break;
         case TimingStateMachine::kTimingFinished_ParentEvents:
+            fBenchmark->postTimingHooks(canvas);
             fOwner->reset();
             fRecords.back().fMeasurements.push_back(fTSM.lastMeasurement());
             if (++fCurrentSample > FLAGS_samples) {
@@ -164,7 +176,7 @@ void VisualLightweightBenchModule::draw(SkCanvas* canvas) {
                 fCurrentSample = 0;
                 fBenchmark.reset(nullptr);
             } else {
-                fTSM.nextSampleWithPrewarm();
+                fHasBeenReset = true;
             }
             break;
     }

tools/VisualBench/VisualLightweightBenchModule.h

@@ -47,6 +47,7 @@ private:
     SkAutoTDelete<VisualBenchmarkStream> fBenchmarkStream;
     SkAutoTUnref<Benchmark> fBenchmark;
     TimingStateMachine fTSM;
+    bool fHasBeenReset;
 
     // support framework
     SkAutoTUnref<VisualBench> fOwner;