From c603c143bb39e8a8dd1b35ad792c4dccb7de3a3a Mon Sep 17 00:00:00 2001 From: joshualitt Date: Thu, 15 Oct 2015 07:18:29 -0700 Subject: [PATCH] small tidy of benchmarkstream BUG=skia: Committed: https://skia.googlesource.com/skia/+/691b6af907e55250a29a7a2a346b63c2026011c3 Review URL: https://codereview.chromium.org/1395703002 --- tools/VisualBench/CpuWrappedBenchmark.h | 8 ++ tools/VisualBench/TimingStateMachine.cpp | 4 +- tools/VisualBench/TimingStateMachine.h | 5 +- tools/VisualBench/VisualBenchmarkStream.cpp | 32 +++++--- tools/VisualBench/VisualBenchmarkStream.h | 2 + .../VisualBench/VisualStreamTimingModule.cpp | 79 ++++++++++--------- tools/VisualBench/VisualStreamTimingModule.h | 13 ++- 7 files changed, 84 insertions(+), 59 deletions(-) diff --git a/tools/VisualBench/CpuWrappedBenchmark.h b/tools/VisualBench/CpuWrappedBenchmark.h index e63187cca4..214ed00e81 100644 --- a/tools/VisualBench/CpuWrappedBenchmark.h +++ b/tools/VisualBench/CpuWrappedBenchmark.h @@ -20,6 +20,12 @@ public: const char* onGetName() override { return fBench->getName(); } const char* onGetUniqueName() override { return fBench->getUniqueName(); } + void onDelayedSetup() override { fBench->delayedSetup(); } + void onPerCanvasPreDraw(SkCanvas* canvas) override { fBench->perCanvasPreDraw(canvas); } + void onPreDraw(SkCanvas* canvas) override { fBench->preDraw(canvas); } + void onPostDraw(SkCanvas* canvas) override { fBench->postDraw(canvas); } + void onPerCanvasPostDraw(SkCanvas* canvas) override { fBench->perCanvasPostDraw(canvas); } + void onDraw(int loops, SkCanvas* canvas) override { // TODO: use onPreDraw() to move offscreen allocation/deallocation out of timing. SkAutoTUnref offscreen(SkSurface::NewRaster(canvas->imageInfo())); @@ -29,6 +35,8 @@ public: canvas->drawImage(image, 0,0); } + virtual SkIPoint onGetSize() override { return fBench->getSize(); } + private: SkAutoTUnref fBench; }; diff --git a/tools/VisualBench/TimingStateMachine.cpp b/tools/VisualBench/TimingStateMachine.cpp index ae03f5d188..85aab42ba9 100644 --- a/tools/VisualBench/TimingStateMachine.cpp +++ b/tools/VisualBench/TimingStateMachine.cpp @@ -91,9 +91,7 @@ void TimingStateMachine::recordMeasurement() { fLastMeasurement = this->elapsed() / (FLAGS_frames * fLoops); } -void TimingStateMachine::nextBenchmark(SkCanvas* canvas, Benchmark* benchmark) { - benchmark->postDraw(canvas); - benchmark->perCanvasPostDraw(canvas); +void TimingStateMachine::nextBenchmark() { fLoops = 1; fInnerState = kTuning_InnerState; fState = kPreWarm_State; diff --git a/tools/VisualBench/TimingStateMachine.h b/tools/VisualBench/TimingStateMachine.h index 22150599ab..3f40b6d31f 100644 --- a/tools/VisualBench/TimingStateMachine.h +++ b/tools/VisualBench/TimingStateMachine.h @@ -38,10 +38,9 @@ public: ParentEvents nextFrame(bool preWarmBetweenSamples); /* - * The caller should call this when they are ready to move to the next benchmark. The caller - * must call this with the *last* benchmark so post draw hooks can be invoked + * The caller should call this when they are ready to move to the next benchmark. */ - void nextBenchmark(SkCanvas*, Benchmark*); + void nextBenchmark(); /* * When TimingStateMachine returns kTimingFinished_ParentEvents, then the owner can call diff --git a/tools/VisualBench/VisualBenchmarkStream.cpp b/tools/VisualBench/VisualBenchmarkStream.cpp index c520eeed05..235200a761 100644 --- a/tools/VisualBench/VisualBenchmarkStream.cpp +++ b/tools/VisualBench/VisualBenchmarkStream.cpp @@ -74,6 +74,11 @@ VisualBenchmarkStream::VisualBenchmarkStream() } } } + + // seed with an initial benchmark + // NOTE the initial benchmark will not have preTimingHooks called, but that is okay because + // it is the warmupbench + this->next(); } bool VisualBenchmarkStream::ReadPicture(const char* path, SkAutoTUnref* pic) { @@ -98,23 +103,24 @@ bool VisualBenchmarkStream::ReadPicture(const char* path, SkAutoTUnrefinnerNext()) && + (SkCommandLineFlags::ShouldSkip(FLAGS_match, bench->getUniqueName()) || + !bench->isSuitableFor(Benchmark::kGPU_Backend))) { + bench->unref(); + } + } + if (bench && FLAGS_cpu) { + bench = new CpuWrappedBenchmark(bench); } - Benchmark* bench; - - // skips non matching benches - while ((bench = this->innerNext()) && - (SkCommandLineFlags::ShouldSkip(FLAGS_match, bench->getUniqueName()) || - !bench->isSuitableFor(Benchmark::kGPU_Backend))) { - bench->unref(); - } - if (FLAGS_cpu) { - return new CpuWrappedBenchmark(bench); - } - return bench; + fBenchmark.reset(bench); + return fBenchmark; } Benchmark* VisualBenchmarkStream::innerNext() { diff --git a/tools/VisualBench/VisualBenchmarkStream.h b/tools/VisualBench/VisualBenchmarkStream.h index 9e5d4590cb..89ca632821 100644 --- a/tools/VisualBench/VisualBenchmarkStream.h +++ b/tools/VisualBench/VisualBenchmarkStream.h @@ -23,6 +23,7 @@ public: static bool ReadPicture(const char* path, SkAutoTUnref* pic); Benchmark* next(); + Benchmark* current() { return fBenchmark.get(); } private: Benchmark* innerNext(); @@ -30,6 +31,7 @@ private: const BenchRegistry* fBenches; const skiagm::GMRegistry* fGMs; SkTArray fSKPs; + SkAutoTUnref fBenchmark; const char* fSourceType; // What we're benching: bench, GM, SKP, ... const char* fBenchType; // How we bench it: micro, playback, ... diff --git a/tools/VisualBench/VisualStreamTimingModule.cpp b/tools/VisualBench/VisualStreamTimingModule.cpp index db75890d11..0a57e67221 100644 --- a/tools/VisualBench/VisualStreamTimingModule.cpp +++ b/tools/VisualBench/VisualStreamTimingModule.cpp @@ -10,61 +10,66 @@ #include "SkCanvas.h" VisualStreamTimingModule::VisualStreamTimingModule(VisualBench* owner, bool preWarmBeforeSample) - : fReinitializeBenchmark(false) + : fInitState(kReset_InitState) , fPreWarmBeforeSample(preWarmBeforeSample) , fOwner(owner) { fBenchmarkStream.reset(new VisualBenchmarkStream); } -bool VisualStreamTimingModule::nextBenchmarkIfNecessary(SkCanvas* canvas) { - if (fBenchmark) { - return true; +inline void VisualStreamTimingModule::handleInitState(SkCanvas* canvas) { + switch (fInitState) { + case kNewBenchmark_InitState: + fOwner->clear(canvas, SK_ColorWHITE, 2); + fBenchmarkStream->current()->delayedSetup(); + // fallthrough + case kReset_InitState: + fBenchmarkStream->current()->preTimingHooks(canvas); + break; + case kNone_InitState: + break; } - - fBenchmark.reset(fBenchmarkStream->next()); - if (!fBenchmark) { - return false; - } - - fOwner->clear(canvas, SK_ColorWHITE, 2); - - fBenchmark->delayedSetup(); - fBenchmark->preTimingHooks(canvas); - return true; + fInitState = kNone_InitState; } -void VisualStreamTimingModule::draw(SkCanvas* canvas) { - if (!this->nextBenchmarkIfNecessary(canvas)) { - SkDebugf("Exiting VisualBench successfully\n"); - fOwner->closeWindow(); - return; - } - - if (fReinitializeBenchmark) { - fReinitializeBenchmark = false; - fBenchmark->preTimingHooks(canvas); - } - - this->renderFrame(canvas, fBenchmark, fTSM.loops()); - fOwner->present(); - TimingStateMachine::ParentEvents event = fTSM.nextFrame(fPreWarmBeforeSample); +inline void VisualStreamTimingModule::handleTimingEvent(SkCanvas* canvas, + TimingStateMachine::ParentEvents event) { switch (event) { case TimingStateMachine::kReset_ParentEvents: - fBenchmark->postTimingHooks(canvas); + fBenchmarkStream->current()->postTimingHooks(canvas); fOwner->reset(); - fReinitializeBenchmark = true; + fInitState = kReset_InitState; break; case TimingStateMachine::kTiming_ParentEvents: break; case TimingStateMachine::kTimingFinished_ParentEvents: - fBenchmark->postTimingHooks(canvas); + fBenchmarkStream->current()->postTimingHooks(canvas); fOwner->reset(); - if (this->timingFinished(fBenchmark, fTSM.loops(), fTSM.lastMeasurement())) { - fTSM.nextBenchmark(canvas, fBenchmark); - fBenchmark.reset(nullptr); + if (this->timingFinished(fBenchmarkStream->current(), fTSM.loops(), + fTSM.lastMeasurement())) { + fTSM.nextBenchmark(); + if (!fBenchmarkStream->next()) { + SkDebugf("Exiting VisualBench successfully\n"); + fOwner->closeWindow(); + } else { + fInitState = kNewBenchmark_InitState; + } } else { - fReinitializeBenchmark = true; + fInitState = kReset_InitState; } break; } } + +void VisualStreamTimingModule::draw(SkCanvas* canvas) { + if (!fBenchmarkStream->current()) { + // this should never happen but just to be safe + // TODO research why this does happen on mac + return; + } + + this->handleInitState(canvas); + this->renderFrame(canvas, fBenchmarkStream->current(), fTSM.loops()); + fOwner->present(); + TimingStateMachine::ParentEvents event = fTSM.nextFrame(fPreWarmBeforeSample); + this->handleTimingEvent(canvas, event); +} diff --git a/tools/VisualBench/VisualStreamTimingModule.h b/tools/VisualBench/VisualStreamTimingModule.h index ac06ed4307..5dbec693fa 100644 --- a/tools/VisualBench/VisualStreamTimingModule.h +++ b/tools/VisualBench/VisualStreamTimingModule.h @@ -14,6 +14,8 @@ #include "VisualBench.h" #include "VisualBenchmarkStream.h" +class SkCanvas; + /* * VisualStreamTimingModule is the base class for modules which want to time a stream of Benchmarks. * @@ -31,12 +33,17 @@ private: // subclasses should return true to advance the stream virtual bool timingFinished(Benchmark*, int loops, double measurement)=0; - bool nextBenchmarkIfNecessary(SkCanvas*); + inline void handleInitState(SkCanvas*); + inline void handleTimingEvent(SkCanvas*, TimingStateMachine::ParentEvents); TimingStateMachine fTSM; SkAutoTDelete fBenchmarkStream; - SkAutoTUnref fBenchmark; - bool fReinitializeBenchmark; + enum InitState { + kNone_InitState, + kReset_InitState, + kNewBenchmark_InitState, + }; + InitState fInitState; bool fPreWarmBeforeSample; // support framework