DM tweaks

- Don't print status updates for skipped tasks or count them as pending tasks.
 - Refactor DMReporter a bit for better symmetry, be more explicit about
   how we read atomics (that is, approximately) in printStatus() (née finish()).
 - Remove mutex locking from printStatus().

BUG=skia:
R=halcanary@google.com, mtklein@google.com

Author: mtklein@chromium.org

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

git-svn-id: http://skia.googlecode.com/svn/trunk@14977 2bbb7eff-a529-9590-31e7-b0007b416f81

dm/DM.cpp

@@ -182,10 +182,7 @@ static void kick_off_skps(DM::Reporter* reporter, DM::TaskRunner* tasks) {
     }
 }
 
-static void report_failures(const DM::Reporter& reporter) {
-    SkTArray<SkString> failures;
-    reporter.getFailures(&failures);
-
+static void report_failures(const SkTArray<SkString>& failures) {
     if (failures.count() == 0) {
         return;
     }
@@ -194,6 +191,7 @@ static void report_failures(const DM::Reporter& reporter) {
     for (int i = 0; i < failures.count(); i++) {
         SkDebugf("  %s\n", failures[i].c_str());
     }
+    SkDebugf("%d failures.\n", failures.count());
 }
 
 template <typename T, typename Registry>
@@ -208,7 +206,7 @@ static void append_matching_factories(Registry* head, SkTDArray<typename Registr
 
 int tool_main(int argc, char** argv);
 int tool_main(int argc, char** argv) {
-    SkGraphics::Init();
+    SkAutoGraphics ag;
     SkCommandLineFlags::Parse(argc, argv);
 #if SK_ENABLE_INST_COUNT
     gPrintInstCount = FLAGS_leaks;
@@ -260,11 +258,11 @@ int tool_main(int argc, char** argv) {
     tasks.wait();
 
     SkDebugf("\n");
-    report_failures(reporter);
 
-    SkGraphics::Term();
-
-    return reporter.failed() > 0;
+    SkTArray<SkString> failures;
+    reporter.getFailures(&failures);
+    report_failures(failures);
+    return failures.count() > 0;
 }
 
 #if !defined(SK_BUILD_FOR_IOS) && !defined(SK_BUILD_FOR_NACL)

dm/DMReporter.cpp

@@ -1,5 +1,6 @@
 #include "DMReporter.h"
 
+#include "SkDynamicAnnotations.h"
 #include "SkCommandLineFlags.h"
 #include "OverwriteLine.h"
 
@@ -8,18 +9,17 @@ DEFINE_bool2(verbose, v, false, "If true, print status updates one-per-line.");
 
 namespace DM {
 
-void Reporter::finish(SkString name, SkMSec timeMs) {
-    sk_atomic_inc(&fFinished);
-
+void Reporter::printStatus(SkString name, SkMSec timeMs) const {
     if (FLAGS_quiet) {
         return;
     }
 
+    // It's okay if these are a little off---they're just for show---so we can read unprotectedly.
+    const int32_t failed  = SK_ANNOTATE_UNPROTECTED_READ(fFailed);
+    const int32_t pending = SK_ANNOTATE_UNPROTECTED_READ(fPending) - 1;
+
     SkString status;
-    status.printf("%s%d tasks left",
-                  FLAGS_verbose ? "\n" : kSkOverwriteLine,
-                  this->started() - this->finished());
-    const int failed = this->failed();
+    status.printf("%s%d tasks left", FLAGS_verbose ? "\n" : kSkOverwriteLine, pending);
     if (failed > 0) {
         status.appendf(", %d failed", failed);
     }
@@ -29,12 +29,9 @@ void Reporter::finish(SkString name, SkMSec timeMs) {
     SkDebugf("%s", status.c_str());
 }
 
-int32_t Reporter::failed() const {
-    SkAutoMutexAcquire reader(&fMutex);
-    return fFailures.count();
-}
-
 void Reporter::fail(SkString msg) {
+    sk_atomic_inc(&fFailed);
+
     SkAutoMutexAcquire writer(&fMutex);
     fFailures.push_back(msg);
 }

dm/DMReporter.h

@@ -12,20 +12,19 @@ namespace DM {
 
 class Reporter : SkNoncopyable {
 public:
-    Reporter() : fStarted(0), fFinished(0) {}
+    Reporter() : fPending(0), fFailed(0) {}
 
-    void start()  { sk_atomic_inc(&fStarted); }
-    void finish(SkString name, SkMSec timeMs);
+    void taskCreated()   { sk_atomic_inc(&fPending); }
+    void taskDestroyed() { sk_atomic_dec(&fPending); }
     void fail(SkString msg);
 
-    int32_t started()  const { return fStarted; }
-    int32_t finished() const { return fFinished; }
-    int32_t failed()   const;
+    void printStatus(SkString name, SkMSec timeMs) const;
 
     void getFailures(SkTArray<SkString>*) const;
 
 private:
-    int32_t fStarted, fFinished;
+    int32_t fPending; // atomic
+    int32_t fFailed;  // atomic, == fFailures.count().
 
     mutable SkMutex fMutex;  // Guards fFailures.
     SkTArray<SkString> fFailures;

dm/DMTask.cpp

@@ -11,14 +11,18 @@ Task::Task(Reporter* reporter, TaskRunner* taskRunner)
     : fReporter(reporter)
     , fTaskRunner(taskRunner)
     , fDepth(0) {
-    fReporter->start();
+    fReporter->taskCreated();
 }
 
 Task::Task(const Task& parent)
     : fReporter(parent.fReporter)
     , fTaskRunner(parent.fTaskRunner)
     , fDepth(parent.depth() + 1) {
-    fReporter->start();
+    fReporter->taskCreated();
+}
+
+Task::~Task() {
+    fReporter->taskDestroyed();
 }
 
 void Task::fail(const char* msg) {
@@ -34,7 +38,7 @@ void Task::start() {
 }
 
 void Task::finish() {
-    fReporter->finish(this->name(), SkTime::GetMSecs() - fStart);
+    fReporter->printStatus(this->name(), SkTime::GetMSecs() - fStart);
 }
 
 void Task::spawnChildNext(CpuTask* task) {
@@ -45,11 +49,11 @@ CpuTask::CpuTask(Reporter* reporter, TaskRunner* taskRunner) : Task(reporter, ta
 CpuTask::CpuTask(const Task& parent) : Task(parent) {}
 
 void CpuTask::run() {
-    this->start();
     if (FLAGS_cpu && !this->shouldSkip()) {
+        this->start();
         this->draw();
+        this->finish();
     }
-    this->finish();
     SkDELETE(this);
 }
 
@@ -63,11 +67,11 @@ void CpuTask::spawnChild(CpuTask* task) {
 GpuTask::GpuTask(Reporter* reporter, TaskRunner* taskRunner) : Task(reporter, taskRunner) {}
 
 void GpuTask::run(GrContextFactory& factory) {
-    this->start();
     if (FLAGS_gpu && !this->shouldSkip()) {
+        this->start();
         this->draw(&factory);
+        this->finish();
     }
-    this->finish();
     SkDELETE(this);
 }
 

dm/DMTask.h

@@ -30,7 +30,7 @@ public:
 protected:
     Task(Reporter* reporter, TaskRunner* taskRunner);
     Task(const Task& parent);
-    virtual ~Task() {}
+    virtual ~Task();
 
     void start();
     void fail(const char* msg = NULL);